> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation. ## The Problem Since PR#119 a CI build and a pull-request target build of one repository run at the same time. On mih09, in the repository `werkrevisor` with its PR#5, both started at 15:56:17 and needed the same rootfs, which was not imported yet. Both called `werkdock import … werkator-buildenv-f426341f4a1b`: - The CI build failed with exit code 125: `open …/images/werkator-buildenv-f426341f4a1b/latest.tmp/image.json: no such file or directory`. - The pull-request target build ran on a half-imported image: `bwrap: execvp /bin/sh: Permission denied`. werkdock stages an import in `<image>.tmp` and renames it into place, so an interrupted import leaves no image. But the staging directory has a fixed name, and an import removes it first: the second import deleted the first one's files under it. Two repositories naming the same new rootfs could hit this before PR#119 too, with `maxConcurrent` above 1. The pull-request target build `ai-review` then reported only "build failed after 00:37" to Gitea. The reviewer saw a red gate without a comment, because werkrevisor never started; nothing said that a restart would help. ## Non-Goals - No change in werkdock here; see [Follow-up PRs](#follow-up-prs). - No new `BuildStatus`: the result stays `failed`, only the status text and the log say what failed. A new status would have to be known by the UI, the API, retention and the restart rules, for a difference that only the reader of the status needs. - No lock between two Werkator instances of the same OS user. ## The Scenarios ### Feature: one import per image #### Scenario#121.01: two builds that need the same new image import it once So that neither build fails in the import, nor runs on a half-imported image. - **Given** two builds, of the same or of two repositories of the instance, with the same werkdock rootfs - **and** werkdock does not have the image yet - **When** both start at the same moment - **Then** one of them imports the image - **and** the other waits for that import, finds the image and imports nothing - **and** both start their command ##### Verified by - [WerkdockBuildRunnerUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/WerkdockBuildRunnerUnitTest.kt) `two builds that need the same new image import it once, also from two repositories (Scenario#121.01)` The test fails without the lock: both builds import. #### Scenario#121.02: a build that waited for a failed import imports the image itself So that one failed import costs one build, not every build that waited for it. - **Given** two builds wait for the same new image - **When** the first import fails - **Then** that build fails - **and** the other build imports the image itself and starts its command ##### Verified by - [WerkdockBuildRunnerUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/WerkdockBuildRunnerUnitTest.kt) `a build that waited for a failed import imports the image itself (Scenario#121.02)` ### Feature: a failed environment is told apart from a failed build #### Scenario#121.03: a build that fails before its commands give a result reports a failed environment So that a reader of the status knows there is no verdict of the build, and that a restart may help. - **Given** a build - **When** its runtime fails before the command has a process, for example in the image import - **or** `docker run` or `werkdock run` ends with exit code 125, their code for an error of their own - **Then** the build ends `failed` - **and** its Gitea status says `build environment failed after mm:ss` - **and** its log says that the build's commands did not run, for a failure before the command - **When** the command itself ends with any other exit code - **Then** the status says `build failed after mm:ss` as before ##### Verified by - [BuildExecutorUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorUnitTest.kt) `a build whose runtime fails before its command runs reports a failed environment, not a failed build (Scenario#121.03)` - [GiteaStatusTextUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/GiteaStatusTextUnitTest.kt) `a build whose environment failed says so, since it has no verdict of its own (Scenario#121.03)` - [DispatchingBuildRunnerUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/DispatchingBuildRunnerUnitTest.kt) `the runtime that runs the build decides whether its exit code is its own failure (Scenario#121.03)` - [WerkdockBuildRunnerUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/WerkdockBuildRunnerUnitTest.kt) `only werkdock's own exit code 125 is a failure of the runtime (Scenario#121.03)` ## The Solution `WerkdockBuildRunner.ensureImage` asks `werkdock images` first, without a lock, as before. If the image is missing, it takes a lock per image name, asks again, and only then downloads and imports. The locks live in the runner's companion object: one per image for the whole JVM, because the image store belongs to the OS user and not to a repository. `BuildRunner.isRuntimeFailure(exitCode, branchConfig)` says whether an exit code is the runtime's own failure. The native runner says never; the Docker and the werkdock runner say so for 125, their documented code for an error of their own; the dispatcher asks the runner that runs the build. The executor marks a build `environmentFailed`: - when an exception ends the run before a command of the build had a process (`commandStarted`), or - when a command's exit code is the runtime's own failure. `GiteaStatusText.of` words a `failed` build with that mark as `build environment failed`. `bwrap: execvp /bin/sh: Permission denied` itself exits with bwrap's code 1, which werkdock passes through like any command's code. Werkator cannot tell it from a failed command; with the lock, the half-imported image that caused it no longer occurs. A local PIT run of the changed classes: | Method | Mutation-CRAP | Complexity | Mutation score | |----------------------------------------|---------------|------------|----------------| | `GiteaStatusText.of` | 10 | 10 | 1.00 | | `BuildExecutor.crashed` | 4 | 4 | 1.00 | | `WerkdockBuildRunner.ensureImage` | 3.14 | 3 | 0.75 | | `WerkdockBuildRunner.isRuntimeFailure` | 2 | 2 | 1.00 | | `DockerBuildRunner.isRuntimeFailure` | 2 | 2 | 1.00 | | `BuildExecutor.runCommand` | 1.42 | 1 | 0.25 | No method of these classes is above 15; the gate stays at 31. ## Additional Changes - `docs/configuration.md`: one sentence on the import in the werkdock notes, one on the status text beside the status context. - Version 1.37.2 with its entry in `releases.html`. ## Follow-up PRs For `mi/werkdock`, not started: - `Store.Import` stages in a directory of its own (`<image>.<random>.tmp`), so concurrent imports from two Werkator instances or a shell cannot remove each other's files; the rename stays the atomic step, and an import that finds the image already in place reports it. - `werkdock run` answers 126 or 127 like docker when bwrap cannot execute the command, so the caller can tell a sandbox that cannot start from a command that failed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: mhoennig <michael@hoennig.de> Reviewed-on: #121
132 lines
7.6 KiB
Markdown
132 lines
7.6 KiB
Markdown
# PR#121: One Import per Werkdock Image, and a Status Text for a Failed Build Environment
|
|
|
|
> **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
|
|
|
|
Since PR#119 a CI build and a pull-request target build of one repository run at the same time.
|
|
On mih09, in the repository `werkrevisor` with its PR#5, both started at 15:56:17 and needed the same rootfs, which was not imported yet.
|
|
Both called `werkdock import … werkator-buildenv-f426341f4a1b`:
|
|
|
|
- The CI build failed with exit code 125: `open …/images/werkator-buildenv-f426341f4a1b/latest.tmp/image.json: no such file or directory`.
|
|
- The pull-request target build ran on a half-imported image: `bwrap: execvp /bin/sh: Permission denied`.
|
|
|
|
werkdock stages an import in `<image>.tmp` and renames it into place, so an interrupted import leaves no image.
|
|
But the staging directory has a fixed name, and an import removes it first: the second import deleted the first one's files under it.
|
|
Two repositories naming the same new rootfs could hit this before PR#119 too, with `maxConcurrent` above 1.
|
|
|
|
The pull-request target build `ai-review` then reported only "build failed after 00:37" to Gitea.
|
|
The reviewer saw a red gate without a comment, because werkrevisor never started; nothing said that a restart would help.
|
|
|
|
## Non-Goals
|
|
|
|
- No change in werkdock here; see [Follow-up PRs](#follow-up-prs).
|
|
- No new `BuildStatus`: the result stays `failed`, only the status text and the log say what failed.
|
|
A new status would have to be known by the UI, the API, retention and the restart rules, for a difference that only the reader of the status needs.
|
|
- No lock between two Werkator instances of the same OS user.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: one import per image
|
|
|
|
#### Scenario#121.01: two builds that need the same new image import it once
|
|
|
|
So that neither build fails in the import, nor runs on a half-imported image.
|
|
|
|
- **Given** two builds, of the same or of two repositories of the instance, with the same werkdock rootfs
|
|
- **and** werkdock does not have the image yet
|
|
- **When** both start at the same moment
|
|
- **Then** one of them imports the image
|
|
- **and** the other waits for that import, finds the image and imports nothing
|
|
- **and** both start their command
|
|
|
|
##### Verified by
|
|
|
|
- [WerkdockBuildRunnerUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/WerkdockBuildRunnerUnitTest.kt) `two builds that need the same new image import it once, also from two repositories (Scenario#121.01)`
|
|
|
|
The test fails without the lock: both builds import.
|
|
|
|
#### Scenario#121.02: a build that waited for a failed import imports the image itself
|
|
|
|
So that one failed import costs one build, not every build that waited for it.
|
|
|
|
- **Given** two builds wait for the same new image
|
|
- **When** the first import fails
|
|
- **Then** that build fails
|
|
- **and** the other build imports the image itself and starts its command
|
|
|
|
##### Verified by
|
|
|
|
- [WerkdockBuildRunnerUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/WerkdockBuildRunnerUnitTest.kt) `a build that waited for a failed import imports the image itself (Scenario#121.02)`
|
|
|
|
### Feature: a failed environment is told apart from a failed build
|
|
|
|
#### Scenario#121.03: a build that fails before its commands give a result reports a failed environment
|
|
|
|
So that a reader of the status knows there is no verdict of the build, and that a restart may help.
|
|
|
|
- **Given** a build
|
|
- **When** its runtime fails before the command has a process, for example in the image import
|
|
- **or** `docker run` or `werkdock run` ends with exit code 125, their code for an error of their own
|
|
- **Then** the build ends `failed`
|
|
- **and** its Gitea status says `build environment failed after mm:ss`
|
|
- **and** its log says that the build's commands did not run, for a failure before the command
|
|
- **When** the command itself ends with any other exit code
|
|
- **Then** the status says `build failed after mm:ss` as before
|
|
|
|
##### Verified by
|
|
|
|
- [BuildExecutorUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorUnitTest.kt) `a build whose runtime fails before its command runs reports a failed environment, not a failed build (Scenario#121.03)`
|
|
- [GiteaStatusTextUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/GiteaStatusTextUnitTest.kt) `a build whose environment failed says so, since it has no verdict of its own (Scenario#121.03)`
|
|
- [DispatchingBuildRunnerUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/DispatchingBuildRunnerUnitTest.kt) `the runtime that runs the build decides whether its exit code is its own failure (Scenario#121.03)`
|
|
- [WerkdockBuildRunnerUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/WerkdockBuildRunnerUnitTest.kt) `only werkdock's own exit code 125 is a failure of the runtime (Scenario#121.03)`
|
|
|
|
## The Solution
|
|
|
|
`WerkdockBuildRunner.ensureImage` asks `werkdock images` first, without a lock, as before.
|
|
If the image is missing, it takes a lock per image name, asks again, and only then downloads and imports.
|
|
The locks live in the runner's companion object: one per image for the whole JVM, because the image store belongs to the OS user and not to a repository.
|
|
|
|
`BuildRunner.isRuntimeFailure(exitCode, branchConfig)` says whether an exit code is the runtime's own failure.
|
|
The native runner says never; the Docker and the werkdock runner say so for 125, their documented code for an error of their own; the dispatcher asks the runner that runs the build.
|
|
|
|
The executor marks a build `environmentFailed`:
|
|
|
|
- when an exception ends the run before a command of the build had a process (`commandStarted`), or
|
|
- when a command's exit code is the runtime's own failure.
|
|
|
|
`GiteaStatusText.of` words a `failed` build with that mark as `build environment failed`.
|
|
|
|
`bwrap: execvp /bin/sh: Permission denied` itself exits with bwrap's code 1, which werkdock passes through like any command's code.
|
|
Werkator cannot tell it from a failed command; with the lock, the half-imported image that caused it no longer occurs.
|
|
|
|
A local PIT run of the changed classes:
|
|
|
|
| Method | Mutation-CRAP | Complexity | Mutation score |
|
|
|----------------------------------------|---------------|------------|----------------|
|
|
| `GiteaStatusText.of` | 10 | 10 | 1.00 |
|
|
| `BuildExecutor.crashed` | 4 | 4 | 1.00 |
|
|
| `WerkdockBuildRunner.ensureImage` | 3.14 | 3 | 0.75 |
|
|
| `WerkdockBuildRunner.isRuntimeFailure` | 2 | 2 | 1.00 |
|
|
| `DockerBuildRunner.isRuntimeFailure` | 2 | 2 | 1.00 |
|
|
| `BuildExecutor.runCommand` | 1.42 | 1 | 0.25 |
|
|
|
|
No method of these classes is above 15; the gate stays at 31.
|
|
|
|
## Additional Changes
|
|
|
|
- `docs/configuration.md`: one sentence on the import in the werkdock notes, one on the status text beside the status context.
|
|
- Version 1.37.2 with its entry in `releases.html`.
|
|
|
|
## Follow-up PRs
|
|
|
|
For `mi/werkdock`, not started:
|
|
|
|
- `Store.Import` stages in a directory of its own (`<image>.<random>.tmp`), so concurrent imports from two Werkator instances or a shell cannot remove each other's files; the rename stays the atomic step, and an import that finds the image already in place reports it.
|
|
- `werkdock run` answers 126 or 127 like docker when bwrap cannot execute the command, so the caller can tell a sandbox that cannot start from a command that failed.
|
|
|
|
🤖 Generated with [Claude Code](https://claude.com/claude-code)
|