Rename the machine configuration along with its directory
Found by the deployment to vm4006: the move renames the directory and leaves the file inside it alone, so the machine configuration ended up at `.git/werkator/.gittally.yml` — a pair of names the lookup did not expect, because it pairs directory and file name. The instance resolved empty credentials, no public URL and none of the host's build definitions, and said nothing about it. That is the exact failure this change exists to prevent, produced by the change itself. `StateDirMigration` now renames the configuration with the directory, unless one under the current name is already there. `ConfigFiles` carries `.git/werkator/.gittally.yml` as a third candidate as well, for a directory somebody moved by hand, where the migration never runs and so can rename nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
8499bb7a53
commit
39dac4b51b
@@ -108,6 +108,22 @@ So that an operator who moved the directory by hand does not lose the live state
|
|||||||
- [StateDirMigrationTest: "leaves both alone when the current directory already exists"](../../src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt)
|
- [StateDirMigrationTest: "leaves both alone when the current directory already exists"](../../src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt)
|
||||||
- [StateDirMigrationTest: "does nothing where there is no pre-rename directory"](../../src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt)
|
- [StateDirMigrationTest: "does nothing where there is no pre-rename directory"](../../src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt)
|
||||||
|
|
||||||
|
#### Scenario#1.06: The machine configuration survives a directory that moved without it
|
||||||
|
|
||||||
|
So that the move cannot produce a pair of names its own lookup does not expect.
|
||||||
|
|
||||||
|
- **Given** a state directory that has moved to `.git/werkator/`
|
||||||
|
- **and** the machine configuration inside it still named `.gittally.yml`
|
||||||
|
- **When** the configuration is loaded
|
||||||
|
- **Then** its settings take effect
|
||||||
|
- **and** where the move did it, the file carries the current name afterwards
|
||||||
|
|
||||||
|
##### Verified by
|
||||||
|
|
||||||
|
- [StateDirMigrationTest: "renames the machine configuration inside the moved directory"](../../src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt)
|
||||||
|
- [StateDirMigrationTest: "never overwrites a machine configuration that already carries the current name"](../../src/test/kotlin/de/hoennig/werkator/StateDirMigrationTest.kt)
|
||||||
|
- [ConfigLoaderTest: "finds the machine config left under its old name in an already-moved directory"](../../src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt)
|
||||||
|
|
||||||
## The Solution
|
## The Solution
|
||||||
|
|
||||||
**One spelling rule.**
|
**One spelling rule.**
|
||||||
@@ -120,6 +136,11 @@ This is what turns a rename into a decidable question instead of a matter of tas
|
|||||||
The current name wins and the old file is then ignored rather than merged — two files side by side are a half-done rename, not a layering.
|
The current name wins and the old file is then ignored rather than merged — two files side by side are a half-done rename, not a layering.
|
||||||
Error messages name the file that was actually read, so they never point at a file that does not exist.
|
Error messages name the file that was actually read, so they never point at a file that does not exist.
|
||||||
|
|
||||||
|
**A move that finishes the job.**
|
||||||
|
The deployment to vm4006 found the gap the hard way: the move renames the *directory* and leaves the file inside it alone, so the machine configuration ended up at `.git/werkator/.gittally.yml` — a pair of names the lookup did not expect.
|
||||||
|
The instance came up with empty credentials and no host build definitions, without an error, which is the failure this PR exists to prevent.
|
||||||
|
`StateDirMigration` now renames the configuration along with the directory, unless one under the current name is already there, and `ConfigFiles` carries the intermediate path as a third candidate for a directory somebody moved by hand.
|
||||||
|
|
||||||
**A one-time move for the state.**
|
**A one-time move for the state.**
|
||||||
[`StateDirMigration`](../../src/main/kotlin/de/hoennig/werkator/StateDirMigration.kt) renames `.git/gittally` to `.git/werkator` from `CliRunner`, before any command resolves a path under it and therefore before the second Spring context of `server` exists.
|
[`StateDirMigration`](../../src/main/kotlin/de/hoennig/werkator/StateDirMigration.kt) renames `.git/gittally` to `.git/werkator` from `CliRunner`, before any command resolves a path under it and therefore before the second Spring context of `server` exists.
|
||||||
A fallback was rejected here: unlike a configuration, the state is written, so a fallback would have to decide on every write which of two directories wins, where a one-time move decides once and leaves a single path behind.
|
A fallback was rejected here: unlike a configuration, the state is written, so a fallback would have to decide on every write which of two directories wins, where a one-time move decides once and leaves a single path behind.
|
||||||
|
|||||||
@@ -170,7 +170,10 @@ tar xzf ~/werkator-runtime-linux-x64.tar.gz -C ~/opt
|
|||||||
# 4. artifact root: a move on the same filesystem, instant in both directions
|
# 4. artifact root: a move on the same filesystem, instant in both directions
|
||||||
mv ~/.local/state/gittally ~/.local/state/werkator
|
mv ~/.local/state/gittally ~/.local/state/werkator
|
||||||
|
|
||||||
# 5. let the state directory move itself, and read what it says
|
# 5. let the state directory move itself, and read what it says.
|
||||||
|
# Check the output: publicBaseUrl, git.account and the host's build
|
||||||
|
# definitions must be there. Empty ones mean the machine configuration
|
||||||
|
# was not found — nothing fails on its own in that case.
|
||||||
cd ~/hs.hsadmin.ng
|
cd ~/hs.hsadmin.ng
|
||||||
~/opt/werkator/jre/bin/java -jar ~/opt/werkator/lib/werkator.jar config:print --full
|
~/opt/werkator/jre/bin/java -jar ~/opt/werkator/lib/werkator.jar config:print --full
|
||||||
|
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
package de.hoennig.werkator
|
package de.hoennig.werkator
|
||||||
|
|
||||||
|
import de.hoennig.werkator.config.ConfigFiles
|
||||||
import org.slf4j.LoggerFactory
|
import org.slf4j.LoggerFactory
|
||||||
import java.io.IOException
|
import java.io.IOException
|
||||||
import java.nio.file.Files
|
import java.nio.file.Files
|
||||||
@@ -44,10 +45,29 @@ object StateDirMigration {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
log.info("moved {} to {}: build history, control token and configuration kept", LEGACY_DIR, DIR)
|
log.info("moved {} to {}: build history, control token and configuration kept", LEGACY_DIR, DIR)
|
||||||
|
renameMachineConfig(current)
|
||||||
dropWorktrees(current)
|
dropWorktrees(current)
|
||||||
warnAboutUnits(current)
|
warnAboutUnits(current)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Moving the directory leaves the file inside it under its old name, and the pair
|
||||||
|
* of names is then one the lookup does not expect. Renaming it here is what makes
|
||||||
|
* the move complete instead of half-done.
|
||||||
|
*/
|
||||||
|
private fun renameMachineConfig(stateDir: Path) {
|
||||||
|
val legacy = stateDir.resolve(ConfigFiles.LEGACY_COMMITTED)
|
||||||
|
val current = stateDir.resolve(ConfigFiles.COMMITTED)
|
||||||
|
if (!Files.isRegularFile(legacy) || Files.exists(current)) return
|
||||||
|
try {
|
||||||
|
Files.move(legacy, current)
|
||||||
|
} catch (e: IOException) {
|
||||||
|
log.warn("could not rename {} to {} in {}: {}", ConfigFiles.LEGACY_COMMITTED, ConfigFiles.COMMITTED, stateDir, e.message)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
log.info("renamed the machine configuration to {}", ConfigFiles.COMMITTED)
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The moved worktrees point at their old path in both directions, so they are dropped
|
* The moved worktrees point at their old path in both directions, so they are dropped
|
||||||
* rather than repaired: the next build of a branch creates its worktree again, and
|
* rather than repaired: the next build of a branch creates its worktree again, and
|
||||||
|
|||||||
@@ -19,14 +19,24 @@ object ConfigFiles {
|
|||||||
/** The machine-specific configuration inside `.git`; secrets live here. */
|
/** The machine-specific configuration inside `.git`; secrets live here. */
|
||||||
const val REPO_INSTALL = ".git/werkator/$COMMITTED"
|
const val REPO_INSTALL = ".git/werkator/$COMMITTED"
|
||||||
|
|
||||||
private const val LEGACY_COMMITTED = ".gittally.yml"
|
/** The name the committed configuration had before the rename. */
|
||||||
|
const val LEGACY_COMMITTED = ".gittally.yml"
|
||||||
|
|
||||||
private const val LEGACY_REPO_INSTALL = ".git/gittally/$LEGACY_COMMITTED"
|
private const val LEGACY_REPO_INSTALL = ".git/gittally/$LEGACY_COMMITTED"
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The state directory moves without touching the file inside it, so between the
|
||||||
|
* move and the rename the machine configuration sits under the old name in the
|
||||||
|
* new directory. `StateDirMigration` closes that gap where it moves the directory
|
||||||
|
* itself; this candidate covers a directory somebody moved by hand.
|
||||||
|
*/
|
||||||
|
private const val MOVED_REPO_INSTALL = ".git/werkator/$LEGACY_COMMITTED"
|
||||||
|
|
||||||
/** Both names of the committed configuration, current first. */
|
/** Both names of the committed configuration, current first. */
|
||||||
val committed = listOf(COMMITTED, LEGACY_COMMITTED)
|
val committed = listOf(COMMITTED, LEGACY_COMMITTED)
|
||||||
|
|
||||||
/** Both paths of the machine-specific configuration, current first. */
|
/** Every path the machine-specific configuration can sit at, current first. */
|
||||||
val repoInstall = listOf(REPO_INSTALL, LEGACY_REPO_INSTALL)
|
val repoInstall = listOf(REPO_INSTALL, MOVED_REPO_INSTALL, LEGACY_REPO_INSTALL)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The first of [candidates] that exists under [dir], or the current name when none
|
* The first of [candidates] that exists under [dir], or the current name when none
|
||||||
|
|||||||
@@ -25,6 +25,27 @@ class StateDirMigrationTest : FunSpec() {
|
|||||||
dir.resolve(".git/werkator/build-results.json").toFile().readText() shouldBe "[]"
|
dir.resolve(".git/werkator/build-results.json").toFile().readText() shouldBe "[]"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
test("renames the machine configuration inside the moved directory") {
|
||||||
|
val dir = repoWithLegacyState()
|
||||||
|
dir.resolve(".git/gittally/.gittally.yml").toFile().writeText("git:\n account: ci-user\n")
|
||||||
|
|
||||||
|
StateDirMigration.migrateIfNeeded(dir)
|
||||||
|
|
||||||
|
Files.exists(dir.resolve(".git/werkator/.gittally.yml")).shouldBeFalse()
|
||||||
|
dir.resolve(".git/werkator/.werkator.yml").toFile().readText() shouldBe "git:\n account: ci-user\n"
|
||||||
|
}
|
||||||
|
|
||||||
|
test("never overwrites a machine configuration that already carries the current name") {
|
||||||
|
val dir = repoWithLegacyState()
|
||||||
|
dir.resolve(".git/gittally/.gittally.yml").toFile().writeText("git:\n account: old\n")
|
||||||
|
dir.resolve(".git/gittally/.werkator.yml").toFile().writeText("git:\n account: current\n")
|
||||||
|
|
||||||
|
StateDirMigration.migrateIfNeeded(dir)
|
||||||
|
|
||||||
|
dir.resolve(".git/werkator/.werkator.yml").toFile().readText() shouldBe "git:\n account: current\n"
|
||||||
|
Files.exists(dir.resolve(".git/werkator/.gittally.yml")).shouldBeTrue()
|
||||||
|
}
|
||||||
|
|
||||||
test("drops the moved worktrees, because they point at their old path") {
|
test("drops the moved worktrees, because they point at their old path") {
|
||||||
val dir = repoWithLegacyState()
|
val dir = repoWithLegacyState()
|
||||||
Files.createDirectories(dir.resolve(".git/gittally/worktrees/main"))
|
Files.createDirectories(dir.resolve(".git/gittally/worktrees/main"))
|
||||||
|
|||||||
@@ -74,6 +74,18 @@ class ConfigLoaderTest : FunSpec() {
|
|||||||
loader.load(dir).git.account shouldBe "ci-user"
|
loader.load(dir).git.account shouldBe "ci-user"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
test("finds the machine config left under its old name in an already-moved directory") {
|
||||||
|
val dir = Files.createTempDirectory("werkator-test")
|
||||||
|
Files.createDirectories(dir.resolve(".git/werkator"))
|
||||||
|
dir.resolve(".git/werkator/.gittally.yml").toFile().writeText(
|
||||||
|
"""
|
||||||
|
git:
|
||||||
|
account: ci-user
|
||||||
|
""".trimIndent(),
|
||||||
|
)
|
||||||
|
loader.load(dir).git.account shouldBe "ci-user"
|
||||||
|
}
|
||||||
|
|
||||||
test("the current name wins where both exist, so a half-done rename is not merged") {
|
test("the current name wins where both exist, so a half-done rename is not merged") {
|
||||||
val dir = Files.createTempDirectory("werkator-test")
|
val dir = Files.createTempDirectory("werkator-test")
|
||||||
dir.resolve(".werkator.yml").toFile().writeText("gitea:\n owner: current\n")
|
dir.resolve(".werkator.yml").toFile().writeText("gitea:\n owner: current\n")
|
||||||
|
|||||||
Reference in New Issue
Block a user