fix(runner-image): retire old bake generations, and detect the leak the quota gauge cannot see #346

Sammanfogat

supernaut sammanfogade 3 incheckningar från fix/boot-volume-leak-detector in i main 2026-08-02 17:48:02 +00:00

Konversation 1 Incheckningar 3 Ändrade filer 9 +762 -34

supernaut kommenterad 2026-08-02 15:56:13 +00:00

Ägare

Kopiera länk

Refs #320 — does not close it; reclaiming the stranded volumes is still an operator action.

Note

Merge after the runner capacity/latency PR. Both touch controller.py,
test_controller_logic.py and the runners dashboard. The changes are additive and the conflict is
mechanical, but the combined test suite should be re-run after resolution.

The producer is identified, which was #320's blocking question. These are image-bake
leftovers, not runner-job leaks. Baking the runner image snapshots the staging VM's boot volume;
the snapshot holds a reference, so Cinder refuses to delete the volume, and the sweep then correctly
skips it — it excludes any volume with snapshots precisely so it cannot destroy image or backup
infrastructure. Each stranded volume pairs one-to-one with a bake snapshot, and the timestamps line
up. The sweep is working as designed.

Root cause confirmed in scripts/bake-runner-image.sh: its exit trap ran a volume delete under
set +e, which Cinder always refuses once the volume has been snapshotted, and --replace deleted
only the old image, never its snapshot or volume.

Part 1 — make this class of leak visible

The existing quota gauge cannot see it: ~160 GB over five weeks disappears inside a band that swings
by several hundred GB during normal CI churn, so a slow leak always reads clean. Adds a direct signal
with three mutually-exclusive classes:

• snapshot_pinned — unattached and snapshot-referenced: unsweepable by design, one per retained
  bake generation. This is the #320 signal.
• sweepable — the sweep's own target; should return to zero within one window.
• named — deliberate retention, shown but never alerted, so out-of-scope waste cannot make the new
  alerts permanently noisy.

Every class publishes -1 for unknown, never 0, when a read fails — a failed read must not read as
"clean", which is the failure mode that hid this. The survey reuses listings the sweep already makes.

Part 2 — stop it at the source

The bake now prunes generations after promoting and verifying the new image, keeping a
configurable number (default 2 = live plus one back). Two hard safety conditions: a snapshot
referenced by any image's block-device-mapping is never deleted, which protects the live pair
automatically rather than by remembering to; and a volume is deleted only if Cinder reports it
available and unnamed — the same two conditions the sweep requires. Adds --prune-only,
--dry-run and --no-prune.

Expect one alert to fire on first deploy

RunnerSnapshotPinnedVolumesHigh will trip until the reclaim is done, because the standing count
exceeds the retention threshold. That is intentional: calibrating the threshold to accept today's
waste would repeat the original mistake. Raise it temporarily if you want quiet until then.

Operator actions after merge

1. Apply the runner-controller and monitoring roles.
2. scripts/bake-runner-image.sh --prune-only --dry-run first — it prints which snapshot each image
   boots from. One pair is load-bearing and deleting it leaves the image unbootable.
3. The retained predrill-* pair will show as named — visible, un-alerted, out of scope here.

Verification

33 checks pass, up from 17. ruff, shellcheck, Prettier, markdownlint all clean; ansible-lint
0 failures across 191 files; gitleaks clean over 272 commits. The alert template renders and parses,
and both expressions parse against the metrics backend. The prune path was exercised offline against
a stub CLI: live snapshot kept, in-use volume refused, named volume refused, clean leftovers
reclaimed. No OpenStack mutation was performed.

