diff --git a/internal/imagecache/README.md b/internal/imagecache/README.md index b67ae20c9..46e52c909 100644 --- a/internal/imagecache/README.md +++ b/internal/imagecache/README.md @@ -62,14 +62,23 @@ anywhere. fs/ the unpacked layer tree (an overlay lowerdir) whiteouts.json whiteout state recorded at unpack time finalized marker written by FinalizeLayer (consumer side) - manifests/sha256/.json image config + ordered diffID list + size byte count recorded at unpack (lazily + backfilled for older layers), so sizing + the pool never walks trees + layers/sha256/.tmp-*/ in-flight unpack (swept at startup) + layers/sha256/.rm-*/ retired by eviction, awaiting async + removal (swept at startup) + manifests/sha256/.json image config + ordered diffID list; the + file's mtime doubles as the image's + last-use timestamp ``` A layer directory that exists is always complete: unpack streams into a `.tmp-*` sibling and moves it into place with a single atomic rename. -Startup recovery (`New`) therefore only has to sweep orphaned temp dirs and -verify the layout version. An "image" is nothing but a manifest record -listing layer diffIDs in order — layers shared by N images exist once. +Startup recovery (`New`) sweeps leftover `.tmp-*` and `.rm-*` dirs, +verifies the layout version, and reclaims orphaned layers (see Garbage +collection below). An "image" is nothing but a manifest record listing +layer diffIDs in order — layers shared by N images exist once. ## Pull path (atelet: `Store.EnsureImage`) @@ -155,27 +164,84 @@ an actor's bundle directory (via `/proc/self/mountinfo`) before atelet wipes it — called from the checkpoint cleanup path in ateom-gvisor and `teardownActor` in ateom-microvm. -## Not implemented yet: garbage collection - -**There is no eviction. Layers and manifest records, once cached, are never -deleted from the node VM.** Disk usage grows monotonically with the set of -distinct layers ever pulled on the node, bounded only by the size of the -volume backing the cache root. Operators should size the -`--image-cache-dir` volume accordingly (and note that on GKE, disk size also -gates IOPS, which directly bounds unpack throughput). If a node fills up, -deleting the cache root entirely (or any individual -`layers/sha256/` directory plus the `manifests/` records) while no -actors are starting is safe — the store re-pulls whatever is missing. - -This is Phase 2 of [#463](https://github.com/agent-substrate/substrate/issues/463): -watermark-driven eviction (start evicting at a disk high-watermark, stop at -the low-watermark) with a protection hierarchy — layers referenced by -actively mounted images are never evicted, then preload-pinned images, then -LRU by image last-use — plus cache metrics. Phase 3 adds the control-plane -surface (reporting cached digests for scheduling affinity, and a -`PreloadImage` API with expiring pins). The layer-materializer seam is also -designed so a lazy-pull backend (eStargz/SOCI-style FUSE) can replace the -untar backend later without restructuring. +## Garbage collection (engine; the periodic loop lands next) + +`gc.go` holds the eviction engine. Nothing in production calls +`Store.EvictUnused` yet — the watermark-driven periodic loop and its +flags (period, high/low watermarks, max-bytes cap, min-age, dry-run) +arrive with the next change — so cached data still only grows for now. +The one behavior `New` gains today is the startup orphan scan below. + +**One pass** (`Store.EvictUnused(ctx, targetBytes, dryRun)`): + +1. **Root set** (`Store.InUse`): scan every bundle's + `rootfs-overlay.json` under the actors dir (`WithActorsDir`). + Overlay mounts live in the ateom pods' mount namespaces, so atelet + cannot see them in its own `/proc/mounts`; the bundle specs are + written by atelet itself before any ateom is asked to mount and + removed only after unmount, so they are the authoritative "actively + mounted" set. A spec roots its image digest, each layer dir it + names, and its *exact* layer set — the last also roots the + multi-arch twin record and records of digestless (pre-`imageDigest`) + specs. +2. **Refcount** layers across *all* image records, and list unrooted + records older than min-age as eviction candidates, LRU-ordered by + last use (the record's mtime — refreshed on every cache hit and on + every completed layer of an in-flight pull). +3. **Evict** candidates until ~targetBytes is freed: delete the record + (after a freshness re-check under the same lock the cache-hit path + holds), then retire each layer the removal left unreferenced. If any + layer must be kept — still referenced by another record, rooted by a + spec, younger than min-age, or its retirement failed — the record is + restored byte-exact and the image simply is not evicted this pass: a + layer is never left on disk without a record explaining it. + +The pass reaches layers **only through records**: a layer is deleted +exactly when its last referencing record goes, never by an independent +scan of the pool. + +**Everything fails toward retention.** If the image records or the +bundle specs cannot be fully enumerated (an unreadable file or +directory), the pass does nothing and logs at ERROR naming the culprit: +refcounts and roots computed from partial data would retire layers that +unread records still reference or running actors still mount. Dry-run +mutates nothing at all — not even the lazy size-file backfill. + +**In-flight pulls need no separate protection.** `EnsureImage` writes +the image record **before** unpacking (the way Go allocates black during +GC and containerd creates its ingest record before the bytes land), so +every layer a pull produces is referenced — and kept fresh by a +per-layer progress touch — from before it exists on disk. An interrupted +pull's record is resumable progress, not garbage: the next pull of that +digest re-fetches only the missing layers, and a pull that never resumes +ages out through ordinary LRU. + +**Startup recovery.** A layer no record references can only be crash +debris (eviction interrupted between record-delete and layer-rename) or +operator damage; `New` reclaims such orphans once, at startup +(`Store.RecoverOrphans`), when no pull can be racing the scan — and +skips the scan entirely, conservatively, if any record or bundle spec +fails to read. There is no online whole-pool scan (ext4's split: bounded +recovery at mount, fsck offline). + +**Deletion is two-phase.** A layer is atomically renamed to `.rm-*` +inside the layer's singleflight (one `rename(2)` — eviction can never +stall a pull), then removed asynchronously; a crash in between leaves +the dir for the startup sweep. This matters because the kernel offers no +protection here: deleting a directory that is a live overlay lowerdir in +another mount namespace succeeds silently, leaves the overlay's behavior +undefined, and doesn't even free the space until the mount goes away. + +Deleting the cache root by hand (while no actors are starting) remains +safe — the store re-pulls whatever is missing. + +This is Phase 2 of [#463](https://github.com/agent-substrate/substrate/issues/463); +the watermark loop, flags, and cache metrics complete it. Phase 3 adds +the control-plane surface (reporting cached digests for scheduling +affinity, and a `PreloadImage` API with expiring pins). The +layer-materializer seam is also designed so a lazy-pull backend +(eStargz/SOCI-style FUSE) can replace the untar backend later without +restructuring. ## Testing diff --git a/internal/imagecache/gc.go b/internal/imagecache/gc.go new file mode 100644 index 000000000..1b4e7633a --- /dev/null +++ b/internal/imagecache/gc.go @@ -0,0 +1,722 @@ +// 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 imagecache + +// Garbage collection for the layer pool. +// +// The pool is a two-level DAG — image records reference layers by diffID — +// so liveness is plain refcounting, recomputed from disk on every pass; +// there is nothing to maintain between passes and nothing to rebuild after +// a restart. An image (and then any layer its removal leaves unreferenced) +// is evictable unless vetoed by, in order: +// +// 1. the root set — bundle overlay specs under the actors dir, the same +// authority that hands out mounts (atelet cannot see the ateoms' mount +// namespaces); +// 2. min-age — records and layers younger than minAge are never touched, +// covering the pull → spec-write → mount window; +// 3. per-victim re-checks against current disk state immediately before +// acting. +// +// The periodic pass reaches layers ONLY through records: pull writes the +// image record before unpacking (see Store.pull), so every layer is +// referenced — and thereby protected — before it can exist on disk. +// Unexplained layers can therefore only be crash debris, reclaimed once at +// startup by RecoverOrphans; there is no online whole-pool scan. +// +// Deletion is two-phase: the only steps that contend with the pull path +// are one os.Remove of a record and one rename of a layer dir to a ".rm-*" +// name inside the layer's singleflight (see retireLayer); the slow +// RemoveAll of multi-GB trees happens afterwards, on dirs nothing can +// reach by diffID. A crash in between leaves a ".rm-*" dir for the +// startup sweep. + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "os" + "path/filepath" + "sort" + "strings" + "sync" + "time" + + v1 "github.com/google/go-containerregistry/pkg/v1" + "golang.org/x/sync/errgroup" +) + +// --- root set --- + +// RootSet is the set of images and layers that eviction must not touch, +// recomputed from disk at the start of every pass. +type RootSet struct { + // ImageDigests are rooted image digest strings ("sha256:"). + ImageDigests map[string]bool + // LayerHexes are rooted layer diffID hexes (the layer dir base names), + // covering bundle specs written before ImageDigest existed and + // belt-and-suspenders for those written after. + LayerHexes map[string]bool + // LayerSets holds a signature per rooted bundle's *exact* layer set. + // A record whose layer set matches one is rooted too: the multi-arch + // twin and the digestless pre-ImageDigest spec, whose record would + // otherwise be evicted while its layers survive — manufacturing an + // orphan when the bundle goes. Exact match on purpose: rooting every + // subset image (e.g. a running actor's base image) would quietly + // weaken --image-cache-max-bytes under heavy layer sharing. + LayerSets map[string]bool +} + +// layerSetSignature canonicalizes a set of layer hexes for comparison. +func layerSetSignature(hexes []string) string { + uniq := make([]string, 0, len(hexes)) + seen := make(map[string]bool, len(hexes)) + for _, h := range hexes { + if !seen[h] { + seen[h] = true + uniq = append(uniq, h) + } + } + sort.Strings(uniq) + return strings.Join(uniq, ",") +} + +// InUse scans the actors directory for bundle overlay specs and returns +// the images and layers referenced by actors placed on this node. Bundles +// exist exactly while an actor runs or transitions here (spec written +// before any mount, deleted after unmount), so the scan protects actively +// mounted images via the same authority that hands out mounts. Leftover +// bundles from crashed actors over-pin until wiped. +// +// A non-nil error means the root set may be incomplete: an unreadable +// actors dir, bundles dir, or spec. Deleting callers must then do nothing — +// a missing root fails toward retiring a running actor's layers, and for a +// long-running actor the spec is the only protection (its record mtime can +// be arbitrarily old, so min-age would not save it). A missing dir or spec +// file is not an error: no actor was ever placed, the actor is torn down, +// or the bundle predates its spec write. +func (s *Store) InUse() (RootSet, error) { + // Per-item lines are Debug and gated: on a full node this loop emits + // hundreds of them, and ungated slog calls build their attr args even + // when suppressed. + dbg := slog.Default().Enabled(context.Background(), slog.LevelDebug) + rs := RootSet{ImageDigests: map[string]bool{}, LayerHexes: map[string]bool{}, LayerSets: map[string]bool{}} + if s.actorsDir == "" { + return rs, nil + } + actorEntries, err := os.ReadDir(s.actorsDir) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return rs, nil + } + return rs, fmt.Errorf("while listing actors dir %q for root set: %w", s.actorsDir, err) + } + var errs []error + for _, actor := range actorEntries { + if !actor.IsDir() { + continue + } + bundlesDir := filepath.Join(s.actorsDir, actor.Name(), "bundles") + bundles, err := os.ReadDir(bundlesDir) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + continue // no bundles: actor not placed / already torn down + } + errs = append(errs, fmt.Errorf("while listing bundles of actor %q: %w", actor.Name(), err)) + continue + } + for _, bundle := range bundles { + if !bundle.IsDir() { + continue + } + // WriteSpec renames its temp file into place, so a spec read here + // is always whole; a partial read would under-report an actor's + // layers and fail toward deleting them. + spec, err := ReadSpec(filepath.Join(bundlesDir, bundle.Name())) + if err != nil { + errs = append(errs, fmt.Errorf("while reading bundle overlay spec %q: %w", filepath.Join(actor.Name(), "bundles", bundle.Name()), err)) + continue + } + if spec == nil { + continue + } + addSpecRoots(&rs, spec, filepath.Join(actor.Name(), "bundles", bundle.Name()), dbg) + } + } + return rs, errors.Join(errs...) +} + +// addSpecRoots roots one bundle spec's image digest, layers, and exact +// layer-set signature. +func addSpecRoots(rs *RootSet, spec *OverlaySpec, bundle string, dbg bool) { + if spec.ImageDigest != "" { + rs.ImageDigests[spec.ImageDigest] = true + if dbg { + slog.Debug("Image cache root-set: bundle roots image", + slog.String("bundle", bundle), + slog.String("digest", spec.ImageDigest), + slog.Int("layers", len(spec.Layers))) + } + } else if len(spec.Layers) > 0 && dbg { + slog.Debug("Image cache root-set: digestless bundle roots layers only", + slog.String("bundle", bundle), + slog.Int("layers", len(spec.Layers))) + } + hexes := make([]string, 0, len(spec.Layers)) + for _, layerDir := range spec.Layers { + hex := filepath.Base(layerDir) + rs.LayerHexes[hex] = true + hexes = append(hexes, hex) + } + if len(hexes) > 0 { + rs.LayerSets[layerSetSignature(hexes)] = true + } +} + +// --- eviction --- + +// EvictStats reports what an eviction pass did (or, dry-run, would do). +type EvictStats struct { + // FreedBytes sums retired layers' recorded sizes, read from the size + // files that rode along with the rename (walked read-only only when + // absent) — consistent with CacheSize's accounting. An estimate; the + // caller's next statfs self-corrects. + FreedBytes int64 + // EvictedImages / EvictedLayers count deleted records and retired layer + // dirs. + EvictedImages, EvictedLayers int + // Candidates is the number of LRU-ordered eviction candidates after all + // listing-stage vetoes. + Candidates int + // RootedImages counts image records excluded because a bundle overlay + // spec roots them (the "actively placed" protection). + RootedImages int + // SkippedRooted counts layers kept during the pass because a bundle + // spec roots them (rooted images at listing time count into + // RootedImages instead). SkippedFresh counts min-age vetoes, fired at + // listing time or by the per-victim re-check. + SkippedRooted, SkippedFresh int + // OrphanLayers counts layers reclaimed by the startup scan + // (RecoverOrphans) — always zero for periodic passes, which reach + // layers only through records. Bytes are included in FreedBytes. + OrphanLayers int +} + +type evictionCandidate struct { + digest v1.Hash + modTime time.Time + diffIDs []string // unique, in record order + raw []byte // record file bytes, for restoration if a layer must be kept +} + +// EvictUnused evicts least-recently-used unprotected images — and the +// layers their removal leaves unreferenced — until ~targetBytes is freed +// or candidates run out. math.MaxInt64 means "free everything eligible"; +// targetBytes <= 0 evicts nothing. With dryRun nothing is deleted or +// renamed. +// +// Per-item failures are aggregated into the error, not fatal: the pass +// continues and each item retries next pass. Passes are serialized. An +// incomplete record or bundle-spec enumeration skips the pass entirely — +// refcounts and roots from partial data would retire layers that unread +// records still reference or running actors still mount. +func (s *Store) EvictUnused(ctx context.Context, targetBytes int64, dryRun bool) (EvictStats, error) { + var stats EvictStats + s.evictMu.Lock() + defer s.evictMu.Unlock() + + roots, rootsErr := s.InUse() + if rootsErr != nil { + // Same shape as the record gate below: refcounts and roots from a + // partial scan would retire layers a running actor still mounts. + slog.ErrorContext(ctx, "Image cache eviction pass skipped: bundle specs could not be fully enumerated", + slog.Any("err", rootsErr)) + return stats, rootsErr + } + cutoff := time.Now().Add(-s.minAge) + + candidates, refcount, complete, listErr := s.listEviction(roots, cutoff, &stats) + if !complete { + // A layer shared with an unread record would hit refcount zero and + // 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. + slog.ErrorContext(ctx, "Image cache eviction pass skipped: image records could not be fully enumerated", + slog.Any("err", listErr)) + return stats, listErr + } + stats.Candidates = len(candidates) + + slog.InfoContext(ctx, "Image cache eviction pass", + slog.Int64("target_bytes", targetBytes), + slog.Bool("dry_run", dryRun), + slog.Int("rooted_images", stats.RootedImages), + slog.Int("rooted_layers", len(roots.LayerHexes)), + slog.Int("candidates", len(candidates)), + slog.Duration("min_age", s.minAge)) + + var errs []error + dbg := slog.Default().Enabled(ctx, slog.LevelDebug) // see InUse + var retired []string // renamed-aside dirs awaiting RemoveAll + + for _, cand := range candidates { + if targetBytes <= 0 || stats.FreedBytes >= targetBytes { + break + } + if err := ctx.Err(); err != nil { + errs = append(errs, err) + break + } + + skip, err := s.removeStaleRecord(cand, cutoff, dryRun) + if err != nil { + errs = append(errs, err) + continue + } + switch skip { + case "fresh": + if dbg { + slog.DebugContext(ctx, "Image cache eviction skipped image", + slog.String("digest", cand.digest.String()), + slog.String("reason", skip), + slog.Time("last_used", cand.modTime)) + } + stats.SkippedFresh++ + continue + case "gone": + continue + } + slog.InfoContext(ctx, "Image cache evicting image record", + slog.String("digest", cand.digest.String()), + slog.Time("last_used", cand.modTime), + slog.Int("layers", len(cand.diffIDs)), + slog.Bool("dry_run", dryRun)) + + kept, candRetired, retireErrs := s.retireCandidateLayers(ctx, cand, refcount, roots, cutoff, dryRun, dbg, &stats) + errs = append(errs, retireErrs...) + retired = append(retired, candRetired...) + if kept { + // Restore the record so kept layers stay reachable, and put the + // refcounts back so later victims don't under-count shared + // layers. Already-retired layers stay retired — the stale record + // re-pulls only the gaps. + if !dryRun { + if err := s.restoreRecord(cand.digest, cand.raw); err != nil { + errs = append(errs, fmt.Errorf("while restoring record %s after kept layer: %w", cand.digest, err)) + } else { + slog.InfoContext(ctx, "Image cache restored image record: some of its layers must be kept", + slog.String("digest", cand.digest.String())) + } + } + for _, hex := range cand.diffIDs { + refcount[hex]++ + } + continue + } + stats.EvictedImages++ + } + + if len(retired) > 0 { + tRemove := time.Now() + errs = append(errs, removeRetiredDirs(retired)...) + slog.InfoContext(ctx, "Image cache removed retired layer dirs", + slog.Int("count", len(retired)), + slog.Duration("took", time.Since(tRemove))) + } + + return stats, errors.Join(errs...) +} + +// removeStaleRecord deletes the candidate's record unless a re-check +// against current disk state vetoes it — a pull or cache hit may have +// landed since listing. Runs under hitMu held exclusive so a hit's +// last-use touch cannot interleave with the re-check. Returns "gone" or +// "fresh" when the candidate must be skipped, "" to proceed. +func (s *Store) removeStaleRecord(cand evictionCandidate, cutoff time.Time, dryRun bool) (skip string, err error) { + s.hitMu.Lock() + defer s.hitMu.Unlock() + fi, err := os.Stat(s.recordPath(cand.digest)) + if errors.Is(err, os.ErrNotExist) { + return "gone", nil // e.g. removed by its multi-arch twin's pass + } else if err != nil { + return "", err + } + if fi.ModTime().After(cutoff) { + return "fresh", nil // touched since listing: in use moments ago + } + if !dryRun { + if err := os.Remove(s.recordPath(cand.digest)); err != nil && !errors.Is(err, os.ErrNotExist) { + return "", fmt.Errorf("while deleting image record %s: %w", cand.digest, err) + } + } + return "", nil +} + +// retireCandidateLayers retires the layers un-referenced by the +// candidate's removal, crediting stats, and reports kept=true if any +// layer must stay (still referenced, rooted, fresh, or failed to retire). +// The caller must then restore the record: a kept layer with no record is +// unreachable until the next restart. +func (s *Store) retireCandidateLayers(ctx context.Context, cand evictionCandidate, refcount map[string]int, roots RootSet, cutoff time.Time, dryRun, dbg bool, stats *EvictStats) (kept bool, retired []string, errs []error) { + for _, hex := range cand.diffIDs { + refcount[hex]-- + if refcount[hex] > 0 { + if dbg { + slog.DebugContext(ctx, "Image cache keeping layer: still referenced", + slog.String("diffid", hex), slog.Int("refcount", refcount[hex])) + } + continue + } + if roots.LayerHexes[hex] { + // Rooted by a bundle spec (the digestless-spec case): without + // its record this layer would strand when that bundle goes. + if dbg { + slog.DebugContext(ctx, "Image cache keeping layer: rooted by a bundle spec", + slog.String("diffid", hex)) + } + stats.SkippedRooted++ + kept = true + continue + } + var size int64 + var retiredPath string + var st retireStatus + if dryRun { + size, st = s.dryRunRetire(hex, cutoff) + } else { + var rerr error + if retiredPath, st, rerr = s.retireLayer(hex, cutoff); rerr != nil { + errs = append(errs, rerr) + kept = true + continue + } + if st == retireRetired { + // Credit from the retired dir, whose size file rode along + // with the rename (O(1); walked only if absent, read-only + // either way). Sizing after the retire means the eviction + // path never writes a backfill into the live pool. + if size, rerr = s.layerSizeReadOnly(retiredPath); rerr != nil { + size = 0 // unknown size: still evicted, credit nothing + } + } + } + switch st { + case retireGone: + continue + case retireVetoed: + kept = true + continue + } + if dbg { + slog.DebugContext(ctx, "Image cache retiring layer", + slog.String("diffid", hex), + slog.Int64("size_bytes", size), + slog.Bool("dry_run", dryRun)) + } + if retiredPath != "" { + retired = append(retired, retiredPath) + } + stats.FreedBytes += size + stats.EvictedLayers++ + } + return kept, retired, errs +} + +// removeRetiredDirs RemoveAlls renamed-aside layer dirs — the slow half +// of two-phase deletion, run outside every lock: the dirs are unreachable +// by diffID, so this contends with nothing. A crash mid-removal leaves +// ".rm-*" dirs for the startup sweep. +// +// Errors are collected per dir rather than through errgroup.Wait, which +// keeps only the first: every failed dir sits invisible as ".rm-*" until +// the next startup, so each deserves surfacing. +func removeRetiredDirs(dirs []string) []error { + var ( + mu sync.Mutex + errs []error + ) + g := new(errgroup.Group) + g.SetLimit(4) + for _, dir := range dirs { + g.Go(func() error { + if err := RemoveAllWritable(dir); err != nil { + mu.Lock() + errs = append(errs, fmt.Errorf("while removing retired layer %q: %w", dir, err)) + mu.Unlock() + } + return nil + }) + } + _ = g.Wait() // goroutines only ever return nil + return errs +} + +// RecoverOrphans reclaims layer dirs that no image record references. +// Called once from New, before the store serves — the one moment the +// scan is race-free: no pull is in flight, so a layer without a record +// is definitionally garbage. Orphans cannot arise in normal operation +// (pull writes the record first; eviction retires layers in the pass +// that drops their records): this reaps crash debris and operator +// damage, the accepted alternative to an fsck against a live pool every +// pass — mid-life debris leaks until the next restart, logged. +// +// Skipped entirely (ERROR) when the record or bundle-spec enumeration is +// incomplete: refcounts from partial data make referenced layers look +// like garbage, and a missing spec root would sweep a mounted layer. +// Bundle-spec roots and min-age still veto. +func (s *Store) RecoverOrphans(ctx context.Context) (EvictStats, error) { + var stats EvictStats + s.evictMu.Lock() + defer s.evictMu.Unlock() + + roots, rootsErr := s.InUse() + if rootsErr != nil { + slog.ErrorContext(ctx, "Image cache startup orphan scan skipped: bundle specs could not be fully enumerated; orphaned layers (if any) will persist until the specs are repaired", + slog.Any("err", rootsErr)) + return stats, rootsErr + } + cutoff := time.Now().Add(-s.minAge) + _, refcount, complete, listErr := s.listEviction(roots, cutoff, &stats) + if !complete { + slog.ErrorContext(ctx, "Image cache startup orphan scan skipped: image records could not be fully enumerated; orphaned layers (if any) will persist until the records are repaired", + slog.Any("err", listErr)) + return stats, listErr + } + + retired, errs := s.sweepOrphanLayers(ctx, roots, refcount, cutoff, false, &stats) + errs = append(errs, removeRetiredDirs(retired)...) + if stats.OrphanLayers > 0 { + slog.InfoContext(ctx, "Image cache startup scan reclaimed orphan layers", + slog.Int("count", stats.OrphanLayers), + slog.Int64("freed_bytes", stats.FreedBytes)) + } + return stats, errors.Join(errs...) +} + +// sweepOrphanLayers retires layer dirs that no surviving image record +// references. Returns the renamed-aside paths for the caller's async +// removal batch, plus any per-layer errors (never fatal). +func (s *Store) sweepOrphanLayers(ctx context.Context, roots RootSet, refcount map[string]int, cutoff time.Time, dryRun bool, stats *EvictStats) ([]string, []error) { + entries, err := os.ReadDir(s.layersDir()) + if err != nil { + return nil, []error{fmt.Errorf("while listing layer pool for orphan sweep: %w", err)} + } + + dbg := slog.Default().Enabled(ctx, slog.LevelDebug) // see InUse + var retired []string + var errs []error + for _, e := range entries { + hex := e.Name() + // Dot-prefixed dirs are in-flight unpacks (".tmp-") or already + // retired (".rm-"); both are handled elsewhere. Anything that is + // not a well-formed digest was not put there by the store, so the + // sweep leaves it alone rather than deleting an operator's file (or + // panicking on a name too short to abbreviate). + if !e.IsDir() || strings.HasPrefix(hex, ".") || !isLayerDirName(hex) { + continue + } + if refcount[hex] > 0 { + continue // referenced by a surviving record + } + if roots.LayerHexes[hex] { + // A bundle spec references it directly — the digestless-spec + // case, where the record is gone but an actor is still using + // these layers. + continue + } + var size int64 + var retiredPath string + var st retireStatus + if dryRun { + size, st = s.dryRunRetire(hex, cutoff) + } else { + var rerr error + if retiredPath, st, rerr = s.retireLayer(hex, cutoff); rerr != nil { + errs = append(errs, rerr) + continue + } + if st == retireRetired { + // See retireCandidateLayers: size file rode along with the + // rename; read-only, never a write into the live pool. + if size, rerr = s.layerSizeReadOnly(retiredPath); rerr != nil { + size = 0 + } + } + } + if st != retireRetired { + continue // too young, or vanished under us + } + if dbg { + slog.DebugContext(ctx, "Image cache retiring orphan layer", + slog.String("diffid", hex), + slog.Int64("size_bytes", size), + slog.Bool("dry_run", dryRun)) + } + if retiredPath != "" { + retired = append(retired, retiredPath) + } + stats.FreedBytes += size + stats.OrphanLayers++ + } + return retired, errs +} + +// dryRunRetire reports what retireLayer would do, without renaming. +// Sizing is read-only: even a size-file backfill would break dry-run's +// mutate-nothing contract. +func (s *Store) dryRunRetire(hex string, cutoff time.Time) (int64, retireStatus) { + dir := filepath.Join(s.layersDir(), hex) + fi, err := os.Stat(dir) + if err != nil { + return 0, retireGone + } + if fi.ModTime().After(cutoff) { + return 0, retireVetoed + } + size, err := s.layerSizeReadOnly(dir) + if err != nil { + size = 0 + } + return size, retireRetired +} + +// listEviction builds the LRU-ordered candidate list and the layer +// refcounts over ALL image records — rooted and fresh included, since +// 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. +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()) + if err != nil { + return nil, refcount, false, fmt.Errorf("while listing manifest records: %w", err) + } + complete = true + var candidates []evictionCandidate + var errs []error + for _, e := range entries { + name := e.Name() + if strings.HasPrefix(name, ".") || !strings.HasSuffix(name, ".json") { + continue + } + hex := strings.TrimSuffix(name, ".json") + digest := v1.Hash{Algorithm: "sha256", Hex: hex} + + b, err := os.ReadFile(filepath.Join(s.manifestsDir(), name)) + if err != nil { + // This record's layers get no refcounts while the record itself + // survives, so its layers would look like orphans. + complete = false + errs = append(errs, fmt.Errorf("while reading image record %s: %w", digest, err)) + continue + } + var rec imageRecord + if err := json.Unmarshal(b, &rec); err != nil { + // An undecodable record contributes no refcounts, so evicting it + // would strand its (unknown) layers as orphans until the next + // restart. Never a candidate: leave it in place and fail the + // enumeration — the error names the file for the operator. + complete = false + errs = append(errs, fmt.Errorf("while decoding image record %s: %w", digest, err)) + continue + } + + // Dedupe diffIDs per record (images may list a layer twice) so the + // decrement in EvictUnused stays symmetric with this count. + seen := map[string]bool{} + var unique []string + for _, d := range rec.DiffIDs { + diffID, err := v1.NewHash(d) + if err != nil { + // A garbled diffID contributes no refcount, so a layer + // referenced only through it would look unreferenced — the + // same failure direction as an undecodable record. Gate. + complete = false + errs = append(errs, fmt.Errorf("invalid diffID %q in image record %s: %w", d, digest, err)) + continue + } + if !seen[diffID.Hex] { + seen[diffID.Hex] = true + unique = append(unique, diffID.Hex) + refcount[diffID.Hex]++ + } + } + + fi, err := e.Info() + if err != nil { + continue + } + // A record is rooted either by digest (a bundle spec naming it) or + // because its exact layer set matches a rooted bundle's (see + // RootSet.LayerSets — deliberately not "every layer rooted + // somewhere"). The exact-set rule covers the multi-arch twin — + // pull records an image under both the index and per-platform + // child digest, but a bundle spec carries only the requested + // one — and digestless (pre-ImageDigest) specs. Without it the + // twin is evicted and rewritten on every pull of a rooted image: + // harmless but pure churn, and it inflates the eviction counters. + if roots.ImageDigests[digest.String()] || (len(unique) > 0 && roots.LayerSets[layerSetSignature(unique)]) { + stats.RootedImages++ + continue + } + if fi.ModTime().After(cutoff) { + stats.SkippedFresh++ + continue + } + candidates = append(candidates, evictionCandidate{digest: digest, modTime: fi.ModTime(), diffIDs: unique, raw: b}) + } + + // LRU by last use, ties broken by name for determinism. + sort.Slice(candidates, func(i, j int) bool { + if candidates[i].modTime.Equal(candidates[j].modTime) { + return candidates[i].digest.Hex < candidates[j].digest.Hex + } + return candidates[i].modTime.Before(candidates[j].modTime) + }) + return candidates, refcount, complete, errors.Join(errs...) +} + +// restoreRecord atomically rewrites a record from bytes captured at +// listing time, after eviction deleted it but had to keep a layer — +// without the record the kept layer is unreachable until restart. The +// rewrite bumps the mtime, so min-age keeps the image off the next +// candidate list instead of churning delete-and-restore every pass; the +// cost is the true last-use, so a cold restored image sorts as fresh +// until it re-ages. +func (s *Store) restoreRecord(digest v1.Hash, raw []byte) error { + path := s.recordPath(digest) + tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".tmp-*") + if err != nil { + return fmt.Errorf("while creating record temp file: %w", err) + } + defer os.Remove(tmp.Name()) // no-op once the rename succeeds + if _, err := tmp.Write(raw); err != nil { + tmp.Close() + return fmt.Errorf("while writing record: %w", err) + } + if err := tmp.Close(); err != nil { + return fmt.Errorf("while closing record: %w", err) + } + if err := os.Rename(tmp.Name(), path); err != nil { + return fmt.Errorf("while moving record into place: %w", err) + } + return nil +} diff --git a/internal/imagecache/gc_regression_test.go b/internal/imagecache/gc_regression_test.go new file mode 100644 index 000000000..1107137e7 --- /dev/null +++ b/internal/imagecache/gc_regression_test.go @@ -0,0 +1,373 @@ +// 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 imagecache + +// Eviction tests for damaged and in-between pool states — crash debris, +// interrupted or wedged pulls, unreadable records — where the required +// behavior is failing toward retention. Mainline eviction behavior is +// covered in gc_test.go. + +import ( + "archive/tar" + "context" + "io" + "math" + "os" + "path/filepath" + "strings" + "testing" + "time" + + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/tarball" +) + +// A layer left in the pool with no referencing image record must be +// reclaimable. That state is crash debris by definition (pull pre-writes +// its record; eviction retires layers in the pass that drops their +// records), so it is reclaimed by the STARTUP scan — and deliberately NOT +// by the periodic pass, which reaches layers only through records. +func TestOrphanLayerReclaimedAtStartup(t *testing.T) { + _, host := newTestRegistry(t) + ref := host + "/test/orphan:latest" + pushImage(t, ref, v1.Config{}, layerFromEntries(t, []tarEntry{ + {name: "f", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("o", 4096)}, + })) + + store := newTestStore(t) + img := mustEnsure(t, store, ref) + + // Crash-debris state: layer on disk, no record referencing it. + if err := os.Remove(store.recordPath(img.Digest)); err != nil { + t.Fatal(err) + } + backdateStore(t, store, 3*time.Hour) + + // The periodic pass must NOT touch it: no online whole-pool scans. + if _, err := store.EvictUnused(context.Background(), math.MaxInt64, false); err != nil { + t.Fatalf("EvictUnused: %v", err) + } + if _, err := os.Stat(img.LayerDirs[0]); err != nil { + t.Fatalf("periodic pass swept an orphan; that scan belongs to startup only: %v", err) + } + + // "Restart": reopening the store runs RecoverOrphans and reclaims it. + reopened, err := New(store.root) + if err != nil { + t.Fatalf("New (restart): %v", err) + } + if got := layerDirsOnDisk(t, reopened); len(got) != 0 { + t.Errorf("orphan layers survive startup recovery: %v", got) + } + if size, err := reopened.CacheSize(); err != nil || size != 0 { + t.Errorf("CacheSize() = %d, %v after startup recovery; orphan bytes still counted", size, err) + } +} + +// A digestless bundle spec (written before OverlaySpec.ImageDigest +// existed, i.e. an actor running across that upgrade) must not strand its +// layers. The exact-layer-set rooting rule keeps the RECORD alive while +// the bundle exists, so when the bundle goes the record and layers are +// evicted together through the ordinary path — no orphan is ever +// manufactured, and no sweep is needed. +func TestDigestlessSpecLayersReclaimedAfterBundleGone(t *testing.T) { + _, host := newTestRegistry(t) + ref := host + "/test/upgrade:latest" + pushImage(t, ref, v1.Config{}, layerFromEntries(t, []tarEntry{ + {name: "f", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("u", 4096)}, + })) + + actorsDir := t.TempDir() + store := newTestStore(t, WithActorsDir(actorsDir)) + img := mustEnsure(t, store, ref) + + // A bundle written before ImageDigest existed: layers only. + bundle := filepath.Join(actorsDir, "actor-old", "bundles", "main") + if err := os.MkdirAll(bundle, 0o700); err != nil { + t.Fatal(err) + } + if err := WriteSpec(bundle, &OverlaySpec{Layers: img.LayerDirs}); err != nil { + t.Fatal(err) + } + backdateStore(t, store, 3*time.Hour) + + // Pass 1, while the actor runs: the exact-layer-set rule roots the + // record, so record AND layer both survive. + if _, err := store.EvictUnused(context.Background(), math.MaxInt64, false); err != nil { + t.Fatalf("EvictUnused: %v", err) + } + if _, err := os.Stat(img.LayerDirs[0]); err != nil { + t.Fatalf("layer of a running (digestless) actor was evicted: %v", err) + } + if _, err := os.Stat(store.recordPath(img.Digest)); err != nil { + t.Fatalf("record of a running (digestless) actor was evicted — that would strand its layers: %v", err) + } + + // The actor finishes; atelet removes the bundle. + if err := os.RemoveAll(filepath.Join(actorsDir, "actor-old")); err != nil { + t.Fatal(err) + } + backdateStore(t, store, 3*time.Hour) + + if _, err := store.EvictUnused(context.Background(), math.MaxInt64, false); err != nil { + t.Fatalf("EvictUnused: %v", err) + } + if got := layerDirsOnDisk(t, store); len(got) != 0 { + t.Errorf("digestless-spec layers not reclaimed after bundle removal: %v", got) + } +} + +// Layers of an in-flight pull must survive eviction, structurally: pull +// writes the record before unpacking, so a mid-pull layer is held by the +// record's refcount, renewed by the per-layer progress touch for as long +// as the pull advances. The shape here: a fresh record protecting even an +// OLD layer (which may have been in the pool for months from another +// image). +func TestRecordFirstProtectsInFlightPull(t *testing.T) { + store := newTestStore(t) // default min-age: 2m + + diffID := v1.Hash{Algorithm: "sha256", Hex: strings.Repeat("5c", 32)} + pending := v1.Hash{Algorithm: "sha256", Hex: strings.Repeat("6d", 32)} + digest := v1.Hash{Algorithm: "sha256", Hex: strings.Repeat("7e", 32)} + dir := store.layerDir(diffID) + if err := os.MkdirAll(filepath.Join(dir, layerFSDirName), 0o700); err != nil { + t.Fatal(err) + } + rec := imageRecord{Version: 1, DiffIDs: []string{diffID.String(), pending.String()}} + if err := store.writeRecord(digest, rec); err != nil { + t.Fatal(err) + } + backdate(t, dir, 3*time.Hour) + + if _, err := store.EvictUnused(context.Background(), math.MaxInt64, false); err != nil { + t.Fatalf("EvictUnused: %v", err) + } + if _, err := os.Stat(dir); err != nil { + t.Fatalf("eviction reclaimed a layer referenced by a fresh (in-flight) record: %v", err) + } + if _, err := os.Stat(store.recordPath(digest)); err != nil { + t.Fatalf("eviction removed a fresh (in-flight) record: %v", err) + } +} + +// The wedged-pull disposal, with the partial layer FRESH — the realistic +// state, since a wedged pull's landed layers are seconds old. The +// contract: the stale record may be selected, but because min-age vetoes +// the fresh layer, the record is RESTORED — nothing is stranded, and the +// whole unit becomes evictable together once the layer ages. +func TestWedgedPullFreshLayersNotStranded(t *testing.T) { + store := newTestStore(t) // default min-age: 2m + + diffID := v1.Hash{Algorithm: "sha256", Hex: strings.Repeat("5c", 32)} + digest := v1.Hash{Algorithm: "sha256", Hex: strings.Repeat("7e", 32)} + dir := store.layerDir(diffID) + if err := os.MkdirAll(filepath.Join(dir, layerFSDirName), 0o700); err != nil { + t.Fatal(err) + } + rec := imageRecord{Version: 1, DiffIDs: []string{diffID.String()}} + if err := store.writeRecord(digest, rec); err != nil { + t.Fatal(err) + } + // Wedged: the record has seen no progress touch for > min-age, but the + // layer itself landed moments ago. + backdate(t, store.recordPath(digest), 3*time.Hour) + + for i := 0; i < 4; i++ { + if _, err := store.EvictUnused(context.Background(), math.MaxInt64, false); err != nil { + t.Fatalf("EvictUnused pass %d: %v", i, err) + } + } + if _, err := os.Stat(dir); err != nil { + t.Fatalf("fresh partial layer was evicted: %v", err) + } + // The layer must not be stranded: its record must still exist (restored + // after the min-age veto fired), keeping it reachable by the runtime + // pass. + if _, err := os.Stat(store.recordPath(digest)); err != nil { + t.Fatalf("record gone while its fresh layer survives: the layer is stranded until restart: %v", err) + } + + // Once the layer ages past min-age (and the restored record does too), + // the unit is evicted together — reclaimed, not leaked. + backdate(t, store.recordPath(digest), 3*time.Hour) + backdate(t, dir, 3*time.Hour) + if _, err := store.EvictUnused(context.Background(), math.MaxInt64, false); err != nil { + t.Fatalf("EvictUnused: %v", err) + } + if _, err := os.Stat(store.recordPath(digest)); !os.IsNotExist(err) { + t.Errorf("aged wedged-pull record survived: %v", err) + } + if _, err := os.Stat(dir); !os.IsNotExist(err) { + t.Errorf("aged wedged-pull layer survived: %v", err) + } +} + +// The direct restore-on-keep shape: a record whose deletion would strand +// a kept layer must be restored. Here the keep is caused by a rooted layer +// from a digestless bundle spec whose layer set does NOT exactly match the +// record (so LayerSets does not root the record itself). +func TestEvictionRestoresRecordWhenLayerRooted(t *testing.T) { + actorsDir := t.TempDir() + store := newTestStore(t, WithActorsDir(actorsDir)) + + shared := v1.Hash{Algorithm: "sha256", Hex: strings.Repeat("1a", 32)} + private := v1.Hash{Algorithm: "sha256", Hex: strings.Repeat("2b", 32)} + digest := v1.Hash{Algorithm: "sha256", Hex: strings.Repeat("3c", 32)} + for _, d := range []v1.Hash{shared, private} { + if err := os.MkdirAll(filepath.Join(store.layerDir(d), layerFSDirName), 0o700); err != nil { + t.Fatal(err) + } + } + rec := imageRecord{Version: 1, DiffIDs: []string{shared.String(), private.String()}} + if err := store.writeRecord(digest, rec); err != nil { + t.Fatal(err) + } + // A digestless spec roots ONLY the shared layer (subset — LayerSets + // cannot root the record). + bundle := filepath.Join(actorsDir, "actor-x", "bundles", "main") + if err := os.MkdirAll(bundle, 0o700); err != nil { + t.Fatal(err) + } + if err := WriteSpec(bundle, &OverlaySpec{Layers: []string{store.layerDir(shared)}}); err != nil { + t.Fatal(err) + } + backdateStore(t, store, 3*time.Hour) + + if _, err := store.EvictUnused(context.Background(), math.MaxInt64, false); err != nil { + t.Fatalf("EvictUnused: %v", err) + } + // The private layer may go; the rooted layer stays — and therefore the + // record must have been restored, or the rooted layer is stranded the + // moment the bundle disappears. + if _, err := os.Stat(store.layerDir(shared)); err != nil { + t.Fatalf("rooted layer evicted: %v", err) + } + if _, err := os.Stat(store.recordPath(digest)); err != nil { + t.Fatalf("record gone while its rooted layer survives (stranded once the bundle goes): %v", err) + } +} + +// Refcounts derived from a partial record enumeration must never drive +// orphan reclamation: the startup scan skips itself entirely +// (conservative, logged) when any record fails to read or decode — while +// New still succeeds, because a corrupt record must not keep atelet from +// serving actors. +func TestStartupOrphanScanSkippedWhenEnumerationIncomplete(t *testing.T) { + _, host := newTestRegistry(t) + ref := host + "/test/enum:latest" + pushImage(t, ref, v1.Config{}, layerFromEntries(t, []tarEntry{ + {name: "f", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("e", 2048)}, + })) + store := newTestStore(t) + img := mustEnsure(t, store, ref) + backdateStore(t, store, 3*time.Hour) + + // A genuine orphan AND a corrupt record: the orphan must be spared + // because the corrupt record makes the enumeration untrustworthy. + orphan := filepath.Join(store.layersDir(), strings.Repeat("aa", 32)) + if err := os.MkdirAll(filepath.Join(orphan, layerFSDirName), 0o700); err != nil { + t.Fatal(err) + } + backdate(t, orphan, 3*time.Hour) + if err := os.WriteFile(store.recordPath(v1.Hash{Algorithm: "sha256", Hex: strings.Repeat("bb", 32)}), []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + + reopened, err := New(store.root) + if err != nil { + t.Fatalf("New must survive a corrupt record (atelet must still serve): %v", err) + } + if _, err := os.Stat(orphan); err != nil { + t.Errorf("startup scan ran on an incomplete enumeration and swept a layer: %v", err) + } + // The intact image is untouched either way. + for _, d := range img.LayerDirs { + if _, err := os.Stat(d); err != nil { + t.Errorf("intact image layer swept: %v", err) + } + } + _ = reopened +} + +// A directory name that is not a layer digest must never be treated as a +// layer — not by the periodic pass, and not by the startup scan (which +// would otherwise also panic abbreviating a short name for rename-aside). +func TestNonLayerDirsIgnored(t *testing.T) { + store := newTestStore(t, WithMinAge(0)) + junk := filepath.Join(store.layersDir(), "notadigest") + if err := os.MkdirAll(junk, 0o700); err != nil { + t.Fatal(err) + } + stats, err := store.EvictUnused(context.Background(), math.MaxInt64, false) + if err != nil { + t.Fatalf("EvictUnused: %v", err) + } + if stats.OrphanLayers != 0 { + t.Errorf("periodic pass swept something: %+v", stats) + } + if _, err := New(store.root, WithMinAge(0)); err != nil { + t.Fatalf("New: %v", err) + } + if _, err := os.Stat(junk); err != nil { + t.Errorf("non-layer dir was removed: %v", err) + } +} + +// The record must exist BEFORE unpacking — the load-bearing ordering. A +// pull that fails partway therefore leaves a record (resumable progress +// that ages out via LRU), never unexplained layers. +func TestFailedPullLeavesResumableRecordNotOrphans(t *testing.T) { + _, host := newTestRegistry(t) + ref := host + "/test/badlayer:latest" + good := layerFromEntries(t, []tarEntry{ + {name: "ok", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("g", 2048)}, + }) + // Valid blob, invalid tar: unpack fails after download succeeds. + bad, err := tarball.LayerFromOpener(func() (io.ReadCloser, error) { + return io.NopCloser(strings.NewReader("this is not a tar archive at all")), nil + }) + if err != nil { + t.Fatal(err) + } + pushImage(t, ref, v1.Config{}, good, bad) + + store := newTestStore(t) + _, err = store.EnsureImage(context.Background(), ref) + if err == nil { + t.Fatal("EnsureImage succeeded on an image with an untarrable layer") + } + + recs, err := os.ReadDir(store.manifestsDir()) + if err != nil { + t.Fatal(err) + } + if len(recs) == 0 { + t.Fatal("failed pull left no record: its landed layers are unexplained orphans") + } + // Whatever layers landed are referenced by that record — assert none is + // an orphan by running the startup scan and checking nothing is swept. + backdateStore(t, store, 3*time.Hour) + for _, r := range recs { + backdate(t, filepath.Join(store.manifestsDir(), r.Name()), 0) // keep records fresh + } + before := layerDirsOnDisk(t, store) + reopened, err := New(store.root) + if err != nil { + t.Fatalf("New: %v", err) + } + if after := layerDirsOnDisk(t, reopened); len(after) != len(before) { + t.Errorf("startup scan swept layers of a failed-but-recorded pull: %v -> %v", before, after) + } +} diff --git a/internal/imagecache/gc_test.go b/internal/imagecache/gc_test.go new file mode 100644 index 000000000..35357c3fd --- /dev/null +++ b/internal/imagecache/gc_test.go @@ -0,0 +1,516 @@ +// 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 imagecache + +import ( + "archive/tar" + "context" + "math" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + v1 "github.com/google/go-containerregistry/pkg/v1" +) + +// backdateStore ages every image record and layer dir so the min-age veto +// no longer applies, making eviction tests deterministic without sleeps. +func backdateStore(t *testing.T, s *Store, age time.Duration) { + t.Helper() + for _, dir := range []string{s.manifestsDir(), s.layersDir()} { + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("listing %q: %v", dir, err) + } + for _, e := range entries { + if strings.HasPrefix(e.Name(), ".") { + continue + } + backdate(t, filepath.Join(dir, e.Name()), age) + } + } +} + +func layerDirsOnDisk(t *testing.T, s *Store) []string { + t.Helper() + entries, err := os.ReadDir(s.layersDir()) + if err != nil { + t.Fatalf("listing layer pool: %v", err) + } + var out []string + for _, e := range entries { + if !strings.HasPrefix(e.Name(), ".") { + out = append(out, e.Name()) + } + } + return out +} + +func TestEvictUnusedMinAgeVeto(t *testing.T) { + _, host := newTestRegistry(t) + ref := host + "/test/fresh:latest" + pushImage(t, ref, v1.Config{}, layerFromEntries(t, []tarEntry{ + {name: "f", typeflag: tar.TypeReg, mode: 0o644, body: "hi"}, + })) + + store := newTestStore(t) // default minAge = 2m; everything is younger + mustEnsure(t, store, ref) + + stats, err := store.EvictUnused(context.Background(), math.MaxInt64, false) + if err != nil { + t.Fatalf("EvictUnused: %v", err) + } + if stats.EvictedImages != 0 || stats.EvictedLayers != 0 { + t.Errorf("evicted fresh content: %+v", stats) + } + if got := layerDirsOnDisk(t, store); len(got) == 0 { + t.Error("fresh layers were removed") + } +} + +func TestEvictUnusedLRUSharedLayersAndRepull(t *testing.T) { + _, host := newTestRegistry(t) + shared := layerFromEntries(t, []tarEntry{ + {name: "shared", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("s", 2048)}, + }) + onlyA := layerFromEntries(t, []tarEntry{ + {name: "a", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("a", 2048)}, + }) + onlyB := layerFromEntries(t, []tarEntry{ + {name: "b", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("b", 2048)}, + }) + refA := host + "/test/a:latest" + refB := host + "/test/b:latest" + pushImage(t, refA, v1.Config{}, shared, onlyA) + pushImage(t, refB, v1.Config{}, shared, onlyB) + + store := newTestStore(t) + imgA := mustEnsure(t, store, refA) + imgB := mustEnsure(t, store, refB) + if imgA.LayerDirs[0] != imgB.LayerDirs[0] { + t.Fatalf("shared layer not deduplicated: %q vs %q", imgA.LayerDirs[0], imgB.LayerDirs[0]) + } + + backdateStore(t, store, 3*time.Hour) + // Make A strictly older than B so LRU picks A first. + backdate(t, store.recordPath(imgA.Digest), 4*time.Hour) + + // A small target: only the LRU image (A) should go. Its private layer is + // deleted; the shared layer survives via B's reference. + stats, err := store.EvictUnused(context.Background(), 1, false) + if err != nil { + t.Fatalf("EvictUnused(1): %v", err) + } + if stats.EvictedImages != 1 { + t.Fatalf("evicted %d images, want 1 (stats %+v)", stats.EvictedImages, stats) + } + if stats.FreedBytes <= 0 { + t.Errorf("FreedBytes = %d, want > 0", stats.FreedBytes) + } + if _, err := os.Stat(store.recordPath(imgA.Digest)); !os.IsNotExist(err) { + t.Error("LRU image record A still present") + } + if _, err := os.Stat(store.recordPath(imgB.Digest)); err != nil { + t.Errorf("newer image record B missing: %v", err) + } + if _, err := os.Stat(imgA.LayerDirs[1]); !os.IsNotExist(err) { + t.Error("A's private layer not evicted") + } + if _, err := os.Stat(imgA.LayerDirs[0]); err != nil { + t.Errorf("shared layer evicted while B still references it: %v", err) + } + + // Free-everything pass: B goes too, and the pool empties. + if _, err := store.EvictUnused(context.Background(), math.MaxInt64, false); err != nil { + t.Fatalf("EvictUnused(max): %v", err) + } + if got := layerDirsOnDisk(t, store); len(got) != 0 { + t.Errorf("layers remain after free-everything pass: %v", got) + } + + // The registry is still up: an evicted image is simply a re-pull away. + imgA2 := mustEnsure(t, store, refA) + if _, err := os.Stat(filepath.Join(imgA2.LayerDirs[1], layerFSDirName, "a")); err != nil { + t.Errorf("re-pulled image content missing: %v", err) + } +} + +func TestEvictUnusedRootSet(t *testing.T) { + _, host := newTestRegistry(t) + refRooted := host + "/test/rooted:latest" + refLayerRooted := host + "/test/layer-rooted:latest" + refLoose := host + "/test/loose:latest" + for _, r := range []string{refRooted, refLayerRooted, refLoose} { + pushImage(t, r, v1.Config{}, layerFromEntries(t, []tarEntry{ + {name: "f-" + r[len(r)-8:], typeflag: tar.TypeReg, mode: 0o644, body: r}, + })) + } + + actorsDir := t.TempDir() + store := newTestStore(t, WithActorsDir(actorsDir)) + imgRooted := mustEnsure(t, store, refRooted) + imgLayerRooted := mustEnsure(t, store, refLayerRooted) + imgLoose := mustEnsure(t, store, refLoose) + + // A modern bundle spec (with digest) roots imgRooted entirely. + bundle1 := filepath.Join(actorsDir, "actor-1", "bundles", "main") + if err := os.MkdirAll(bundle1, 0o700); err != nil { + t.Fatal(err) + } + if err := WriteSpec(bundle1, &OverlaySpec{ImageDigest: imgRooted.Digest.String(), Layers: imgRooted.LayerDirs}); err != nil { + t.Fatal(err) + } + // A digestless (pre-ImageDigest) spec roots only imgLayerRooted's layers. + bundle2 := filepath.Join(actorsDir, "actor-2", "bundles", "main") + if err := os.MkdirAll(bundle2, 0o700); err != nil { + t.Fatal(err) + } + if err := WriteSpec(bundle2, &OverlaySpec{Layers: imgLayerRooted.LayerDirs}); err != nil { + t.Fatal(err) + } + + rs, err := store.InUse() + if err != nil { + t.Fatalf("InUse: %v", err) + } + if !rs.ImageDigests[imgRooted.Digest.String()] { + t.Error("InUse missing digest-rooted image") + } + if !rs.LayerHexes[filepath.Base(imgLayerRooted.LayerDirs[0])] { + t.Error("InUse missing layer-rooted layer") + } + + backdateStore(t, store, 3*time.Hour) + stats, err := store.EvictUnused(context.Background(), math.MaxInt64, false) + if err != nil { + t.Fatalf("EvictUnused: %v", err) + } + + // Rooted image: record and layers untouched. + if _, err := os.Stat(store.recordPath(imgRooted.Digest)); err != nil { + t.Errorf("digest-rooted image record evicted: %v", err) + } + if _, err := os.Stat(imgRooted.LayerDirs[0]); err != nil { + t.Errorf("digest-rooted image layer evicted: %v", err) + } + // Layer-rooted (old, digestless spec): the layers must stay, and so must + // the record — a record all of whose layers are rooted is itself treated + // as rooted. Without that rule the record is evicted while its layers + // survive, which manufactures an orphan the moment the bundle goes away + // (see TestDigestlessSpecLayersReclaimedAfterBundleGone) and churns the + // record on every pull. + if _, err := os.Stat(imgLayerRooted.LayerDirs[0]); err != nil { + t.Errorf("layer-rooted layer evicted: %v", err) + } + if _, err := os.Stat(store.recordPath(imgLayerRooted.Digest)); err != nil { + t.Errorf("record with all layers rooted was evicted: %v", err) + } + if stats.RootedImages < 2 { + t.Errorf("RootedImages = %d, want >= 2 (digest-rooted + all-layers-rooted), stats %+v", stats.RootedImages, stats) + } + // The loose image is fully evicted. + if _, err := os.Stat(store.recordPath(imgLoose.Digest)); !os.IsNotExist(err) { + t.Error("unrooted image record survived a free-everything pass") + } + if _, err := os.Stat(imgLoose.LayerDirs[0]); !os.IsNotExist(err) { + t.Error("unrooted layer survived a free-everything pass") + } +} + +// An unreadable record must gate the whole pass: its refcounts are +// invisible, so a layer it shares with a readable candidate would hit +// zero and be retired while the unreadable record still names it. +func TestEvictUnusedSkipsPassOnUnreadableRecord(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("chmod 0000 does not block reads for root") + } + _, host := newTestRegistry(t) + shared := layerFromEntries(t, []tarEntry{ + {name: "shared", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("s", 2048)}, + }) + onlyA := layerFromEntries(t, []tarEntry{ + {name: "a", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("a", 2048)}, + }) + onlyB := layerFromEntries(t, []tarEntry{ + {name: "b", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("b", 2048)}, + }) + refA := host + "/test/unreadable-a:latest" + refB := host + "/test/unreadable-b:latest" + pushImage(t, refA, v1.Config{}, shared, onlyA) + pushImage(t, refB, v1.Config{}, shared, onlyB) + + store := newTestStore(t) + imgA := mustEnsure(t, store, refA) + imgB := mustEnsure(t, store, refB) + backdateStore(t, store, 3*time.Hour) + + if err := os.Chmod(store.recordPath(imgA.Digest), 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(store.recordPath(imgA.Digest), 0o600) }) + + stats, err := store.EvictUnused(context.Background(), math.MaxInt64, false) + if err == nil { + t.Fatal("EvictUnused returned no error on an unreadable record") + } + if stats.EvictedImages != 0 || stats.EvictedLayers != 0 || stats.FreedBytes != 0 { + t.Errorf("gated pass still evicted: %+v", stats) + } + // B and the shared layer survive: evicting B on partial refcounts would + // have retired the shared layer out from under A. + if _, err := os.Stat(store.recordPath(imgB.Digest)); err != nil { + t.Errorf("record B evicted during a gated pass: %v", err) + } + if _, err := os.Stat(imgB.LayerDirs[0]); err != nil { + t.Errorf("shared layer retired while an unreadable record references it: %v", err) + } +} + +// An unreadable actors dir, bundles dir, or bundle spec must gate the +// whole pass, exactly like an unreadable record: a missing root fails +// toward retiring a running actor's layers, and for a long-running actor +// the spec is the only protection (its record mtime can be arbitrarily +// old, so min-age cannot save it). +func TestEvictUnusedSkipsPassOnUnreadableRoots(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("chmod 0000 does not block reads for root") + } + cases := []struct { + name string + deny func(t *testing.T, actorsDir, bundleDir string) + }{ + {"actors dir unreadable", func(t *testing.T, actorsDir, _ string) { + denyRead(t, actorsDir) + }}, + {"bundles dir unreadable", func(t *testing.T, actorsDir, _ string) { + denyRead(t, filepath.Join(actorsDir, "actor-1", "bundles")) + }}, + {"bundle spec unreadable", func(t *testing.T, _, bundleDir string) { + denyRead(t, filepath.Join(bundleDir, OverlaySpecFileName)) + }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, host := newTestRegistry(t) + ref := host + "/test/rootgate:latest" + pushImage(t, ref, v1.Config{}, layerFromEntries(t, []tarEntry{ + {name: "f", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("g", 1024)}, + })) + + actorsDir := t.TempDir() + store := newTestStore(t, WithActorsDir(actorsDir)) + img := mustEnsure(t, store, ref) + + // A running actor roots the image; its record is arbitrarily old. + bundleDir := filepath.Join(actorsDir, "actor-1", "bundles", "main") + if err := os.MkdirAll(bundleDir, 0o700); err != nil { + t.Fatal(err) + } + if err := WriteSpec(bundleDir, &OverlaySpec{ImageDigest: img.Digest.String(), Layers: img.LayerDirs}); err != nil { + t.Fatal(err) + } + backdateStore(t, store, 3*time.Hour) + + tc.deny(t, actorsDir, bundleDir) + + stats, err := store.EvictUnused(context.Background(), math.MaxInt64, false) + if err == nil { + t.Fatal("EvictUnused returned no error with an unenumerable root set") + } + if stats.EvictedImages != 0 || stats.EvictedLayers != 0 { + t.Errorf("gated pass still evicted: %+v", stats) + } + if _, err := os.Stat(store.recordPath(img.Digest)); err != nil { + t.Errorf("record evicted while its actor's roots were unreadable: %v", err) + } + if _, err := os.Stat(img.LayerDirs[0]); err != nil { + t.Errorf("mounted layer retired while its actor's roots were unreadable: %v", err) + } + }) + } +} + +// denyRead removes all permissions and restores them at cleanup (so +// t.TempDir removal works). +func denyRead(t *testing.T, path string) { + t.Helper() + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if err := os.Chmod(path, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(path, fi.Mode().Perm()) }) +} + +// A record whose layer references cannot be established — undecodable +// JSON, or a garbled diffID inside valid JSON — contributes no refcounts, +// so a layer referenced only through it would look unreferenced. Either +// shape must gate the pass, and the record must survive it: evicting the +// record would strand its unknown layers as orphans until restart. +func TestEvictUnusedSkipsPassOnBadRecord(t *testing.T) { + cases := []struct { + name string + badRecord string + }{ + {"undecodable JSON", `{not json`}, + {"garbled diffID", `{"version":1,"diffIDs":["not-a-hash"]}`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + store := newTestStore(t) + layer := filepath.Join(store.layersDir(), strings.Repeat("cc", 32)) + if err := os.MkdirAll(filepath.Join(layer, layerFSDirName), 0o700); err != nil { + t.Fatal(err) + } + badPath := store.recordPath(v1.Hash{Algorithm: "sha256", Hex: strings.Repeat("dd", 32)}) + if err := os.WriteFile(badPath, []byte(tc.badRecord), 0o600); err != nil { + t.Fatal(err) + } + backdateStore(t, store, 3*time.Hour) + + stats, err := store.EvictUnused(context.Background(), math.MaxInt64, false) + if err == nil { + t.Fatal("EvictUnused returned no error on a bad record") + } + if stats.EvictedImages != 0 || stats.EvictedLayers != 0 { + t.Errorf("gated pass still evicted: %+v", stats) + } + if _, err := os.Stat(badPath); err != nil { + t.Errorf("bad record was evicted (its unknown layers would be stranded): %v", err) + } + if _, err := os.Stat(layer); err != nil { + t.Errorf("layer removed during a gated pass: %v", err) + } + }) + } +} + +// Dry-run must not write anything — including the size-file backfill for +// layers unpacked before sizes were recorded, the exact population a +// dry-run soak on upgraded nodes exists to observe. +func TestEvictUnusedDryRunDoesNotBackfillSizes(t *testing.T) { + _, host := newTestRegistry(t) + ref := host + "/test/nobackfill:latest" + pushImage(t, ref, v1.Config{}, layerFromEntries(t, []tarEntry{ + {name: "f", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("n", 1024)}, + })) + + store := newTestStore(t) + img := mustEnsure(t, store, ref) + sizePath := filepath.Join(img.LayerDirs[0], layerSizeFileName) + if err := os.Remove(sizePath); err != nil { + t.Fatal(err) + } + backdateStore(t, store, 3*time.Hour) + + stats, err := store.EvictUnused(context.Background(), math.MaxInt64, true) + if err != nil { + t.Fatalf("EvictUnused(dry): %v", err) + } + if stats.FreedBytes <= 0 { + t.Errorf("FreedBytes = %d, want > 0 (walked size despite missing size file)", stats.FreedBytes) + } + if _, err := os.Stat(sizePath); !os.IsNotExist(err) { + t.Error("dry run wrote a size file into the layer pool") + } + if _, err := os.Stat(img.LayerDirs[0]); err != nil { + t.Errorf("dry run deleted the layer: %v", err) + } +} + +func TestEvictUnusedDryRun(t *testing.T) { + _, host := newTestRegistry(t) + ref := host + "/test/dryrun:latest" + pushImage(t, ref, v1.Config{}, layerFromEntries(t, []tarEntry{ + {name: "f", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("d", 1024)}, + })) + + store := newTestStore(t) + img := mustEnsure(t, store, ref) + backdateStore(t, store, 3*time.Hour) + + stats, err := store.EvictUnused(context.Background(), math.MaxInt64, true) + if err != nil { + t.Fatalf("EvictUnused(dry): %v", err) + } + if stats.EvictedImages != 1 || stats.EvictedLayers != 1 || stats.FreedBytes <= 0 { + t.Errorf("dry-run stats = %+v, want 1 image / 1 layer / >0 bytes", stats) + } + if _, err := os.Stat(store.recordPath(img.Digest)); err != nil { + t.Errorf("dry run deleted the record: %v", err) + } + if _, err := os.Stat(img.LayerDirs[0]); err != nil { + t.Errorf("dry run deleted the layer: %v", err) + } +} + +// TestConcurrentEnsureImageAndEvict races cache hits, re-pulls, and +// free-everything eviction passes. The invariant under test: whatever +// interleaving happens, EnsureImage never returns an Image whose layer +// dirs have been (or will be) retired — either its touch wins and the +// evictor skips, or the evictor wins and EnsureImage re-pulls fresh dirs. +// Run with -race. +func TestConcurrentEnsureImageAndEvict(t *testing.T) { + _, host := newTestRegistry(t) + ref := host + "/test/race:latest" + pushImage(t, ref, v1.Config{}, layerFromEntries(t, []tarEntry{ + {name: "f", typeflag: tar.TypeReg, mode: 0o644, body: strings.Repeat("r", 512)}, + })) + + store := newTestStore(t) + digestRef := host + "/test/race@" + mustEnsure(t, store, ref).Digest.String() + + for i := 0; i < 25; i++ { + backdateStore(t, store, 3*time.Hour) + + var wg sync.WaitGroup + var img *Image + var ensureErr error + wg.Add(2) + go func() { + defer wg.Done() + img, ensureErr = store.EnsureImage(context.Background(), digestRef) + }() + go func() { + defer wg.Done() + if _, err := store.EvictUnused(context.Background(), math.MaxInt64, false); err != nil { + t.Errorf("EvictUnused: %v", err) + } + }() + wg.Wait() + + if ensureErr != nil { + t.Fatalf("iteration %d: EnsureImage: %v", i, ensureErr) + } + // The returned image must reference live, complete layer dirs. Its + // record must be fresh (either the hit's touch or the re-pull wrote + // it), which is what protects it until an ateom would mount it. + for _, dir := range img.LayerDirs { + if _, err := os.Stat(filepath.Join(dir, layerFSDirName, "f")); err != nil { + t.Fatalf("iteration %d: returned layer dir unusable: %v", i, err) + } + } + if fi, err := os.Stat(store.recordPath(img.Digest)); err != nil { + t.Fatalf("iteration %d: record of returned image missing: %v", i, err) + } else if time.Since(fi.ModTime()) > time.Minute { + t.Fatalf("iteration %d: returned image's record is stale (mtime %v)", i, fi.ModTime()) + } + } +} diff --git a/internal/imagecache/imagecache.go b/internal/imagecache/imagecache.go index e84983ea5..b5b6f0eab 100644 --- a/internal/imagecache/imagecache.go +++ b/internal/imagecache/imagecache.go @@ -84,6 +84,9 @@ const ( // across nodes. layerSizeFileName = "size" + // defaultMinAge is the default eviction minimum age (see WithMinAge). + defaultMinAge = 2 * time.Minute + // layerPullConcurrency bounds concurrent layer download+unpack streams per // image pull. Memory use is O(stream buffers) per slot, independent of // layer size. @@ -107,12 +110,26 @@ type Store struct { // nodes it validates for. platform *v1.Platform + // actorsDir is scanned by InUse for bundle overlay specs; empty disables + // the scan (the root set is then empty). + actorsDir string + + // minAge vetoes eviction of any layer or image record younger than this, + // covering the window between a pull (or cache-hit stat) and the bundle + // spec write / ateom mount that roots it. + minAge time.Duration + imageSF singleflight.Group layerSF singleflight.Group - // hitMu makes the hit path's record read + last-use touch atomic with - // respect to eviction, which will hold it exclusive around record - // removal. No exclusive holder yet, so effectively free. + // evictMu serializes EvictUnused passes (concurrent passes would fight + // over the same candidates for no benefit). + evictMu sync.Mutex + + // hitMu closes the hit-vs-evict window: held shared by the hit path + // (cachedImageHit), exclusive by eviction's record removal + // (removeStaleRecord), so a hit's last-use touch and eviction's final + // re-check can never interleave. Uncontended except during a pass. hitMu sync.RWMutex } @@ -137,6 +154,20 @@ func WithPlatform(p v1.Platform) Option { return func(s *Store) { s.platform = &p } } +// WithActorsDir points the eviction root-set scan at the node's actors +// directory (the per-actor state dirs under ateompath.BasePath). Each +// //bundles//rootfs-overlay.json roots its +// image and layers against eviction. Empty disables the scan. +func WithActorsDir(dir string) Option { + return func(s *Store) { s.actorsDir = dir } +} + +// WithMinAge overrides the eviction minimum age (default 2m): layers and +// image records younger than this are never evicted. +func WithMinAge(d time.Duration) Option { + return func(s *Store) { s.minAge = d } +} + // Image describes one cached, ready-to-compose image. type Image struct { // Digest is the manifest digest the caller's ref resolved to (for a @@ -162,7 +193,7 @@ type imageRecord struct { // startup recovery: verifying the layout version and sweeping temp dirs left // by unpacks that were in flight when a previous atelet died. func New(root string, opts ...Option) (*Store, error) { - s := &Store{root: root} + s := &Store{root: root, minAge: defaultMinAge} for _, o := range opts { o(s) } @@ -192,6 +223,12 @@ func New(root string, opts ...Option) (*Store, error) { if err := s.sweepTempDirs(); err != nil { return nil, err } + // Startup-only orphan recovery (see RecoverOrphans for why it must not + // run during normal operation). Failure is logged inside, never fatal: + // a corrupt record must not keep atelet from serving actors. + if _, err := s.RecoverOrphans(context.Background()); err != nil { + slog.Warn("Image cache startup orphan recovery incomplete", slog.Any("err", err)) + } return s, nil } @@ -280,13 +317,7 @@ func (s *Store) EnsureImage(ctx context.Context, ref string) (*Image, error) { digest = desc.Digest } - s.hitMu.RLock() - img, err := s.cachedImage(digest) - if err == nil && img != nil { - // Record last-use for eviction's LRU ordering. - s.touchRecord(digest) - } - s.hitMu.RUnlock() + img, err := s.cachedImageHit(digest) if err != nil { return nil, err } @@ -309,6 +340,22 @@ func (s *Store) EnsureImage(ctx context.Context, ref string) (*Image, error) { return v.(*Image), nil } +// cachedImageHit is the hit side of the hitMu contract: it verifies the +// cached image and records last-use for eviction's LRU ordering, atomic +// with respect to eviction's record removal (removeStaleRecord holds +// hitMu exclusive). Refreshing the mtime also renews the min-age veto, so +// an image in active use cannot age into eviction between this stat and +// the ateom's mount. +func (s *Store) cachedImageHit(digest v1.Hash) (*Image, error) { + s.hitMu.RLock() + defer s.hitMu.RUnlock() + img, err := s.cachedImage(digest) + if err == nil && img != nil { + s.touchRecord(digest) + } + return img, err +} + // cachedImage returns the cached image for digest, or nil if the record or // any of its layer dirs is missing (in which case the caller re-pulls; only // the missing layers cost anything). diff --git a/internal/imagecache/pull_gated_test.go b/internal/imagecache/pull_gated_test.go index 3d565029d..e605cd873 100644 --- a/internal/imagecache/pull_gated_test.go +++ b/internal/imagecache/pull_gated_test.go @@ -196,8 +196,8 @@ func TestPullRewritesRecordEvictedMidPull(t *testing.T) { // The per-layer progress touch keeps a slow pull's record fresh mid-flight: // freshness must come from layer completions, not only from the final -// rewrite. (The eviction pass asserting min-age against that freshness is -// covered with the eviction engine's tests.) +// rewrite — and that freshness must veto an eviction pass running while +// the pull is still in flight. func TestProgressTouchKeepsSlowPullRecordFresh(t *testing.T) { reg := newGatedRegistry(t) free, gated1, gated2 := gatedTestLayers(t) @@ -244,6 +244,14 @@ func TestProgressTouchKeepsSlowPullRecordFresh(t *testing.T) { return err == nil && time.Since(fi.ModTime()) < time.Hour }) + // The pull is still mid-flight: the freshened record must veto eviction. + if _, err := store.EvictUnused(context.Background(), 1<<62, false); err != nil { + t.Fatalf("EvictUnused: %v", err) + } + if _, err := os.Stat(recPath); err != nil { + t.Fatalf("freshly-touched mid-pull record evicted: %v", err) + } + // Unblock the last layer and drain the pull. release2() select { diff --git a/internal/imagecache/sizes.go b/internal/imagecache/sizes.go index bae433cda..b1f92ead1 100644 --- a/internal/imagecache/sizes.go +++ b/internal/imagecache/sizes.go @@ -58,17 +58,47 @@ func (s *Store) touchRecord(digest v1.Hash) { // layerSize returns the layer's recorded byte count, backfilling the size // file for layers unpacked before sizes were recorded. func (s *Store) layerSize(layerDir string) (int64, error) { + if n, ok, err := recordedLayerSize(layerDir); err != nil || ok { + return n, err + } + total := walkLayerSize(layerDir) + // Best-effort write; the walked total is valid regardless. The write + // bumps the dir mtime and deliberately does NOT restore it: restoring + // could rewind a concurrent ensureLayer reuse-touch and defeat + // retireLayer's freshness veto. The bump just makes a freshly + // backfilled layer read as recently used for one min-age window. + _ = os.WriteFile(filepath.Join(layerDir, layerSizeFileName), []byte(strconv.FormatInt(total, 10)+"\n"), 0o600) + return total, nil +} + +// layerSizeReadOnly is layerSize without the backfill write, for dry-run +// eviction: "dry run mutates nothing" includes derived metadata. +func (s *Store) layerSizeReadOnly(layerDir string) (int64, error) { + if n, ok, err := recordedLayerSize(layerDir); err != nil || ok { + return n, err + } + return walkLayerSize(layerDir), nil +} + +// recordedLayerSize reads the layer's size file. ok is false when the file +// is absent or corrupt (the caller falls back to walking the tree). +func recordedLayerSize(layerDir string) (n int64, ok bool, err error) { b, err := os.ReadFile(filepath.Join(layerDir, layerSizeFileName)) - if err == nil { - n, perr := strconv.ParseInt(strings.TrimSpace(string(b)), 10, 64) - if perr == nil { - return n, nil + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return 0, false, nil } - // Corrupt size file: fall through to backfill. - } else if !errors.Is(err, os.ErrNotExist) { - return 0, fmt.Errorf("while reading layer size: %w", err) + return 0, false, fmt.Errorf("while reading layer size: %w", err) + } + n, perr := strconv.ParseInt(strings.TrimSpace(string(b)), 10, 64) + if perr != nil { + return 0, false, nil } + return n, true, nil +} +// walkLayerSize sums regular-file sizes under the layer's fs tree. +func walkLayerSize(layerDir string) int64 { var total int64 fsRoot := filepath.Join(layerDir, layerFSDirName) // The callback swallows every error (skip-and-under-count contract), @@ -90,13 +120,7 @@ func (s *Store) layerSize(layerDir string) (int64, error) { } return nil }) - // Preserve the dir mtime (the eviction age signal) across the write. - if fi, statErr := os.Stat(layerDir); statErr == nil { - if err := os.WriteFile(filepath.Join(layerDir, layerSizeFileName), []byte(strconv.FormatInt(total, 10)+"\n"), 0o600); err == nil { - _ = os.Chtimes(layerDir, fi.ModTime(), fi.ModTime()) - } - } - return total, nil + return total } // CacheSize returns the sum of recorded sizes of every layer in the pool diff --git a/internal/imagecache/sizes_test.go b/internal/imagecache/sizes_test.go index 164e10051..572c3a8f5 100644 --- a/internal/imagecache/sizes_test.go +++ b/internal/imagecache/sizes_test.go @@ -86,8 +86,8 @@ func TestLayerSizeRecordedAndBackfilled(t *testing.T) { t.Errorf("backfill did not rewrite the size file: %v", err) } after, _ := os.Stat(img.LayerDirs[0]) - if !after.ModTime().Equal(before.ModTime()) { - t.Errorf("backfill changed the layer dir mtime (the eviction age signal): %v -> %v", before.ModTime(), after.ModTime()) + if !after.ModTime().After(before.ModTime()) { + t.Errorf("backfill must bump the layer dir mtime (not restore it — a restore could rewind a concurrent reuse-touch): %v -> %v", before.ModTime(), after.ModTime()) } // Corrupt size file: healed the same way as a missing one.