From 80a0183567b679d0af66bcf9e624ea193eca382d Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 24 Oct 2026 04:17:01 +0200 Subject: [PATCH 1/3] Build a done key again when the artifact store no longer holds its blob, and on the controller's artifacts-gone (issue 1937) The store's nightly collector deleted the bare blobs of node-tools', mesh-host's and the forge bridge's done keys, which the controller had never taken in, and the builder kept them done: their next delivery would refuse. The builder now asks the controller's held before it leaves a done trunk key done (once per key per process, at its start and on each module value), claims a key whose blob is gone building as a new revision and says so in its log and the key's step, and hears artifacts-gone, said when a take-in found a gone blob. No timer: a start, a value or an event. --- .../gone_integration_test.go | 185 ++++++++++++++++ .../cmd/mesh-delivery-builder/main.go | 28 +++ .../cmd/mesh-delivery-builder/stored.go | 61 ++++++ modules/mesh-delivery-builder/go.mod | 2 +- modules/mesh-delivery-builder/go.sum | 4 +- .../internal/build/claim.go | 54 +++++ .../internal/build/gone_test.go | 204 ++++++++++++++++++ .../internal/build/run.go | 70 +++++- .../mesh-delivery-builder/manifest_test.go | 13 +- modules/mesh-delivery-builder/module.json | 6 +- 10 files changed, 616 insertions(+), 11 deletions(-) create mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/gone_integration_test.go create mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/stored.go create mode 100644 modules/mesh-delivery-builder/internal/build/gone_test.go diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/gone_integration_test.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/gone_integration_test.go new file mode 100644 index 0000000..7a6de73 --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/gone_integration_test.go @@ -0,0 +1,185 @@ +//go:build integration + +package main + +import ( + "encoding/json" + "strings" + "sync" + "testing" + "time" + + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// A done key whose blob the artifact store no longer holds is built again, by the builder whole against the bus +// (novox/hq issue 1937): on 2026-10-24 the store's 03:30 collection deleted the bare blobs of three done keys the +// controller had never taken in, and the builder kept them done. + +// heldVerb answers the controller seat's verb held on the bus, as the serving controller does: {result: {output, ok, +// answer}}; a reference is gone while gone says so. +type heldVerb struct { + mu sync.Mutex + gone func(reference string) bool + asked []string +} + +func (h *heldVerb) serve(t *testing.T, b *bed) { + t.Helper() + sub, err := b.js.Conn().Subscribe("mesh.seat.mesh-controller.call.held.>", func(m *nats.Msg) { + var args struct { + References []string `json:"references"` + } + _ = json.Unmarshal(m.Data, &args) + a := delivery.HeldAnswer{Store: "store.test:5100", Held: []string{}, Gone: []string{}} + h.mu.Lock() + for _, r := range args.References { + h.asked = append(h.asked, r) + if h.gone(r) { + a.Gone = append(a.Gone, r) + } else { + a.Held = append(a.Held, r) + } + } + h.mu.Unlock() + reply, _ := json.Marshal(map[string]any{"result": map[string]any{"output": "held", "ok": true, "answer": a}, "node": "ctl"}) + _ = m.Respond(reply) + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = sub.Unsubscribe() }) +} + +func (h *heldVerb) asks() int { + h.mu.Lock() + defer h.mu.Unlock() + return len(h.asked) +} + +// eventsConsumer makes EVENTS and the builder's durable consumer of artifacts-gone on it, as the controller makes them +// from the manifest's consumes. +func eventsConsumer(t *testing.T, b *bed, node string) { + t.Helper() + if _, err := b.js.CreateStream(b.ctx, jetstream.StreamConfig{Name: eventsStream, Subjects: []string{"mesh.seat.>"}}); err != nil { + t.Fatal(err) + } + if _, err := b.js.CreateOrUpdateConsumer(b.ctx, eventsStream, jetstream.ConsumerConfig{Durable: node + "_mesh-delivery-builder", + AckPolicy: jetstream.AckExplicitPolicy, FilterSubjects: []string{delivery.ArtifactsGoneSubject}}); err != nil { + t.Fatal(err) + } +} + +// doneKey writes builds.. done, naming one archive at reference, and answers its revision. +func doneKey(t *testing.T, b *bed, module, manifest, reference, digest string) uint64 { + t.Helper() + key, _ := delivery.BuilderBuildKey(module, fp) + data, err := delivery.EncodeBuilderBuild(delivery.BuilderBuild{State: delivery.BuildDone, By: "n2.mesh-delivery-builder", + Manifest: manifest, Step: "built mesh-build-go on n2", + Artifacts: []delivery.BuiltArtifact{{Name: "files", Kind: "archive", Digest: digest, Reference: reference}}}) + if err != nil { + t.Fatal(err) + } + rev, err := b.builds.Put(b.ctx, key, data) + if err != nil { + t.Fatal(err) + } + return rev +} + +func revisionOf(t *testing.T, b *bed, module string) uint64 { + t.Helper() + key, _ := delivery.BuilderBuildKey(module, fp) + e, err := b.builds.Get(b.ctx, key) + if err != nil { + t.Fatal(err) + } + return e.Revision() +} + +// 1937/AC3 (integration): the builder, started whole, finds the done key of the trunk's fingerprint naming a blob held +// answers gone, builds it again as a new revision whose step says so, and leaves the rebuilt key done. +func TestADoneKeyWhoseBlobIsGoneIsBuiltAgainAtStart(t *testing.T) { + b := startBed(t) + eventsConsumer(t, b, "n1") + manifest := b.archive(t, "ca-trust", "modules/ca-trust", commit) + old := digestOf("collected last night") + oldRef := "artifact-store://ca-trust/files/blobs/" + old + rev := doneKey(t, b, "ca-trust", manifest, oldRef, old) + h := &heldVerb{gone: func(r string) bool { return r == oldRef }} + h.serve(t, b) + + b.start(t, "n1", nil) + v := b.await(t, "ca-trust", func(v delivery.BuilderBuild) bool { + return v.State == delivery.BuildDone && len(v.Artifacts) == 1 && v.Artifacts[0].Digest == digestOf("files") + }) + if !strings.Contains(v.Step, "built again: the artifact store no longer held "+oldRef) { + t.Errorf("the rebuilt key's step %q does not say why it was built again", v.Step) + } + if got := revisionOf(t, b, "ca-trust"); got <= rev { + t.Errorf("revision %d, was %d: want a new revision", got, rev) + } + if n := b.fake.count("mesh-build-go.build_go_build"); n != 1 { + t.Errorf("the archive was built %d times, want once", n) + } + if h.asks() == 0 { + t.Error("the builder never asked held") + } +} + +// 1937/AC4 (integration): a done key whose blob held answers held at start stays done; when the controller says +// artifacts-gone for it, the builder asks again and builds it again. +func TestArtifactsGoneLeadsTheBuilderToBuildTheKeyAgain(t *testing.T) { + b := startBed(t) + eventsConsumer(t, b, "n1") + manifest := b.archive(t, "ca-trust", "modules/ca-trust", commit) + d := digestOf("files") + ref := "artifact-store://ca-trust/files/blobs/" + d + rev := doneKey(t, b, "ca-trust", manifest, ref, d) + var mu sync.Mutex + collected := false + h := &heldVerb{gone: func(r string) bool { + mu.Lock() + defer mu.Unlock() + if r == ref && collected { + collected = false // gone once: the rebuild pushes it again + return true + } + return false + }} + h.serve(t, b) + + b.start(t, "n1", nil) + for deadline := time.Now().Add(20 * time.Second); h.asks() == 0 && time.Now().Before(deadline); time.Sleep(50 * time.Millisecond) { + } + if h.asks() == 0 { + t.Fatal("the builder never asked held at its start") + } + time.Sleep(300 * time.Millisecond) + if got := revisionOf(t, b, "ca-trust"); got != rev || b.fake.count("mesh-build-go.build_go_build") != 0 { + t.Fatalf("a key whose blob is held was built again (revision %d, was %d)", got, rev) + } + + mu.Lock() + collected = true + mu.Unlock() + key, _ := delivery.BuilderBuildKey("ca-trust", fp) + body, err := delivery.EncodeArtifactsGone(delivery.ArtifactsGone{Module: "ca-trust", Key: key, Fingerprint: fp, + Gone: []string{ref}, At: time.Now().UTC()}) + if err != nil { + t.Fatal(err) + } + if _, err := b.js.Publish(b.ctx, delivery.ArtifactsGoneSubject, body); err != nil { + t.Fatal(err) + } + v := b.await(t, "ca-trust", func(v delivery.BuilderBuild) bool { + return v.State == delivery.BuildDone && strings.Contains(v.Step, "built again") + }) + if got := revisionOf(t, b, "ca-trust"); got <= rev || b.fake.count("mesh-build-go.build_go_build") != 1 { + t.Errorf("build = %+v at revision %d (was %d), %d builds: want built again once on the event", v, got, rev, + b.fake.count("mesh-build-go.build_go_build")) + } +} diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go index 6e9d5db..6ff9aed 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go @@ -26,6 +26,7 @@ import ( "github.com/nats-io/nats.go/jetstream" "git.novox.be/novox/mesh-sdk/go/delivery" + "git.novox.be/novox/mesh-sdk/go/process" "mesh-delivery-builder/internal/build" ) @@ -153,6 +154,17 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro // Each build module's part of a build is an ask on its build seat's stream, pulled by a build node with a free slot // (novox/hq ADR 0525): the builder chooses no node, so it reads neither presence nor have and holds no capacity. asks := buildrun.JetStreamAsks{JS: js} + // The builder asks the controller, as its own account, whether the artifact store still holds a done key's blobs + // (the seat verb held, novox/hq issue 1937) before it leaves the key done. + account, err := process.ReadAccount(path) + if err != nil { + return err + } + calls, err := process.Connect(account, nats.Name(cred.Node+"."+cred.Module+".calls"), nats.RetryOnFailedConnect(true)) + if err != nil { + return err + } + defer calls.Close() b := &build.Builder{ Claimer: build.Claimer{Store: kvStore{builds}, By: cred.Node + "." + cred.Module}, Asks: buildrun.Asker{Publisher: asks, Waiter: asks, Bound: bound}, @@ -160,6 +172,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro Toolchains: tools, Manifests: manifestReader{manifests}, Logf: logger.Printf, + Stored: storedVia(calls.Call), } // **Said once every build module answered a reference, taken away when stopped** (issue 1886, ADR 0522): the // controller delivers from this builder's keys only while an instance says it writes references. A bucket not there @@ -203,6 +216,21 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro } defer cc.Stop() } + // The controller says artifacts-gone when its take-in found a key's blob gone (issue 1937): the builder's durable + // consumer on EVENTS, made by the controller from the manifest's consumes. Where it is not there yet the builder says + // so; it still asks held at its start and at every module value. + switch cons, err := js.Consumer(ctx, eventsStream, cred.Node+"_"+cred.Module); { + case err != nil: + logger.Printf("the consumer %s_%s on %s does not exist yet, so %s is not heard until the builder starts again: %v", + cred.Node, cred.Module, eventsStream, delivery.ArtifactsGoneEvent, err) + default: + cc, err := cons.Consume(func(m jetstream.Msg) { heardGone(ctx, b, m, logger.Printf) }, + jetstream.ConsumeErrHandler(func(_ jetstream.ConsumeContext, err error) { logger.Printf("hearing the bus: %v", err) })) + if err != nil { + return err + } + defer cc.Stop() + } each := func(e jetstream.KeyValueEntry) { if opOf(e) { b.Module(ctx, e.Key(), e.Value()) diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/stored.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/stored.go new file mode 100644 index 0000000..3f2bbef --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/stored.go @@ -0,0 +1,61 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "sort" + "strings" + + "github.com/nats-io/nats.go/jetstream" + + "git.novox.be/novox/mesh-sdk/go/delivery" + + "mesh-delivery-builder/internal/build" +) + +// eventsStream is the stream every module's and role's events are on, where the controller makes the builder's durable +// consumer of artifacts-gone (mesh-controller internal/broker EventsStream). +const eventsStream = "EVENTS" + +// toolCaller calls a tool or a seat verb as the builder's own account (mesh-sdk process.Conn.Call). +type toolCaller func(key string, body any) (json.RawMessage, error) + +// storedVia is build.Builder's Stored: it asks the controller's verb held (mesh-sdk delivery.HeldVerb; novox/hq issue +// 1937) and answers the references the store no longer holds. One it could not ask about, with none gone, is an error: +// the key is then left done, and the controller's take-in still refuses a gone blob. +func storedVia(call toolCaller) func(ctx context.Context, references []string) ([]string, error) { + return func(_ context.Context, references []string) ([]string, error) { + raw, err := call(delivery.HeldVerb, map[string]any{"references": references}) + if err != nil { + return nil, fmt.Errorf("asking %s: %w", delivery.HeldVerb, err) + } + a, err := delivery.DecodeHeldAnswer(raw, references) + if err != nil { + return nil, err + } + if len(a.Gone) == 0 && len(a.Unknown) > 0 { + var why []string + for r, w := range a.Unknown { + why = append(why, r+": "+w) + } + sort.Strings(why) + return nil, fmt.Errorf("the artifact store could not be asked about %s", strings.Join(why, "; ")) + } + return a.Gone, nil + } +} + +// heardGone takes one artifacts-gone event in: acknowledged, then handed to the builder, which asks the store and builds +// the key again. A body that is not one is said and acknowledged, so it is not handed over again. +func heardGone(ctx context.Context, b *build.Builder, m jetstream.Msg, logf func(string, ...any)) { + e, err := delivery.DecodeArtifactsGone(m.Data()) + if ackErr := m.Ack(); ackErr != nil { + logf("acknowledging %s: %v", m.Subject(), ackErr) + } + if err != nil { + logf("%s holds what is not %s, left: %v", m.Subject(), delivery.ArtifactsGoneEvent, err) + return + } + b.ArtifactsGone(ctx, e) +} diff --git a/modules/mesh-delivery-builder/go.mod b/modules/mesh-delivery-builder/go.mod index d5b8384..615cce0 100644 --- a/modules/mesh-delivery-builder/go.mod +++ b/modules/mesh-delivery-builder/go.mod @@ -3,7 +3,7 @@ module mesh-delivery-builder go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261023150321-a2c813a6b468 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024020722-34f210e5eba1 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-delivery-builder/go.sum b/modules/mesh-delivery-builder/go.sum index 2a3e9f3..d648ca0 100644 --- a/modules/mesh-delivery-builder/go.sum +++ b/modules/mesh-delivery-builder/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261023150321-a2c813a6b468 h1:UcMee+IV2ii5E4dRZdQUaSaeh9IedTFYumuQbBiB0k0= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261023150321-a2c813a6b468/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024020722-34f210e5eba1 h1:fzOkrz9CSp3mEjeV7NfZx8yoivI1sD1AOtIPtyyF8RI= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024020722-34f210e5eba1/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-delivery-builder/internal/build/claim.go b/modules/mesh-delivery-builder/internal/build/claim.go index 21b1316..d931a16 100644 --- a/modules/mesh-delivery-builder/internal/build/claim.go +++ b/modules/mesh-delivery-builder/internal/build/claim.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "slices" "sort" "strings" @@ -37,6 +38,11 @@ type Claim struct { Key string Rev uint64 Build delivery.BuilderBuild + // Gone are the references of a done key the artifact store no longer holds, for which it was claimed again + // (ClaimOptions.Stored; issue 1937): said in the log and the key's step. + Gone []string + // Unasked is why the store could not be asked about a done key taken as it stands to publish: said in the log. + Unasked error } // ClaimOptions say what the builder knows about the moment of a claim. @@ -55,6 +61,30 @@ type ClaimOptions struct { // a package's reference. A key queued waiting for a version (BuilderBuild.Waits) is taken only once it is (ADR 0500 // clause 6). Nil means none is. Published func(ref string) bool + // Stored answers which of the references given the artifact store no longer holds (the controller's verb held, + // mesh-sdk delivery.HeldVerb; novox/hq issue 1937). A done key of the trunk's fingerprint is asked before it is left + // done: the store's nightly collector deletes a blob nothing holds, and a key the controller never took in holds + // nothing, so a key whose blob is gone is claimed building as a new revision and built again. Nil asks nothing. + Stored func(ctx context.Context, references []string) (gone []string, err error) +} + +// StoredReferences are the references of a done build's bundles, archives and upstream copies that name a blob in the +// artifact store (`artifact-store:////blobs/`): what the store's collector can delete. An image +// is a manifest, which the collector keeps, and a package lives in the package registry. +func StoredReferences(b delivery.BuilderBuild) []string { + var out []string + for _, a := range b.Artifacts { + if strings.HasPrefix(a.Reference, "artifact-store://") && strings.Contains(a.Reference, "/blobs/") && + !slices.Contains(out, a.Reference) { + out = append(out, a.Reference) + } + } + return out +} + +// GoneStep is the step a key claimed again for gone blobs says (issue 1937). +func GoneStep(gone []string) string { + return "claimed again: the artifact store no longer holds " + strings.Join(gone, ", ") } // Unpublished is whether a done build holds an image built without a push or a package built without a publish: it has @@ -118,6 +148,8 @@ func Unreferenced(b delivery.BuilderBuild) bool { // names it (ClaimOptions.Trunk): that is taken as it stands, with the done value, and its builder writes the new // revision when the publishing build ends (ADR 0498 clause 6). From R4e one node builds each fingerprint (ADR 0478 // clause 4), so a key built on one node is never built again for a second; +// - a done key of the trunk's fingerprint whose bundle, archive or upstream copy the artifact store no longer holds +// (ClaimOptions.Stored, issue 1937) is claimed building as a new revision and built whole, its step saying which; // - a failed key is taken again as a new revision of the same key (ADR 0429 clause 18), except one failed for a kind no // builder builds, for an upstream pinned by no digest or a package version taken (delivery.FailedUntilChanged: ADR 0478 // clause 2, ADR 0498 clauses 4 and 5) or for digests that differ (buildrun.DigestsDifferPrefix, clause 4), @@ -149,6 +181,28 @@ func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string if Unreferenced(cur) { return c.write(ctx, key, building, func(data []byte) (uint64, error) { return c.Store.Update(ctx, key, data, rev) }) } + if opt.Trunk && opt.Stored != nil { + if refs := StoredReferences(cur); len(refs) > 0 { + gone, err := opt.Stored(ctx, refs) + if err != nil { + err = fmt.Errorf("%s: whether the artifact store still holds %s cannot be asked, so the key is "+ + "left done (the controller's take-in refuses it if one is gone): %w", key, strings.Join(refs, ", "), err) + if len(publishable(cur, opt.ImageBuilder)) > 0 { + return &Claim{Key: key, Rev: rev, Build: cur, Unasked: err}, nil + } + return nil, err + } + if len(gone) > 0 { + again := building + again.Step = GoneStep(gone) + cl, err := c.write(ctx, key, again, func(data []byte) (uint64, error) { return c.Store.Update(ctx, key, data, rev) }) + if cl != nil { + cl.Gone = gone + } + return cl, err + } + } + } if opt.Trunk && len(publishable(cur, opt.ImageBuilder)) > 0 { return &Claim{Key: key, Rev: rev, Build: cur}, nil } diff --git a/modules/mesh-delivery-builder/internal/build/gone_test.go b/modules/mesh-delivery-builder/internal/build/gone_test.go new file mode 100644 index 0000000..499cdbc --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/gone_test.go @@ -0,0 +1,204 @@ +package build + +import ( + "context" + "errors" + "fmt" + "slices" + "strings" + "sync" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// A done key whose blob the artifact store no longer holds is built again (novox/hq issue 1937): on 2026-10-24 the +// store's 03:30 collection deleted the bare blobs of node-tools', mesh-host's and the forge bridge's done keys, which the +// controller had never taken in, and the builder kept them done. + +// fakeHeld answers the controller's verb held: the references in gone are gone. +type fakeHeld struct { + mu sync.Mutex + gone map[string]bool + err error + asked [][]string +} + +func (f *fakeHeld) stored(_ context.Context, refs []string) ([]string, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.asked = append(f.asked, slices.Clone(refs)) + if f.err != nil { + return nil, f.err + } + var out []string + for _, r := range refs { + if f.gone[r] { + out = append(out, r) + } + } + return out, nil +} + +func (f *fakeHeld) asks() int { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.asked) +} + +// logged keeps what the builder says. +type logged struct { + mu sync.Mutex + lines []string +} + +func (l *logged) logf(format string, args ...any) { + l.mu.Lock() + defer l.mu.Unlock() + l.lines = append(l.lines, fmt.Sprintf(format, args...)) +} + +func (l *logged) has(words ...string) bool { + l.mu.Lock() + defer l.mu.Unlock() + for _, line := range l.lines { + all := true + for _, w := range words { + all = all && strings.Contains(line, w) + } + if all { + return true + } + } + return false +} + +const goneRef = "artifact-store://m/tool/blobs/" + digest + +func doneKeyNaming(ref string) delivery.BuilderBuild { + return delivery.BuilderBuild{State: delivery.BuildDone, By: "shanks.builder", Manifest: manifestD, Step: "built mesh-build-go on shanks", + Artifacts: []delivery.BuiltArtifact{{Name: "tool", Kind: "bundle", Digest: digest, Reference: ref}}} +} + +// 1937/AC3 (unit, the claim): a done key of the trunk's fingerprint whose blob is gone is claimed building as a new +// revision, its step naming the gone reference; a candidate's done key is not asked about. +func TestADoneTrunkKeyWhoseBlobIsGoneIsClaimedAgain(t *testing.T) { + st := newMem() + c := Claimer{Store: st, By: "novox.builder"} + key, _ := delivery.BuilderBuildKey("m", fp1) + rev := putBuild(t, st, key, doneKeyNaming(goneRef)) + held := &fakeHeld{gone: map[string]bool{goneRef: true}} + + cl, err := c.Claim(t.Context(), "m", fp1, manifestD, ClaimOptions{Stored: held.stored}) + if err != nil || cl != nil || held.asks() != 0 { + t.Fatalf("a candidate's done key: claim %+v, %v, %d asks; want nothing and no ask", cl, err, held.asks()) + } + cl, err = c.Claim(t.Context(), "m", fp1, manifestD, ClaimOptions{Trunk: true, Stored: held.stored}) + if err != nil || cl == nil { + t.Fatalf("claim = %+v, %v; want the key claimed again", cl, err) + } + if cl.Rev <= rev || cl.Build.State != delivery.BuildBuilding || !slices.Equal(cl.Gone, []string{goneRef}) { + t.Errorf("claim = %+v (revision was %d); want building as a new revision naming the gone reference", cl, rev) + } + v := valueOf(t, st, key) + if v.State != delivery.BuildBuilding || !strings.Contains(v.Step, "no longer holds") || !strings.Contains(v.Step, goneRef) { + t.Errorf("the key holds %+v; want building, its step naming the gone reference", v) + } +} + +// 1937/AC5 (unit, the claim): a done trunk key whose blobs are all held stays done; a store that cannot be asked leaves +// it done and says why; images and packages are not asked about. +func TestADoneTrunkKeyStaysDoneWhileTheStoreHoldsItsBlobs(t *testing.T) { + st := newMem() + c := Claimer{Store: st, By: "novox.builder"} + key, _ := delivery.BuilderBuildKey("m", fp1) + b := doneKeyNaming(goneRef) + b.Artifacts = append(b.Artifacts, delivery.BuiltArtifact{Name: "img", Kind: "image", Digest: digest, Reference: "registry.example/m/img@" + digest}) + rev := putBuild(t, st, key, b) + held := &fakeHeld{gone: map[string]bool{}} + if cl, err := c.Claim(t.Context(), "m", fp1, manifestD, ClaimOptions{Trunk: true, Stored: held.stored}); err != nil || cl != nil { + t.Fatalf("claim = %+v, %v; want nothing: the store holds the blob", cl, err) + } + if len(held.asked) != 1 || !slices.Equal(held.asked[0], []string{goneRef}) { + t.Errorf("asked %v; want the bundle's reference alone", held.asked) + } + held.err = errors.New("the controller did not answer") + cl, err := c.Claim(t.Context(), "m", fp1, manifestD, ClaimOptions{Trunk: true, Stored: held.stored}) + if cl != nil || err == nil || !strings.Contains(err.Error(), "cannot be asked") || !strings.Contains(err.Error(), "left done") { + t.Errorf("claim = %+v, %v; want the key left done and said", cl, err) + } + if st.revs[key] != rev { + t.Errorf("the key moved to revision %d (was %d)", st.revs[key], rev) + } +} + +// 1937/AC3 (unit, the builder): the builder's look at a module whose done key's blob is gone builds it again, says so +// in a log line and in the done key's step, and asks the store once per process while it answers held. +func TestTheBuilderBuildsAgainADoneKeyWhoseBlobIsGone(t *testing.T) { + r := newRig(t, goBundle) + log := &logged{} + r.b.Logf = log.logf + held := &fakeHeld{gone: map[string]bool{goneRef: true}} + r.b.Stored = held.stored + rev := putBuild(t, r.st, r.key, doneKeyNaming(goneRef)) + v := r.consider(t, "modules/m") + if v.State != delivery.BuildDone || r.st.revs[r.key] <= rev || len(r.bm.callsOf("build_go_build")) == 0 { + t.Fatalf("build = %+v at revision %d (was %d); want built again", v, r.st.revs[r.key], rev) + } + if !strings.Contains(v.Step, "built again: the artifact store no longer held "+goneRef) { + t.Errorf("the done key's step %q does not say it was built again for the gone blob", v.Step) + } + if !log.has(r.key, "built again", goneRef) { + t.Errorf("no log line says the key was built again: %v", log.lines) + } + // The new revision's blob is held: asked once, then not again in this process. + held.mu.Lock() + held.gone = map[string]bool{} + held.mu.Unlock() + rev = r.st.revs[r.key] + asks := held.asks() + r.consider(t, "modules/m") + r.consider(t, "modules/m") + if r.st.revs[r.key] != rev { + t.Errorf("a key whose blob is held was built again") + } + if held.asks() != asks+1 { + t.Errorf("asked the store %d times for two looks at a held key; want once", held.asks()-asks) + } +} + +// 1937/AC4 (unit, the builder): artifacts-gone for the module's fingerprint makes the builder ask again, even for a key +// it found held before, and build it again; one for another fingerprint builds nothing. +func TestArtifactsGoneMakesTheBuilderAskAndBuildAgain(t *testing.T) { + r := newRig(t, goBundle) + log := &logged{} + r.b.Logf = log.logf + held := &fakeHeld{gone: map[string]bool{}} + r.b.Stored = held.stored + putBuild(t, r.st, r.key, doneKeyNaming(goneRef)) + if v := r.consider(t, "modules/m"); v.State != delivery.BuildDone || len(r.bm.callsOf("build_go_build")) != 0 { + t.Fatalf("a key whose blob is held was built: %+v", v) + } + rev := r.st.revs[r.key] + + held.mu.Lock() + held.gone[goneRef] = true // collected after the builder's look; the controller's take-in found it + held.mu.Unlock() + r.b.ArtifactsGone(context.Background(), delivery.ArtifactsGone{Event: delivery.ArtifactsGoneEvent, Module: "m", + Fingerprint: "src2:" + strings.Repeat("9", 64), Gone: []string{goneRef}}) + r.b.Wait() + if r.st.revs[r.key] != rev || !log.has("a fingerprint the module does not have now") { + t.Fatalf("an event for another fingerprint moved the key (revision %d, was %d): %v", r.st.revs[r.key], rev, log.lines) + } + + r.b.ArtifactsGone(context.Background(), delivery.ArtifactsGone{Event: delivery.ArtifactsGoneEvent, Module: "m", + Key: r.key, Fingerprint: fp1, Gone: []string{goneRef}}) + r.b.Wait() + v := valueOf(t, r.st, r.key) + if v.State != delivery.BuildDone || r.st.revs[r.key] <= rev || len(r.bm.callsOf("build_go_build")) == 0 { + t.Fatalf("build = %+v at revision %d (was %d); want built again on the event", v, r.st.revs[r.key], rev) + } + if !log.has("heard artifacts-gone") || !log.has(r.key, "built again", goneRef) { + t.Errorf("the builder did not say what it heard and did: %v", log.lines) + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index f4f2035..1d140d7 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -50,6 +50,9 @@ type Builder struct { // Answered is told the build module whose step answered a reference (issue 1886): what the builder says it writes // rests on it. Nil tells nobody. Answered func(buildModule string) + // Stored answers which references the artifact store no longer holds (the controller's verb held; issue 1937): a done + // key of the trunk's fingerprint is asked before it is left done (ClaimOptions.Stored). Nil asks nothing. + Stored func(ctx context.Context, references []string) (gone []string, err error) once sync.Once sem chan struct{} @@ -66,6 +69,9 @@ type Builder struct { // published are the package versions `@` a done key of this bucket names as a package's reference: a key // waiting for one is built once it is here (ADR 0500 clause 6). published map[string]bool + // held are the done keys' references the store answered held, by ` `, asked once per process: a + // blob collected later is found by the take-in, whose artifacts-gone forgets the key here (issue 1937). + held map[string]bool } type planned struct { @@ -82,7 +88,7 @@ func (b *Builder) init() { b.sem = make(chan struct{}, limit) b.cands, b.via, b.trunkFP = map[string]delivery.Candidate{}, map[string]via{}, map[string]bool{} b.known, b.inflight, b.again, b.plans, b.tried = map[string]delivery.Module{}, map[string]bool{}, map[string]delivery.Module{}, map[string]planned{}, map[string]bool{} - b.published = map[string]bool{} + b.published, b.held = map[string]bool{}, map[string]bool{} }) } @@ -202,6 +208,55 @@ func (b *Builder) Forget(key string) { b.mu.Unlock() } +// storedOnce is Stored for one key, asked once per process while the store answers every reference held. +func (b *Builder) storedOnce(key string) func(ctx context.Context, references []string) ([]string, error) { + if b.Stored == nil { + return nil + } + return func(ctx context.Context, references []string) ([]string, error) { + id := key + " " + strings.Join(references, " ") + b.mu.Lock() + known := b.held[id] + b.mu.Unlock() + if known { + return nil, nil + } + gone, err := b.Stored(ctx, references) + if err == nil && len(gone) == 0 { + b.mu.Lock() + b.held[id] = true + b.mu.Unlock() + } + return gone, err + } +} + +// ArtifactsGone takes the controller's event artifacts-gone in (issue 1937): a take-in found that the store does not +// hold what the module's key names. The module is looked at again, which asks the store (ClaimOptions.Stored) and +// builds the key again; an event for a fingerprint the module no longer has is said and left. The event only wakes the +// builder: what is built rests on the store's answer, never on the event alone. +func (b *Builder) ArtifactsGone(ctx context.Context, e delivery.ArtifactsGone) { + b.init() + b.mu.Lock() + m, ok := b.known[e.Module] + if key, err := delivery.BuilderBuildKey(e.Module, e.Fingerprint); err == nil { + for id := range b.held { + if strings.HasPrefix(id, key+" ") { + delete(b.held, id) + } + } + } + b.mu.Unlock() + if !ok || m.FP2 != e.Fingerprint { + b.logf("%s: heard %s for %s, a fingerprint the module does not have now: nothing to build", e.Module, + delivery.ArtifactsGoneEvent, e.Fingerprint) + return + } + b.logf("%s: heard %s (%s): asking the artifact store and building the key again", e.Module, + delivery.ArtifactsGoneEvent, strings.Join(e.Gone, ", ")) + b.consider(ctx, e.Module, m) +} + // Reconsider looks at every module again: the toolchains changed, so a build that failed for them may build now. func (b *Builder) Reconsider(ctx context.Context) { b.init() @@ -312,7 +367,8 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { first := !b.tried[key] b.tried[key] = true b.mu.Unlock() - cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest, ClaimOptions{TakeOver: first, ImageBuilder: imageSeat, Trunk: trunk, Published: b.isPublished}) + cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest, ClaimOptions{TakeOver: first, ImageBuilder: imageSeat, Trunk: trunk, Published: b.isPublished, + Stored: b.storedOnce(key)}) if err != nil { b.logf("%s: %v", module, err) return @@ -320,6 +376,13 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { if cl == nil { return } + if cl.Unasked != nil { + b.logf("%v", cl.Unasked) + } + if len(cl.Gone) > 0 { + // Said aloud (issue 1937): the key was done, and the store's collector deleted what it names. + b.logf("%s: built again: the artifact store no longer holds %s", cl.Key, strings.Join(cl.Gone, ", ")) + } // A done key taken again builds only what is missing: the image or the package built unpublished as a candidate's; the // other artifacts stay as the done key has them (ADR 0487 clauses 5 and 8, ADR 0498 clause 6). var prior *delivery.BuilderBuild @@ -350,6 +413,9 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { return } result.By, result.Manifest = b.Claimer.By, m.Manifest + if len(cl.Gone) > 0 && result.State == delivery.BuildDone { + result.Step += " (built again: the artifact store no longer held " + strings.Join(cl.Gone, ", ") + ")" + } if publishing && result.State == delivery.BuildFailed { // The trunk's image or package was built only as a candidate's and its push or publish failed: the key says so, so the builds view, the // checker and the failed-key retake see it, and a look does not build the image again for nothing. diff --git a/modules/mesh-delivery-builder/manifest_test.go b/modules/mesh-delivery-builder/manifest_test.go index 930b0d8..11be676 100644 --- a/modules/mesh-delivery-builder/manifest_test.go +++ b/modules/mesh-delivery-builder/manifest_test.go @@ -96,11 +96,14 @@ func TestManifestDeclarations(t *testing.T) { // 1607/AC1 if want := []string{"node-build-go", "node-build-image", "node-build-node"}; !reflect.DeepEqual(sorted(m.Uses), want) { t.Errorf("uses = %v, want the three build seats", m.Uses) } - if len(m.Invokes) != 0 || len(m.Config) != 0 { - t.Errorf("invokes = %v, config = %v, want none: no tool call from the builder and no capacity", m.Invokes, m.Config) + // 1937/AC3: the builder asks the controller whether the artifact store still holds a done key's blobs (held), and no + // other tool; it holds no capacity. + if want := []string{"seat:mesh-controller.held"}; !reflect.DeepEqual(m.Invokes, want) || len(m.Config) != 0 { + t.Errorf("invokes = %v, config = %v, want %v and no config", m.Invokes, m.Config, want) } - // The builder is woken by a build module's report of a toolchain, as the indexer is: the bucket may not exist when it starts. - if want := []string{"mesh-build-go.toolchain.reported", "mesh-build-node.toolchain.reported"}; !reflect.DeepEqual(sorted(m.Consumes), want) { + // The builder is woken by a build module's report of a toolchain, as the indexer is: the bucket may not exist when it + // starts; and by the controller's artifacts-gone, a take-in refused for a gone blob (1937/AC4). + if want := []string{"mesh-build-go.toolchain.reported", "mesh-build-node.toolchain.reported", "mesh-controller.artifacts-gone"}; !reflect.DeepEqual(sorted(m.Consumes), want) { t.Errorf("consumes = %v, want %v", m.Consumes, want) } if b, ok := m.OwnSecrets["broker"]; !ok || len(m.OwnSecrets) != 1 || b != "${dir:state}/broker" { @@ -109,7 +112,7 @@ func TestManifestDeclarations(t *testing.T) { // 1607/AC1 if m.SecretsOwner != "mesh-delivery-builder" { t.Errorf("secrets-owner = %q", m.SecretsOwner) } - for _, k := range []string{"streams", "logs", "emits", "claims", "provides", "requires", "defines-seats", "tools", "seat", "seats", "bus", "invokes", "config"} { + for _, k := range []string{"streams", "logs", "emits", "claims", "provides", "requires", "defines-seats", "tools", "seat", "seats", "bus", "config"} { if _, ok := top[k]; ok { t.Errorf("the manifest declares %q: the builder has none", k) } diff --git a/modules/mesh-delivery-builder/module.json b/modules/mesh-delivery-builder/module.json index 798cb79..0b7a72f 100644 --- a/modules/mesh-delivery-builder/module.json +++ b/modules/mesh-delivery-builder/module.json @@ -4,7 +4,11 @@ "slug": "builder", "consumes": [ "mesh-build-go.toolchain.reported", - "mesh-build-node.toolchain.reported" + "mesh-build-node.toolchain.reported", + "mesh-controller.artifacts-gone" + ], + "invokes": [ + "seat:mesh-controller.held" ], "state": [ { "name": "builds", "history": 5 }, From f0c5be8c3029dbcb4af2d9e3f041fa672ca74d12 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 24 Oct 2026 04:19:17 +0200 Subject: [PATCH 2/3] Say which artifacts the store check asks about: bundles and archives, whose blobs the collector deletes (issue 1937) --- modules/mesh-delivery-builder/internal/build/claim.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/mesh-delivery-builder/internal/build/claim.go b/modules/mesh-delivery-builder/internal/build/claim.go index d931a16..406a62c 100644 --- a/modules/mesh-delivery-builder/internal/build/claim.go +++ b/modules/mesh-delivery-builder/internal/build/claim.go @@ -68,9 +68,9 @@ type ClaimOptions struct { Stored func(ctx context.Context, references []string) (gone []string, err error) } -// StoredReferences are the references of a done build's bundles, archives and upstream copies that name a blob in the -// artifact store (`artifact-store:////blobs/`): what the store's collector can delete. An image -// is a manifest, which the collector keeps, and a package lives in the package registry. +// StoredReferences are the references of a done build's bundles and archives that name a blob in the artifact store +// (`artifact-store:////blobs/`): what the store's collector can delete. An image or an upstream +// copy is a manifest, which the collector keeps, and a package lives in the package registry. func StoredReferences(b delivery.BuilderBuild) []string { var out []string for _, a := range b.Artifacts { @@ -148,7 +148,7 @@ func Unreferenced(b delivery.BuilderBuild) bool { // names it (ClaimOptions.Trunk): that is taken as it stands, with the done value, and its builder writes the new // revision when the publishing build ends (ADR 0498 clause 6). From R4e one node builds each fingerprint (ADR 0478 // clause 4), so a key built on one node is never built again for a second; -// - a done key of the trunk's fingerprint whose bundle, archive or upstream copy the artifact store no longer holds +// - a done key of the trunk's fingerprint whose bundle or archive blob the artifact store no longer holds // (ClaimOptions.Stored, issue 1937) is claimed building as a new revision and built whole, its step saying which; // - a failed key is taken again as a new revision of the same key (ADR 0429 clause 18), except one failed for a kind no // builder builds, for an upstream pinned by no digest or a package version taken (delivery.FailedUntilChanged: ADR 0478 From 28ee22ef27e2f67355be3dc890ce7abed65b7cbd Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 24 Oct 2026 04:25:19 +0200 Subject: [PATCH 3/3] Give the integration test's EVENTS stream the artifacts-gone subject alone, so it does not overlap the build seats' streams (issue 1937) --- .../cmd/mesh-delivery-builder/gone_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/gone_integration_test.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/gone_integration_test.go index 7a6de73..4ccc867 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/gone_integration_test.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/gone_integration_test.go @@ -64,7 +64,7 @@ func (h *heldVerb) asks() int { // from the manifest's consumes. func eventsConsumer(t *testing.T, b *bed, node string) { t.Helper() - if _, err := b.js.CreateStream(b.ctx, jetstream.StreamConfig{Name: eventsStream, Subjects: []string{"mesh.seat.>"}}); err != nil { + if _, err := b.js.CreateStream(b.ctx, jetstream.StreamConfig{Name: eventsStream, Subjects: []string{delivery.ArtifactsGoneSubject}}); err != nil { t.Fatal(err) } if _, err := b.js.CreateOrUpdateConsumer(b.ctx, eventsStream, jetstream.ConsumerConfig{Durable: node + "_mesh-delivery-builder",