Refs #320 — does not close it; reclaiming the stranded volumes is still an operator action. > [!NOTE] > **Merge after the runner capacity/latency PR.** Both touch `controller.py`, > `test_controller_logic.py` and the runners dashboard. The changes are additive and the conflict is > mechanical, but the combined test suite should be re-run after resolution. **The producer is identified**, which was #320's blocking question. These are **image-bake leftovers**, not runner-job leaks. Baking the runner image snapshots the staging VM's boot volume; the snapshot holds a reference, so Cinder refuses to delete the volume, and the sweep then correctly skips it — it excludes any volume with snapshots precisely so it cannot destroy image or backup infrastructure. Each stranded volume pairs one-to-one with a bake snapshot, and the timestamps line up. **The sweep is working as designed.** Root cause confirmed in `scripts/bake-runner-image.sh`: its exit trap ran a volume delete under `set +e`, which Cinder always refuses once the volume has been snapshotted, and `--replace` deleted only the old image, never its snapshot or volume. ### Part 1 — make this class of leak visible The existing quota gauge cannot see it: ~160 GB over five weeks disappears inside a band that swings by several hundred GB during normal CI churn, so a slow leak always reads clean. Adds a direct signal with three mutually-exclusive classes: - `snapshot_pinned` — unattached and snapshot-referenced: unsweepable by design, one per retained bake generation. This is the #320 signal. - `sweepable` — the sweep's own target; should return to zero within one window. - `named` — deliberate retention, shown but never alerted, so out-of-scope waste cannot make the new alerts permanently noisy. Every class publishes `-1` for unknown, never `0`, when a read fails — a failed read must not read as "clean", which is the failure mode that hid this. The survey reuses listings the sweep already makes. ### Part 2 — stop it at the source The bake now prunes generations after promoting **and** verifying the new image, keeping a configurable number (default 2 = live plus one back). Two hard safety conditions: a snapshot referenced by **any** image's block-device-mapping is never deleted, which protects the live pair automatically rather than by remembering to; and a volume is deleted only if Cinder reports it available **and** unnamed — the same two conditions the sweep requires. Adds `--prune-only`, `--dry-run` and `--no-prune`. ### Expect one alert to fire on first deploy `RunnerSnapshotPinnedVolumesHigh` will trip until the reclaim is done, because the standing count exceeds the retention threshold. That is intentional: calibrating the threshold to accept today's waste would repeat the original mistake. Raise it temporarily if you want quiet until then. ### Operator actions after merge 1. Apply the runner-controller and monitoring roles. 2. `scripts/bake-runner-image.sh --prune-only --dry-run` first — it prints which snapshot each image boots from. **One pair is load-bearing** and deleting it leaves the image unbootable. 3. The retained `predrill-*` pair will show as `named` — visible, un-alerted, out of scope here. ### Verification 33 checks pass, up from 17. `ruff`, shellcheck, Prettier, markdownlint all clean; `ansible-lint` 0 failures across 191 files; gitleaks clean over 272 commits. The alert template renders and parses, and both expressions parse against the metrics backend. The prune path was exercised offline against a stub CLI: live snapshot kept, in-use volume refused, named volume refused, clean leftovers reclaimed. **No OpenStack mutation was performed.**

supernaut lade till 2 incheckningar 2026-08-02 15:56:13 +00:00

feat(monitoring): detect unattached-volume leaks the quota gauge cannot see ... 34f1bc5d90

`os_volume_gb_used` is a quota trend that swings ~380 GiB with ordinary CI
churn, so a leak of ~36 GiB/week disappears into the noise band and reads
clean for weeks — which is how eight stranded 20 GiB volumes accumulated
unnoticed. Unattached is a state, not a trend: in a project where every
legitimate disk is named and attached, a volume in Cinder state `available`
is either in-flight, waste, or deliberately kept.

The controller now surveys those volumes from the listings the orphan sweep
already makes (no extra Cinder reads) and publishes them by class:

  - sweepable       unnamed, no snapshot — the sweep's own target, back to 0
                    within an hour on a healthy project
  - snapshot_pinned a snapshot references it, so Cinder refuses to delete it
                    and the sweep skips it BY DESIGN — one per retained
                    runner-image bake generation
  - named           deliberately retained by an operator; shown, never alerted

