fix(ci): make leaked CI containers visible and reapable on the shared Docker host - #563
Merged
Merged
Conversation
… Docker host
A Playwright container from run 30553500384 (ui-tests-matrix, sqlite leg,
2026-07-30) was still running on the shared Docker host two weeks later,
even though its `Cleanup` step reported success.
Root cause: the host was over its ZFS quota that afternoon — sibling legs
in the same run died with "disk quota exceeded" from containerd's meta.db.
`docker rm -f` needs that same write, so cleanup failed too. But it was
written `docker rm -fv "$X" 2>/dev/null || true`, so the error was
discarded and the step went green.
Nothing could collect it afterwards, for four independent reasons:
- the container carried no labels, so ci.yml's run_id reap and its
`org.testcontainers.managed-by` orphan sweep were both blind to it;
- docker-host-gc pruned volumes, images, build cache and networks, but
never containers;
- container names are run-id-scoped, so no later run could match it;
- it pinned the ~2GB Playwright image against `image prune`, so the leak
actively degraded the one sweep that did run.
Fixes:
- Label every container the UI workflows start with `hfs-ci=true`,
`github.run_id` and `github.workflow`.
- Add an age-guarded orphan reap to docker-host-gc, scoped to the
`hfs-ci=true` label (default 180m). Runs after the volume sweep, which
is what unwedges a quota-choked host, and before the image prune, which
a surviving container would otherwise block. Label-scoped rather than a
blanket `container prune` so it cannot collect a stopped container a
human intends to restart.
- Replace the silencing `2>/dev/null || true` removals with a `reap()`
that verifies by inspect and emits `::warning::` when a container
survives. Non-fatal: a leak should not turn a passing suite red.
- `docker builder prune --keep-storage` is deprecated in favour of
`--reserved-space`. The flag is parsed by the client, so the runner pool
decides which name works and the pool is not uniform — probe for it.
Verified by extracting the shell out of the YAML and replaying it against
the real daemon and against fake daemons reproducing the quota error, a
daemon that exits 0 without removing, and an unreachable daemon.
Claude-Session: https://claude.ai/code/session_01XVgTcegkXoCzz1Y2LRan9i
…copy
RUSTSEC-2026-0258 (published 2026-08-17) flags unbounded buffering of
empty HTTP/2 DATA frames as a memory-exhaustion DoS, turning the Security
Audit job red on every branch. The lock carried two h2 copies:
* 0.4.13, the one our own server actually speaks HTTP/2 through
(axum -> hyper 1.x, plus tonic and reqwest). Bumped to the patched
0.4.16 -- this is the copy that matters, since it is the one facing
untrusted FHIR clients.
* 0.3.27, reached only via the legacy hyper 0.14 fallback client inside
aws-smithy-http-client 1.1.10. The 0.3 line is unsupported upstream
(0.3.27 is its final release; the fix lands in >= 0.4.16 only), so no
upgrade exists. `cargo tree -i hyper@0.14.32` finds nothing under the
default feature set -- it links only under `--all-features`, the same
legacy smithy path already covered by the rustls-webpki ignores -- and
it is a client talking to AWS endpoints, so reaching the bug would
require AWS itself to send the hostile frame stream. Ignored with that
rationale recorded next to the existing entries.
The lock edit is deliberately confined to the four h2 references and the
package block: `cargo update --precise` also re-resolves unrelated entries
(it downgraded socket2 0.6.3 -> 0.5.10 and windows-sys 0.61.2 -> 0.52.0,
neither of which is yanked or advisory-affected), and that churn does not
belong in a security fix. `cargo check --workspace --all-targets --locked`
and `cargo metadata --all-features --locked` both pass on the result.
Claude-Session: https://claude.ai/code/session_011VbASUJbXY1kDoPLgUMgkH
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What happened
docker pson the shared Docker host showed a Playwright container still running two weeks after its job ended:It is from run 30553500384 (
UI Tests (backend matrix),sqliteleg, 2026-07-30) — whoseCleanupstep reported success.Root cause
The host was over its ZFS quota that afternoon. Sibling legs in the same run died with:
docker rm -fneeds that same meta.db write, so the cleanup failed too — but it was written:stderr discarded,
|| trueon the end. The step went green. The job log showsRemoving Playwright container...at14:58:38.654and the next echo 93 ms later, with no container ID echoed back — it never removed anything. The container still showsUpdespite its command beingsleep 3600because containerd could not persist the exit state either.Why nothing swept it in two weeks
Four independent gaps, all of which had to hold — and did:
ui-tests.yml/ui-tests-matrix.ymlstart containers with no--label, so ci.yml's "Reap this run's testcontainers" (label=github.run_id=…) and "Sweep orphaned testcontainers from dead runs" (label=org.testcontainers.managed-by=testcontainers) were both blind to it. Those only see testcontainers-rs containers.docker-host-gcnever pruned containers — volumes, images, build cache, networks only.docker rm -f "$PW_CONTAINER"could never match an older leak.docker image prune -af --filter until=48hcould not reclaim the ~2 GB Playwright image.Net: on a healthy host cleanup works; the moment the host is sick, it fails silently and the leak becomes permanently unreapable.
Changes
hfs-ci=true,github.run_idandgithub.workflow.docker-host-gcorphan reap — new age-guarded sweep (newcontainer-max-age-mininput, default 180m) scoped tohfs-ci=true. Placed after the volume sweep (which is what unwedges a quota-choked host) and before the image prune (which a surviving container blocks). Label-scoped rather than a blanketdocker container prune, following the same defence-in-depth reasoning as the existing anonymous-volume filter: the host runs long-lived services, and a stopped container a human intends to restart must never be collected.2>/dev/null || truereplaced with areap()that verifies bydocker inspectand emits::warning::when a container survives. Deliberately still non-fatal: a leak should not turn a passing suite red.reap()verifies by inspect rather than trusting the exit code, becausedocker rm -fexits 0 for a container that does not exist and a wedged daemon can report success without removing anything.--keep-storage→--reserved-space— the old spelling is deprecated and now warns. The flag is parsed by the client, not the remote daemon, so the runner pool decides which spelling works and the pool is not uniform; the action probes--helprather than pinning either name.Verification
The host itself has been cleaned manually (zombie removed, ~1.2 GB reclaimed, now 0 B reclaimable).
For the code: the shell was extracted straight out of the YAML and replayed against the real daemon and against fake daemons reproducing each failure mode, under GitHub's actual
bash --noprofile --norc -eo pipefail.::warning::, step continuesrmexits 0 but container survives (wedged daemon)::warning::, step continuesES_CONTAINER(empty arg):-nonedefault, which asked the daemon to remove a container literally namednonealready goneRemoved, confirmed gonehfs-ci=truecontainer; unlabelled one untouched::warning::+ skip, rather than reporting a clean "Reaped 0"--keep-storage; probe works with no daemon reachableAll three files pass YAML parse and
bash -non everyrun:block.Note
ui-tests-matrix.ymlis nightly +workflow_dispatchonly, so this PR will not exercise it.ui-tests.ymlruns on PRs touchingcrates/ui/**, which this PR does not — worth a manualworkflow_dispatchof both after merge to see the new labels land.https://claude.ai/code/session_01XVgTcegkXoCzz1Y2LRan9i