fix(runner-image): the prune reads the wrong snapshot column, so no boot volume is ever deleted #352

Nytt ärende

Stängd

öppnade 2026-08-02 18:05:04 +00:00 av supernaut · 0 kommentarer

supernaut kommenterad

2026-08-02 18:05:04 +00:00

Ägare

Found by running the reclaim dry run that #346's description recommends:

scripts/bake-runner-image.sh --prune-only --dry-run
[prune] retiring generation d301abd1-… (), volume 2026-07-20T18:01:11.000000
[prune]   volume 2026-07-20T18:01:11.000000 is already gone

The volume field holds a timestamp, the created field is empty, and every generation reports its
volume as "already gone". Run for real, the prune would delete the snapshots and leave every boot
volume behind — half the reclaim, and the half that holds 20 GiB apiece.

Two bugs, compounding

1. The jq reads a column the CLI does not return. openstack volume snapshot list --long -f json
names it Volume; bake_generations() reads ."Volume ID" // .volume_id // "", so the value is
always empty. Verified:

has "Volume ID": false    has "Volume": true    value: f1ce224d-…

2. A tab-delimited read collapses the empty field. Tab is IFS whitespace, so bash treats runs
of tabs as one delimiter and drops empties — the timestamp slides 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|

Bug 1 empties the field; bug 2 disguises it as a plausible-looking value, which is why it reads as
"already gone" rather than as an obvious error.

Fix

Read .Volume first (keeping the other spellings as fallbacks for other CLI versions), and switch
the row delimiter from @tsv to a pipe, which is not IFS whitespace so empty fields survive. Neither
a UUID nor an ISO-8601 timestamp can contain one.

After the fix the dry run resolves real volumes and still protects what it must:

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.

Note on blast radius

Not silently destructive: the two safety conditions in prune_generation_volume (available and
unnamed) are unaffected, and a snapshot referenced by a live image is still never touched. The
failure mode was under-deletion, not over-deletion. The volumes would in fact have been reclaimed
eventually — once their snapshot was gone they become unpinned, and the controller's sweep takes
unnamed available volumes — but by accident, via a different mechanism, and the script's own output
would have said the job was done when it was not.

controller.py is unaffected: it reads volume_id off the openstacksdk object, which is the correct
attribute there, and its published counts match the live state.

Done when

The dry run prints a volume UUID for every retired generation, and a timestamp in the created
column
shellcheck and bash -n clean

Found by running the reclaim dry run that #346's description recommends: ```text scripts/bake-runner-image.sh --prune-only --dry-run [prune] retiring generation d301abd1-… (), volume 2026-07-20T18:01:11.000000 [prune] volume 2026-07-20T18:01:11.000000 is already gone ``` The volume field holds a *timestamp*, the created field is empty, and every generation reports its volume as "already gone". Run for real, the prune would delete the snapshots and leave every boot volume behind — half the reclaim, and the half that holds 20 GiB apiece. ## Two bugs, compounding **1. The jq reads a column the CLI does not return.** `openstack volume snapshot list --long -f json` names it `Volume`; `bake_generations()` reads `."Volume ID" // .volume_id // ""`, so the value is always empty. Verified: ```text has "Volume ID": false has "Volume": true value: f1ce224d-… ``` **2. A tab-delimited read collapses the empty field.** Tab is IFS *whitespace*, so bash treats runs of tabs as one delimiter and drops empties — the timestamp slides 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| ``` Bug 1 empties the field; bug 2 disguises it as a plausible-looking value, which is why it reads as "already gone" rather than as an obvious error. ## Fix Read `.Volume` first (keeping the other spellings as fallbacks for other CLI versions), and switch the row delimiter from `@tsv` to a pipe, which is not IFS whitespace so empty fields survive. Neither a UUID nor an ISO-8601 timestamp can contain one. After the fix the dry run resolves real volumes and still protects what it must: ```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. ## Note on blast radius Not silently destructive: the two safety conditions in `prune_generation_volume` (available **and** unnamed) are unaffected, and a snapshot referenced by a live image is still never touched. The failure mode was under-deletion, not over-deletion. The volumes would in fact have been reclaimed eventually — once their snapshot was gone they become unpinned, and the controller's sweep takes unnamed available volumes — but by accident, via a different mechanism, and the script's own output would have said the job was done when it was not. `controller.py` is unaffected: it reads `volume_id` off the openstacksdk object, which is the correct attribute there, and its published counts match the live state. ## Done when - [ ] The dry run prints a volume UUID for every retired generation, and a timestamp in the created column - [ ] `shellcheck` and `bash -n` clean

supernaut refererade till detta ärende från en incheckning

2026-08-02 18:05:36 +00:00

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

supernaut hänvisade till detta ärende från en ändringsförfrågan som kommer att stänga det

2026-08-02 18:05:52 +00:00

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

supernaut lade till etiketterna

area/ci

area/infra

area/storage

ready-for-implementation

type