Conflating the first two is what hid the leak: the sweep logged a refusal on
every restart while `orphan_volumes_swept` sat at 0, and nothing separated
"nothing to sweep" from "volumes it is forbidden to sweep". Every class
publishes -1 when the Cinder read fails, so an unknown never reads as clean.

Adds RunnerOrphanVolumesUnswept (sweep not reclaiming) and
RunnerSnapshotPinnedVolumesHigh (bake generations piling up), both with
configurable thresholds, plus a dashboard panel and 16 new logic checks.

Refs #320

fix(runner-image): retire old bake generations instead of stranding them ... 554b6c0181

One bake of the `gitborg-runner` image leaves a builder boot volume, a Cinder
snapshot of it, and a Glance image that boots FROM that snapshot. The snapshot
pins the volume — Cinder will not delete a volume that has one — so the exit
trap's `volume delete` always failed, and the runner-controller's orphan sweep
refuses snapshotted volumes on purpose (so it can never destroy image or backup
infrastructure). Nothing else ever pruned them, so every bake stranded 20 GiB.

The bake now retires generations itself after promoting and verifying the new
image: newest-first, keeping RETAIN_GENERATIONS (default 2 = the live image
plus one to roll back to), deleting the snapshot, waiting for it to actually
go, then its volume. Retention is now a stated decision rather than whatever
accumulated.

Two safety lines, both hard:
  - a snapshot referenced by ANY image's block_device_mapping is never deleted
    (that is what the live image boots from — deleting it breaks all CI);
  - a volume is deleted only if Cinder reports it `available` AND unnamed, the
    same two conditions the controller's sweep requires.

Adds --prune-only (operator reclaim path), --dry-run and --no-prune, and stops
the exit trap from pretending to delete a volume it cannot. Also corrects the
misleading `runner_controller_os_boot_volume_size` comment: the image's
block_device_mapping declares volume_size 20 and wins, so the 40 is inert.

Refs #320

supernaut tvångsskickade fix/boot-volume-leak-detector från 554b6c0181 till e57fdaec68 2026-08-02 16:28:31 +00:00 Jämför

supernaut tvångsskickade fix/boot-volume-leak-detector från e57fdaec68 till 6f348fc1f0 2026-08-02 16:35:48 +00:00 Jämför

supernaut lade till 1 incheckning 2026-08-02 17:05:45 +00:00

merge main into fix/boot-volume-leak-detector ... 218e8b5302

#345 landed the capacity-formula fix and pickup-latency instrumentation in the
same four files this branch touches. Every conflict was additive on both sides;
nothing was dropped and no behaviour was invented.

controller.py — kept both. VOLUME_CLASSES and _render_volume_class_series sit
alongside the latency block; write_metrics carries available_volumes,
available_volume_gb AND latency in both its signature and its call site, and its
body emits both metric families.

test_controller_logic.py — git had aligned the two new test groups on shared
'))' scaffolding, which would have interleaved them. Rebuilt from the three
versions instead: main's file, plus this branch's _vol helper and its #320 group
inserted whole. 63/63 checks pass (17 pre-existing + 30 from #345 + 16 here).

gitborg-runners.json — both sides used panel id 13. Kept #345's pickup-latency
row at 13-19 and renumbered this branch's volume timeseries to 20, placed at
y=36 after the latency graph and before the Controller logs row, which shifts
down by its height. Verified: ids unique, no grid overlap. The on-call text
panel lists all four alerts — taking main's copy wholesale had silently dropped
the two added here.

Checks: 63/63 controller tests, ansible-lint 0 failures over 191 files,
syntax-check, shellcheck, prettier and markdownlint all clean.

supernaut kommenterad 2026-08-02 17:06:08 +00:00

Upphovsperson

Ägare

Kopiera länk

Rebased base caught up: main (now including #345) is merged in, and the branch is 0 behind.

#345 landed the capacity-formula fix and pickup-latency instrumentation in the same four files this
branch touches. Three files conflicted; every conflict was additive on both sides, so nothing was
dropped and no behaviour was invented.

controller.py — kept both sides. VOLUME_CLASSES and _render_volume_class_series sit
alongside the latency block, and write_metrics carries available_volumes, available_volume_gb
and latency in its signature and at its call site. Its body — which auto-merged rather than
conflicting, so it was checked separately — emits both metric families.

test_controller_logic.py — git had aligned the two new test groups on shared )) scaffolding,
so a line-wise resolution would have interleaved them into something that parsed but tested nonsense.
Rebuilt from the three versions instead: main's file, plus this branch's _vol helper and its whole
#320 group inserted as a unit. 63/63 checks pass — 17 pre-existing + 30 from #345 + 16 from here,
which is the arithmetic you would expect if nothing was lost.

