Runner: report infrastructure failure distinctly from job failure (#375) (#56)
Companion to fangorn/anvil #238 for #375 (REQ-CI-114, REQ-CI-118, REQ-CI-119).
## Why
On 2026-07-30 `carl` went down for a reboot. For ~90 seconds before the host died its Docker daemon was already gone while the runner process kept polling, and it claimed four jobs in thirteen seconds, destroying each in about one second:
```
Warning: docker pull failed: error during connect: Post "http://%2Fvar%2Frun%2Fdocker.sock/v1.45/images/create?...": EOF
docker: error during connect: Head "http://%2Fvar%2Frun%2Fdocker.sock/_ping": EOF.
Prepare failed: Prepare commands failed with exit code 125
```
Every one of those was reported to the server as a plain exit-1 job failure, indistinguishable from "your test suite is broken", because `report_result` collapsed its whole `Err(e)` arm to `("failed", Some(1), None)`. The server had no way to know it should requeue them on a healthy peer.
## What changed
`src/runner/loop_runner.rs`:
- `job_needs_container/1` — a step with no image never talks to the daemon, so a dead daemon can never explain its failure. It asks exactly the question `executor::execute` asks: `image.is_some()`.
- `docker_daemon_reachable/0` — a direct probe (`docker version --format {{.Server.Version}}`). This checks the actual condition rather than pattern-matching error strings, and a missing `docker` binary counts as unreachable: a runner asked to run a container image and unable to is broken either way. Kept separate from the decision so the decision is testable without a daemon.
- `failure_signal/1` — the failure to classify, from *either* arm of `execute_job` (see below).
- `classify_execution_error/3` — pure. Container job + unreachable daemon means infrastructure; anything else stays an ordinary failure.
- `update_infra_failure/4` — `PATCH /runners/jobs/:id/status` with `status: "infra_failed"` and an `infra_failure_reason`. Deliberately not routed through `update_job_status`: there is no exit code to send, because nothing ran.
`src/commands/ci.rs`: `ci job-view` renders `Infra Failure` and, when it is above 1, `Attempts` — so an operator sees *why* instead of a bare `Exit Code -1` they have to correlate against runner logs by hand.
`src/commands/runner.rs`: `runner list` grows a QUARANTINED column and `runner view` a Quarantined line, from the `quarantined_until` the server now returns (REQ-CI-118).
The prepare and executor paths are untouched; this is only the reporting boundary.
## Review fixes (second commit)
**Classification now runs on `Ok(exit_code)`, not only on `Err`.** This was the substantive defect: the first version classified only the `Err` arm, and almost nothing reaches it.
- `executor::run_and_stream` returns `Ok(ExecResult { exit_code: 125 })` when `docker run` cannot reach the daemon — literally the first line of the outage log above.
- `execute_docker` downgrades a failed `docker pull` to a logged warning.
- The only path that yields `Err` is `prepare::prepare_image`, and prepare is attached server-side **only when the pipeline declares a top-level `prepare:` block** (`lib/anvil/ci.ex:1033`).
So for any repo without a `prepare:` block the outage reproduced unchanged: `exit_code: 125`, `failed`, no requeue, no quarantine. `failure_signal/1` now supplies the signal from either arm. A timeout or a runner-initiated cancel DID run, so both stay terminal and are never candidates for requeueing.
**`Some("")` was a genuine wire mismatch.** `job_needs_container` treated an empty image string as bare while `executor::execute` routes any `Some(_)` to `execute_docker` — a job that could only ever have failed on the daemon was never blamed on it. Both now ask `image.is_some()`.
The `"bare"` string is no longer special-cased. `image: bare` is resolved to no image server-side (REQ-CI-102), so a runner never sees the literal. **This is a deliberate change to what two existing tests assert** — `a_bare_job_does_not` and `a_bare_job_is_never_blamed_on_docker` were both exercising a payload the server does not send; they now use the payload it does (no `image` key at all).
**A 409 is not "this server predates #375".** anvil#238 now fences a stale infrastructure report with 409, and falling back to a plain failure report there would be the same stale write in a different envelope. Only a 422 means "unknown status". `InfraReport { Accepted, Fenced, Unsupported }` makes the three cases explicit, and `infra_report_outcome/1` is a pure mapping so the fallback rule is testable without a server.
## Tests
The five original tests, against pre-fix code, did not compile — `classify_execution_error` and `job_needs_container` did not exist.
The eight added in the review pass fail on assertions, not compilation. `failure_signal` was first introduced with the old semantics (`Err` only) so the gap shows as behaviour:
```
running 12 tests
test ...::an_empty_image_is_still_a_container_job ... FAILED
test ...::a_dead_daemon_behind_exit_125_is_infrastructure ... FAILED
test ...::a_nonzero_exit_is_a_failure_signal ... FAILED
---- an_empty_image_is_still_a_container_job stdout ----
assertion failed: job_needs_container(&json!({"image": ""}))
---- a_dead_daemon_behind_exit_125_is_infrastructure stdout ----
panicked at src/runner/loop_runner.rs:814:53: exit 125 is a failure
test result: FAILED. 9 passed; 3 failed
```
After:
```
running 13 tests
test ...::a_bare_job_does_not ... ok
test ...::a_bare_job_is_never_blamed_on_docker ... ok
test ...::a_containerized_job_needs_the_daemon ... ok
test ...::a_dead_daemon_behind_exit_125_is_infrastructure ... ok
test ...::a_dead_daemon_under_a_container_job_is_infrastructure ... ok
test ...::a_live_daemon_means_the_job_itself_failed ... ok
test ...::a_nonzero_exit_is_a_failure_signal ... ok
test ...::a_passing_job_is_not_a_failure_signal ... ok
test ...::a_runner_cancel_is_not_a_failure_signal ... ok
test ...::a_timed_out_job_is_not_a_failure_signal ... ok
test ...::an_empty_image_is_still_a_container_job ... ok
test ...::an_execution_error_is_still_a_failure_signal ... ok
test ...::only_a_422_means_the_server_predates_this ... ok
test result: ok. 13 passed; 0 failed
```
Plus four for the quarantine column (`quarantine_column::*`), including a lapsed quarantine rendering nothing — `quarantined_until` is absolute, so once it is in the past the runner is claiming again and saying otherwise would send an operator chasing a healthy host.
Full suite: **460 passed, 0 failed.** `cargo fmt --check` and `cargo clippy --all-targets -- -D warnings` are clean (the only remaining note is a pre-existing future-incompat warning from `proc-macro-error2`).
## Compatibility
A runner on this version talking to a server that predates #375 gets a 422 for `infra_failed` and falls back to reporting an ordinary failure — the same outcome as today, rather than leaving the job `running` until the watchdog times it out half an hour later. Against a current server, a 409 means the job is no longer this runner's to report on and the report is dropped. There is no deploy-ordering constraint in either direction.
SHA:
de31c3bdcaca301659aee4c0433c760adfefd7b2
Author:
Anvil <noreply@anvil.fangorn.io>
Date:
2026-07-31 05:14
Parents:
b84c623
3 files changed
+484
-7
| Type | ||
|---|---|---|
|
|
src/commands/ci.rs | +15 −0 |
|
||
|
|
src/commands/runner.rs | +89 −1 |
|
||
|
|
src/runner/loop_runner.rs | +380 −6 |
|
||