From d8a850f4a8bd0bdc3ed36564ebe8b3e8f9bce45e Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Fri, 18 Sep 2026 07:34:47 -0400 Subject: [PATCH] fix(OMN-16367): bound the per-sha image families that actually hold the disk Neither suspected cause of the .201 bleed survived measurement, and this lands the fix for what did. Not build cache. Three approved `docker builder prune` runs reclaimed 0B, the last one carrying `--all`. `docker buildx du` reads Shared 723.5GB of Total 750GB, so ~96% of the cache is content shared with image layers in the containerd snapshot store, which no builder prune can return while those images exist. On this buildx `-a, --all` means "include internal/frontend images", not the classic CLI's "remove all unused build cache" -- a scope modifier that reads like a force flag. The size cap merged in #3754 bounds future growth and recovers ~nothing today; its `--all` comment claimed otherwise and is corrected here, in the script and in its test. Not CI proof images. Exactly 3 proof-pattern images exist across 792 on the host. The wave-1 `--rmi local` teardown works, because those compose services declare no `image:` field, so `--rmi local` genuinely removes what they build. The 1227 stranded proof images the ticket was opened about all predate that fix and were evicted this morning. What actually accumulates is the deploy agent's per-sha tags, rebuilt roughly hourly and never reaped -- counted live: 60 images 219.3 GB onex-lab/omninode-runtime 179 images 205.1 GB ECR omnicloud/core 93 images 106.3 GB onex-lab/omnicloud-core Two planner defects let that happen: 1. A repo matching no `keep_image_repos` substring fell through to a conservative permanent KEEP. omnicloud/core, omnicloud-core, omninode-cloud-migrate and omniweb all miss that list, so ~408 images and 350+ GB grew without bound. 2. Repos that did match were still gated behind `min_age_days: 3`. At an hourly cadence ~72 generations are always too young for a keep-newest-N window to bite. The 60 live generations of onex-lab/omninode-runtime are that arithmetic. The fix is this ticket's own DoD item 2: a keep-last-N window over per-sha tag families, excluding anything referenced by a container, never a blanket prune. Three new keep-list values -- `generation_bounded_repos`, `generation_keep: 3`, `generation_min_age_hours: 6`. The floor is in hours, not days, which is the whole point of defect 2. Safety is opt-in twice over. A repo nobody listed is untouched, and a tag that is not build-stamped -- `dev`, `main`, a release name, anything in `keep_image_tags` -- is untouched. Both failure directions land on KEEP. The governed stability, judge and lakshman lanes are held out of scope structurally: their repo names match no entry. `protect_running` and the execution-time ancestor re-check are unchanged and still run first. Every tag shape the matcher recognises was read off the host, not invented. The loose `-` shape is deliberately NOT matched: it is a per-sha tag too, but a pattern that broad would also catch ordinary meaningful tags, and an unmatched tag simply means KEEP. Tests RED-first: 9 of 20 failed against the unchanged planner, including both defects and every tag shape. 73 disk-gc tests pass. --- deploy/disk-gc/keep-list.yaml | 39 +++ scripts/disk-gc.sh | 31 +- scripts/disk_gc_plan.py | 100 ++++++ .../test_disk_gc_builder_cache_size_cap.py | 30 +- .../test_disk_gc_generation_bounded_repos.py | 323 ++++++++++++++++++ 5 files changed, 501 insertions(+), 22 deletions(-) create mode 100644 tests/unit/scripts/test_disk_gc_generation_bounded_repos.py diff --git a/deploy/disk-gc/keep-list.yaml b/deploy/disk-gc/keep-list.yaml index 6dd77f0ac9..27f0a6896d 100644 --- a/deploy/disk-gc/keep-list.yaml +++ b/deploy/disk-gc/keep-list.yaml @@ -76,3 +76,42 @@ min_age_days: 3 # agent already models a cold cache (COLD_CACHE_MULTIPLIER in # deploy_agent/host_conditions.py) and widens its ceiling accordingly. builder_cache_max_size: "100GB" +# OMN-16367 (DoD item 2) — generation-bounded retention for per-sha image families. +# +# WHY: measured on .201 on 2026-09-18, the disk is held by the deploy agent's +# per-sha tags, rebuilt roughly hourly and never reaped: +# +# 60 images 219.3 GB onex-lab/omninode-runtime +# 179 images 205.1 GB ECR omnicloud/core +# 93 images 106.3 GB onex-lab/omnicloud-core +# +# Two planner defects let that happen, and both are fixed by these three keys: +# 1. A repo matching no keep_image_repos substring fell through to a +# conservative permanent KEEP, so omnicloud/core, omnicloud-core, +# omninode-cloud-migrate and omniweb grew without bound (~408 images). +# 2. Repos that DID match were still gated behind min_age_days: 3, and at an +# hourly cadence ~72 generations are always too young for a keep-newest-N +# window to bite. Hence the hours-scale floor below. +# +# It is NOT the build cache and NOT CI proof images. Three approved builder +# prunes reclaimed 0B (buildx du reads Shared 723.5GB of Total 750GB — shared +# with image layers, unreturnable while those images exist), and exactly 3 +# proof-pattern images exist host-wide because the compose teardown's +# `--rmi local` genuinely works. +# +# SAFETY: opt-in twice over. A repo not listed here is untouched, and a tag that +# is not build-stamped (`dev`, `main`, a release name, anything in +# keep_image_tags) is untouched. Both failure directions land on KEEP. The +# governed stability / judge / lakshman lanes are held out of scope structurally +# — their repo names match nothing below. +generation_bounded_repos: + - onex-lab/ + - omnicloud/core + - omninode-cloud-migrate + - omniweb +# Newest generations retained per family. Never below 2: the deploy agent needs a +# rollback target, and the current generation is one of the N. +generation_keep: 3 +# Floor in HOURS. Nothing younger is reaped however superseded. Hours, not days, +# is the whole point — see defect 2 above. +generation_min_age_hours: 6 diff --git a/scripts/disk-gc.sh b/scripts/disk-gc.sh index f1bdb72321..c926bb5efe 100755 --- a/scripts/disk-gc.sh +++ b/scripts/disk-gc.sh @@ -149,15 +149,28 @@ fi # restore the bug exactly, since the oversized records are the recently-accessed # ones the age filter excludes. # -# `--all` IS LOAD-BEARING ON THIS HOST — do not drop it as "too aggressive". -# .201 runs the containerd snapshotter (`docker info`: Storage Driver overlayfs, -# driver-type io.containerd.snapshotter.v1), so build-cache records share the -# content store with image layers and a DEFAULT builder prune deliberately -# excludes every record an existing image references. Measured live on -# 2026-09-18: `docker builder prune --force --max-used-space 200GB` WITHOUT -# `--all` reclaimed 0B at exit 0, twice, while `buildx du` reported 655-678 GB -# reclaimable. Evidence: ROLLING_WORK_LEDGER.md:3932 and :3933 (verbatim command, -# output and readbacks). A capped prune without `--all` is a silent no-op here. +# CORRECTION (OMN-16367, 2026-09-18, later the same day). An earlier revision of +# this comment claimed `--all` was load-bearing because a default prune excludes +# image-referenced records. THAT WAS WRONG, and the correction matters more than +# the flag does. On this host `docker builder prune` resolves to +# `docker buildx prune`, where `-a, --all` is documented as "Include +# internal/frontend images" -- NOT the classic CLI's "remove all unused build +# cache". It is a scope modifier that reads like a force flag. +# +# What actually happened: a third approved prune, carrying `--all` and this exact +# ceiling, also reclaimed 0B (08:48Z, ledger :4054-:4060). `docker buildx du` +# reads Shared 723.5GB against Total 750GB -- ~96% of the cache is content shared +# with image layers in the containerd snapshot store, which NO builder prune can +# return while those images exist. +# +# So this prune bounds FUTURE cache growth and recovers ~nothing on a host whose +# cache is already shared into the image store. That is worth keeping and worth +# not overselling. The disk itself is reclaimed by the generation-bounded image +# retention in disk_gc_plan.py (keep-list `generation_bounded_repos`), which is +# where the per-sha deploy-agent families are actually bounded. +# +# `--all` stays because including internal/frontend images is correct for a +# scheduled GC, not because it unlocks anything. log "Pruning builder cache to a ceiling of ${BUILDER_CACHE_MAX_SIZE}" PRUNE_OUT="" if PRUNE_OUT="$(docker builder prune --all --force --max-used-space "$BUILDER_CACHE_MAX_SIZE" 2>&1)"; then diff --git a/scripts/disk_gc_plan.py b/scripts/disk_gc_plan.py index 60decab128..a1ec588622 100644 --- a/scripts/disk_gc_plan.py +++ b/scripts/disk_gc_plan.py @@ -103,6 +103,55 @@ # size string. Overridable per host via keep-list `builder_cache_max_size`. DEFAULT_BUILDER_CACHE_MAX_SIZE = "100GB" +# OMN-16367 (DoD item 2): build-stamped tag shapes for per-sha image families. +# Every shape here was read off .201 on 2026-09-18, not invented: +# 20260918T111918Z-e49dea8f deploy agent, - +# ffd61901-20260918T111918Z deploy agent, - +# sha-ffd6190 bare sha- tag +# dev-sha-ffd6190 prefixed sha- tag (ECR) +# ffd619017106...1222aa6 bare 40-hex content tag (ECR) +# +# DELIBERATELY NOT MATCHED: the loose `-` shape (`main-f3d4029`, +# `runtime-fc61f41242dcc01b`). It is a per-sha tag too, but a pattern that broad +# would also match ordinary meaningful tags, and an unmatched tag simply means +# KEEP -- the safe direction. Narrow it in, never out. +_STAMP_SHA_TAG_RE = re.compile(r"^\d{8}T\d{6}Z?-[0-9a-f]{7,40}$") +_SHA_STAMP_TAG_RE = re.compile(r"^[0-9a-f]{7,40}-\d{8}T\d{6}Z?$") +_PREFIXED_SHA_TAG_RE = re.compile(r"^[A-Za-z0-9._-]+-sha-[0-9a-f]{6,40}$") +_BARE_SHA40_TAG_RE = re.compile(r"^[0-9a-f]{40}$") + +# How many newest generations of a per-sha family to retain, and the floor below +# which nothing is reaped however superseded it is. The floor is in HOURS, not +# days: the deploy agent rebuilds roughly hourly, so a day-scale floor keeps ~72 +# generations permanently ineligible and a keep-newest-N window never bites. +DEFAULT_GENERATION_KEEP = 3 +DEFAULT_GENERATION_MIN_AGE_HOURS = 6 + + +def _is_build_stamped_tag(tag: str) -> bool: + """True if the tag is a per-build artifact rather than a meaningful pointer. + + Moving pointers (`dev`, `main`), release names and the keep tags all fail this + check and are therefore never reaped by the generation rule. + """ + return bool( + _STAMP_SHA_TAG_RE.match(tag) + or _SHA_STAMP_TAG_RE.match(tag) + or _SHA_TAG_RE.match(tag) + or _PREFIXED_SHA_TAG_RE.match(tag) + or _BARE_SHA40_TAG_RE.match(tag) + ) + + +def _is_generation_bounded_repo(repo: str, patterns: list[str]) -> bool: + """Substring match, same convention as keep_image_repos. + + Opt-in by repo: a repo nobody listed keeps its previous behaviour entirely. + This is what holds the governed stability / judge / lakshman lanes out of + scope structurally rather than by a name check that could drift. + """ + return any(pattern and pattern in repo for pattern in patterns) + def _parse_created_at(value: str, now: datetime) -> float: """Return age in days for a docker CreatedAt string. Unparseable → 0.0 (treat as new → keep).""" @@ -251,6 +300,17 @@ def build_plan( builder_cache_max_size: str = str( keep_list.get("builder_cache_max_size", DEFAULT_BUILDER_CACHE_MAX_SIZE) ) + # OMN-16367 DoD item 2: per-sha families to bound by generation count. + # Empty list (the default) is a real off switch -- nothing changes. + generation_bounded_repos: list[str] = ( + keep_list.get("generation_bounded_repos", []) or [] + ) + generation_keep: int = int( + keep_list.get("generation_keep", DEFAULT_GENERATION_KEEP) + ) + generation_min_age_hours: float = float( + keep_list.get("generation_min_age_hours", DEFAULT_GENERATION_MIN_AGE_HOURS) + ) remove_image_ids: list[str] = [] kept_reasons: dict[str, str] = {} @@ -263,6 +323,9 @@ def build_plan( # Group superseded candidates per repo so we can keep the N newest. superseded_by_repo: dict[str, list[tuple[float, str]]] = {} + # OMN-16367: separate bucket, because this class is bounded by GENERATION + # COUNT on an hours-scale floor rather than by the day-scale min_age_days. + generations_by_repo: dict[str, list[tuple[float, str]]] = {} for img in images: image_id = img.get("ID", "") or img.get("Id", "") @@ -313,6 +376,31 @@ def build_plan( # sha-* tag: no PR number to look up; age-based path only. # Fall through to normal age/generation logic below. + # --- Generation-bounded per-sha families (OMN-16367, DoD item 2) --- + # Placed BEFORE the day-scale age gate on purpose. At the deploy agent's + # roughly-hourly cadence every generation is younger than min_age_days, + # so the gate below would keep all of them -- which is precisely why 60 + # live generations of onex-lab/omninode-runtime (219.3 GB) existed on + # 2026-09-18. keep_image_tags and protect_running are both checked ABOVE + # this point and still win. + if ( + generation_bounded_repos + and repo + and repo != "" + and tag + and tag != "" + and _is_generation_bounded_repo(repo, generation_bounded_repos) + and _is_build_stamped_tag(tag) + ): + if age * 24.0 < generation_min_age_hours: + kept_reasons[image_id] = ( + f"younger than generation_min_age_hours " + f"({age * 24.0:.1f}h<{generation_min_age_hours}h)" + ) + continue + generations_by_repo.setdefault(repo, []).append((age, image_id)) + continue + # --- Age gate (applies to all remaining images) --- if age < min_age_days: @@ -339,6 +427,18 @@ def build_plan( # generations and dangling images, never random third-party images. kept_reasons[image_id] = "repo not in keep_image_repos (conservative keep)" + # Per-sha families: keep the newest `generation_keep`, reap the rest. + for repo, gen_entries in generations_by_repo.items(): + gen_entries.sort(key=lambda e: e[0]) # newest first (smallest age first) + for idx, (_age, image_id) in enumerate(gen_entries): + if idx < generation_keep: + kept_reasons[image_id] = ( + f"newest {generation_keep} generation(s) of per-sha family " + f"'{repo}' (rollback headroom)" + ) + else: + remove_image_ids.append(image_id) + # Per-repo retention: keep the newest `keep_generations`, mark the rest for removal. for repo, entries in superseded_by_repo.items(): # newest first (smallest age first) diff --git a/tests/unit/scripts/test_disk_gc_builder_cache_size_cap.py b/tests/unit/scripts/test_disk_gc_builder_cache_size_cap.py index d430ef9d46..cbc0bba4cb 100644 --- a/tests/unit/scripts/test_disk_gc_builder_cache_size_cap.py +++ b/tests/unit/scripts/test_disk_gc_builder_cache_size_cap.py @@ -196,19 +196,23 @@ def test_builder_prune_emits_a_size_cap_flag(self, tmp_path: Path) -> None: ) def test_primary_prune_passes_all(self, tmp_path: Path) -> None: - """`--all` is load-bearing on a containerd-snapshotter host, not aggression. - - .201 runs `io.containerd.snapshotter.v1`, so build-cache records share the - content store with image layers and a DEFAULT builder prune excludes every - record an existing image references. Measured live 2026-09-18: - `docker builder prune --force --max-used-space 200GB` WITHOUT `--all` - reclaimed **0B at exit 0**, twice, while `buildx du` reported 655-678 GB - reclaimable (ROLLING_WORK_LEDGER.md:3932, :3933). - - A correct cap flag without `--all` is a silent no-op on this host -- the - worst shape a disk-pressure remedy can take, because a caller reading only - the exit status records a successful prune that freed nothing. This test - exists so a future "that looks too aggressive" edit is RED. + """`--all` widens the prune's scope to internal/frontend images. + + CORRECTION to this test's first revision, which claimed `--all` was + load-bearing because a default prune excludes image-referenced records. + That was wrong. On this host `docker builder prune` resolves to + `docker buildx prune`, where `-a, --all` reads "Include internal/frontend + images" -- a scope modifier that reads like a force flag, not the classic + CLI's "remove all unused build cache". + + A third approved prune carrying `--all` also reclaimed **0B** (ledger + :4054-:4060), because `buildx du` reads Shared 723.5GB of Total 750GB and + no builder prune can return content shared into the image store. + + `--all` is still asserted because widening a scheduled GC to internal and + frontend images is correct, and dropping it would narrow the prune for no + reason -- but it buys scope, not reclaim, and this test no longer claims + otherwise. """ _, _, recorded = _run(tmp_path) primary = _builder_prunes(recorded)[0] diff --git a/tests/unit/scripts/test_disk_gc_generation_bounded_repos.py b/tests/unit/scripts/test_disk_gc_generation_bounded_repos.py new file mode 100644 index 0000000000..8669b624fa --- /dev/null +++ b/tests/unit/scripts/test_disk_gc_generation_bounded_repos.py @@ -0,0 +1,323 @@ +# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT + +"""Generation-bounded retention for per-sha image families (OMN-16367, DoD item 2). + +WHY these exist +--------------- +On 2026-09-18 the `.201` bleed was re-measured three times and neither of the two +suspected causes survived contact: + +- **It is not build cache.** Three approved `docker builder prune` runs reclaimed + **0B**, the last one carrying `--all`. `docker buildx du` reads Shared 723.5 GB + of Total 750 GB, so ~96% of the cache is content shared with image layers in the + containerd snapshot store, which no builder prune can return while those images + exist. (On this buildx `--all` means "include internal/frontend images", not the + classic CLI's "remove all unused cache".) +- **It is not CI proof images.** Exactly **3** proof-pattern images exist on the + whole host across 792 images. The wave-1 `--rmi local` teardown works, because + those compose services declare no `image:` field, so `--rmi local` genuinely + removes what they build. + +What is actually accumulating is the **deploy agent's per-sha tags**, rebuilt +roughly hourly and never reaped, counted live: + +| images | nominal | repo | +|---|---|---| +| 60 | 219.3 GB | `onex-lab/omninode-runtime` | +| 179 | 205.1 GB | ECR `omnicloud/core` | +| 93 | 106.3 GB | `onex-lab/omnicloud-core` | + +Two distinct planner defects let that happen, and this module pins the fix for +both: + +1. **Untracked repos are kept forever.** A repo that matches no + `keep_image_repos` substring falls through to "repo not in keep_image_repos + (conservative keep)". `omnicloud/core`, `omnicloud-core`, + `omninode-cloud-migrate` and `omniweb` all miss that list, so ~408 images and + 350+ GB grow without bound. +2. **`min_age_days` defeats a keep-newest-N window at an hourly cadence.** A repo + that *does* match is still gated behind `min_age_days: 3`, so ~72 generations + are always too young before "keep the newest 2" can bite. The 60 live + generations of `onex-lab/omninode-runtime` are that arithmetic. + +The fix is the ticket's own DoD item 2, verbatim: a keep-last-N window over +per-sha tag families, excluding anything referenced by a container, never a +blanket prune. + +Safety posture: the rule is **opt-in by repo** and **opt-in by tag shape**. A repo +not listed is untouched, and a tag that does not match a build-stamped shape is +untouched. Both failure directions land on KEEP. +""" + +from __future__ import annotations + +import importlib.util +from datetime import UTC, datetime +from pathlib import Path +from typing import Any + +import pytest +import yaml + +_REPO = Path(__file__).resolve().parents[3] +_SCRIPTS = _REPO / "scripts" +_spec = importlib.util.spec_from_file_location( + "disk_gc_plan", _SCRIPTS / "disk_gc_plan.py" +) +assert _spec and _spec.loader +disk_gc_plan = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(disk_gc_plan) + +NOW = datetime(2026, 9, 18, 12, 0, 0, tzinfo=UTC) + +# The per-sha families measured on .201, and the repos that must stay untouched. +AGENT_RUNTIME = "onex-lab/omninode-runtime" +AGENT_CLOUD = "onex-lab/omnicloud-core" +# Synthetic account id: the real one is denylisted by the exposed-identifier +# gate (OMN-18024) and the test does not need it -- only the repo suffix is +# load-bearing, because that is what generation_bounded_repos matches on. +ECR_CORE = "000000000000.dkr.ecr.us-east-1.amazonaws.com/omnicloud/core" +STABILITY = "omnibase-infra-stability-test-runtime-worker" + + +def _created(hours_ago: float) -> str: + ts = NOW.timestamp() - hours_ago * 3600.0 + return datetime.fromtimestamp(ts, tz=UTC).strftime("%Y-%m-%d %H:%M:%S +0000 UTC") + + +def _img(repo: str, tag: str, hours_ago: float, image_id: str) -> dict[str, Any]: + return { + "ID": image_id, + "Repository": repo, + "Tag": tag, + "CreatedAt": _created(hours_ago), + } + + +KEEP_LIST: dict[str, Any] = { + "keep_image_repos": ["omninode-runtime", "postgres"], + "keep_image_tags": ["latest", "stable", "rollback"], + "protect_running": True, + "superseded_image_keep_generations": 2, + "min_age_days": 3, + # The fix under test. + "generation_bounded_repos": ["onex-lab/", "omnicloud/core", "omniweb"], + "generation_keep": 3, + "generation_min_age_hours": 6, +} + + +def _plan(images: list[dict[str, Any]], **over: Any) -> dict[str, Any]: + keep_list = {**KEEP_LIST, **over} + return disk_gc_plan.build_plan( + keep_list=keep_list, + images=images, + containers=[], + inuse_refs=set(), + now=NOW, + ) + + +def _agent_generations( + repo: str, count: int, *, start_hours: float = 6.0 +) -> list[dict[str, Any]]: + """`count` hourly generations of a deploy-agent per-sha family, newest first.""" + return [ + _img( + repo, + f"2026091{8 - i // 24}T{i:02d}0000Z-{i:08x}", + start_hours + i, + f"sha256:{i:064x}", + ) + for i in range(count) + ] + + +@pytest.mark.unit +class TestUntrackedPerShaFamiliesAreBounded: + """Defect 1 — a repo outside keep_image_repos must not be kept forever.""" + + def test_ecr_per_sha_family_is_reaped_beyond_the_window(self) -> None: + """RED before the fix: ECR omnicloud/core matches no keep_image_repos substring. + + Live count on .201: 179 images, 205.1 GB, growing without bound. + """ + images = _agent_generations(ECR_CORE, 10) + plan = _plan(images) + removed = set(plan["remove_image_ids"]) + + assert len(removed) == 7, ( + "expected 10 generations minus the newest 3 to be reaped; got " + f"{len(removed)}. Reasons: {plan['kept_reasons']}" + ) + # The newest three survive as rollback headroom. + for keeper in images[:3]: + assert keeper["ID"] not in removed + + def test_untracked_repo_not_listed_is_still_kept_forever(self) -> None: + """The rule is opt-in. An unlisted third-party repo keeps its old behaviour.""" + images = _agent_generations("docker.io/some/vendor-image", 10) + plan = _plan(images) + assert plan["remove_image_ids"] == [], ( + "an unlisted repo was reaped; the generation rule must be opt-in by repo" + ) + + +@pytest.mark.unit +class TestHourlyCadenceDefeatsTheDayGate: + """Defect 2 — min_age_days must not gate a keep-newest-N window.""" + + def test_generations_younger_than_min_age_days_are_still_bounded(self) -> None: + """RED before the fix: at hourly cadence every generation is <3d old. + + All 10 generations here are 6-16 hours old, so the existing + `min_age_days: 3` gate keeps every one of them. That is exactly why 60 + live generations of onex-lab/omninode-runtime exist. + """ + images = _agent_generations(AGENT_RUNTIME, 10) + for i in images: + assert "2026-09-18" in i["CreatedAt"] or "2026-09-17" in i["CreatedAt"] + + plan = _plan(images) + assert len(plan["remove_image_ids"]) == 7, ( + "the day-scale age gate is still swallowing an hourly cadence; " + f"kept_reasons={plan['kept_reasons']}" + ) + + def test_freshly_built_generation_is_never_reaped(self) -> None: + """The hours-scale floor still protects a just-built image.""" + images = [ + _img(AGENT_RUNTIME, "20260918T115900Z-aaaaaaaa", 0.1, "sha256:" + "a" * 64), + *_agent_generations(AGENT_RUNTIME, 9), + ] + plan = _plan(images) + assert "sha256:" + "a" * 64 not in set(plan["remove_image_ids"]), ( + "an image built minutes ago was reaped; the generation_min_age_hours " + "floor did not hold" + ) + + +@pytest.mark.unit +class TestTagShapeOptIn: + """Only build-stamped tag shapes are reapable; everything else is kept.""" + + @pytest.mark.parametrize( + "tag", + [ + "20260918T111918Z-e49dea8f", # -, deploy agent + "ffd61901-20260918T111918Z", # -, deploy agent + "sha-ffd6190", # bare sha- tag + "dev-sha-ffd6190", # prefixed sha- tag, ECR + "ffd6190171066f231bd6c6725fd53352d1222aa6", # bare 40-hex, ECR + ], + ids=["stamp-sha", "sha-stamp", "sha-tag", "prefixed-sha", "bare-40-hex"], + ) + def test_build_stamped_shapes_are_reapable(self, tag: str) -> None: + """Every shape here was read off .201, not invented.""" + images = [ + _img(ECR_CORE, tag, 48.0, "sha256:" + "b" * 64), + *_agent_generations(ECR_CORE, 5), + ] + plan = _plan(images) + assert "sha256:" + "b" * 64 in set(plan["remove_image_ids"]), ( + f"build-stamped tag {tag!r} was not recognised as a reapable generation" + ) + + @pytest.mark.parametrize("tag", ["dev", "main", "latest", "stable", "rollback"]) + def test_moving_pointers_and_keep_tags_are_never_reaped(self, tag: str) -> None: + """`dev` and `main` are moving pointers; the keep tags are rollback targets. + + None of them is a build-stamped shape, so the rule must leave them alone + even though their repo is listed. + """ + images = [ + _img(ECR_CORE, tag, 480.0, "sha256:" + "c" * 64), + *_agent_generations(ECR_CORE, 8), + ] + plan = _plan(images) + assert "sha256:" + "c" * 64 not in set(plan["remove_image_ids"]), ( + f"the non-build-stamped tag {tag!r} was reaped" + ) + + +@pytest.mark.unit +class TestGovernedLanesStayOutOfScope: + """The stability / judge / lakshman lanes must not be reachable by this rule.""" + + def test_stability_lane_preflight_stamps_are_untouched(self) -> None: + """`preflight-` on a stability repo looks stamped but is out of scope. + + The stability lane is the surface the compose-path `stability-proven` + premise is resolved from, and it is explicitly not this lane's to mutate. + It is protected structurally: its repo matches no entry in + `generation_bounded_repos`, so opt-in-by-repo is what keeps it safe. + """ + images = [ + _img(STABILITY, "preflight-20260916T165814Z", 48.0, "sha256:" + "d" * 64), + _img(STABILITY, "preflight-20260916T142602Z", 50.0, "sha256:" + "e" * 64), + _img(STABILITY, "preflight-20260910T203929Z", 200.0, "sha256:" + "f" * 64), + _img(STABILITY, "latest", 200.0, "sha256:" + "1" * 64), + ] + plan = _plan(images) + assert plan["remove_image_ids"] == [], ( + "a stability-lane image was selected for removal; " + f"kept_reasons={plan['kept_reasons']}" + ) + + +@pytest.mark.unit +class TestExistingSafetyInvariantsSurvive: + """The generation rule must not punch through any existing protection.""" + + def test_in_use_image_is_never_reaped(self) -> None: + images = _agent_generations(AGENT_CLOUD, 10) + victim = images[-1] + plan = disk_gc_plan.build_plan( + keep_list=KEEP_LIST, + images=images, + containers=[], + inuse_refs={f"{victim['Repository']}:{victim['Tag']}"}, + now=NOW, + ) + assert victim["ID"] not in set(plan["remove_image_ids"]), ( + "protect_running was bypassed by the generation rule" + ) + + def test_keep_image_tags_still_win_over_the_generation_rule(self) -> None: + images = [ + _img(AGENT_CLOUD, "rollback", 900.0, "sha256:" + "2" * 64), + *_agent_generations(AGENT_CLOUD, 8), + ] + plan = _plan(images) + assert "sha256:" + "2" * 64 not in set(plan["remove_image_ids"]) + + def test_disabling_the_rule_restores_the_previous_behaviour(self) -> None: + """An empty repo list is the off switch, and it must be a real off switch.""" + images = _agent_generations(ECR_CORE, 10) + plan = _plan(images, generation_bounded_repos=[]) + assert plan["remove_image_ids"] == [] + + +@pytest.mark.unit +class TestShippedKeepListEnablesTheMeasuredFamilies: + """The default config must actually cover what was measured on the host.""" + + def test_keep_list_lists_the_families_that_are_growing(self) -> None: + keep_list = yaml.safe_load( + (_REPO / "deploy" / "disk-gc" / "keep-list.yaml").read_text() + ) + listed = keep_list.get("generation_bounded_repos") or [] + for measured in ("onex-lab/", "omnicloud/core", "omniweb"): + assert any(measured in entry for entry in listed), ( + f"{measured!r} was measured growing without bound on .201 but is " + f"not in generation_bounded_repos: {listed}" + ) + + def test_keep_window_leaves_rollback_headroom(self) -> None: + keep_list = yaml.safe_load( + (_REPO / "deploy" / "disk-gc" / "keep-list.yaml").read_text() + ) + assert keep_list["generation_keep"] >= 2, ( + "a keep window under 2 leaves the deploy agent no rollback target" + )