bitborg-runners.json — both sides used panel id 13, which is a genuine collision since ids
must be unique. Kept #345's pickup-latency row at 13–19 and renumbered this branch's volume
timeseries to 20, placed at y=36 — after the latency graph, before the Controller logs row,
which shifts down by its height. Verified afterwards: ids unique across 20 panels, and no two panels
overlap on the 24-column grid.

One thing worth flagging, because it would have been a silent loss: taking main's copy of the
dashboard wholesale dropped this branch's additions to the on-call text panel. Both PRs had appended
their alert names to the same sentence. The panel now lists all four — RunnerQueueStalled,
RunnerPickupSlow, RunnerOrphanVolumesUnswept, RunnerSnapshotPinnedVolumesHigh.

Also confirmed intact after the auto-merges: all three alert rules, all three thresholds in
monitoring/defaults, the bake script, and this branch's correction to the misleading
runner_controller_os_boot_volume_size comment.

Checks on the merged tree: 63/63 controller tests · ansible-lint 0 failures over 191 files
(profile production) · site.yml --syntax-check clean · shellcheck clean · Prettier clean ·
markdownlint 0 issues. ruff could not be run locally (not installed on this machine) — the merge
introduced no new long lines (longest is 115 against a 120 limit, identical to main) and no
blank-line or trailing-whitespace changes, but CI is the authority there.

The operator actions in the description are unchanged: apply the two roles, then reclaim the stale
pairs with --prune-only --dry-run first, and expect RunnerSnapshotPinnedVolumesHigh to fire until
that is done.

