Compare commits
9
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
fb1db30f2a | ||
|
|
2cddd76c88 | ||
|
|
7fd473b88b | ||
|
|
092183ca30 | ||
|
|
0761a274db | ||
|
|
5c73c4cc21 | ||
|
|
16543f038b | ||
|
|
3ccc901d1b | ||
|
|
9221550a2a |
@@ -0,0 +1,42 @@
|
||||
> **WARNING:** This document describes only the change applied in this PR.
|
||||
> It may already be outdated once the next PR is merged.
|
||||
> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation.
|
||||
|
||||
## The Problem
|
||||
|
||||
The build for commit `7028ca8` on `main` failed on the mih09 production instance with
|
||||
`BuildExecutorTest > with maxConcurrent 1 a second branch stays PENDING until the first finished`,
|
||||
while a retry of the very same commit passed, and the test passes locally.
|
||||
|
||||
The test was timing-dependent.
|
||||
It started a build for `branch-a` whose build command was `sleep 1`, immediately started a second build for `branch-b`,
|
||||
and then asserted — without any synchronization at all — that `branch-b` was still `PENDING`.
|
||||
|
||||
That assertion only held as long as the test thread reached it within the one second `branch-a` slept.
|
||||
On a shared host under CPU contention the executor can get through `branch-a` entirely (queued, running, slept, succeeded) first,
|
||||
and `branch-b` is then already `RUNNING` or `SUCCESS` when the assertion runs.
|
||||
The failure therefore says nothing about the executor; it is pure scheduling noise that costs a build and a retry every time it hits.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- No change to production code — `BuildExecutor` is not touched, its queueing behaviour is unchanged.
|
||||
- No sweep of the other timing-sensitive tests in the suite; only the one that actually flaked is fixed.
|
||||
|
||||
## The Solution
|
||||
|
||||
`branch-a` no longer sleeps for a fixed time, it blocks until the test says so:
|
||||
its build command is `until [ -f gate ]; do sleep 0.05; done`, and the build workspace is the test's working directory.
|
||||
|
||||
The test now
|
||||
1. waits (via `eventually`) until `branch-a` is `RUNNING`, so the single executor slot is provably occupied,
|
||||
2. asserts that `branch-b` is `PENDING` — which cannot race anything, because `branch-a` cannot finish before the gate file exists,
|
||||
3. creates the gate file, and only then awaits both builds' `SUCCESS`.
|
||||
|
||||
The assertion on the event transitions (`branch-b` goes `RUNNING` only after `branch-a` reached `SUCCESS`) is unchanged.
|
||||
A blocking gate was chosen over a mocked `BuildRunner` because it keeps the test on the real `ProcessBuildRunner`,
|
||||
so it still covers the actual process handling rather than only the executor's bookkeeping.
|
||||
|
||||
Verified by running `BuildExecutorTest` five times on an idle machine and three more times with twice `nproc` busy-loops saturating the CPU,
|
||||
which is the condition that produced the original failure.
|
||||
|
||||
- [BuildExecutorTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorTest.kt)
|
||||
@@ -0,0 +1,30 @@
|
||||
> **WARNING:** This document describes only the change applied in this PR.
|
||||
> It may already be outdated once the next PR is merged.
|
||||
> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation.
|
||||
|
||||
## The Problem
|
||||
|
||||
The repository's home is `https://git.javagil.de/mi/werkator.git` since the move to the own Gitea instance,
|
||||
but `tools/remote` still defaulted `WERKATOR_REPO_URL` to the GitHub mirror.
|
||||
|
||||
The mih09 production instance was cloned from that mirror and consequently watched a `main` that nobody pushes to any more:
|
||||
it kept reporting the last GitHub state as green while three merged pull requests sat unbuilt on the real `main`.
|
||||
The failure that started this — a flaky test already fixed on Gitea's `main` — could not be re-verified live,
|
||||
because the instance had no way to see the fix.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- The existing clone on mih09 is not touched by this change; its remote was repointed by hand (`git remote set-url`), and `repo-init` skips an existing clone.
|
||||
- The generic placeholder URLs in `docs/deployment.md` stay as they are — they describe cloning *any* watched repository, not werkator's own.
|
||||
- No decision about mirroring to GitHub; the mirror simply stops being the source an instance builds from.
|
||||
|
||||
## The Solution
|
||||
|
||||
`REPO_URL` in [tools/remote](../../tools/remote) defaults to the Gitea URL, and the usage comment says so.
|
||||
|
||||
The value stays overridable via `WERKATOR_REPO_URL` in the instance's env file,
|
||||
so an installation that deliberately watches a different remote is unaffected.
|
||||
Anonymous HTTPS works against Gitea exactly as it did against GitHub, so no deploy key or token is involved.
|
||||
|
||||
Note that pull requests opened via AGit-Flow create no branch in Gitea,
|
||||
so an instance watching this remote sees `main` only — branch builds require pushing real branches.
|
||||
@@ -0,0 +1,90 @@
|
||||
> **WARNING:** This document describes only the change applied in this PR.
|
||||
> It may already be outdated once the next PR is merged.
|
||||
> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation.
|
||||
|
||||
## The Problem
|
||||
|
||||
`init --systemd` generates the host integration — the systemd unit's resource limits, the Apache `.htaccess` and the maintenance page — from the effective configuration.
|
||||
It read that configuration through a helper with two independent defects, both of which fail silently.
|
||||
|
||||
**It swallowed every error.**
|
||||
The load sat in `try { … } catch (_: Exception) { ServerConfig() }`.
|
||||
Any configuration error at all — a missing required field, a malformed layer, a version floor violation — produced a default `ServerConfig` with a blank `publicBaseUrl`.
|
||||
A repository with a broken `.werkator.yml` then looked exactly like one that simply has no public base URL configured:
|
||||
the `.htaccess` and the maintenance page were skipped without a word.
|
||||
This surfaced while verifying [PR#17](2026-09-03-PR%2317-maintenance-page.md) on mih09, where the missing files looked like an unconfigured `publicBaseUrl` and were in fact an unrelated validation error.
|
||||
|
||||
**It read from the wrong directory.**
|
||||
The helper called `configLoader.load(Paths.get("."))` — the process's current directory — while everything else in the command works off the git top level resolved by `GitService.getTopLevel`.
|
||||
`ConfigLoader.loadRaw` resolves the layers directly under the directory it is given and does not walk up to the repository root, so the two agree only when `init` happens to be invoked from the root itself.
|
||||
From a subdirectory the command read another repository's configuration, or none.
|
||||
That also broke `--apply`: the fragment is installed into the repository root deliberately before the systemd files are written, so that its port and limits reach the generated unit, and a current-directory read does not see it.
|
||||
|
||||
`init --systemd` runs during initial deployment setup, which is exactly when a silent wrong answer is most expensive.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- The fallback itself is kept: a configuration that cannot be loaded is not fatal for `init`, the units are still generated with the defaults.
|
||||
During the very first bootstrap there is legitimately nothing to load yet.
|
||||
- No change to `ConfigLoader`, to the configuration schema, or to any other command.
|
||||
- No sweep for catch-all exception handlers elsewhere in the code base;
|
||||
the two other `catch` blocks in `InitCommand` already print an `Error:` and abort, so they were only checked, not changed.
|
||||
|
||||
## The Scenarios
|
||||
|
||||
### Feature: init reports what it read and where it read it from
|
||||
|
||||
#### Background
|
||||
|
||||
- The *repository root* is the git top level as resolved by `GitService.getTopLevel`, the directory holding `.werkator.yml`, `.git/werkator/.werkator.yml` and an applied fragment.
|
||||
- The *current directory* is the process working directory, which is the repository root only when `init` is invoked there.
|
||||
|
||||
#### Scenario#22.01: A broken configuration is named, not defaulted over
|
||||
|
||||
So that a validation error during deployment setup is not mistaken for an unconfigured installation.
|
||||
|
||||
- **Given** a repository whose effective configuration cannot be loaded
|
||||
- **When** `init --systemd` runs
|
||||
- **Then** the exception message is printed as a warning
|
||||
- **and** the unit files are still generated with the default settings
|
||||
- **and** the warning appears exactly once, although three settings are read from the configuration
|
||||
|
||||
##### Verified by
|
||||
|
||||
- [InitCommandTest: `--systemd warns once when the effective configuration cannot be loaded`](../../src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt)
|
||||
|
||||
#### Scenario#22.02: The configuration is read from the repository root
|
||||
|
||||
So that the generated host integration reflects the repository being initialized, whatever directory `init` was invoked from.
|
||||
|
||||
- **Given** a repository whose root configuration sets `server.publicBaseUrl` and `server.port`
|
||||
- **and** a current directory that is not that repository root
|
||||
- **When** `init --systemd` runs
|
||||
- **Then** the `.htaccess` and the maintenance page are generated
|
||||
- **and** the `.htaccess` proxies to the port from the root configuration
|
||||
|
||||
##### Verified by
|
||||
|
||||
- [InitCommandTest: `--systemd reads the configuration from the repository root, not the current directory`](../../src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt)
|
||||
|
||||
## The Solution
|
||||
|
||||
The catch-all now prints the exception message before falling back:
|
||||
|
||||
```
|
||||
Warning: the effective configuration could not be loaded (<message>)
|
||||
continuing with default server settings — check the generated unit and host files
|
||||
```
|
||||
|
||||
The configuration is read three times while the systemd files are written (`memoryMax`, `tasksMax`, `publicBaseUrl`), which would repeat the warning three times.
|
||||
It is therefore loaded once per run and cached in the command, and the cache is reset at the top of `run()` so a reused instance — the command is a Spring singleton — re-reads.
|
||||
|
||||
The repository root is passed down into the two accessors instead of `Paths.get(".")`.
|
||||
This matches every other caller of `ConfigLoader.load` in the code base, all of which pass an explicit working directory;
|
||||
`InitCommand` was the only one relying on the process's current directory.
|
||||
|
||||
Both fixes are the same failure in two forms — the command answered from a configuration it never actually read — which is why they are in one PR.
|
||||
|
||||
## Additional Changes
|
||||
|
||||
- None.
|
||||
@@ -48,6 +48,7 @@ class InitCommand(
|
||||
internal var javaExecutableResolver: () -> Path = { Paths.get(System.getProperty("java.home"), "bin", "java") }
|
||||
|
||||
override fun run() {
|
||||
cachedServerConfig = null
|
||||
val normalizedWorkingDir = workingDir.toAbsolutePath().normalize()
|
||||
val root =
|
||||
try {
|
||||
@@ -296,14 +297,31 @@ class InitCommand(
|
||||
* already loadable (re-running `init --systemd` on an installed instance); during
|
||||
* the very first bootstrap they stay unset and the defaults (no directives) apply.
|
||||
*/
|
||||
private fun loadedSystemdConfig(): de.hoennig.werkator.config.SystemdConfig = loadedServerConfig().systemd
|
||||
private fun loadedSystemdConfig(root: Path): de.hoennig.werkator.config.SystemdConfig = loadedServerConfig(root).systemd
|
||||
|
||||
private fun loadedServerConfig(): de.hoennig.werkator.config.ServerConfig =
|
||||
/** Loaded once per run, so a broken configuration is reported once and not per caller. */
|
||||
private var cachedServerConfig: de.hoennig.werkator.config.ServerConfig? = null
|
||||
|
||||
/**
|
||||
* Read from the repository root like every other file this command touches — the
|
||||
* layers sit there, not in whatever directory the process happens to run in, and
|
||||
* an applied fragment must reach the generated unit even when `init` is invoked
|
||||
* from a subdirectory.
|
||||
*
|
||||
* A configuration error here is not fatal — the units are still generated with defaults —
|
||||
* but it must not pass for "nothing configured": without the warning a broken `.werkator.yml`
|
||||
* looks exactly like an unset `publicBaseUrl` and the host integration is skipped silently.
|
||||
*/
|
||||
private fun loadedServerConfig(root: Path): de.hoennig.werkator.config.ServerConfig =
|
||||
cachedServerConfig ?: run {
|
||||
try {
|
||||
configLoader.load(Paths.get(".")).server
|
||||
} catch (_: Exception) {
|
||||
configLoader.load(root).server
|
||||
} catch (e: Exception) {
|
||||
println("Warning: the effective configuration could not be loaded (${e.message})")
|
||||
println(" continuing with default server settings — check the generated unit and host files")
|
||||
de.hoennig.werkator.config
|
||||
.ServerConfig()
|
||||
}.also { cachedServerConfig = it }
|
||||
}
|
||||
|
||||
private fun createSystemdFiles(
|
||||
@@ -327,8 +345,8 @@ class InitCommand(
|
||||
javaExecutable = javaExecutableResolver(),
|
||||
jarPath = jarPath,
|
||||
envFile = envFile,
|
||||
memoryMax = loadedSystemdConfig().memoryMax,
|
||||
tasksMax = loadedSystemdConfig().tasksMax,
|
||||
memoryMax = loadedSystemdConfig(root).memoryMax,
|
||||
tasksMax = loadedSystemdConfig(root).tasksMax,
|
||||
),
|
||||
)
|
||||
println("created ${unitFile.toFile().relativeTo(normalizedWorkingDir.toFile())}")
|
||||
@@ -351,7 +369,7 @@ class InitCommand(
|
||||
|
||||
// generated host integration like the units: only meaningful behind a web
|
||||
// frontend, so it needs a public base URL; unused elsewhere and harmless
|
||||
val server = loadedServerConfig()
|
||||
val server = loadedServerConfig(root)
|
||||
if (server.publicBaseUrl.isNotBlank()) {
|
||||
val htaccessFile = werkatorDir.resolve(SystemdServiceFiles.HTACCESS_NAME)
|
||||
htaccessFile.toFile().writeText(SystemdServiceFiles.htaccessContent(server.port))
|
||||
|
||||
@@ -505,6 +505,8 @@ class BuildExecutorTest : FunSpec() {
|
||||
}
|
||||
|
||||
test("with maxConcurrent 1 a second branch stays PENDING until the first finished") {
|
||||
// branch-a blocks on a gate file the test creates, so the PENDING assertion
|
||||
// below cannot race the first build finishing on a loaded machine
|
||||
val h =
|
||||
Harness(
|
||||
"""
|
||||
@@ -512,7 +514,7 @@ class BuildExecutorTest : FunSpec() {
|
||||
maxConcurrent: 1
|
||||
branches:
|
||||
branch-a:
|
||||
buildCommand: "sleep 1"
|
||||
buildCommand: "until [ -f gate ]; do sleep 0.05; done"
|
||||
cleanCommand: ""
|
||||
branch-b:
|
||||
buildCommand: "echo ok"
|
||||
@@ -523,8 +525,13 @@ class BuildExecutorTest : FunSpec() {
|
||||
h.executor.startBuild(h.repo, "branch-a", "sha-a")
|
||||
h.executor.startBuild(h.repo, "branch-b", "sha-b")
|
||||
|
||||
eventually(30.seconds) {
|
||||
h.repository.latestFor("branch-a")?.status shouldBe BuildStatus.RUNNING
|
||||
}
|
||||
h.repository.latestFor("branch-b")?.status shouldBe BuildStatus.PENDING
|
||||
|
||||
Files.createFile(h.workingDir.resolve("gate"))
|
||||
|
||||
awaitStatus(h, "branch-b", BuildStatus.SUCCESS)
|
||||
awaitStatus(h, "branch-a", BuildStatus.SUCCESS)
|
||||
val transitions = h.events.map { it.result.branch to it.result.status }
|
||||
|
||||
@@ -252,5 +252,50 @@ class InitCommandTest : FunSpec() {
|
||||
// This should not throw IllegalArgumentException
|
||||
initCommand.run()
|
||||
}
|
||||
|
||||
test("--systemd reads the configuration from the repository root, not the current directory") {
|
||||
val tempDir = Files.createTempDirectory("werkator-init-test")
|
||||
// written before the run, so `init` keeps it instead of creating a template
|
||||
tempDir.resolve(".werkator.yml").toFile().writeText(
|
||||
"server:\n publicBaseUrl: \"https://werkator.example.org/\"\n port: 18099\n",
|
||||
)
|
||||
initCommand.workingDir = tempDir
|
||||
initCommand.systemd = true
|
||||
initCommand.jarPathResolver = { Paths.get("/home/ci/bin/werkator.jar") }
|
||||
initCommand.javaExecutableResolver = { Paths.get("/usr/bin/java") }
|
||||
|
||||
every { gitService.getTopLevel(tempDir) } returns tempDir
|
||||
every { gitService.getOriginUrl(tempDir) } returns "https://git.example.org/my-org/my-repo.git"
|
||||
|
||||
initCommand.run()
|
||||
|
||||
// the host integration is generated only when the root's config has a public base URL
|
||||
val htaccess = tempDir.resolve(".git/werkator/${SystemdServiceFiles.HTACCESS_NAME}")
|
||||
htaccess.toFile().shouldExist()
|
||||
htaccess.toFile().readText() shouldContain "18099"
|
||||
tempDir.resolve(".git/werkator/${SystemdServiceFiles.MAINTENANCE_PAGE_NAME}").toFile().shouldExist()
|
||||
}
|
||||
|
||||
test("--systemd warns once when the effective configuration cannot be loaded") {
|
||||
val tempDir = Files.createTempDirectory("werkator-init-test")
|
||||
val brokenLoader = mockk<de.hoennig.werkator.config.ConfigLoader>()
|
||||
every { brokenLoader.load(any()) } throws IllegalStateException("gitea.owner is required")
|
||||
val command = InitCommand(gitService, brokenLoader)
|
||||
command.workingDir = tempDir
|
||||
command.systemd = true
|
||||
command.jarPathResolver = { Paths.get("/home/ci/bin/werkator.jar") }
|
||||
command.javaExecutableResolver = { Paths.get("/usr/bin/java") }
|
||||
|
||||
every { gitService.getTopLevel(tempDir) } returns tempDir
|
||||
every { gitService.getOriginUrl(tempDir) } returns "https://git.example.org/my-org/my-repo.git"
|
||||
|
||||
val console = captureConsole { command.run() }
|
||||
|
||||
console.stdout shouldContain "gitea.owner is required"
|
||||
// the three readers of the configuration must not repeat the warning
|
||||
console.stdout.windowed("Warning:".length).count { it == "Warning:" } shouldBe 1
|
||||
// the units are still written with the defaults
|
||||
tempDir.resolve(".git/werkator/${SystemdServiceFiles.unitName(tempDir)}").toFile().shouldExist()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+2
-2
@@ -44,7 +44,7 @@
|
||||
# Optional in the env file:
|
||||
# WERKATOR_INIT_CONFIG the init fragment to apply (repo-init, instance-start)
|
||||
# WERKATOR_REPO_URL https clone URL of the watched repository
|
||||
# (default: https://github.com/mhoennig/werkator.git)
|
||||
# (default: https://git.javagil.de/mi/werkator.git)
|
||||
# WERKATOR_REPO_DIR directory of the watched repository, absolute or relative to
|
||||
# WERKATOR_PATH (default: werkator); it also names the systemd
|
||||
# unit, exactly as `init --systemd` derives it
|
||||
@@ -122,7 +122,7 @@ require_env WERKATOR_REMOTE WERKATOR_PATH
|
||||
HOST="$WERKATOR_REMOTE"
|
||||
TARGET_DIR="$WERKATOR_PATH"
|
||||
ROOTFS="${WERKATOR_ROOTFS:-$REPO_ROOT/build/werkator-buildenv-trixie-java-go-node.tar.zst}"
|
||||
REPO_URL="${WERKATOR_REPO_URL:-https://github.com/mhoennig/werkator.git}"
|
||||
REPO_URL="${WERKATOR_REPO_URL:-https://git.javagil.de/mi/werkator.git}"
|
||||
|
||||
# The host layout is three values, not one convention: an installation that grew
|
||||
# before this script existed puts them elsewhere, and the defaults are exactly what
|
||||
|
||||
Reference in New Issue
Block a user