ci: reap testcontainer volumes, not just containers - #452
Conversation
Every CI cleanup path force-removed its containers with `docker rm -f`, which does not remove anonymous volumes. The postgres, elasticsearch, mongo, minio and keycloak images all declare VOLUME in their Dockerfiles, so each testcontainer created one — and each reap orphaned it permanently. The shared self-hosted Docker host is an LXC container on a ZFS dataset with a 200G refquota. The orphans filled it until containerd could not write io.containerd.metadata.v1.bolt/meta.db, at which point *every* container start failed with "disk quota exceeded" (EDQUOT, not ENOSPC — the dataset quota, not a full disk). `docker system df` on the host still showed 144 volumes with only 4 active after a manual prune. Add `-v` to all 84 call sites across 16 workflows. This removes only anonymous volumes; named volumes are never touched by `docker rm -v`, and no workflow creates or mounts one — every volume in play here is anonymous. This fixes the leak at the source. The nightly janitor in #451 remains worth having as a backstop for runs that die before reaching cleanup, but a 05:30 UTC sweep could not have saved a run that failed at 16:03.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
…eap each other The reaper excluded this run's own containers by run id, which covers the sqlite/postgres matrix legs (they share a run id). It did not cover two *different* runs: the `concurrency` group only cancels in-progress runs on the same ref, so a dispatch on main and one on a PR branch run side by side with different run ids — and each would have reaped the other's containers mid-benchmark. Adds the age guard ci.yml already uses for the same reason. 120 minutes is comfortably longer than a full leg (import alone is ~22 min), so a running benchmark is never in range. The canary covers the window where an orphan is younger than the guard but still holding the host's space. Found by reading #452, which fixes the same class of leak repo-wide.
|
Heads-up on an overlap — #453 (opened just before this, for the benchmark specifically) touches the same four lines in We reached the same conclusion independently, which is reassuring. Your commit has a detail I didn't have and that I've now cited: the host's 200G ZFS quota, and that the leak is anonymous volumes because the postgres/ES/mongo images declare No disagreement in content — #453 already does Suggested order: merge this one first. It's broader (16 files) and lower-risk, and it fixes workflows #453 doesn't touch. I'll rebase #453 on top — its One thing reading this PR fixed in mine: your |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
main's #452 added `-v` to 84 `docker rm` call sites across 16 workflows, so reaped testcontainers no longer orphan the anonymous volumes their images declare via VOLUME. That leak filled the shared Docker host's 200G ZFS refquota until containerd could not write meta.db and every container start failed with EDQUOT — which is what took this PR's `Test Rust` and `Redis Cluster Tests` red at 16:03 with zero assertions run. The sweep could not reach three call sites, because all three arrive with this branch and so were not in main's tree: redis-cluster-tests.yml:77 reap step (whole workflow is new in #269) cluster-smoke.yml:432,592 Elasticsearch teardown for the E1 nightly Both images leak: redis declares VOLUME /data and elasticsearch declares VOLUME /usr/share/elasticsearch/data — elasticsearch is named explicitly in #452's commit message. Left alone, merging #269 would reintroduce the leak it just got bitten by, on the workflow that reports the symptom. Merge itself was conflict-free.
|
Quantified evidence for this PR's leak, from a benchmark dispatch I just ran on the same host (run 30576511473): 187 volumes, 22.13 GB reclaimable — 93%. Against the 200 G quota you cite, that is the bulk of the problem, and it is exactly the anonymous-volume leak Also worth noting for the record: two containers orphaned by benchmark run 30550776427 were still present 5½ hours later (ages 333m/334m) when my reaper removed them. So the host recovering enough for CI to go green earlier today did not mean it had been cleaned — it just had enough headroom again. Some manual pruning on the host is still worth doing; neither PR retroactively reclaims those 22 GB. |
smunini
changed the title
smunini
changed the title
#452 landed first, as suggested — it changed `docker rm -f` → `docker rm -fv` across 16 workflows, including the four sites in fhir-benchmark.yml this branch rewrites. All four conflicts were the same shape: identical semantics, different spelling, with this branch adding lines around them. Resolved by keeping this branch's version (a strict superset for this file: the same `-v` plus the named PG data volume, the labels the reaper matches on, and the volume teardown) while adopting main's `-fv` spelling, so all five `docker rm` sites here read the same as the other fifteen workflows. Verified after resolution: YAML parses, every `run:` block passes `bash -n`, the step order is unchanged, and the net diff against the new main is purely additive — no `-fv` regressed back to `-f`.
Companion to #452/#454, which fixed the volume leak that choked the Docker host and produced the nightly ES signature (#449: readiness curl passes, then the mapped port drops SYNs while ES stalls on fsync). What made that incident expensive was the blindness: nothing recorded the ES container's fate. On a wait-for-HFS failure the step now dumps the container state, OOM flag, log tail, and a live reachability probe. Also defensive, and logged either way: NO_PROXY names the backend hosts exactly, because reqwest does not honor CIDR entries the way the curl probes do — a proxy-configured runner would otherwise time out HFS's ES traffic while every probe passes. Closes #449
2 participants
The bug
Every CI cleanup path force-removes its containers with
docker rm -f, which does not remove anonymous volumes — that requires-v.The postgres, elasticsearch, mongo, minio and keycloak images all declare
VOLUMEin their Dockerfiles, so every testcontainer gets an anonymous volume, and every reap orphans it permanently. This has been leaking on every run, on every workflow, for as long as the reaps have existed.How it surfaced
The shared self-hosted Docker host is an LXC container on a ZFS dataset (
nvme1/subvol-106-disk-0) with a 200G refquota. The orphaned volumes filled it until containerd could no longer writeio.containerd.metadata.v1.bolt/meta.db, at which point every container start failed:Note
EDQUOT("disk quota exceeded"), notENOSPC— the ZFS dataset quota, not a full disk. The failures are protean, because anything needing a container dies: see #449 for the Elasticsearch variant.Seen in run 30559660973 (
Test Rust,postgres_bootstrap_lock.rs), and it is not PR-specific — any run landing in that window fails the same way.The fingerprint on the host, after a manual prune had already recovered it:
144 volumes, 4 active. 140 orphans.
The change
docker rm -f→docker rm -fvat 84 call sites across 16 workflows, plus a comment at the canonical reap site inci.ymlrecording why-vis load-bearing.Safety
docker rm -vremoves only anonymous volumes; named volumes are never touched. No workflow in this repo runsdocker volume createor mounts a named volume — every volume in play is anonymous, so all of them were pure leak.actionlintwas diffed againstmain: identical findings, no new ones. The only delta is three pre-existingSC2086warnings shifting column 16→17, from the extra character on those lines.Follow-ups
This stops new orphans, but an already-orphaned volume has no container left, so no
docker rm -fvwill ever reclaim it. The backlog still needs sweeping — see the per-run GC in the follow-up PR, which supersedes #451's nightly janitor.#451 also carries an unrelated real fix (
ci(ui-matrix): route the Elasticsearch self-traffic around the runner proxy) that is worth keeping.