4a76e4afc4
Processor time: a Docker build's work runs in containers below the daemon, not below the `docker run` client Werkator started, so sampling that tree measured the log relaying. An eight-minute build recorded cpu=0 and the page would have read "used no processor". BuildRunner now says whether its tree is the build's own, the dispatcher routes that question the same way it routes start(), and an unmeasurable runtime records nothing at all. File names: artifacts went out under Spring's reflected-file-download default, `inline; filename=f.txt`, so a build's jar was saved as f.txt. The response now carries the artifact's own name — inline for what a browser renders, attachment for the rest. The unit: a stop sends SIGTERM and the JVM exits 143, which systemd was journalling as `Failed with result 'exit-code'` — every clean stop, a deployment's swap included, looked like a crash. SuccessExitStatus=143. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: mhoennig <michael@hoennig.de> Reviewed-on: #73
115 lines
6.6 KiB
Markdown
115 lines
6.6 KiB
Markdown
> **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.
|
|
|
|
## Related Links
|
|
|
|
- [PR#72](2026-09-15-PR%2372-a-key-nobody-could-set.md) — the socket fix that made vm4006 build again, and the update that surfaced all three findings here.
|
|
- [PR#67](2026-09-15-PR%2367-what-a-build-actually-cost.md) — processor time per build, which this PR teaches to say "not measurable here".
|
|
|
|
## The Problem
|
|
|
|
vm4006 — the only Docker host, updated from 1.2.0 to the current version — said three things in one evening.
|
|
None of them is related to the others except in how they were found, and each is a figure or a file that lies quietly.
|
|
|
|
**A build of eight minutes reported no processor time.**
|
|
The first green build after the update recorded `cpu=0` beside a duration of 474 seconds.
|
|
`ProcessTreeCpuTime` samples the tree below the process Werkator started; under Docker that process is the `docker run` client, and the build's own processes are children of the daemon.
|
|
The client only relays log streams, so the tree burns a fraction of a second, and the artifact page would have shown `0:00 (0.0x)` — a build that ran for eight minutes and used no processor.
|
|
That is the distinction PR#63 already had to learn once at the quota probe: "could not measure" and "measured nothing" are different answers, and only one of them is true here.
|
|
|
|
**Every downloadable artifact was named `f.txt`.**
|
|
Artifacts are served as `ResponseEntity<Resource>` without a `Content-Disposition`, so Spring writes its own: `inline; filename=f.txt`, its guard against reflected file downloads.
|
|
A browser saving `hsadmin-ng-0.0.1-SNAPSHOT.jar` from the artifact page would therefore save `f.txt` — 85 MB under a name that says nothing, for every non-HTML artifact of every instance.
|
|
|
|
**Every clean stop was journalled as a crash.**
|
|
`systemctl stop` sends SIGTERM and the JVM exits 143.
|
|
The unit `init --systemd` generates declares no `SuccessExitStatus`, so systemd records `Failed with result 'exit-code'` each time — including the stop in the middle of every deployment swap.
|
|
This is how an `instance-update` that worked perfectly printed `failed`.
|
|
|
|
## Non-Goals
|
|
|
|
- No attempt to measure a Docker build's processor time by another route (reading the container's cgroup, asking the daemon).
|
|
Recording nothing is the honest answer; a figure the host cannot produce is not worth inventing a mechanism for until someone needs it.
|
|
- No change to the artifact sandbox (`Content-Security-Policy: sandbox allow-scripts`), which was verified as correct while these were found: the Gradle test report and the JaCoCo report both render and sort fine under it.
|
|
- Existing units are not rewritten by this PR; a host gets the new one when `init --systemd` writes it again.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: three figures that must not lie
|
|
|
|
#### Scenario#73.01: A build whose runtime is not measurable records no processor time
|
|
|
|
So that a missing measurement reads as missing and not as zero.
|
|
|
|
- **Given** a build runtime whose process tree is not the build's own
|
|
- **When** a build runs and burns real processor time outside that tree
|
|
- **Then** the stored result records no processor time at all
|
|
|
|
##### Verified by
|
|
|
|
- [BuildExecutorTest — a build whose runtime does not account processor time records none](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorTest.kt)
|
|
|
|
#### Scenario#73.04: Only Docker says its tree is not the build's
|
|
|
|
So that the answer can never disagree with the runtime that actually runs the build.
|
|
|
|
- **Given** a branch configured for Docker, for the sandbox, or for neither
|
|
- **When** the dispatcher is asked whether the processor time is accounted
|
|
- **Then** only the Docker case says no
|
|
- **and** the question takes the same route through the dispatcher as starting the build does
|
|
|
|
##### Verified by
|
|
|
|
- [DispatchingBuildRunnerTest — only the Docker runtime says the process tree is not the build's](../../src/test/kotlin/de/hoennig/werkator/build/DispatchingBuildRunnerTest.kt)
|
|
|
|
#### Scenario#73.02: An artifact is served under its own name
|
|
|
|
So that a downloaded build output is the file it says it is.
|
|
|
|
- **Given** a stored artifact a browser cannot display, such as a jar
|
|
- **When** it is served
|
|
- **Then** it is offered as a download under its own file name
|
|
- **And given** a report page instead
|
|
- **Then** it stays inline, also under its own name
|
|
|
|
##### Verified by
|
|
|
|
- [ArtifactFileControllerTest — a file a browser cannot show is offered under its own name](../../src/test/kotlin/de/hoennig/werkator/server/ArtifactFileControllerTest.kt)
|
|
- [ArtifactFileControllerTest — a report page stays inline, under its own name](../../src/test/kotlin/de/hoennig/werkator/server/ArtifactFileControllerTest.kt)
|
|
|
|
#### Scenario#73.03: A clean stop is a clean stop
|
|
|
|
So that the journal of a host says what happened.
|
|
|
|
- **Given** the unit `init --systemd` generates
|
|
- **When** it is written
|
|
- **Then** it declares `SuccessExitStatus=143`
|
|
|
|
##### Verified by
|
|
|
|
- [SystemdServiceFilesTest — unit file runs the server jar in the repository with restart and environment file](../../src/test/kotlin/de/hoennig/werkator/commands/SystemdServiceFilesTest.kt)
|
|
|
|
## The Solution
|
|
|
|
**Processor time.** `BuildRunner` gains `accountsProcessorTime(branchConfig)`, default true; `DockerBuildRunner` answers false.
|
|
`DispatchingBuildRunner` routes the question through the same `runnerFor` it now uses to route `start`, so the answer cannot drift from the runtime that runs the build — the drift that PR#72 was about.
|
|
The executor samples nothing when the answer is no, and a build with nothing sampled already records `null` rather than zero.
|
|
|
|
**File names.** The artifact response sets a `Content-Disposition` carrying the file's own name: `inline` for what a browser renders — a test report is meant to be read where it is — and `attachment` for everything else.
|
|
This takes over the job Spring's `f.txt` default was doing: the media type is explicit, `nosniff` is set, and the sandbox header stands, so naming the file adds nothing an attacker can use.
|
|
|
|
**The unit.** One line, `SuccessExitStatus=143`, with the reason beside it; `docs/deployment.md` says what it is for and that an older installation keeps its unit until `init --systemd` runs again.
|
|
|
|
## Additional Changes
|
|
|
|
- None.
|
|
|
|
## Prerequisite PRs
|
|
|
|
- [PR#72](2026-09-15-PR%2372-a-key-nobody-could-set.md) — without it vm4006 had no green build to measure.
|
|
|
|
## Follow-up PRs
|
|
|
|
- mih09 and vm4006 need `init --systemd` (or `instance-start`) once for the unit line to take effect; until then their journals keep calling a clean stop a failure.
|