Rebased base caught up: `main` (now including #345) is merged in, and the branch is 0 behind. #345 landed the capacity-formula fix and pickup-latency instrumentation in the same four files this branch touches. Three files conflicted; every conflict was additive on both sides, so nothing was dropped and no behaviour was invented. **`controller.py`** — kept both sides. `VOLUME_CLASSES` and `_render_volume_class_series` sit alongside the latency block, and `write_metrics` carries `available_volumes`, `available_volume_gb` **and** `latency` in its signature and at its call site. Its body — which auto-merged rather than conflicting, so it was checked separately — emits both metric families. **`test_controller_logic.py`** — git had aligned the two new test groups on shared `))` scaffolding, so a line-wise resolution would have interleaved them into something that parsed but tested nonsense. Rebuilt from the three versions instead: main's file, plus this branch's `_vol` helper and its whole #320 group inserted as a unit. **63/63 checks pass** — 17 pre-existing + 30 from #345 + 16 from here, which is the arithmetic you would expect if nothing was lost. **`bitborg-runners.json`** — both sides used **panel id 13**, which is a genuine collision since ids must be unique. Kept #345's pickup-latency row at 13–19 and renumbered this branch's volume timeseries to **20**, placed at `y=36` — after the latency graph, before the *Controller logs* row, which shifts down by its height. Verified afterwards: ids unique across 20 panels, and no two panels overlap on the 24-column grid. One thing worth flagging, because it would have been a silent loss: taking main's copy of the dashboard wholesale dropped this branch's additions to the on-call text panel. Both PRs had appended their alert names to the same sentence. The panel now lists all four — `RunnerQueueStalled`, `RunnerPickupSlow`, `RunnerOrphanVolumesUnswept`, `RunnerSnapshotPinnedVolumesHigh`. Also confirmed intact after the auto-merges: all three alert rules, all three thresholds in `monitoring/defaults`, the bake script, and this branch's correction to the misleading `runner_controller_os_boot_volume_size` comment. **Checks on the merged tree:** 63/63 controller tests · `ansible-lint` 0 failures over 191 files (profile `production`) · `site.yml --syntax-check` clean · shellcheck clean · Prettier clean · markdownlint 0 issues. `ruff` could not be run locally (not installed on this machine) — the merge introduced no new long lines (longest is 115 against a 120 limit, identical to main) and no blank-line or trailing-whitespace changes, but CI is the authority there. The operator actions in the description are unchanged: apply the two roles, then reclaim the stale pairs with `--prune-only --dry-run` first, and expect `RunnerSnapshotPinnedVolumesHigh` to fire until that is done.

supernaut sammanfogade incheckning b0da48e8c6 till main 2026-08-02 17:48:02 +00:00

supernaut tog bort grenen fix/boot-volume-leak-detector 2026-08-02 17:48:03 +00:00

supernaut refererade denna ändringsförfrågan från en incheckning 2026-08-02 17:48:04 +00:00

fix(runner-image): retire old bake generations, and detect the leak the quota gauge cannot see (#346)

supernaut refererade till denna ändringsförfrågan 2026-08-02 18:05:04 +00:00

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

supernaut refererade till denna ändringsförfrågan 2026-08-02 18:05:52 +00:00

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

supernaut refererade denna ändringsförfrågan från en incheckning 2026-08-02 18:15:59 +00:00

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

supernaut refererade till denna ändringsförfrågan 2026-08-02 18:27:22 +00:00

8 orphaned 20 GB boot volumes (160 GB) accumulating since June — the leak metric cannot see it #320

supernaut refererade denna ändringsförfrågan från en incheckning 2026-08-03 09:41:35 +00:00

fix(runner-image): retire old bake generations, and detect the leak the quota gauge cannot see (#346)

supernaut refererade denna ändringsförfrågan från en incheckning 2026-08-03 09:41:35 +00:00

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

Logga in för att delta i denna konversation.

Granskare

Inga granskare

Etiketter

Rensa etiketter

  

area/backups

Backup & restore

  

area/ci

Actions / runners

  

area/control-panel

Logged-in self-service area

  

area/identity

Kanidm / OIDC / entitlements

  

area/infra

Infrastructure (OpenTofu/Ansible/host)

  

area/observability

Monitoring / logs / analytics

  

area/payments

Billing / payment providers

  

area/security

Security & compliance (incl. ISO 27001)

  

area/storage

Volumes / data / LFS

  

area/web

Public web portal (gitborg-web)

  

blocked

Evaluated and specified, but waiting on an external dependency

  

needs-info

Waiting on the reporter for more information

  

needs-triage

Not yet evaluated

  

ready-for-implementation

Fully specified, ready to be picked up

  

type

bug

Something is broken

  

type

chore

Maintenance / housekeeping

  

type

docs

Documentation

  

type

epic

High-level roadmap item

  

type

feature

New capability

  

type

task

Concrete unit of work

  

wontfix

Will not be actioned

Inga etiketter

area/backups

area/ci

area/control-panel

area/identity

area/infra

area/observability

area/payments

area/security

area/storage

area/web

blocked

needs-info

needs-triage

ready-for-implementation

type

bug

type

chore

type

docs

type

epic

type

feature

type

task

wontfix

Milstolpe

Rensa milstolpe

Inga objekt

Ingen milstolpe

Projekt

Rensa projekt

Inga objekt

Inget projekt

Tilldelade

Rensa tilldelade

Inga tilldelade

1 deltagare

Notiser

Prenumerera

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!346

Ingen beskrivning angiven.