Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions deploy/disk-gc/keep-list.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
31 changes: 22 additions & 9 deletions scripts/disk-gc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
100 changes: 100 additions & 0 deletions scripts/disk_gc_plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, <stamp>-<sha>
# ffd61901-20260918T111918Z deploy agent, <sha>-<stamp>
# 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 `<word>-<hex>` 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)."""
Expand Down Expand Up @@ -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] = {}
Expand All @@ -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", "")
Expand Down Expand Up @@ -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 != "<none>"
and tag
and tag != "<none>"
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:
Expand All @@ -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)
Expand Down
30 changes: 17 additions & 13 deletions tests/unit/scripts/test_disk_gc_builder_cache_size_cap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading
Loading