diff --git a/cmd/atelet/imagegc.go b/cmd/atelet/imagegc.go index 1bc7e1dac..4eb8b63f3 100644 --- a/cmd/atelet/imagegc.go +++ b/cmd/atelet/imagegc.go @@ -42,11 +42,11 @@ import ( ) var ( - imageCacheGCPeriod = pflag.Duration("image-cache-gc-period", 5*time.Minute, "How often to run the image cache eviction pass. 0 disables eviction entirely.") + imageCacheGCPeriod = pflag.Duration("image-cache-gc-period", 5*time.Minute, "How often to run the image cache eviction pass. 0 disables the periodic pass (startup orphan recovery still runs at every atelet start).") imageCacheHighPct = pflag.Int("image-cache-high-percent", 85, "Cache-volume usage percentage above which eviction starts.") imageCacheLowPct = pflag.Int("image-cache-low-percent", 80, "Cache-volume usage percentage eviction frees down to. Must be lower than --image-cache-high-percent.") imageCacheMaxBytes = pflag.Int64("image-cache-max-bytes", 0, "Absolute cap on the summed size of cached layers, evicted down to independently of the volume watermarks. 0 means no cap.") - imageCacheMinAge = pflag.Duration("image-cache-min-age", 2*time.Minute, "Layers and image records younger than this are never evicted (protects images pulled but not yet mounted).") + imageCacheMinAge = pflag.Duration("image-cache-min-age", 2*time.Minute, "Layers and image records younger than this are never evicted (protects images pulled but not yet mounted). Governs startup orphan recovery too, so it is live even with the periodic pass disabled.") imageCacheGCDryRun = pflag.Bool("image-cache-gc-dry-run", false, "Compute and log eviction decisions without deleting anything.") ) @@ -60,6 +60,11 @@ const ( ) func validateImageCacheGCFlags() error { + if *imageCacheGCPeriod < 0 { + // A negative period would silently disable the loop (it fails the + // > 0 guard at the launch site, which also protects the ticker). + return fmt.Errorf("--image-cache-gc-period %v must be >= 0", *imageCacheGCPeriod) + } if *imageCacheHighPct < 1 || *imageCacheHighPct > 100 { return fmt.Errorf("--image-cache-high-percent %d out of range [1,100]", *imageCacheHighPct) } @@ -71,11 +76,7 @@ func validateImageCacheGCFlags() error { // future), making just-pulled layers evictable. return fmt.Errorf("--image-cache-min-age %v must be >= 0", *imageCacheMinAge) } - // Outside BasePath the watermarks measure a different volume than - // actor state, so the pass would chase pressure the cache doesn't - // contribute to. Warn, don't fail: a separate cache volume is - // legitimate (recommended for IOPS) — it just wants its own numbers. - if !strings.HasPrefix(filepath.Clean(*imageCacheDir), ateompath.BasePath+string(os.PathSeparator)) { + if imageCacheDirOutsideBasePath(*imageCacheDir) { slog.Warn("Image cache dir is outside the ateom base path; its volume watermarks are measured separately from actor state", slog.String("image_cache_dir", *imageCacheDir), slog.String("actors_dir", ateompath.ActorsDir)) @@ -83,6 +84,18 @@ func validateImageCacheGCFlags() error { return nil } +// imageCacheDirOutsideBasePath reports whether the cache dir is outside +// the ateom base path — the watermarks then measure a different volume +// than actor state. Warn-worthy, not an error: a separate cache volume is +// legitimate (recommended for IOPS). +func imageCacheDirOutsideBasePath(dir string) bool { + abs, err := filepath.Abs(dir) + if err != nil { + abs = filepath.Clean(dir) + } + return !strings.HasPrefix(abs, ateompath.BasePath+string(os.PathSeparator)) +} + // imageCacheGCTarget computes the bytes a pass should free: the larger // of the watermark shortfall (kubelet's formula — usage at highPct frees // down to lowPct) and the pool's overage past maxBytes, but never more @@ -121,11 +134,18 @@ func imageCacheGCTarget(capacity, available uint64, cacheSize, maxBytes int64, h return target } +// gcStore is what the loop needs from *imagecache.Store — a seam so +// runPass's skip and recovery paths are testable without a real pool. +type gcStore interface { + CacheSize() (int64, error) + EvictUnused(ctx context.Context, targetBytes int64, dryRun bool) (imagecache.EvictStats, error) +} + // imageCacheGC is the loop's state: configuration snapshotted from the // flags at construction (the pass logic never reads globals, so it is // testable without flag juggling) plus the shortfall-backoff counter. type imageCacheGC struct { - store *imagecache.Store + store gcStore cacheDir string period time.Duration highPct int @@ -151,6 +171,10 @@ func newImageCacheGC(store *imagecache.Store, cacheDir string) *imageCacheGC { // Run executes eviction passes on the configured period until ctx is // done. Passes are strictly serialized: a slow pass delays the next tick // rather than overlapping it. +// +// atelet passes its root context (the StartMetricsServer convention), so +// the loop dies with the process; a pass cut off there leaves only .rm-* +// dirs for the startup sweep. Cancellation is honored for tests. func (g *imageCacheGC) Run(ctx context.Context) { // First pass immediately: a node booting under disk pressure must not // wait a full period (startup recovery reclaims debris, not pressure). @@ -221,7 +245,13 @@ func (g *imageCacheGC) runPass(ctx context.Context) { slog.Bool("dry_run", g.dryRun), slog.Duration("took", time.Since(tStart)), } - outcome := classifyGCPass(err, target, stats.FreedBytes) + g.noteOutcome(ctx, classifyGCPass(err, target, stats.FreedBytes), err, attrs) +} + +// noteOutcome logs one finished pass and advances the shortfall backoff. +// The counter survives a gated pass (which says nothing about whether the +// cache can meet a target) and resets when a target is met or absent. +func (g *imageCacheGC) noteOutcome(ctx context.Context, outcome gcPassOutcome, err error, attrs []any) { if outcome == gcPassSkipped { slog.ErrorContext(ctx, "Image cache GC pass skipped", append(attrs, slog.Any("err", err))...) return diff --git a/cmd/atelet/imagegc_test.go b/cmd/atelet/imagegc_test.go index 86e418012..99df8d3a3 100644 --- a/cmd/atelet/imagegc_test.go +++ b/cmd/atelet/imagegc_test.go @@ -15,11 +15,17 @@ package main import ( + "bytes" + "context" "errors" "fmt" + "log/slog" + "path/filepath" + "strings" "testing" "time" + "github.com/agent-substrate/substrate/internal/ateompath" "github.com/agent-substrate/substrate/internal/imagecache" ) @@ -112,38 +118,224 @@ func TestImageCacheGCTarget(t *testing.T) { } func TestValidateImageCacheGCFlags(t *testing.T) { - setFlags := func(high, low int, minAge time.Duration) { + setFlags := func(period time.Duration, high, low int, minAge time.Duration) { + *imageCacheGCPeriod = period *imageCacheHighPct = high *imageCacheLowPct = low *imageCacheMinAge = minAge } - t.Cleanup(func() { setFlags(85, 80, 2*time.Minute) }) + t.Cleanup(func() { setFlags(5*time.Minute, 85, 80, 2*time.Minute) }) cases := []struct { name string + period time.Duration high, low int minAge time.Duration wantErr bool }{ - {"defaults", 85, 80, 2 * time.Minute, false}, - {"boundary high=100 low=0", 100, 0, 0, false}, - {"high over 100", 101, 80, 0, true}, - {"low equals high", 85, 85, 0, true}, - {"low above high", 85, 90, 0, true}, - {"negative low", 85, -1, 0, true}, - {"negative min-age inverts the veto", 85, 80, -time.Second, true}, + {"defaults", 5 * time.Minute, 85, 80, 2 * time.Minute, false}, + {"boundary high=100 low=0", 5 * time.Minute, 100, 0, 0, false}, + {"zero period disables the periodic pass", 0, 85, 80, 0, false}, + {"negative period would silently disable the loop", -5 * time.Minute, 85, 80, 0, true}, + {"high over 100", 5 * time.Minute, 101, 80, 0, true}, + {"low equals high", 5 * time.Minute, 85, 85, 0, true}, + {"low above high", 5 * time.Minute, 85, 90, 0, true}, + {"negative low", 5 * time.Minute, 85, -1, 0, true}, + {"negative min-age inverts the veto", 5 * time.Minute, 85, 80, -time.Second, true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - setFlags(tc.high, tc.low, tc.minAge) + setFlags(tc.period, tc.high, tc.low, tc.minAge) err := validateImageCacheGCFlags() if (err != nil) != tc.wantErr { - t.Errorf("high=%d low=%d minAge=%v: err=%v, wantErr=%v", tc.high, tc.low, tc.minAge, err, tc.wantErr) + t.Errorf("period=%v high=%d low=%d minAge=%v: err=%v, wantErr=%v", tc.period, tc.high, tc.low, tc.minAge, err, tc.wantErr) } }) } } +func TestImageCacheDirOutsideBasePath(t *testing.T) { + cases := []struct { + name string + dir string + want bool + }{ + {"inside", filepath.Join(ateompath.BasePath, "image-cache"), false}, + {"inside with doubled separator", ateompath.BasePath + "//image-cache", false}, + {"inside via dot-dot", ateompath.BasePath + "/x/../image-cache", false}, + {"base path itself is not inside", ateompath.BasePath, true}, + {"sibling with the base path as name prefix", ateompath.BasePath + "-other/image-cache", true}, + {"outside", "/var/lib/elsewhere/image-cache", true}, + {"dot-dot escaping the base path", ateompath.BasePath + "/../elsewhere/image-cache", true}, + {"relative resolves against the cwd, not the base path", "image-cache", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := imageCacheDirOutsideBasePath(tc.dir); got != tc.want { + t.Errorf("imageCacheDirOutsideBasePath(%q) = %v, want %v", tc.dir, got, tc.want) + } + }) + } +} + +type fakeGCStore struct { + size int64 + sizeErr error + sizeCalls int + evictCalls int + gotTarget int64 + gotDryRun bool + evictErr error + stats imagecache.EvictStats + panicOnEvict bool +} + +func (f *fakeGCStore) CacheSize() (int64, error) { + f.sizeCalls++ + return f.size, f.sizeErr +} + +func (f *fakeGCStore) EvictUnused(_ context.Context, target int64, dryRun bool) (imagecache.EvictStats, error) { + f.evictCalls++ + f.gotTarget = target + f.gotDryRun = dryRun + if f.panicOnEvict { + panic("boom") + } + return f.stats, f.evictErr +} + +func TestRunPassSkipsOnStatfsFailure(t *testing.T) { + fake := &fakeGCStore{} + g := &imageCacheGC{store: fake, cacheDir: filepath.Join(t.TempDir(), "missing"), highPct: 85, lowPct: 80} + g.runPass(context.Background()) + if fake.sizeCalls != 0 || fake.evictCalls != 0 { + t.Errorf("statfs failure: sizeCalls=%d evictCalls=%d, want 0/0", fake.sizeCalls, fake.evictCalls) + } +} + +func TestRunPassSkipsOnCacheSizeFailure(t *testing.T) { + fake := &fakeGCStore{sizeErr: errors.New("unreadable size file")} + g := &imageCacheGC{store: fake, cacheDir: t.TempDir(), highPct: 85, lowPct: 80} + g.runPass(context.Background()) + if fake.evictCalls != 0 { + t.Errorf("CacheSize failure: evictCalls=%d, want 0", fake.evictCalls) + } +} + +func TestRunPassEvictsAndPassesDryRun(t *testing.T) { + // high=100 sidelines the watermark on any volume with >=1% free, so + // the max-bytes overage (99) is the target; a near-full host volume + // can lift it to the cacheSize cap, hence >= not ==. + fake := &fakeGCStore{size: 100, stats: imagecache.EvictStats{FreedBytes: 100}} + g := &imageCacheGC{store: fake, cacheDir: t.TempDir(), highPct: 100, lowPct: 0, maxBytes: 1, dryRun: true} + g.consecutiveShortfalls = 5 // a met target must reset it + g.runPass(context.Background()) + if fake.evictCalls != 1 || fake.gotTarget < 99 || !fake.gotDryRun { + t.Errorf("evictCalls=%d target=%d dryRun=%v, want 1/>=99/true", fake.evictCalls, fake.gotTarget, fake.gotDryRun) + } + if g.consecutiveShortfalls != 0 { + t.Errorf("consecutiveShortfalls=%d after met target, want 0", g.consecutiveShortfalls) + } +} + +func TestRunFirstPassIsImmediate(t *testing.T) { + // A cancelled context and an hour-long period: the single call can + // only be the immediate first pass, never a tick. + fake := &fakeGCStore{} + g := &imageCacheGC{store: fake, cacheDir: t.TempDir(), highPct: 100, lowPct: 0, period: time.Hour} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + g.Run(ctx) + if fake.evictCalls != 1 { + t.Errorf("evictCalls=%d, want exactly 1 (the immediate first pass)", fake.evictCalls) + } +} + +func TestRunTicks(t *testing.T) { + fake := &fakeGCStore{} + g := &imageCacheGC{store: fake, cacheDir: t.TempDir(), highPct: 100, lowPct: 0, period: 10 * time.Millisecond} + ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) + defer cancel() + g.Run(ctx) + if fake.evictCalls < 2 { + t.Errorf("evictCalls=%d, want >=2 (first pass plus at least one tick)", fake.evictCalls) + } +} + +func TestRunPassRecoversPanic(t *testing.T) { + g := &imageCacheGC{store: &fakeGCStore{panicOnEvict: true}, cacheDir: t.TempDir(), highPct: 100, lowPct: 0} + g.runPass(context.Background()) // must not propagate the panic +} + +// TestNoteOutcomeShortfallBackoff drives the shortfall cadence end to end: +// warn on the first shortfallWarnLimit consecutive shortfalls, then only +// every shortfallReminderEvery-th, streak preserved across a gated pass, +// reset (re-arming the warnings) on a met or absent target. +func TestNoteOutcomeShortfallBackoff(t *testing.T) { + var buf bytes.Buffer + prev := slog.Default() + slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil))) + t.Cleanup(func() { slog.SetDefault(prev) }) + + ctx := context.Background() + g := &imageCacheGC{} + logCount := func(msg string) int { return strings.Count(buf.String(), msg) } + + for range shortfallWarnLimit { + g.noteOutcome(ctx, gcPassShortfall, nil, nil) + } + if got := logCount("could not reach target"); got != shortfallWarnLimit { + t.Errorf("initial warns = %d, want %d", got, shortfallWarnLimit) + } + + buf.Reset() + for g.consecutiveShortfalls < 2*shortfallReminderEvery { + g.noteOutcome(ctx, gcPassShortfall, nil, nil) + } + if got := logCount("still short of target"); got != 2 { + t.Errorf("reminders through streak %d = %d, want 2", g.consecutiveShortfalls, got) + } + if got := logCount("could not reach target"); got != 0 { + t.Errorf("warns past the limit = %d, want 0", got) + } + + streak := g.consecutiveShortfalls + buf.Reset() + g.noteOutcome(ctx, gcPassSkipped, errors.New("gated"), nil) + if g.consecutiveShortfalls != streak { + t.Errorf("streak after gated pass = %d, want %d (preserved)", g.consecutiveShortfalls, streak) + } + if got := logCount("Image cache GC pass skipped"); got != 1 { + t.Errorf("skip logs = %d, want 1", got) + } + + buf.Reset() + g.noteOutcome(ctx, gcPassComplete, errors.New("one dir failed"), nil) + if g.consecutiveShortfalls != 0 { + t.Errorf("streak after complete pass = %d, want 0", g.consecutiveShortfalls) + } + if got := logCount("pass complete"); got != 1 { + t.Errorf("complete logs = %d, want 1", got) + } + if got := logCount("finished with errors"); got != 1 { + t.Errorf("per-item error warns = %d, want 1", got) + } + + buf.Reset() + g.noteOutcome(ctx, gcPassShortfall, nil, nil) + if got := logCount("could not reach target"); got != 1 { + t.Errorf("warns after reset = %d, want 1 (re-armed)", got) + } + + g.consecutiveShortfalls = shortfallWarnLimit + 1 + buf.Reset() + g.noteOutcome(ctx, gcPassQuiet, nil, nil) + if g.consecutiveShortfalls != 0 || buf.Len() != 0 { + t.Errorf("quiet pass: streak=%d buf=%q, want silent reset", g.consecutiveShortfalls, buf.String()) + } +} + func TestClassifyGCPass(t *testing.T) { gated := fmt.Errorf("pass gated: %w", imagecache.ErrIncompleteEnumeration) perItem := errors.New("while removing retired layer: permission denied") diff --git a/internal/imagecache/README.md b/internal/imagecache/README.md index 428edeb97..621ee2752 100644 --- a/internal/imagecache/README.md +++ b/internal/imagecache/README.md @@ -167,7 +167,8 @@ it — called from the checkpoint cleanup path in ateom-gvisor and ## Garbage collection `gc.go` holds the eviction engine; atelet drives it as a periodic pass -(`--image-cache-gc-period`, default 5m; `0` disables it). Each tick +(`--image-cache-gc-period`, default 5m; `0` disables the periodic pass, +but startup orphan recovery still runs at every atelet start). Each tick measures the cache volume with `statfs` and the pool's own size from the per-layer `size` files, then computes a byte target: free down to `--image-cache-low-percent` when volume usage reaches @@ -178,6 +179,18 @@ to fix disk pressure it didn't cause. `--image-cache-gc-dry-run` computes and logs every decision while mutating nothing: the recommended way to soak the policy on a live fleet. +Two caveats on those numbers. The cap is the pool's *total* size, not +the evictable subset (rooted and fresh layers can never be freed), so +under **sustained** foreign pressure the target stays unreachable and +every pass evicts everything unrooted and older than min-age — hit rate +goes to zero until the pressure clears. The "could not reach target" +WARNs are the signal; if this bites in practice, a retention floor +(never evict below N bytes) is the intended extension. And usage is +computed against the volume's raw capacity — kubelet's formula, so +operator intuition transfers — which counts ext4's ~5% reserved blocks +as used: eviction starts about five points below the configured +percentage as `df` reports it. + **One pass** (`Store.EvictUnused(ctx, targetBytes, dryRun)`): 1. **Root set** (`Store.InUse`): scan every bundle's diff --git a/internal/imagecache/gc.go b/internal/imagecache/gc.go index cddd3624a..8a27804fb 100644 --- a/internal/imagecache/gc.go +++ b/internal/imagecache/gc.go @@ -251,7 +251,7 @@ func (s *Store) EvictUnused(ctx context.Context, targetBytes int64, dryRun bool) // partial scan would retire layers a running actor still mounts. // Not logged here — the caller logs the gated pass once, with its // own context (see ErrIncompleteEnumeration). - return stats, errors.Join(ErrIncompleteEnumeration, rootsErr) + return stats, fmt.Errorf("%w: %w", ErrIncompleteEnumeration, rootsErr) } cutoff := time.Now().Add(-s.minAge) @@ -261,11 +261,13 @@ func (s *Store) EvictUnused(ctx context.Context, targetBytes int64, dryRun bool) // be retired while that record still names it. Fail the whole pass // toward retention; the error names the records to repair or // delete, and every later pass retries. - return stats, errors.Join(ErrIncompleteEnumeration, listErr) + return stats, fmt.Errorf("%w: %w", ErrIncompleteEnumeration, listErr) } stats.Candidates = len(candidates) - slog.InfoContext(ctx, "Image cache eviction pass", + // Debug: the driving caller logs the pass outcome once, and a + // no-target pass should be silent. + slog.DebugContext(ctx, "Image cache eviction pass", slog.Int64("target_bytes", targetBytes), slog.Bool("dry_run", dryRun), slog.Int("rooted_images", stats.RootedImages), @@ -489,12 +491,12 @@ func (s *Store) RecoverOrphans(ctx context.Context) (EvictStats, error) { roots, rootsErr := s.InUse() if rootsErr != nil { - return stats, errors.Join(ErrIncompleteEnumeration, rootsErr) + return stats, fmt.Errorf("%w: %w", ErrIncompleteEnumeration, rootsErr) } cutoff := time.Now().Add(-s.minAge) _, refcount, complete, listErr := s.listEviction(roots, cutoff, &stats) if !complete { - return stats, errors.Join(ErrIncompleteEnumeration, listErr) + return stats, fmt.Errorf("%w: %w", ErrIncompleteEnumeration, listErr) } retired, errs := s.sweepOrphanLayers(ctx, roots, refcount, cutoff, false, &stats) @@ -599,10 +601,11 @@ func (s *Store) dryRunRetire(hex string, cutoff time.Time) (int64, retireStatus) // their references are what keep shared layers alive — counting // listing-stage vetoes into stats. // -// complete reports whether every record was read and decoded. Refcounts -// from a partial listing understate references — a layer shared with an -// unread record looks unreferenced — so both callers gate on it and do -// nothing with a partial listing. +// complete reports whether every record was read and decoded, and is +// false only alongside a non-nil err (the gates wrap that err with %w, +// which a nil would garble). Refcounts from a partial listing understate +// references — a layer shared with an unread record looks unreferenced — +// so both callers gate on it and do nothing with a partial listing. func (s *Store) listEviction(roots RootSet, cutoff time.Time, stats *EvictStats) (cands []evictionCandidate, refcount map[string]int, complete bool, err error) { refcount = map[string]int{} entries, err := os.ReadDir(s.manifestsDir()) diff --git a/internal/imagecache/imagecache.go b/internal/imagecache/imagecache.go index da6fb04e4..115f526d7 100644 --- a/internal/imagecache/imagecache.go +++ b/internal/imagecache/imagecache.go @@ -105,9 +105,8 @@ type Store struct { localhostRegistryReplacement string - // platform overrides the default pull platform (linux/GOARCH). Used by - // validation tooling that runs on a different architecture than the - // nodes it validates for. + // platform overrides the default pull platform (linux/GOARCH), for + // callers pulling on a different architecture than the images' target. platform *v1.Platform // actorsDir is scanned by InUse for bundle overlay specs; empty disables diff --git a/tools/validate-image-cache/README.md b/tools/validate-image-cache/README.md index 54f28f81d..9d93e792a 100644 --- a/tools/validate-image-cache/README.md +++ b/tools/validate-image-cache/README.md @@ -23,9 +23,9 @@ whiteouts, oversized layer counts, ...). `domain:google.com` grant) is not necessarily readable by a *service account* — validate with the identity that production will use. - Disk: unpacked layers are 2–3× their compressed size. The tool bounds - usage by evicting idle cached layers when the cache volume's free space - drops below `--min-free-gb`, but give it room to breathe (tens of GB - minimum). + usage by asking the cache's own eviction engine to reclaim space when the + cache volume's free space drops below `--min-free-gb`, but give it room + to breathe (tens of GB minimum). - Runs anywhere Go runs, including macOS (unpack is pure file I/O). Caveat: a case-insensitive filesystem (default macOS APFS) unpacks case-colliding paths slightly differently than Linux — silently, not as an error. Linux @@ -68,15 +68,17 @@ go run ./tools/validate-image-cache \ | Flag | Default | Meaning | |---|---|---| -| `--refs-file` | (required) | file with one image ref per line | +| `--refs-file` | (required unless `--evict-all`) | file with one image ref per line | | `--cache-dir` | (required) | cache root; reused (and cache-hit) across runs | | `--sample` | 0 (= all) | validate a random sample of N refs | | `--seed` | 1 | sampling seed; same seed + file ⇒ same sample | | `--out` | `validate-results.csv` | results CSV, written incrementally | | `--parallel` | 3 | images validated concurrently (each pulls up to 4 layers in parallel) | | `--timeout` | 20m | per-image timeout | -| `--min-free-gb` | 150 | evict oldest idle layers below this free-space floor | -| `--evict-idle` | 10m | only evict layers idle at least this long; must be far below disk-fill time on small disks | +| `--min-free-gb` | 150 | reclaim the shortfall below this free-space floor via the eviction engine | +| `--evict-idle` | 10m | eviction min-age: layers and records younger than this are never evicted (minimum 1m on a node with an actors dir); must be far below disk-fill time on small disks | +| `--evict-all` | false | evict everything evictable and exit (mutually exclusive with `--refs-file`); rooted images and anything younger than `--evict-idle` survive | +| `--force` | false | allow eviction on a node with an actors dir (required for both modes there) | | `--platform` | `linux/amd64` | image platform to pull | ## Output and rerunning @@ -89,10 +91,22 @@ non-zero if any image failed. Reruns are cheap by design: completed layers stay in the cache (and even an interrupted pull keeps every layer that finished), so re-running the same sample — or just the failed refs — mostly re-validates from local disk. -Eviction only removes layers idle for at least `--evict-idle`, so in-flight -images are not raced; on a small disk with high throughput, set it well -below the time the corpus needs to fill the disk, or nothing will be -evictable while it fills. +Eviction never removes layers or records younger than `--evict-idle`, so +this process's own in-flight images are not raced; on a small disk with +high throughput, set it well below the time the corpus needs to fill the +disk, or nothing will be evictable while it fills. + +## Live nodes + +On a node with an actors dir, **both modes require `--force`** and +`--evict-idle` is floored at 1m. The engine's locks are per-process, so a +run there is not synchronized with the node agent's own GC and pulls. +Bundle-spec rooting protects placed actors' images and min-age protects +fresh pulls, but a layer that ages past `--evict-idle` and is reused +mid-pass can be evicted out from under an actor: a not-yet-mounted start +fails once and heals on re-pull; an already-mounted actor can take I/O +errors from a removed lowerdir. Note that refs mode evicts too — below +`--min-free-gb` (default 150) it flushes repeatedly, not once. ## Scaling out diff --git a/tools/validate-image-cache/main.go b/tools/validate-image-cache/main.go index 3800611d9..2178fd035 100644 --- a/tools/validate-image-cache/main.go +++ b/tools/validate-image-cache/main.go @@ -26,27 +26,40 @@ // // gcloud artifacts docker images list REPO --format="value[separator='@'](package,version)" // -// Disk is bounded by evicting the oldest cached layers when the cache -// volume's free space drops below --min-free-gb. Only layers idle for more -// than 30 minutes are evicted, so in-flight images are never raced. +// Disk is bounded by the cache's own eviction engine: below +// --min-free-gb free space, the tool asks Store.EvictUnused to reclaim +// the shortfall. --evict-all instead empties everything evictable and +// exits. Bundle-spec rooting (scanned from ateompath.ActorsDir) protects +// placed actors' mounted images. The engine's locks are per-process, so a +// run beside a live atelet is unsynchronized with its GC and pulls — the +// pool cannot be corrupted (record-first pulls, two-phase retirement), +// but a layer idle past --evict-idle can be retired just as atelet +// reuses it: a not-yet-mounted actor start fails once and heals on +// re-pull; an already-mounted actor can take EIO from a removed +// lowerdir. min-age is the only cross-process guard, hence the floor on +// --evict-idle when an actors dir exists. (The end state is an +// atelet-owned flush — RPC or trigger file — not a second process in +// the pool.) Run as the user that owns the cache and actors dirs — an +// unreadable actors dir gates every pass. package main import ( "bufio" "context" "encoding/csv" + "errors" "flag" "fmt" "log" + "math" "math/rand" "os" - "path/filepath" - "sort" "strconv" "strings" "sync" "time" + "github.com/agent-substrate/substrate/internal/ateompath" "github.com/agent-substrate/substrate/internal/imagecache" v1 "github.com/google/go-containerregistry/pkg/v1" googlecontainerauth "github.com/google/go-containerregistry/pkg/v1/google" @@ -54,18 +67,97 @@ import ( ) var ( - refsFile = flag.String("refs-file", "", "File with one image ref per line (required)") + refsFile = flag.String("refs-file", "", "File with one image ref per line (required unless --evict-all)") sample = flag.Int("sample", 0, "Validate a random sample of N refs (0 = all)") seed = flag.Int64("seed", 1, "Seed for reproducible sampling") cacheDir = flag.String("cache-dir", "", "Cache root (required); reused across runs") outCSV = flag.String("out", "validate-results.csv", "Results CSV path") parallel = flag.Int("parallel", 3, "Images validated concurrently (each pulls up to 4 layers in parallel)") timeout = flag.Duration("timeout", 20*time.Minute, "Per-image timeout") - minFreeGB = flag.Uint64("min-free-gb", 150, "Evict oldest idle cached layers when the cache volume has less free space than this") - evictIdle = flag.Duration("evict-idle", 10*time.Minute, "Only evict layers idle for at least this long (must exceed the time any in-flight image needs a just-unpacked layer; small disks + high throughput need small values)") + minFreeGB = flag.Uint64("min-free-gb", 150, "Ask the eviction engine to reclaim disk when the cache volume has less free space than this") + evictIdle = flag.Duration("evict-idle", 10*time.Minute, "Eviction min-age: layers and records younger than this are never evicted. Minimum 1m on a node with an actors dir. NOTE: if the corpus unpacks faster than this window elapses on a small disk, nothing is evictable while the disk fills — size it well below disk-fill time") + evictAll = flag.Bool("evict-all", false, "Evict everything evictable from the cache and exit (no refs file needed). Rooted images and anything younger than --evict-idle survive. Requires --force on a node with an actors dir") + force = flag.Bool("force", false, "Allow eviction on a node with an actors dir") platform = flag.String("platform", "linux/amd64", "Image platform to pull") ) +// looksLikeLiveNode reports whether dir — the node's actors dir, the +// same authority the eviction root set is scanned from — exists. +// Anything but ENOENT counts: an unreadable actors dir is still a node. +func looksLikeLiveNode(dir string) bool { + _, err := os.Stat(dir) + return err == nil || !errors.Is(err, os.ErrNotExist) +} + +// liveNodeIdleFloor: min-age is the only protection that works across +// processes (the engine's locks are per-process), so on a live node it +// must not be tuned away. Validation hosts keep full freedom. +const liveNodeIdleFloor = time.Minute + +// errUsage marks validation failures that should print flag usage. +var errUsage = errors.New("usage") + +// runConfig is the flag state validate checks — a plain struct so the +// combination matrix is table-testable. +type runConfig struct { + cacheDir, refsFile string + evictAll, force bool + evictIdle time.Duration + minFreeGB uint64 + live bool + setFlags []string // flag names given explicitly +} + +func (c runConfig) validate() error { + if c.cacheDir == "" || (c.refsFile == "" && !c.evictAll) || (c.refsFile != "" && c.evictAll) { + return fmt.Errorf("%w: need --cache-dir plus exactly one of --refs-file or --evict-all", errUsage) + } + if c.evictIdle < 0 { + // A negative min-age inverts the veto (the cutoff lands in the + // future), making even in-flight pulls' layers evictable. + return fmt.Errorf("--evict-idle %v must be >= 0", c.evictIdle) + } + if c.minFreeGB > math.MaxUint64/uint64(1e9) { + return fmt.Errorf("--min-free-gb %d overflows a byte count", c.minFreeGB) + } + if c.evictAll { + var stray []string + for _, name := range c.setFlags { + switch name { + case "cache-dir", "evict-idle", "evict-all", "force": + // The whole flush-mode surface; new flags fail closed. + default: + stray = append(stray, "--"+name) + } + } + if len(stray) > 0 { + return fmt.Errorf("%w: %s: only valid with --refs-file", errUsage, strings.Join(stray, ", ")) + } + } + if c.live { + if c.evictIdle < liveNodeIdleFloor { + return fmt.Errorf("--evict-idle=%v is below %v with %s present: min-age is the only protection that applies across processes using the cache", + c.evictIdle, liveNodeIdleFloor, ateompath.ActorsDir) + } + // Both modes evict here (evictIfLow is a low-water flush on a + // loop), unsynchronized with every other user of the pool. + if !c.force { + return fmt.Errorf("%s exists — this looks like a live node, and evictions are not synchronized with other processes using the cache; re-run with --force to proceed", ateompath.ActorsDir) + } + } + return nil +} + +// newStore opens the cache with the options both modes share: min-age +// from --evict-idle, and bundle-spec rooting from the node's actors dir +// (absent on a validation host, which InUse treats as an empty root set). +func newStore(extra ...imagecache.Option) (*imagecache.Store, error) { + return imagecache.New(*cacheDir, append([]imagecache.Option{ + imagecache.WithMinAge(*evictIdle), + imagecache.WithActorsDir(ateompath.ActorsDir), + }, extra...)...) +} + type result struct { ref string digest string @@ -76,12 +168,48 @@ type result struct { func main() { flag.Parse() - if *refsFile == "" || *cacheDir == "" { - flag.Usage() - os.Exit(2) + cfg := runConfig{ + cacheDir: *cacheDir, refsFile: *refsFile, + evictAll: *evictAll, force: *force, + evictIdle: *evictIdle, minFreeGB: *minFreeGB, + live: looksLikeLiveNode(ateompath.ActorsDir), + } + flag.Visit(func(f *flag.Flag) { cfg.setFlags = append(cfg.setFlags, f.Name) }) + if err := cfg.validate(); err != nil { + if errors.Is(err, errUsage) { + fmt.Fprintln(os.Stderr, err) + flag.Usage() + os.Exit(2) + } + log.Fatal(err) } ctx := context.Background() + if *evictAll { + // Flush mode: no refs, no registry auth. New also reclaims any + // crash-debris orphans before the pass. + store, err := newStore() + if err != nil { + log.Fatalf("opening cache: %v", err) + } + stats, err := store.EvictUnused(ctx, math.MaxInt64, false) + if errors.Is(err, imagecache.ErrIncompleteEnumeration) { + // Gated: nothing was attempted; the error names the unreadable + // or corrupt path. + log.Fatalf("evict-all did nothing: %v", err) + } + if err != nil { + log.Printf("evict-all finished with errors: %v", err) + } + // Print the summary even on partial failure; err decides the exit code. + log.Printf("evict-all: %d images / %d layers evicted, %.1f GB credited (free now %s)", + stats.EvictedImages, stats.EvictedLayers, float64(stats.FreedBytes)/1e9, freeGB(*cacheDir)) + if err != nil { + os.Exit(1) + } + return + } + refs, err := loadRefs(*refsFile) if err != nil { log.Fatalf("loading refs: %v", err) @@ -102,7 +230,7 @@ func main() { log.Fatalf("creating GCP authenticator (need application-default credentials): %v", err) } - store, err := imagecache.New(*cacheDir, + store, err := newStore( imagecache.WithAuthenticator(auth), imagecache.WithPlatform(v1.Platform{OS: osName, Architecture: arch}), ) @@ -132,7 +260,7 @@ func main() { wg.Go(func() { defer func() { <-sem }() - evictIfLow(*cacheDir, *minFreeGB*1e9) + evictIfLow(ctx, store, *cacheDir, *minFreeGB*1e9) r := validateOne(ctx, store, ref, *timeout) @@ -203,57 +331,54 @@ func shortRef(ref string) string { return ref } -// evictIfLow deletes the oldest cached layer trees until the cache volume -// has at least minFree bytes available. Layers touched within the last -// --evict-idle are skipped so an in-flight image's freshly unpacked layers -// are not raced away mid-validation. NOTE: if the corpus unpacks faster -// than the idle window elapses on a small disk, nothing is evictable while -// the disk fills — size --evict-idle well below disk-fill time. -var evictMu sync.Mutex +// evictIfLow asks the eviction engine to reclaim the free-space +// shortfall below minFree. All engine protections apply, so in-flight +// validations are never raced. Unlike atelet's loop, the target is not +// capped at the pool size: on a dedicated validation disk an oversized +// target just runs out of candidates. The statfs shortfall and the +// engine's FreedBytes are different estimates (disk blocks vs recorded +// sizes), so a "met" target can leave free space still short; the next +// worker's re-check converges. +var ( + evictMu sync.Mutex // one attempt per low-water episode; queued workers re-check and return + // lastFruitless backs off when a pass freed nothing — everything + // younger than --evict-idle, or a gated pass. Without it every queued + // worker would run a full engine pass ahead of its validation until + // free space moved. + lastFruitless time.Time +) + +const fruitlessCooldown = 30 * time.Second -func evictIfLow(cacheRoot string, minFree uint64) { +func evictIfLow(ctx context.Context, store *imagecache.Store, cacheRoot string, minFree uint64) { evictMu.Lock() defer evictMu.Unlock() - if freeBytes(cacheRoot) >= minFree { + free := freeBytes(cacheRoot) + if free >= minFree { return } - layersDir := filepath.Join(cacheRoot, "layers", "sha256") - entries, err := os.ReadDir(layersDir) - if err != nil { + if time.Since(lastFruitless) < fruitlessCooldown { return } - type aged struct { - path string - mod time.Time - } - var candidates []aged - for _, e := range entries { - info, err := e.Info() - if err != nil || time.Since(info.ModTime()) < *evictIdle { - continue - } - candidates = append(candidates, aged{filepath.Join(layersDir, e.Name()), info.ModTime()}) + stats, err := store.EvictUnused(ctx, int64(minFree-free), false) + switch { + case errors.Is(err, imagecache.ErrIncompleteEnumeration): + // Gated: nothing was attempted; the error names the path to repair. + // Validation itself goes on — it only needed the disk space. + log.Printf("eviction pass skipped, nothing attempted: %v", err) + case err != nil: + // Per-item failures on a pass that ran; each retries next pass. + log.Printf("eviction pass finished with errors: %v", err) } - sort.Slice(candidates, func(i, j int) bool { return candidates[i].mod.Before(candidates[j].mod) }) - - evicted := 0 - for _, c := range candidates { - if freeBytes(cacheRoot) >= minFree { - break - } - if err := imagecache.RemoveAllWritable(c.path); err == nil { - evicted++ - } + if stats.EvictedImages > 0 || stats.EvictedLayers > 0 { + log.Printf("evicted %d images / %d layers, %.1f GB credited (free now %s)", + stats.EvictedImages, stats.EvictedLayers, float64(stats.FreedBytes)/1e9, freeGB(cacheRoot)) } - // Records referencing evicted layers re-pull only the missing layers, so - // stale manifests are harmless; drop them anyway to keep the dir tidy. - if evicted > 0 { - manifests, _ := filepath.Glob(filepath.Join(cacheRoot, "manifests", "sha256", "*.json")) - for _, m := range manifests { - _ = os.Remove(m) - } - log.Printf("evicted %d idle layers to reclaim disk (free now %.0f GB)", evicted, float64(freeBytes(cacheRoot))/1e9) + // The same counters the log line above gates on — not bytes, which + // credit zero for a layer retired with an unreadable size file. + if stats.EvictedImages == 0 && stats.EvictedLayers == 0 { + lastFruitless = time.Now() } } @@ -264,3 +389,13 @@ func freeBytes(path string) uint64 { } return st.Bavail * uint64(st.Bsize) } + +// freeGB renders free space for logs, naming the statfs-failure sentinel +// instead of printing it as ~18 billion GB. +func freeGB(path string) string { + free := freeBytes(path) + if free == ^uint64(0) { + return "unknown" + } + return fmt.Sprintf("%.0f GB", float64(free)/1e9) +} diff --git a/tools/validate-image-cache/main_test.go b/tools/validate-image-cache/main_test.go new file mode 100644 index 000000000..ce71a8ae7 --- /dev/null +++ b/tools/validate-image-cache/main_test.go @@ -0,0 +1,96 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "errors" + "math" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestRunConfigValidate(t *testing.T) { + valid := runConfig{cacheDir: "/c", refsFile: "r.txt", evictIdle: 10 * time.Minute, minFreeGB: 150} + cases := []struct { + name string + mutate func(*runConfig) + wantErr string // substring of the error; "" means valid + usage bool // wraps errUsage + }{ + {"refs mode defaults", func(c *runConfig) {}, "", false}, + {"missing cache-dir", func(c *runConfig) { c.cacheDir = "" }, "exactly one", true}, + {"neither mode", func(c *runConfig) { c.refsFile = "" }, "exactly one", true}, + {"both modes", func(c *runConfig) { c.evictAll = true }, "exactly one", true}, + {"negative idle rejected everywhere", func(c *runConfig) { c.evictIdle = -time.Hour }, "must be >= 0", false}, + {"low idle fine on a validation host", func(c *runConfig) { c.evictIdle = 10 * time.Second }, "", false}, + {"low idle floored on a live node", func(c *runConfig) { c.evictIdle = 10 * time.Second; c.live = true; c.force = true }, "is below", false}, + {"live node refuses without force", func(c *runConfig) { c.live = true }, "--force", false}, + {"live node with force", func(c *runConfig) { c.live = true; c.force = true }, "", false}, + {"live flush refuses without force", func(c *runConfig) { c.refsFile = ""; c.evictAll = true; c.live = true }, "--force", false}, + {"flush with a refs-mode flag", func(c *runConfig) { + c.refsFile = "" + c.evictAll = true + c.setFlags = []string{"cache-dir", "evict-all", "parallel"} + }, "only valid with --refs-file", true}, + {"flush with its own flags", func(c *runConfig) { + c.refsFile = "" + c.evictAll = true + c.setFlags = []string{"cache-dir", "evict-idle", "evict-all", "force"} + }, "", false}, + {"future flags fail closed", func(c *runConfig) { + c.refsFile = "" + c.evictAll = true + c.setFlags = []string{"some-new-flag"} + }, "only valid with --refs-file", true}, + {"min-free-gb overflow", func(c *runConfig) { c.minFreeGB = math.MaxUint64 }, "overflows", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := valid + tc.mutate(&c) + err := c.validate() + if tc.wantErr == "" { + if err != nil { + t.Fatalf("validate() = %v, want nil", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("validate() = %v, want error containing %q", err, tc.wantErr) + } + if got := errors.Is(err, errUsage); got != tc.usage { + t.Errorf("errors.Is(err, errUsage) = %v, want %v", got, tc.usage) + } + }) + } +} + +func TestLooksLikeLiveNode(t *testing.T) { + dir := t.TempDir() + if !looksLikeLiveNode(dir) { + t.Error("existing dir: want true") + } + if looksLikeLiveNode(filepath.Join(dir, "missing")) { + t.Error("missing dir: want false") + } +} + +func TestFreeGBNamesTheSentinel(t *testing.T) { + if got := freeGB(filepath.Join(t.TempDir(), "missing")); got != "unknown" { + t.Errorf("freeGB(missing path) = %q, want \"unknown\"", got) + } +}