ref:main

Runner: report infrastructure failure distinctly from job failure (#375) #56

merged colechristensen cole.christensen@gmail.com wants to merge fix/375-infra-failure-reporting into main

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/4PATCH /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 asserta_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.

Created Jul 30, 2026 at 18:40 UTC | Merged Jul 31, 2026 at 05:14 UTC by colechristensen cole.christensen@gmail.com