From 13a98ae7025def95ec594e12897ded6e62819d18 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 29 Jul 2026 02:09:59 +0000 Subject: [PATCH] Make a half-scoped reap impossible in ci-cleanup.sh The reap runs two selectors, one on the CI label and one on the compose project, and each flag narrows only its own. So --run-id alone leaves the project sweep broad and --project-prefix alone leaves the label sweep broad, and either way the reap still destroys every concurrent run on the host. Both single-flag forms look scoped and are not. The usage text asserted otherwise. It documented --run-id as leaving other runs alone and --project-prefix as scoping the reap to a single run, and both claims were false for the same reason. A caller passing --run-id alone, on the strength of that line, force-removed the containers of three concurrent runs on 2026-07-29, and its own comment recorded the belief that it was scoped. Rather than ask every caller to remember the pair, close the two half-scoped states here. --run-id now derives the matching project prefix when none is given, built from the existing base so the two cannot drift, and an explicit --project-prefix still wins. --project-prefix without --run-id is refused outright, because there is nothing to derive a label scope from and the quiet failure is someone else's run disappearing. Passing neither flag is untouched: that is the deliberate "reap everything" form a manual cleanup wants. Verified against a decoy container labelled as another run: the scoped reap leaves it up, and reproducing the old broad project sweep removes it, so the check discriminates rather than passing because there was nothing to reap. An earlier version of that check ran when no CI containers existed at all and passed without proving anything. --- testing/ci-cleanup.sh | 51 +++++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/testing/ci-cleanup.sh b/testing/ci-cleanup.sh index 407b11b..010394d 100755 --- a/testing/ci-cleanup.sh +++ b/testing/ci-cleanup.sh @@ -15,9 +15,15 @@ # with that prefix). # # The generic CI label is shared by every run on the host, so an unscoped label -# sweep would tear down a CONCURRENT run's resources. Pass --run-id to narrow -# the label sweep to one run; a run's own teardown always does. Without it the -# label sweep stays broad, which is what a manual "reap everything" wants. +# sweep would tear down a CONCURRENT run's resources. So would an unscoped +# compose-project sweep, and reap_containers runs BOTH. Neither flag alone is +# therefore enough to spare a concurrent run: --run-id narrows only the label +# sweep, --project-prefix only the project sweep, and whichever is left broad +# reaps everything by itself. This cost three concurrent runs on 2026-07-29, +# via a caller that passed --run-id alone and reasonably believed that scoped +# it. --run-id now implies the matching project prefix, and --project-prefix +# without --run-id is refused, so the dangerous half-scoped states are no +# longer reachable. Passing neither is still the broad "reap everything" form. # # Host-namespace veth interfaces are the one non-docker resource reaped here. # The chaos simulation creates each pair in the host namespace and then moves @@ -37,12 +43,17 @@ # # Usage: # ci-cleanup.sh Reap ALL fips-ci resources (any run) +# ci-cleanup.sh --run-id ID Scope the reap to one run: narrows the +# label sweep to ID and, unless +# --project-prefix says otherwise, +# narrows the project sweep to that run +# too. Does NOT scope the host veth +# sweep — see --veth-suffixes # ci-cleanup.sh --project-prefix P Restrict the compose-project sweep to -# names starting with P (scopes the reap -# to a single run). Does NOT scope the -# host veth sweep — see --veth-suffixes -# ci-cleanup.sh --run-id ID Restrict the label sweep to the run -# labelled ID (leaves other runs alone) +# names starting with P. Requires +# --run-id: on its own it leaves the +# label sweep broad, which reaps every +# concurrent run regardless of P # ci-cleanup.sh --label L Override the CI label (default above) # ci-cleanup.sh --images "a b,c" Also `docker rmi -f` these image tags # (space- or comma-separated) @@ -75,11 +86,12 @@ VETH_SUFFIXES="" # empty AND no --run-id: every simulation veth name # only ever run the harness it need not exist at all, and the reap this script # advertises as the remedy for orphaned interfaces would be a permanent no-op. VETH_IMAGE="" +PROJECT_PREFIX_GIVEN=0 while [[ $# -gt 0 ]]; do case "$1" in --label) LABEL="$2"; shift 2 ;; - --project-prefix) PROJECT_PREFIX="$2"; shift 2 ;; + --project-prefix) PROJECT_PREFIX="$2"; PROJECT_PREFIX_GIVEN=1; shift 2 ;; --run-id) RUN_ID="$2"; shift 2 ;; --images) IMAGES="$2"; shift 2 ;; --veth-suffixes) VETH_SUFFIXES="$2"; shift 2 ;; @@ -89,6 +101,22 @@ while [[ $# -gt 0 ]]; do esac done +# A reap scoped on one axis and broad on the other still destroys every +# concurrent run, because both selectors run. Close both half-scoped states +# here rather than trusting each caller to pass the pair. +if [[ -n "$RUN_ID" && "$PROJECT_PREFIX_GIVEN" -eq 0 ]]; then + # Every run's projects are named "_", so the + # run id is all that is needed. Derived from the base rather than a second + # copy of the literal, so the two cannot drift. + PROJECT_PREFIX="${PROJECT_PREFIX}${RUN_ID}" +fi +if [[ -z "$RUN_ID" && "$PROJECT_PREFIX_GIVEN" -eq 1 ]]; then + echo "ci-cleanup.sh: --project-prefix requires --run-id." >&2 + echo " Without it the label sweep stays broad and reaps every concurrent" >&2 + echo " run regardless of the prefix. Pass both, or neither for a full reap." >&2 + exit 2 +fi + # Resolve the image to run ip(8) in: the caller's choice, then the run's own # image, then any surviving fips test image. That last fallback is what keeps # an unscoped `ci-local.sh --reap` working — it execs this script from inside @@ -116,8 +144,9 @@ fi # never wedge a caller (ci-local's signal trap relies on this being bounded). TMO=30 -# Selector for the label sweep. With --run-id it matches only the named run, so -# a concurrent run's resources are left alone; without it, every CI run. +# Selector for the label sweep. With --run-id it matches only the named run; +# without it, every CI run. Note this is only half of what scopes a reap — the +# compose-project sweep below is the other half, and both must be narrow. if [[ -n "$RUN_ID" ]]; then SWEEP_LABEL="${RUN_LABEL_KEY}=${RUN_ID}" else