diff --git a/cmd/mesh-controller/collect.go b/cmd/mesh-controller/collect.go index 936f02b7..dac14eef 100644 --- a/cmd/mesh-controller/collect.go +++ b/cmd/mesh-controller/collect.go @@ -59,6 +59,9 @@ type sweepResult struct { LetGo []string // Skipped is how many references the sweep will not address (novox/hq issue 226). Skipped int + // Indexes is how many eligible indexes a sweep a person did not confirm left for one that is: an + // index goes only with its platform manifests (novox/hq ADR 0257 §4). + Indexes int // Left is how many eligible references were not asked about this time. Left int // Bounded is whether it stopped at its bounds rather than at a refusal. @@ -85,7 +88,7 @@ func sweep(ctx context.Context, inv *inventory.Inventory, store artifacts.Store, // 0257), and a kept index keeps its own: which platform manifests the kept indexes name is read // from the store for every repository the sweep will touch, before the first delete. A single // read that fails stops the sweep before it deletes anything, as a kept archive that cannot be - // held does. The sweep after a build lets an index go alone, as it always has. + // held does. The sweep after a build leaves an eligible index for such a collect, below. store.Spare = nil if bounds.platforms { if inv == nil { @@ -124,6 +127,22 @@ func sweep(ctx context.Context, inv *inventory.Inventory, store artifacts.Store, } break } + if !bounds.platforms { + // **An index waits for a confirmed collect** (novox/hq ADR 0257 §4). Let go of alone, + // its platform manifests would stay in the store and the record would say collected, so + // no later sweep would offer it again and they would stay for ever. Left eligible, the + // next collect a person confirms takes it with them. + index, err := store.IsIndex(within, reference) + if err != nil && !errors.Is(err, artifacts.Gone) && !errors.Is(err, artifacts.ErrNotOurs) { + r.Stopped = fmt.Sprintf("the artifact store could not say whether %s is an index, so nothing more was asked of it: %v", reference, err) + r.Left = len(references) - i + break + } + if index { + r.Indexes++ + continue + } + } err := store.LetGo(within, reference) if err == nil || errors.Is(err, artifacts.Gone) { // Gone is the outcome wanted, already true. Recorded so the next sweep does not ask @@ -215,6 +234,9 @@ func collect(ctx context.Context, inv *inventory.Inventory) { if r.Skipped > 0 { fmt.Fprintf(os.Stderr, "%d artifact(s) the sweep will not address were skipped\n", r.Skipped) } + if r.Indexes > 0 { + fmt.Fprintf(os.Stderr, "%d eligible index(es) wait for a collect a person confirms, which takes their platforms too\n", r.Indexes) + } } // holdKept holds every kept archive by its manifest, stopping at the first refusal by the store. diff --git a/cmd/mesh-controller/mirrors_test.go b/cmd/mesh-controller/mirrors_test.go index ae5e0c06..de85038c 100644 --- a/cmd/mesh-controller/mirrors_test.go +++ b/cmd/mesh-controller/mirrors_test.go @@ -255,14 +255,32 @@ func TestAConfirmedCollectLetsAnIndexGoWithItsOwnPlatformsOnly(t *testing.T) { } } -// The sweep after a build lets an index go alone: its platforms wait for a person's collect. -func TestTheSweepAfterABuildLetsAnIndexGoAlone(t *testing.T) { +// The sweep after a build leaves an eligible index in place and eligible: let go of alone, its +// platforms would stay for ever under a record that says collected. A later collect a person confirms +// takes it with the platform only it names. An image the same sweep reaches is let go of as before. +func TestTheSweepAfterABuildLeavesAnIndexForAConfirmedCollect(t *testing.T) { inv := inventory.ForTest(t) _, eligible, reg := twoCopies(t, inv) + reg.held[copyDigest(5)] = true + image := catalogue.ArtifactStoreScheme + "upstream/docker.io/library/golang@" + copyDigest(5) store := reg.serve(t) - r := sweep(t.Context(), inv, store, []string{eligible}, nil, afterBuild) - if !slices.Equal(r.LetGo, []string{eligible}) || !slices.Equal(reg.deleted, []string{copyDigest(2)}) { - t.Fatalf("after a build: let go %v, deleted %v; want the index alone", r.LetGo, reg.deleted) + r := sweep(t.Context(), inv, store, []string{eligible, image}, nil, afterBuild) + if r.Indexes != 1 || !slices.Equal(r.LetGo, []string{image}) || !slices.Equal(reg.deleted, []string{copyDigest(5)}) { + t.Fatalf("after a build: %d indexes left, let go %v, deleted %v; want the index left and the image gone", + r.Indexes, r.LetGo, reg.deleted) + } + left, err := inv.ToCollect(t.Context()) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(left, []string{eligible}) { + t.Fatalf("after the sweep the records offer %v; want the index still eligible", left) + } + a := runCollect(t.Context(), inv, store, left, nil, true, + sweepBounds{most: 10, budget: 5 * time.Second, platforms: true}) + if !slices.Equal(a.LetGo, []string{eligible}) || + !slices.Equal(reg.deleted, []string{copyDigest(5), copyDigest(13), copyDigest(2)}) { + t.Fatalf("a confirmed collect let go %v, deleted %v; want the index with its own platform", a.LetGo, reg.deleted) } } diff --git a/internal/artifacts/store.go b/internal/artifacts/store.go index 253c428c..b84776b0 100644 --- a/internal/artifacts/store.go +++ b/internal/artifacts/store.go @@ -261,6 +261,21 @@ func split(path string) (repository, kind, digest string, err error) { return "", "", "", fmt.Errorf("%w: %q names nothing the store holds by digest", ErrNotOurs, path) } +// IsIndex is whether a recorded reference is an index the store holds: one that names manifests. A +// blob, or a manifest that names none, is not. Gone when the store does not hold it. +func (s Store) IsIndex(ctx context.Context, reference string) (bool, error) { + path, ours := catalogue.InArtifactStore(reference) + if !ours { + return false, fmt.Errorf("%w: %s", ErrNotOurs, reference) + } + repository, kind, digest, err := split(path) + if err != nil || kind != "manifests" { + return false, err + } + children, err := s.Platforms(ctx, repository, digest) + return len(children) > 0, err +} + // HoldsManifest is whether the store holds the manifest a recorded image reference names. func (s Store) HoldsManifest(ctx context.Context, reference string) (bool, error) { path, ours := catalogue.InArtifactStore(reference) diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index 31de26ba..299fb73c 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -451,7 +451,8 @@ var ControllerVerbs = []Verb{ "listed, nothing held or deleted. With confirm and why: every kept archive held first, then each eligible " + "artifact let go of, oldest first, and recorded collected — a hand act, recorded with its why. A confirmed " + "collect lets an index go with the platform manifests no kept index names (read first; any read that " + - "fails lets nothing go); the sweep after a build lets an index go alone (novox/hq ADR 0257). Bounded by " + + "fails lets nothing go); the sweep after a build leaves an eligible index for such a collect (novox/hq ADR " + + "0257). Bounded by " + "most (500 by default, at most 5000) and 45 seconds; what is left is said. Bytes are reclaimed by the " + "store's nightly collector. Never touches a digest the mesh did not record (novox/hq ADR 0189, ADR 0251).", Input: schema(map[string]string{