From f7616531aa9502ed86d449fcc7b82f3f7761c344 Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 18 May 2026 14:11:43 +0800 Subject: [PATCH 1/4] feat(gc): per-id INFO logs + orchestrator summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #33's "output detail" requirement: `cocoon gc` was previously near-silent, only printing a single "GC completed". Now every collected item is logged at INFO under `gc.` with a structured key=value payload, and the orchestrator ends each cycle with a per-module count + duration. Sample run: INFO gc.snapshot collected id=XEOU... name=ubuntu-hot-testing:v1 bytes=3221225472 last_accessed=2026-04-12T10:30:00Z reason=lru-age INFO gc.cloudhypervisor collected id=ABC123 runDir=/var/lib/cocoon/run/cloudhypervisor/ABC123 logDir=/var/log/cocoon/cloudhypervisor/ABC123 reason=orphan-runDir INFO gc.oci collected blob=b40150c1c2717d... reason=unreferenced INFO gc.cni collected id=JKL netns=cocoon-JKL nics=2 reason=orphan INFO gc.bridge collected id=MNO iface=btMNO-0 reason=orphan-tap INFO gc.Run completed: cloudhypervisor=1 cni=1 oci=4 snapshot=3 (failures: 0, duration: 230ms) Reasons surface why each id was selected: - snapshot: orphan / stale-pending / lru-all / lru-age / lru-keep / lru-size (multi-criterion joins with `+`, e.g. `lru-age+lru-size`) - cloudhypervisor/firecracker: orphan-runDir / orphan-logDir / stale-creating - images (oci, cloudimg): unreferenced - cni: orphan - bridge: orphan-tap Implementation: - pickLRU now returns `map[string]string` (id → reason); tests updated. - snapshot and hypervisor modules carry a `reasons` closure variable populated in Resolve, read in Collect — Module.Collect signature is unchanged so the framework stays untouched for the other modules. - gcCollect (hypervisor) is now private and takes the reasons map; no external callers existed. - GCCollectBlobs takes a `module` parameter so the per-blob log goes to the correct `gc.` namespace. - Orchestrator tracks {module → attempted-id-count} and emits a summary log via formatSummary; counts are attempts (not successes) to keep Collect signatures untouched. Failures count comes from the per-module err. - README documents the format + grep/awk examples. Compatible: no flag changes, no DB shape changes, no Module signature changes. The only visible difference is more log output (INFO level — appears by default). --- README.md | 28 +++++++++++++ cmd/others/handler.go | 7 +--- gc/orchestrator.go | 26 ++++++++++++ gc/orchestrator_test.go | 22 +++++++++++ hypervisor/gc.go | 35 ++++++++++++++--- images/gc.go | 2 +- images/index.go | 14 +++++-- network/bridge/gc_linux.go | 4 +- network/cni/gc.go | 7 ++++ snapshot/localfile/gc.go | 74 +++++++++++++++++++++++------------ snapshot/localfile/gc_test.go | 32 +++++++++++---- 11 files changed, 201 insertions(+), 50 deletions(-) create mode 100644 gc/orchestrator_test.go diff --git a/README.md b/README.md index 333c5268..c8a4e4b4 100644 --- a/README.md +++ b/README.md @@ -831,6 +831,34 @@ State changes are detected via **fsnotify** on the VM index file (sub-second lat This ensures blobs referenced by running VMs or saved snapshots are never deleted. +### Log Output + +Every collected item is logged at INFO level with a structured `key=value` payload under `gc.`, and a summary line ends the cycle. Sample: + +``` +INFO gc.snapshot collected id=XEOU... name=ubuntu-hot-testing:v1 bytes=3221225472 last_accessed=2026-04-12T10:30:00Z reason=lru-age +INFO gc.snapshot collected id=2GQVEA... name= bytes=0 last_accessed=never reason=orphan +INFO gc.cloudhypervisor collected id=ABC123 runDir=/var/lib/cocoon/run/cloudhypervisor/ABC123 logDir=/var/log/cocoon/cloudhypervisor/ABC123 reason=orphan-runDir +INFO gc.oci collected blob=b40150c1c2717d... reason=unreferenced +INFO gc.cni collected id=JKLMN netns=cocoon-JKLMN nics=2 reason=orphan +INFO gc.bridge collected id=MNOPQ iface=btMNOPQ-0 reason=orphan-tap +INFO gc.Run completed: cloudhypervisor=1 cni=1 oci=4 snapshot=3 (failures: 0, duration: 230ms) +``` + +Filter with `awk` / `grep`: + +```bash +journalctl -u cocoon-gc.service --since today | grep "gc.snapshot.*reason=lru-" +journalctl -u cocoon-gc.service --since today | awk '/gc.Run completed/' +``` + +Reasons: +- **snapshot**: `orphan` (dataDir without DB record), `stale-pending` (Create crashed >24h ago), `lru-all` / `lru-age` / `lru-keep` / `lru-size` (multi-criterion uses `+` joiner) +- **cloudhypervisor / firecracker**: `orphan-runDir`, `orphan-logDir`, `stale-creating` +- **images (oci, cloudimg)**: `unreferenced` +- **cni**: `orphan` (netns without active VM) +- **bridge**: `orphan-tap` + ### Snapshot LRU Eviction Bare `cocoon gc` only reclaims **orphans** (on-disk data with no DB record) and **stale pending** records (crashed mid-Create, older than 24h). To also evict healthy snapshots by access recency, pass `--snapshot`: diff --git a/cmd/others/handler.go b/cmd/others/handler.go index 4c14571e..619945aa 100644 --- a/cmd/others/handler.go +++ b/cmd/others/handler.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/docker/go-units" - "github.com/projecteru2/core/log" "github.com/spf13/cobra" cmdcore "github.com/cocoonstack/cocoon/cmd/core" @@ -56,11 +55,7 @@ func (h Handler) GC(cmd *cobra.Command, _ []string) error { netProvider.RegisterGC(o) gc.Register(o, bridge.GCModule(conf.RootDir)) snapBackend.RegisterGC(o) - if err := o.Run(ctx); err != nil { - return err - } - log.WithFunc("cmd.gc").Info(ctx, "GC completed") - return nil + return o.Run(ctx) } func (h Handler) Version(_ *cobra.Command, _ []string) error { diff --git a/gc/orchestrator.go b/gc/orchestrator.go index 4613cd31..2f878ba6 100644 --- a/gc/orchestrator.go +++ b/gc/orchestrator.go @@ -4,7 +4,10 @@ import ( "context" "errors" "fmt" + "maps" + "slices" "strings" + "time" "github.com/projecteru2/core/log" ) @@ -25,6 +28,7 @@ func Register[S any](o *Orchestrator, m Module[S]) { // Run executes one GC cycle: lock all modules, snapshot, resolve, collect. // Fail-closed: any busy lock aborts the cycle so cross-module decisions stay consistent. func (o *Orchestrator) Run(ctx context.Context) error { + start := time.Now() logger := log.WithFunc("gc.Run") // Acquire all locks up front; hold until GC finishes. @@ -75,14 +79,36 @@ func (o *Orchestrator) Run(ctx context.Context) error { // Phase 3: collect (skip modules with no targets). var errs []error + summary := make(map[string]int, len(locked)) + failures := 0 for _, m := range locked { ids := targets[m.getName()] if len(ids) == 0 { continue } if err := m.collect(ctx, ids); err != nil { + failures++ errs = append(errs, fmt.Errorf("gc %s: %w", m.getName(), err)) } + summary[m.getName()] = len(ids) } + logger.Infof(ctx, "completed: %s (failures: %d, duration: %s)", + formatSummary(summary), failures, time.Since(start).Truncate(time.Millisecond)) return errors.Join(errs...) } + +// formatSummary renders the per-module collection counts as `m1=N m2=M`, sorted by module name. +func formatSummary(s map[string]int) string { + if len(s) == 0 { + return "nothing to collect" + } + keys := slices.Sorted(maps.Keys(s)) + var sb strings.Builder + for i, k := range keys { + if i > 0 { + sb.WriteByte(' ') + } + fmt.Fprintf(&sb, "%s=%d", k, s[k]) + } + return sb.String() +} diff --git a/gc/orchestrator_test.go b/gc/orchestrator_test.go new file mode 100644 index 00000000..6a2a055b --- /dev/null +++ b/gc/orchestrator_test.go @@ -0,0 +1,22 @@ +package gc + +import "testing" + +func TestFormatSummary(t *testing.T) { + cases := []struct { + name string + in map[string]int + want string + }{ + {"empty", map[string]int{}, "nothing to collect"}, + {"single", map[string]int{"snapshot": 3}, "snapshot=3"}, + {"sorted", map[string]int{"snapshot": 3, "cloudhypervisor": 1, "oci": 12}, "cloudhypervisor=1 oci=12 snapshot=3"}, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + if got := formatSummary(tt.in); got != tt.want { + t.Errorf("formatSummary(%v) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} diff --git a/hypervisor/gc.go b/hypervisor/gc.go index 99d1f344..ecdcf7f7 100644 --- a/hypervisor/gc.go +++ b/hypervisor/gc.go @@ -3,10 +3,13 @@ package hypervisor import ( "context" "errors" + "fmt" "maps" "slices" "time" + "github.com/projecteru2/core/log" + "github.com/cocoonstack/cocoon/gc" "github.com/cocoonstack/cocoon/types" "github.com/cocoonstack/cocoon/utils" @@ -27,6 +30,7 @@ func (s VMGCSnapshot) ActiveVMIDs() map[string]struct{} { return s.vmIDs } // BuildGCModule builds GC module that scans DB and dirs for orphan VMs. func (b *Backend) BuildGCModule() gc.Module[VMGCSnapshot] { + var reasons map[string]string return gc.Module[VMGCSnapshot]{ Name: b.Typ, Locker: b.Locker, @@ -60,15 +64,31 @@ func (b *Backend) BuildGCModule() gc.Module[VMGCSnapshot] { return snap, nil }, Resolve: func(_ context.Context, snap VMGCSnapshot, _ map[string]any) []string { + reasons = make(map[string]string) // "db" holds vms.json/vms.lock — exclude from orphan scan when RootDir == RunDir. reserved := map[string]struct{}{"db": {}} runOrphans := utils.FilterUnreferenced(snap.runDirs, snap.vmIDs, reserved) logOrphans := utils.FilterUnreferenced(snap.logDirs, snap.vmIDs, reserved) + for _, id := range snap.staleCreate { + reasons[id] = "stale-creating" + } + for _, id := range runOrphans { + if _, ok := reasons[id]; !ok { + reasons[id] = "orphan-runDir" + } + } + for _, id := range logOrphans { + if _, ok := reasons[id]; !ok { + reasons[id] = "orphan-logDir" + } + } candidates := slices.Concat(runOrphans, logOrphans, snap.staleCreate) slices.Sort(candidates) return slices.Compact(candidates) }, - Collect: b.GCCollect, + Collect: func(ctx context.Context, ids []string) error { + return b.gcCollect(ctx, ids, reasons) + }, } } @@ -81,9 +101,9 @@ func (b *Backend) WatchPath() string { return b.Conf.IndexFile() } -// GCCollect kills leftover hypervisor processes and removes orphan dirs/records. -// Runs under the GC orchestrator's flock — uses lock-free DB access. -func (b *Backend) GCCollect(ctx context.Context, ids []string) error { +// gcCollect kills leftover hypervisor processes and removes orphan dirs/records under the orchestrator's flock. +func (b *Backend) gcCollect(ctx context.Context, ids []string, reasons map[string]string) error { + logger := log.WithFunc("gc." + b.Typ) var errs []error for _, id := range ids { runDir, logDir := b.Conf.VMRunDir(id), b.Conf.VMLogDir(id) @@ -95,11 +115,14 @@ func (b *Backend) GCCollect(ctx context.Context, ids []string) error { }) b.killOrphanProcess(ctx, runDir) if err := RemoveVMDirs(runDir, logDir); err != nil { - errs = append(errs, err) + errs = append(errs, fmt.Errorf("remove vm %s: %w", id, err)) + continue } + logger.Infof(ctx, "collected id=%s runDir=%s logDir=%s reason=%s", + id, runDir, logDir, reasons[id]) } if err := b.CleanStalePlaceholders(ctx, ids); err != nil { - errs = append(errs, err) + errs = append(errs, fmt.Errorf("clean stale placeholders: %w", err)) } return errors.Join(errs...) } diff --git a/images/gc.go b/images/gc.go index 6a311abc..07afd427 100644 --- a/images/gc.go +++ b/images/gc.go @@ -70,7 +70,7 @@ func BuildGCModule[I any](cfg GCModuleConfig[I]) gc.Module[ImageGCSnapshot] { return slices.Compact(candidates) }, Collect: func(ctx context.Context, ids []string) error { - return GCCollectBlobs(ctx, cfg.TempDir, cfg.DirOnly, ids, cfg.Removers...) + return GCCollectBlobs(ctx, cfg.Name, cfg.TempDir, cfg.DirOnly, ids, cfg.Removers...) }, } } diff --git a/images/index.go b/images/index.go index 556ff3dd..7838e843 100644 --- a/images/index.go +++ b/images/index.go @@ -3,6 +3,7 @@ package images import ( "context" "errors" + "fmt" "io/fs" "os" "strings" @@ -104,17 +105,24 @@ func GCStaleTemp(ctx context.Context, dir string, dirOnly bool) []error { }) } -// GCCollectBlobs removes temp files and blob artifacts by hex ID. +// GCCollectBlobs removes temp files and blob artifacts by hex ID; module names the gc subsystem for log routing. // removers are called for each hex; fs.ErrNotExist errors are ignored. -func GCCollectBlobs(ctx context.Context, tempDir string, dirOnly bool, ids []string, removers ...func(string) error) error { +func GCCollectBlobs(ctx context.Context, module, tempDir string, dirOnly bool, ids []string, removers ...func(string) error) error { + logger := log.WithFunc("gc." + module) var errs []error errs = append(errs, GCStaleTemp(ctx, tempDir, dirOnly)...) for _, hex := range ids { + var blobErr error for _, rm := range removers { if err := rm(hex); err != nil && !errors.Is(err, fs.ErrNotExist) { - errs = append(errs, err) + blobErr = errors.Join(blobErr, err) } } + if blobErr != nil { + errs = append(errs, fmt.Errorf("remove blob %s: %w", hex, blobErr)) + continue + } + logger.Infof(ctx, "collected blob=%s reason=unreferenced", hex) } return errors.Join(errs...) } diff --git a/network/bridge/gc_linux.go b/network/bridge/gc_linux.go index 8e847142..8de6759b 100644 --- a/network/bridge/gc_linux.go +++ b/network/bridge/gc_linux.go @@ -66,7 +66,7 @@ func GCModule(rootDir string) gc.Module[bridgeSnapshot] { return orphans }, Collect: func(ctx context.Context, prefixes []string) error { - logger := log.WithFunc("bridge.gc.Collect") + logger := log.WithFunc("gc.bridge") orphanSet := make(map[string]struct{}, len(prefixes)) for _, p := range prefixes { @@ -89,7 +89,7 @@ func GCModule(rootDir string) gc.Module[bridgeSnapshot] { if err := netlink.LinkDel(l); err != nil { logger.Warnf(ctx, "delete orphan TAP %s: %v", name, err) } else { - logger.Infof(ctx, "collected orphan TAP %s", name) + logger.Infof(ctx, "collected id=%s iface=%s reason=orphan-tap", prefix, name) } } return nil diff --git a/network/cni/gc.go b/network/cni/gc.go index d6be2ebb..1811461e 100644 --- a/network/cni/gc.go +++ b/network/cni/gc.go @@ -10,6 +10,8 @@ import ( "slices" "strings" + "github.com/projecteru2/core/log" + "github.com/cocoonstack/cocoon/gc" ) @@ -63,6 +65,7 @@ func (c *CNI) GCModule() gc.Module[cniSnapshot] { return orphans }, Collect: func(ctx context.Context, ids []string) error { + logger := log.WithFunc("gc.cni") var errs []error for _, vmID := range ids { // 1. Read CNI records for this VM (lockless — orchestrator holds flock). @@ -82,6 +85,7 @@ func (c *CNI) GCModule() gc.Module[cniSnapshot] { nsName := netnsName(vmID) if err := deleteNetns(ctx, nsName); err != nil && !errors.Is(err, fs.ErrNotExist) { errs = append(errs, fmt.Errorf("remove netns %s: %w", nsName, err)) + continue } // 4. Clean DB records (lockless write). @@ -95,8 +99,11 @@ func (c *CNI) GCModule() gc.Module[cniSnapshot] { return nil }); err != nil { errs = append(errs, fmt.Errorf("clean DB for %s: %w", vmID, err)) + continue } } + logger.Infof(ctx, "collected id=%s netns=%s nics=%d reason=orphan", + vmID, nsName, len(records)) } return errors.Join(errs...) }, diff --git a/snapshot/localfile/gc.go b/snapshot/localfile/gc.go index 127ac065..c296e1a7 100644 --- a/snapshot/localfile/gc.go +++ b/snapshot/localfile/gc.go @@ -84,6 +84,10 @@ type snapshotGCSnapshot struct { func (s snapshotGCSnapshot) UsedBlobIDs() map[string]struct{} { return s.blobIDs } func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker lock.Locker, policy EvictionPolicy) gc.Module[snapshotGCSnapshot] { + var ( + records map[string]snapshotMeta + reasons map[string]string + ) return gc.Module[snapshotGCSnapshot]{ Name: "snapshot", Locker: locker, @@ -126,14 +130,24 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker return snap, nil }, Resolve: func(ctx context.Context, snap snapshotGCSnapshot, _ map[string]any) []string { + records = snap.records + reasons = make(map[string]string) orphans := utils.FilterUnreferenced(snap.dataDirs, snap.snapshotIDs) + for _, id := range orphans { + reasons[id] = "orphan" + } + for _, id := range snap.stalePending { + reasons[id] = "stale-pending" + } candidates := slices.Concat(orphans, snap.stalePending) if snap.policy.Enabled { - evict := pickLRU(snap.records, snap.policy) - logEvictions(ctx, evict, snap.records, snap.policy.DryRun) - if !snap.policy.DryRun { - candidates = append(candidates, evict...) + lruReasons := pickLRU(snap.records, snap.policy) + if snap.policy.DryRun { + logWouldEvict(ctx, lruReasons, snap.records) + } else { + maps.Copy(reasons, lruReasons) + candidates = append(candidates, slices.Collect(maps.Keys(lruReasons))...) } } @@ -141,6 +155,7 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker return slices.Compact(candidates) }, Collect: func(ctx context.Context, ids []string) error { + logger := log.WithFunc("gc.snapshot") var ( errs []error removed = make([]string, 0, len(ids)) @@ -154,6 +169,9 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker errs = append(errs, fmt.Errorf("remove snapshot %s: %w", id, err)) continue } + m := records[id] + logger.Infof(ctx, "collected id=%s name=%s bytes=%d last_accessed=%s reason=%s", + id, m.name, m.sizeBytes, formatTime(m.lastAccessed), reasons[id]) removed = append(removed, id) } if err := cleanResolvedRecords(store, removed); err != nil { @@ -164,30 +182,42 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker } } -// pickLRU returns evict IDs. No sub-criteria → all records; else union of age/keep/size. -func pickLRU(records map[string]snapshotMeta, p EvictionPolicy) []string { +// pickLRU returns evict IDs keyed by reason (lru-all / lru-age / lru-keep / lru-size, joined by "+" on multi-match). +// No sub-criteria → all records with reason "lru-all". +func pickLRU(records map[string]snapshotMeta, p EvictionPolicy) map[string]string { sorted := slices.SortedFunc(maps.Keys(records), func(a, b string) int { return records[a].lastAccessed.Compare(records[b].lastAccessed) }) + reasons := make(map[string]string) + if !p.hasCriteria() { - return sorted + for _, id := range sorted { + reasons[id] = "lru-all" + } + return reasons } - evict := make(map[string]struct{}) + add := func(id, label string) { + if existing, ok := reasons[id]; ok { + reasons[id] = existing + "+" + label + return + } + reasons[id] = label + } if p.MaxAge > 0 { cutoff := time.Now().Add(-p.MaxAge) for _, id := range sorted { if records[id].lastAccessed.Before(cutoff) { - evict[id] = struct{}{} + add(id, "lru-age") } } } if p.KeepLast > 0 && len(sorted) > p.KeepLast { for _, id := range sorted[:len(sorted)-p.KeepLast] { - evict[id] = struct{}{} + add(id, "lru-keep") } } @@ -200,31 +230,25 @@ func pickLRU(records map[string]snapshotMeta, p EvictionPolicy) []string { if total <= p.MaxSize { break } - evict[id] = struct{}{} + add(id, "lru-size") total -= records[id].sizeBytes } } - return slices.Sorted(maps.Keys(evict)) + return reasons } -func logEvictions(ctx context.Context, ids []string, records map[string]snapshotMeta, dryRun bool) { - if len(ids) == 0 { +// logWouldEvict prints a preview row per dry-run LRU candidate; non-dry-run path logs inside Collect after successful removal. +func logWouldEvict(ctx context.Context, reasons map[string]string, records map[string]snapshotMeta) { + if len(reasons) == 0 { return } - logger := log.WithFunc("localfile.gc.lru") - verb := "evicting" - if dryRun { - verb = "would evict" - } - var freed int64 - for _, id := range ids { + logger := log.WithFunc("gc.snapshot") + for _, id := range slices.Sorted(maps.Keys(reasons)) { m := records[id] - freed += m.sizeBytes - logger.Infof(ctx, "%s id=%s name=%s last_accessed=%s size_bytes=%d", - verb, id, m.name, formatTime(m.lastAccessed), m.sizeBytes) + logger.Infof(ctx, "would-evict id=%s name=%s bytes=%d last_accessed=%s reason=%s", + id, m.name, m.sizeBytes, formatTime(m.lastAccessed), reasons[id]) } - logger.Infof(ctx, "%s %d snapshot(s), freeing %d bytes", verb, len(ids), freed) } func formatTime(t time.Time) string { diff --git a/snapshot/localfile/gc_test.go b/snapshot/localfile/gc_test.go index 8c164863..86dc0cbb 100644 --- a/snapshot/localfile/gc_test.go +++ b/snapshot/localfile/gc_test.go @@ -2,6 +2,7 @@ package localfile import ( "fmt" + "maps" "os" "path/filepath" "slices" @@ -17,6 +18,10 @@ func meta(ageHours int, size int64) snapshotMeta { return snapshotMeta{lastAccessed: accessedAt, sizeBytes: size} } +func sortedKeys(m map[string]string) []string { + return slices.Sorted(maps.Keys(m)) +} + func TestPickLRU_NoCriteriaEvictsAll(t *testing.T) { records := map[string]snapshotMeta{ "a": meta(1, 100), @@ -37,9 +42,12 @@ func TestPickLRU_KeepLast(t *testing.T) { "oldester": meta(20, 10), } got := pickLRU(records, EvictionPolicy{Enabled: true, KeepLast: 2}) - if !slices.Equal(got, []string{"oldest", "oldester"}) { + if !slices.Equal(sortedKeys(got), []string{"oldest", "oldester"}) { t.Errorf("KeepLast=2: got %v", got) } + if got["oldest"] != "lru-keep" { + t.Errorf("reason: got %q, want lru-keep", got["oldest"]) + } } func TestPickLRU_KeepLastExceedsAll(t *testing.T) { @@ -56,9 +64,12 @@ func TestPickLRU_MaxAge(t *testing.T) { "stale": meta(48, 10), } got := pickLRU(records, EvictionPolicy{Enabled: true, MaxAge: 24 * time.Hour}) - if !slices.Equal(got, []string{"stale"}) { + if !slices.Equal(sortedKeys(got), []string{"stale"}) { t.Errorf("MaxAge=24h: got %v", got) } + if got["stale"] != "lru-age" { + t.Errorf("reason: got %q, want lru-age", got["stale"]) + } } func TestPickLRU_MaxSize(t *testing.T) { @@ -69,7 +80,7 @@ func TestPickLRU_MaxSize(t *testing.T) { "d": meta(4, 30), } got := pickLRU(records, EvictionPolicy{Enabled: true, MaxSize: 60}) - if !slices.Equal(got, []string{"c", "d"}) { + if !slices.Equal(sortedKeys(got), []string{"c", "d"}) { t.Errorf("MaxSize=60: got %v", got) } } @@ -83,9 +94,8 @@ func TestPickLRU_UnionOfCriteria(t *testing.T) { got := pickLRU(records, EvictionPolicy{ Enabled: true, MaxAge: 24 * time.Hour, MaxSize: 50, }) - want := []string{"fresh-big", "old-small"} - if !slices.Equal(got, want) { - t.Errorf("union: got %v, want %v", got, want) + if !slices.Equal(sortedKeys(got), []string{"fresh-big", "old-small"}) { + t.Errorf("union: got %v", got) } } @@ -95,11 +105,19 @@ func TestPickLRU_ZeroTimeIsOldest(t *testing.T) { "zero": {lastAccessed: time.Time{}, sizeBytes: 10}, } got := pickLRU(records, EvictionPolicy{Enabled: true, KeepLast: 1}) - if !slices.Equal(got, []string{"zero"}) { + if !slices.Equal(sortedKeys(got), []string{"zero"}) { t.Errorf("zero time should be evicted first: got %v", got) } } +func TestPickLRU_NoCriteriaAllReasonAll(t *testing.T) { + records := map[string]snapshotMeta{"a": meta(1, 10)} + got := pickLRU(records, EvictionPolicy{Enabled: true}) + if got["a"] != "lru-all" { + t.Errorf("reason: got %q, want lru-all", got["a"]) + } +} + func TestGCModule_LRUEndToEnd(t *testing.T) { lf := newTestLF(t) ctx := t.Context() From 42e6bd2c25b4893ec0ab06321baf0b49db51afb4 Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 18 May 2026 14:18:58 +0800 Subject: [PATCH 2/4] chore(snapshot): /code apply public-above-private + dedupe evict log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Hoist EvictionPolicy + hasCriteria above backfillSizeBytes (public declarations must precede private per file-organization rule). - Align backfillSizeBytes logger prefix to gc.snapshot (matches the other two log sites in this file; localfile.X is reserved for non-GC callers). - Extract logEvictRow shared by Collect (collected) and logWouldEvict (would-evict) — same field shape was duplicated; inlines formatTime into the single caller. - Trim three godocs of restating chatter. --- snapshot/localfile/gc.go | 53 ++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/snapshot/localfile/gc.go b/snapshot/localfile/gc.go index c296e1a7..9561ba5d 100644 --- a/snapshot/localfile/gc.go +++ b/snapshot/localfile/gc.go @@ -21,9 +21,21 @@ import ( // pendingGCGrace lets a slow-storage snapshot finish before GC reclaims a pending record. const pendingGCGrace = 24 * time.Hour -// backfillSizeBytes computes DirSize for records with SizeBytes==0 and persists via WriteRaw. +// EvictionPolicy controls LRU snapshot eviction; Enabled with zero criteria evicts all non-pending. +type EvictionPolicy struct { + Enabled bool + DryRun bool + KeepLast int + MaxAge time.Duration + MaxSize int64 +} + +func (p EvictionPolicy) hasCriteria() bool { + return p.KeepLast > 0 || p.MaxAge > 0 || p.MaxSize > 0 +} + func backfillSizeBytes(ctx context.Context, conf *Config, store storage.Store[snapshot.SnapshotIndex], records map[string]snapshotMeta) { - logger := log.WithFunc("localfile.gc.backfillSizeBytes") + logger := log.WithFunc("gc.snapshot") var changed bool for id, m := range records { if m.sizeBytes > 0 { @@ -53,19 +65,6 @@ func backfillSizeBytes(ctx context.Context, conf *Config, store storage.Store[sn } } -// EvictionPolicy controls LRU snapshot eviction; Enabled with zero criteria evicts all non-pending. -type EvictionPolicy struct { - Enabled bool - DryRun bool - KeepLast int - MaxAge time.Duration - MaxSize int64 -} - -func (p EvictionPolicy) hasCriteria() bool { - return p.KeepLast > 0 || p.MaxAge > 0 || p.MaxSize > 0 -} - type snapshotMeta struct { name string lastAccessed time.Time @@ -169,9 +168,7 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker errs = append(errs, fmt.Errorf("remove snapshot %s: %w", id, err)) continue } - m := records[id] - logger.Infof(ctx, "collected id=%s name=%s bytes=%d last_accessed=%s reason=%s", - id, m.name, m.sizeBytes, formatTime(m.lastAccessed), reasons[id]) + logEvictRow(ctx, logger, "collected", id, records[id], reasons[id]) removed = append(removed, id) } if err := cleanResolvedRecords(store, removed); err != nil { @@ -182,8 +179,7 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker } } -// pickLRU returns evict IDs keyed by reason (lru-all / lru-age / lru-keep / lru-size, joined by "+" on multi-match). -// No sub-criteria → all records with reason "lru-all". +// pickLRU returns evict IDs keyed by reason ("+" joins multi-match; no criteria → "lru-all"). func pickLRU(records map[string]snapshotMeta, p EvictionPolicy) map[string]string { sorted := slices.SortedFunc(maps.Keys(records), func(a, b string) int { return records[a].lastAccessed.Compare(records[b].lastAccessed) @@ -238,27 +234,26 @@ func pickLRU(records map[string]snapshotMeta, p EvictionPolicy) map[string]strin return reasons } -// logWouldEvict prints a preview row per dry-run LRU candidate; non-dry-run path logs inside Collect after successful removal. func logWouldEvict(ctx context.Context, reasons map[string]string, records map[string]snapshotMeta) { if len(reasons) == 0 { return } logger := log.WithFunc("gc.snapshot") for _, id := range slices.Sorted(maps.Keys(reasons)) { - m := records[id] - logger.Infof(ctx, "would-evict id=%s name=%s bytes=%d last_accessed=%s reason=%s", - id, m.name, m.sizeBytes, formatTime(m.lastAccessed), reasons[id]) + logEvictRow(ctx, logger, "would-evict", id, records[id], reasons[id]) } } -func formatTime(t time.Time) string { - if t.IsZero() { - return "never" +func logEvictRow(ctx context.Context, logger *log.Fields, verb, id string, m snapshotMeta, reason string) { + accessed := "never" + if !m.lastAccessed.IsZero() { + accessed = m.lastAccessed.UTC().Format(time.RFC3339) } - return t.UTC().Format(time.RFC3339) + logger.Infof(ctx, "%s id=%s name=%s bytes=%d last_accessed=%s reason=%s", + verb, id, m.name, m.sizeBytes, accessed, reason) } -// cleanResolvedRecords drops GC-resolved records; pending only if past grace (protects in-flight Create). +// cleanResolvedRecords drops resolved records; pending only past grace. func cleanResolvedRecords(store storage.Store[snapshot.SnapshotIndex], ids []string) error { if len(ids) == 0 { return nil From 77462a04fc89515debcf09d584d2adda88af9dc9 Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 18 May 2026 14:36:41 +0800 Subject: [PATCH 3/4] refactor(gc): lift reasons map from per-module closure to Module[S] contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both snapshot/localfile and hypervisor maintained a closure-captured reasons map[string]string that Resolve wrote and Collect read — the same coupling smell duplicated in two places. Add snap to Collect's signature (gc.Module[S].Collect) so any map field populated by Resolve (reasons in this case, but generally any per-id metadata) is visible to Collect without the closure dance. - snapshot/localfile/gc.go: drop var (records, reasons) outer capture; reasons becomes a field of snapshotGCSnapshot, initialized in ReadDB, populated in Resolve, read in Collect via snap. - hypervisor/gc.go: drop var reasons closure; reasons becomes a field of VMGCSnapshot. - images/network bridge/cni: Collect signature gains a snap parameter; modules that don't need it use _ XSnapshot. - gc/module.go + runner.go + orchestrator.go: thread snap through the collect adapter via the snapshots map already kept for cross-module analysis. Net: same line count, removes duplicated closure-state smell, makes "per-id metadata Resolve→Collect" a first-class GC contract for any future module that needs it. --- gc/module.go | 11 ++++++----- gc/orchestrator.go | 2 +- gc/runner.go | 2 +- hypervisor/gc.go | 19 +++++++++---------- images/gc.go | 2 +- network/bridge/gc_linux.go | 2 +- network/bridge/gc_other.go | 2 +- network/cni/gc.go | 2 +- snapshot/localfile/gc.go | 19 +++++++------------ snapshot/localfile/gc_test.go | 9 +++++---- 10 files changed, 33 insertions(+), 37 deletions(-) diff --git a/gc/module.go b/gc/module.go index 386cc18e..5d86e0d5 100644 --- a/gc/module.go +++ b/gc/module.go @@ -14,11 +14,11 @@ type Module[S any] struct { // ReadDB reads the module's current state (called while the lock is held). ReadDB func(ctx context.Context) (S, error) - // Resolve returns IDs to delete; others holds snapshots from peer modules (cast for cross-module analysis, e.g. VMs pinning images). + // Resolve returns IDs to delete; others holds snapshots from peer modules (cast for cross-module analysis, e.g. VMs pinning images). Map fields written here are visible to Collect via the same snap. Resolve func(ctx context.Context, snap S, others map[string]any) []string - // Collect removes the given IDs (called while the lock is held). - Collect func(ctx context.Context, ids []string) error + // Collect removes the given IDs (called while the lock is held); snap is the same value Resolve saw. + Collect func(ctx context.Context, ids []string, snap S) error } // Module[S] implements runner — internal to the gc package. @@ -37,6 +37,7 @@ func (m Module[S]) resolveTargets(ctx context.Context, snap any, others map[stri return m.Resolve(ctx, typed, others) } -func (m Module[S]) collect(ctx context.Context, ids []string) error { - return m.Collect(ctx, ids) +func (m Module[S]) collect(ctx context.Context, ids []string, snap any) error { + typed, _ := snap.(S) + return m.Collect(ctx, ids, typed) } diff --git a/gc/orchestrator.go b/gc/orchestrator.go index 2f878ba6..62d2a008 100644 --- a/gc/orchestrator.go +++ b/gc/orchestrator.go @@ -86,7 +86,7 @@ func (o *Orchestrator) Run(ctx context.Context) error { if len(ids) == 0 { continue } - if err := m.collect(ctx, ids); err != nil { + if err := m.collect(ctx, ids, snapshots[m.getName()]); err != nil { failures++ errs = append(errs, fmt.Errorf("gc %s: %w", m.getName(), err)) } diff --git a/gc/runner.go b/gc/runner.go index 050c339a..95b48503 100644 --- a/gc/runner.go +++ b/gc/runner.go @@ -13,5 +13,5 @@ type runner interface { getLocker() lock.Locker readSnapshot(ctx context.Context) (any, error) resolveTargets(ctx context.Context, snap any, others map[string]any) []string - collect(ctx context.Context, ids []string) error + collect(ctx context.Context, ids []string, snap any) error } diff --git a/hypervisor/gc.go b/hypervisor/gc.go index ecdcf7f7..979dd34e 100644 --- a/hypervisor/gc.go +++ b/hypervisor/gc.go @@ -22,6 +22,7 @@ type VMGCSnapshot struct { staleCreate []string runDirs []string logDirs []string + reasons map[string]string } func (s VMGCSnapshot) UsedBlobIDs() map[string]struct{} { return s.blobIDs } @@ -30,12 +31,11 @@ func (s VMGCSnapshot) ActiveVMIDs() map[string]struct{} { return s.vmIDs } // BuildGCModule builds GC module that scans DB and dirs for orphan VMs. func (b *Backend) BuildGCModule() gc.Module[VMGCSnapshot] { - var reasons map[string]string return gc.Module[VMGCSnapshot]{ Name: b.Typ, Locker: b.Locker, ReadDB: func(_ context.Context) (VMGCSnapshot, error) { - var snap VMGCSnapshot + snap := VMGCSnapshot{reasons: make(map[string]string)} cutoff := time.Now().Add(-CreatingStateGCGrace) if err := b.DB.ReadRaw(func(idx *VMIndex) error { snap.blobIDs = make(map[string]struct{}, len(idx.VMs)) @@ -64,30 +64,29 @@ func (b *Backend) BuildGCModule() gc.Module[VMGCSnapshot] { return snap, nil }, Resolve: func(_ context.Context, snap VMGCSnapshot, _ map[string]any) []string { - reasons = make(map[string]string) // "db" holds vms.json/vms.lock — exclude from orphan scan when RootDir == RunDir. reserved := map[string]struct{}{"db": {}} runOrphans := utils.FilterUnreferenced(snap.runDirs, snap.vmIDs, reserved) logOrphans := utils.FilterUnreferenced(snap.logDirs, snap.vmIDs, reserved) for _, id := range snap.staleCreate { - reasons[id] = "stale-creating" + snap.reasons[id] = "stale-creating" } for _, id := range runOrphans { - if _, ok := reasons[id]; !ok { - reasons[id] = "orphan-runDir" + if _, ok := snap.reasons[id]; !ok { + snap.reasons[id] = "orphan-runDir" } } for _, id := range logOrphans { - if _, ok := reasons[id]; !ok { - reasons[id] = "orphan-logDir" + if _, ok := snap.reasons[id]; !ok { + snap.reasons[id] = "orphan-logDir" } } candidates := slices.Concat(runOrphans, logOrphans, snap.staleCreate) slices.Sort(candidates) return slices.Compact(candidates) }, - Collect: func(ctx context.Context, ids []string) error { - return b.gcCollect(ctx, ids, reasons) + Collect: func(ctx context.Context, ids []string, snap VMGCSnapshot) error { + return b.gcCollect(ctx, ids, snap.reasons) }, } } diff --git a/images/gc.go b/images/gc.go index 07afd427..b2d50900 100644 --- a/images/gc.go +++ b/images/gc.go @@ -69,7 +69,7 @@ func BuildGCModule[I any](cfg GCModuleConfig[I]) gc.Module[ImageGCSnapshot] { slices.Sort(candidates) return slices.Compact(candidates) }, - Collect: func(ctx context.Context, ids []string) error { + Collect: func(ctx context.Context, ids []string, _ ImageGCSnapshot) error { return GCCollectBlobs(ctx, cfg.Name, cfg.TempDir, cfg.DirOnly, ids, cfg.Removers...) }, } diff --git a/network/bridge/gc_linux.go b/network/bridge/gc_linux.go index 8de6759b..f626116a 100644 --- a/network/bridge/gc_linux.go +++ b/network/bridge/gc_linux.go @@ -65,7 +65,7 @@ func GCModule(rootDir string) gc.Module[bridgeSnapshot] { slices.Sort(orphans) return orphans }, - Collect: func(ctx context.Context, prefixes []string) error { + Collect: func(ctx context.Context, prefixes []string, _ bridgeSnapshot) error { logger := log.WithFunc("gc.bridge") orphanSet := make(map[string]struct{}, len(prefixes)) diff --git a/network/bridge/gc_other.go b/network/bridge/gc_other.go index a0c76c3b..28f3f9d8 100644 --- a/network/bridge/gc_other.go +++ b/network/bridge/gc_other.go @@ -25,7 +25,7 @@ func GCModule(rootDir string) gc.Module[bridgeSnapshot] { Resolve: func(_ context.Context, _ bridgeSnapshot, _ map[string]any) []string { return nil }, - Collect: func(_ context.Context, _ []string) error { + Collect: func(_ context.Context, _ []string, _ bridgeSnapshot) error { return nil }, } diff --git a/network/cni/gc.go b/network/cni/gc.go index 1811461e..89095455 100644 --- a/network/cni/gc.go +++ b/network/cni/gc.go @@ -64,7 +64,7 @@ func (c *CNI) GCModule() gc.Module[cniSnapshot] { slices.Sort(orphans) return orphans }, - Collect: func(ctx context.Context, ids []string) error { + Collect: func(ctx context.Context, ids []string, _ cniSnapshot) error { logger := log.WithFunc("gc.cni") var errs []error for _, vmID := range ids { diff --git a/snapshot/localfile/gc.go b/snapshot/localfile/gc.go index 9561ba5d..837ae9b6 100644 --- a/snapshot/localfile/gc.go +++ b/snapshot/localfile/gc.go @@ -77,21 +77,18 @@ type snapshotGCSnapshot struct { dataDirs []string stalePending []string records map[string]snapshotMeta + reasons map[string]string policy EvictionPolicy } func (s snapshotGCSnapshot) UsedBlobIDs() map[string]struct{} { return s.blobIDs } func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker lock.Locker, policy EvictionPolicy) gc.Module[snapshotGCSnapshot] { - var ( - records map[string]snapshotMeta - reasons map[string]string - ) return gc.Module[snapshotGCSnapshot]{ Name: "snapshot", Locker: locker, ReadDB: func(ctx context.Context) (snapshotGCSnapshot, error) { - snap := snapshotGCSnapshot{policy: policy} + snap := snapshotGCSnapshot{policy: policy, reasons: make(map[string]string)} cutoff := time.Now().Add(-pendingGCGrace) if err := store.ReadRaw(func(idx *snapshot.SnapshotIndex) error { snap.blobIDs = make(map[string]struct{}) @@ -129,14 +126,12 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker return snap, nil }, Resolve: func(ctx context.Context, snap snapshotGCSnapshot, _ map[string]any) []string { - records = snap.records - reasons = make(map[string]string) orphans := utils.FilterUnreferenced(snap.dataDirs, snap.snapshotIDs) for _, id := range orphans { - reasons[id] = "orphan" + snap.reasons[id] = "orphan" } for _, id := range snap.stalePending { - reasons[id] = "stale-pending" + snap.reasons[id] = "stale-pending" } candidates := slices.Concat(orphans, snap.stalePending) @@ -145,7 +140,7 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker if snap.policy.DryRun { logWouldEvict(ctx, lruReasons, snap.records) } else { - maps.Copy(reasons, lruReasons) + maps.Copy(snap.reasons, lruReasons) candidates = append(candidates, slices.Collect(maps.Keys(lruReasons))...) } } @@ -153,7 +148,7 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker slices.Sort(candidates) return slices.Compact(candidates) }, - Collect: func(ctx context.Context, ids []string) error { + Collect: func(ctx context.Context, ids []string, snap snapshotGCSnapshot) error { logger := log.WithFunc("gc.snapshot") var ( errs []error @@ -168,7 +163,7 @@ func gcModule(conf *Config, store storage.Store[snapshot.SnapshotIndex], locker errs = append(errs, fmt.Errorf("remove snapshot %s: %w", id, err)) continue } - logEvictRow(ctx, logger, "collected", id, records[id], reasons[id]) + logEvictRow(ctx, logger, "collected", id, snap.records[id], snap.reasons[id]) removed = append(removed, id) } if err := cleanResolvedRecords(store, removed); err != nil { diff --git a/snapshot/localfile/gc_test.go b/snapshot/localfile/gc_test.go index 86dc0cbb..81e03cff 100644 --- a/snapshot/localfile/gc_test.go +++ b/snapshot/localfile/gc_test.go @@ -154,7 +154,7 @@ func TestGCModule_LRUEndToEnd(t *testing.T) { if len(ids) != 2 { t.Errorf("want 2 evictions, got %v", ids) } - if err := mod.Collect(ctx, ids); err != nil { + if err := mod.Collect(ctx, ids, snap); err != nil { t.Fatalf("Collect: %v", err) } @@ -214,7 +214,7 @@ func TestGCModule_BareSnapshotEvictsAll(t *testing.T) { t.Fatal(err) } ids := mod.Resolve(ctx, snap, map[string]any{}) - if err := mod.Collect(ctx, ids); err != nil { + if err := mod.Collect(ctx, ids, snap); err != nil { t.Fatal(err) } @@ -310,7 +310,8 @@ func TestGCModule_RemovalFailureKeepsDBRecord(t *testing.T) { t.Cleanup(func() { _ = os.Chmod(parent, 0o750) }) mod := gcModule(lf.conf, lf.store, lf.locker, EvictionPolicy{Enabled: true}) - if err := mod.Collect(ctx, ids); err == nil { + snap, _ := mod.ReadDB(ctx) + if err := mod.Collect(ctx, ids, snap); err == nil { t.Fatal("expected Collect to error on chmod-protected parent") } for i, name := range []string{"a", "b"} { @@ -338,7 +339,7 @@ func TestGCModule_OrphanDirCleaned(t *testing.T) { if !slices.Contains(ids, "ORPHAN_ID_NO_DB") { t.Errorf("orphan dir should be picked, got %v", ids) } - if err := mod.Collect(ctx, ids); err != nil { + if err := mod.Collect(ctx, ids, snap); err != nil { t.Fatal(err) } if _, err := os.Stat(orphanDir); !os.IsNotExist(err) { From 5ccab28dfe08f3b802a3e1adc021e1f1bd5d7bf3 Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 18 May 2026 14:39:33 +0800 Subject: [PATCH 4/4] chore(gc): apply /simplify findings on reasons-lift refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - gc/module.go: trim Resolve+Collect godoc narration that restated the signature contract. - gc/module.go: collect adapter type-asserts symmetrically with resolveTargets — return nil on mismatch instead of silently passing zero S. - hypervisor/gc.go: gcCollect now takes snap VMGCSnapshot (consistent with snapshot/localfile/gc.go which also reads snap.reasons inside Collect); reasons-map plumbing folded into the snap contract. - snapshot/localfile/gc_test.go: ReadDB called BEFORE chmod 0500 on parent so the test exercises the intended path (read-then-fail-on- remove) instead of accidentally depending on chmod 0500 still permitting parent dir reads. --- gc/module.go | 9 ++++++--- hypervisor/gc.go | 6 +++--- snapshot/localfile/gc_test.go | 8 ++++++-- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/gc/module.go b/gc/module.go index 5d86e0d5..6c162d0f 100644 --- a/gc/module.go +++ b/gc/module.go @@ -14,10 +14,10 @@ type Module[S any] struct { // ReadDB reads the module's current state (called while the lock is held). ReadDB func(ctx context.Context) (S, error) - // Resolve returns IDs to delete; others holds snapshots from peer modules (cast for cross-module analysis, e.g. VMs pinning images). Map fields written here are visible to Collect via the same snap. + // Resolve returns IDs to delete; others holds snapshots from peer modules (cast for cross-module analysis, e.g. VMs pinning images). Resolve func(ctx context.Context, snap S, others map[string]any) []string - // Collect removes the given IDs (called while the lock is held); snap is the same value Resolve saw. + // Collect removes the given IDs (called while the lock is held). Collect func(ctx context.Context, ids []string, snap S) error } @@ -38,6 +38,9 @@ func (m Module[S]) resolveTargets(ctx context.Context, snap any, others map[stri } func (m Module[S]) collect(ctx context.Context, ids []string, snap any) error { - typed, _ := snap.(S) + typed, ok := snap.(S) + if !ok { + return nil + } return m.Collect(ctx, ids, typed) } diff --git a/hypervisor/gc.go b/hypervisor/gc.go index 979dd34e..4bdf7d04 100644 --- a/hypervisor/gc.go +++ b/hypervisor/gc.go @@ -86,7 +86,7 @@ func (b *Backend) BuildGCModule() gc.Module[VMGCSnapshot] { return slices.Compact(candidates) }, Collect: func(ctx context.Context, ids []string, snap VMGCSnapshot) error { - return b.gcCollect(ctx, ids, snap.reasons) + return b.gcCollect(ctx, ids, snap) }, } } @@ -101,7 +101,7 @@ func (b *Backend) WatchPath() string { } // gcCollect kills leftover hypervisor processes and removes orphan dirs/records under the orchestrator's flock. -func (b *Backend) gcCollect(ctx context.Context, ids []string, reasons map[string]string) error { +func (b *Backend) gcCollect(ctx context.Context, ids []string, snap VMGCSnapshot) error { logger := log.WithFunc("gc." + b.Typ) var errs []error for _, id := range ids { @@ -118,7 +118,7 @@ func (b *Backend) gcCollect(ctx context.Context, ids []string, reasons map[strin continue } logger.Infof(ctx, "collected id=%s runDir=%s logDir=%s reason=%s", - id, runDir, logDir, reasons[id]) + id, runDir, logDir, snap.reasons[id]) } if err := b.CleanStalePlaceholders(ctx, ids); err != nil { errs = append(errs, fmt.Errorf("clean stale placeholders: %w", err)) diff --git a/snapshot/localfile/gc_test.go b/snapshot/localfile/gc_test.go index 81e03cff..ebec41af 100644 --- a/snapshot/localfile/gc_test.go +++ b/snapshot/localfile/gc_test.go @@ -303,14 +303,18 @@ func TestGCModule_RemovalFailureKeepsDBRecord(t *testing.T) { } } + mod := gcModule(lf.conf, lf.store, lf.locker, EvictionPolicy{Enabled: true}) + snap, err := mod.ReadDB(ctx) + if err != nil { + t.Fatalf("ReadDB: %v", err) + } + parent := lf.conf.DataDir() if err := os.Chmod(parent, 0o500); err != nil { t.Skipf("chmod failed: %v", err) } t.Cleanup(func() { _ = os.Chmod(parent, 0o750) }) - mod := gcModule(lf.conf, lf.store, lf.locker, EvictionPolicy{Enabled: true}) - snap, _ := mod.ReadDB(ctx) if err := mod.Collect(ctx, ids, snap); err == nil { t.Fatal("expected Collect to error on chmod-protected parent") }