fix(runner-image): read the snapshot's real volume column when pruning #353

Sammanfogat
supernaut sammanfogade 1 incheckning från fix/bake-prune-volume-field in i main 2026-08-02 18:15:59 +00:00
Ägare

Closes #352. Found by running the reclaim dry run that #346's own description recommends.

Before — the volume column holds a timestamp, created is empty, and every generation claims its
volume is already gone:

[prune] retiring generation d301abd1-… (), volume 2026-07-20T18:01:11.000000
[prune]   volume 2026-07-20T18:01:11.000000 is already gone

Run for real, that deletes the snapshots and leaves every boot volume behind — the half of the
reclaim that actually frees quota.

Two bugs, compounding

The jq reads a column the CLI does not return. openstack volume snapshot list --long -f json
names it Volume; the code read ."Volume ID" // .volume_id // "", so the value was always empty.
Verified against the live API: has "Volume ID": false, has "Volume": true.

A tab-delimited read then hid it. Tab is IFS whitespace, so bash collapses runs of tabs and
drops empty fields — the timestamp slid one position left into vol_id:

printf 'ID\t\t2026-07-20T18:01:11\n' | while IFS=$'\t' read -r a b c; do echo "$a|$b|$c"; done
ID|2026-07-20T18:01:11|

The first bug empties the field; the second disguises it as a plausible value. That is why it
surfaced as a calm "already gone" rather than an error.

Fix

Read .Volume first, keeping the other spellings as fallbacks for other CLI versions, and delimit
rows with a pipe instead of @tsv. A pipe is not IFS whitespace, so empty fields survive, and
neither a UUID nor an ISO-8601 timestamp can contain one.

After

keeping 4daabf1b-… (2026-07-29T21:12:18) — a Glance image boots from it
keeping a500b034-… (2026-07-22T14:15:16) — within RETAIN_GENERATIONS=2
would delete volume e8dd5800-…   4d4a7cd2-…   40781d11-…   e610f170-…

Those four are exactly the stranded volumes identified in #320, and the live image's pair is still
protected.

Blast radius

Not silently destructive — the failure mode was under-deletion. prune_generation_volume's two
safety conditions (available and unnamed) are untouched, and a snapshot referenced by a live
image is still never a candidate. The volumes would in fact have been reclaimed eventually, since
deleting the snapshot unpins them and the controller's sweep takes unnamed available volumes — but
by accident, and the script would have reported success while leaving 80 GiB behind.

controller.py is unaffected: it reads volume_id off the openstacksdk object, the correct
attribute there, and its published counts match live state (snapshot_pinned 6, 120 GiB).

Checks

shellcheck exit 0 (the first attempt at this fix regressed it — a comment containing shell tab
syntax inside the jq string tripped SC1012; reworded), bash -n clean, Prettier clean, and the dry
run re-run against production to confirm the output above. Nothing was deleted.

Closes #352. Found by running the reclaim dry run that #346's own description recommends. Before — the volume column holds a timestamp, created is empty, and every generation claims its volume is already gone: ```text [prune] retiring generation d301abd1-… (), volume 2026-07-20T18:01:11.000000 [prune] volume 2026-07-20T18:01:11.000000 is already gone ``` Run for real, that deletes the snapshots and leaves every boot volume behind — the half of the reclaim that actually frees quota. ## Two bugs, compounding **The jq reads a column the CLI does not return.** `openstack volume snapshot list --long -f json` names it `Volume`; the code read `."Volume ID" // .volume_id // ""`, so the value was always empty. Verified against the live API: `has "Volume ID": false`, `has "Volume": true`. **A tab-delimited read then hid it.** Tab is IFS *whitespace*, so bash collapses runs of tabs and drops empty fields — the timestamp slid one position left into `vol_id`: ```text printf 'ID\t\t2026-07-20T18:01:11\n' | while IFS=$'\t' read -r a b c; do echo "$a|$b|$c"; done ID|2026-07-20T18:01:11| ``` The first bug empties the field; the second disguises it as a plausible value. That is why it surfaced as a calm "already gone" rather than an error. ## Fix Read `.Volume` first, keeping the other spellings as fallbacks for other CLI versions, and delimit rows with a pipe instead of `@tsv`. A pipe is not IFS whitespace, so empty fields survive, and neither a UUID nor an ISO-8601 timestamp can contain one. ## After ```text keeping 4daabf1b-… (2026-07-29T21:12:18) — a Glance image boots from it keeping a500b034-… (2026-07-22T14:15:16) — within RETAIN_GENERATIONS=2 would delete volume e8dd5800-… 4d4a7cd2-… 40781d11-… e610f170-… ``` Those four are exactly the stranded volumes identified in #320, and the live image's pair is still protected. ## Blast radius Not silently destructive — the failure mode was under-deletion. `prune_generation_volume`'s two safety conditions (available **and** unnamed) are untouched, and a snapshot referenced by a live image is still never a candidate. The volumes would in fact have been reclaimed eventually, since deleting the snapshot unpins them and the controller's sweep takes unnamed available volumes — but by accident, and the script would have reported success while leaving 80 GiB behind. `controller.py` is unaffected: it reads `volume_id` off the openstacksdk object, the correct attribute there, and its published counts match live state (`snapshot_pinned 6`, 120 GiB). ## Checks `shellcheck` exit 0 (the first attempt at this fix regressed it — a comment containing shell tab syntax inside the jq string tripped SC1012; reworded), `bash -n` clean, Prettier clean, and the dry run re-run against production to confirm the output above. Nothing was deleted.
supernaut lade till 1 incheckning 2026-08-02 18:05:52 +00:00
fix(runner-image): read the snapshot's real volume column when pruning
Alla kontroller lyckades
ci / ci (pull_request) Successful in 1m3s
de071e975b
The prune resolved no boot volume at all, so it would have deleted the
snapshots and left every 20 GiB volume behind — the half of the reclaim that
actually frees quota.

Two bugs compounding. The CLI names the column Volume, not "Volume ID", so the
jq always produced an empty field. Tab is IFS whitespace, so the tab-separated
read then collapsed the two adjacent tabs and shifted the timestamp into the
volume variable — which made an empty field look like a real value and
'already gone' look like a reasonable answer.

Read .Volume first, keeping the other spellings as fallbacks, and delimit rows
with a pipe: it is not IFS whitespace, so empty fields survive, and neither a
UUID nor an ISO-8601 timestamp can contain one.

The safety conditions are untouched — a volume is still deleted only when Cinder
reports it available and unnamed, and a snapshot a live image boots from is
still never a candidate. The failure mode here was under-deletion.

Closes #352
supernaut sammanfogade incheckning 872cd44925 till main 2026-08-02 18:15:59 +00:00
supernaut tog bort grenen fix/bake-prune-volume-field 2026-08-02 18:15:59 +00:00
Logga in för att delta i denna konversation.
Inga granskare
Ingen milstolpe
Inget projekt
Inga tilldelade
1 deltagare
Notiser
Förfallodatum
Förfallodatumet är ogiltigt eller utanför gränserna. Använd formatet "åååå-mm-dd".

Inget förfallodatum satt.

Beroenden

Inga beroenden satta

Referens
bitborg/bitborg-infra!353
Ingen beskrivning angiven.