From f26ff8f8bbcd923d0178b977052e95b4591665bc Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 17:57:01 +0200 Subject: [PATCH] =?UTF-8?q?Write=20a=20key=20done=20only=20when=20every=20?= =?UTF-8?q?bundle=20and=20archive=20names=20where=20it=20is=20stored,=20a?= =?UTF-8?q?=20candidate's=20too,=20so=20a=20merge=20builds=20nothing=20its?= =?UTF-8?q?=20check=20built=20(issue=201587,=20B1,=20ADR=200498=20=C2=A71-?= =?UTF-8?q?=C2=A73)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins mesh-sdk feat/r4e-sdk a90f523 (R4e's group member). A trunk key's builds are asked to publish; a build answering no reference fails the key 'not stored:', and a step's own 'not stored:', 'not pinned:', 'version taken:' or 'refused kind:' opens the key's error. --- modules/mesh-delivery-builder/go.mod | 2 +- modules/mesh-delivery-builder/go.sum | 2 + .../internal/build/r4e_test.go | 76 +++++++++++++++++++ .../internal/build/run.go | 55 ++++++++++++-- .../internal/build/run_test.go | 11 ++- 5 files changed, 136 insertions(+), 10 deletions(-) create mode 100644 modules/mesh-delivery-builder/internal/build/r4e_test.go diff --git a/modules/mesh-delivery-builder/go.mod b/modules/mesh-delivery-builder/go.mod index 59b27dd..a96fd50 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.20261021192148-78364e318bdc + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261022154726-a90f523edc75 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 3515fb8..d5ce48e 100644 --- a/modules/mesh-delivery-builder/go.sum +++ b/modules/mesh-delivery-builder/go.sum @@ -1,5 +1,7 @@ git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021192148-78364e318bdc h1:ZJScJSnF6LgFl55/VJ9RT/lNroVWun469sXtx0nlVYw= git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021192148-78364e318bdc/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261022154726-a90f523edc75 h1:e/aOJLHaNSBPFFHHjUF+0fAfjmtMUy/vDWqetauWp/U= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261022154726-a90f523edc75/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/r4e_test.go b/modules/mesh-delivery-builder/internal/build/r4e_test.go new file mode 100644 index 0000000..6d77b9a --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/r4e_test.go @@ -0,0 +1,76 @@ +package build + +import ( + "context" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// R4e: references in the key (novox/hq ADR 0498), upstream copies, packages published from the trunk, the package wait +// (ADR 0500 clause 6) and one node per fingerprint (ADR 0478 clause 4). + +func TestACandidatesBundleKeyNamesItsReferenceAndTheTrunkBuildsItNoMore(t *testing.T) { // B1, ADR 0498 clause 3 and its check 3 + r := newRig(t, goBundle, archiveA) + trunkModule(t, r) + r.b.Candidate(context.Background(), "feat%2Fx", candidateValue(t, "feat/x", 7, headX)) + r.b.Wait() + v := valueOf(t, r.st, r.key) + if v.State != delivery.BuildDone { + t.Fatalf("build = %+v, want done", v) + } + if ok, missing := v.Referenced(); !ok { + t.Fatalf("a candidate's bundle and archive name no reference: %v (%+v)", missing, v.Artifacts) + } + rev := r.st.revs[r.key] + r.bm.mu.Lock() + r.bm.calls = nil + r.bm.mu.Unlock() + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) // the merge: the trunk now names the same fingerprint + r.b.Wait() + if n := len(r.bm.calls); n != 0 || r.st.revs[r.key] != rev { + t.Errorf("the trunk built its candidate's fingerprint again: %d calls, revision %d (was %d)", n, r.st.revs[r.key], rev) + } +} + +func TestABundleWhosePushAnsweredNoReferenceFailsNotStoredAndIsBuiltAgain(t *testing.T) { // B1, ADR 0498 clauses 1 and 2 + r := newRig(t, goBundle) + r.bm.noRef["ace/tool"] = true + v := r.consider(t, "modules/m") + if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, delivery.NotStored) || !strings.Contains(v.Error, "tool") { + t.Fatalf("build = %+v, want failed opening %q and naming the artifact", v, delivery.NotStored) + } + delete(r.bm.noRef, "ace/tool") + if v = r.consider(t, "modules/m"); v.State != delivery.BuildDone { + t.Errorf("build = %+v, want a not-stored key built again as a new revision", v) + } +} + +func TestAPushRefusedByTheStoreFailsTheKeyOpeningNotStored(t *testing.T) { // B1, ADR 0498 clause 2 + r := newRig(t, goBundle) + r.bm.fails["ace/build_go_build"] = delivery.NotStored + "tool: the artifact store answered 507 insufficient storage" + v := r.consider(t, "modules/m") + if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, delivery.NotStored) || !strings.Contains(v.Error, "507") || !strings.Contains(v.Error, "ace") { + t.Fatalf("build = %+v, want failed opening %q with the store's words and the node", v, delivery.NotStored) + } +} + +func TestOnlyATrunkKeysBuildsAreAskedToPublish(t *testing.T) { // B1, the lead's interface (a): publish only for a trunk key + r := newRig(t, goBundle) + r.consider(t, "modules/m") + for _, c := range r.bm.callsOf("build_go_build") { + if c.body.(map[string]any)["publish"] != true { + t.Errorf("trunk build args = %+v, want publish", c.body) + } + } + c := newRig(t, goBundle) + trunkModule(t, c) + c.b.Candidate(context.Background(), "feat%2Fx", candidateValue(t, "feat/x", 7, headX)) + c.b.Wait() + for _, call := range c.bm.callsOf("build_go_build") { + if _, ok := call.body.(map[string]any)["publish"]; ok { + t.Errorf("candidate build args = %+v, want no publish", call.body) + } + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index ea67174..1607a71 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -483,7 +483,7 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p } } if len(errs) > 0 { - return failed("%s", strings.Join(errs, "; ")) + return failed("%s", keyError(errs)) } var artifacts []delivery.BuiltArtifact keptModule := map[string]bool{} @@ -515,7 +515,16 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p } built := delivery.BuiltArtifact{Name: t.Artifact, Kind: t.Kind, Digest: d} for _, l := range lanes { - if r := l.references[t.Artifact]; r != "" && l.module == t.Module { + if l.module != t.Module { + continue + } + r := l.references[t.Artifact] + if r == "" && stored(t.Kind) { + // A bundle, an archive or an upstream copy is done only when its push was answered with where it is (ADR 0498 + // clause 1); a candidate's included (clause 3). + return failed("%s%s on %s: the build answered no reference, so its push to the artifact store was not answered", delivery.NotStored, t.Artifact, l.node) + } + if r != "" { built.Reference = r } } @@ -537,7 +546,40 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p if len(used) == 0 { used = nil } - return delivery.BuilderBuild{State: delivery.BuildDone, Step: step, Artifacts: artifacts, Toolchains: used} + done := delivery.BuilderBuild{State: delivery.BuildDone, By: b.Claimer.By, Manifest: m.Manifest, Step: step, Artifacts: artifacts, Toolchains: used} + if _, err := delivery.EncodeBuilderBuild(done); err != nil { + // A reference the build module answered that does not name the artifact and its digest is no reference. + return failed("%s%v", delivery.NotStored, err) + } + return done +} + +// stored is whether an artifact of this kind is in the artifact store once built, a candidate's too, so its done key +// names its reference (ADR 0498 clauses 1, 3 and 4); an image or a package names one only when built for the trunk. +func stored(kind string) bool { + return kind == catalogue.ArtifactBundle || kind == catalogue.ArtifactArchive || kind == catalogue.ArtifactUpstream +} + +// keyPrefixes are the errors a build module's step may open its reason with that the key's error must open with too, so +// the claim reads them (delivery.FailedUntilChanged), first the ones not built again. +var keyPrefixes = []string{delivery.NotPinned, delivery.VersionTaken, delivery.RefusedKind, delivery.NotStored} + +// keyError is the error of a key from its lanes' errors: when one names a prefix of keyPrefixes inside the step's words +// (` on : the step failed: not stored: ...`), the key's error opens with that prefix, so a reader of the +// key need not search for it. +func keyError(errs []string) string { + for _, prefix := range keyPrefixes { + for i, e := range errs { + if strings.HasPrefix(e, prefix) { + return strings.Join(errs, "; ") + } + if at := strings.Index(e, prefix); at >= 0 { + rest := append([]string{prefix + e[:at] + e[at+len(prefix):]}, append(append([]string(nil), errs[:i]...), errs[i+1:]...)...) + return strings.Join(rest, "; ") + } + } + } + return strings.Join(errs, "; ") } func (b *Builder) online(node string) bool { return b.Presence == nil || b.Presence.Online(node) } @@ -657,8 +699,9 @@ func (b *Builder) runLane(ctx context.Context, l *lane, repository string, m del } for _, t := range l.tasks { args := map[string]any{"workspace": fetched.Workspace, "module": m.Path, "artifact": t.Artifact} - if t.Kind == catalogue.ArtifactImage && !candidate { - // Only a trunk key pushes the image to the registry; a candidate's build never does (ADR 0487 clause 12). + if !candidate { + // Only a trunk key's build pushes an image or publishes a package; a candidate's never does (ADR 0487 clause 12, + // ADR 0498 clauses 5 and 6). A bundle or an archive is pushed either way (clause 3). args["publish"] = true } raw, err := b.step(ctx, l.module, l.node, "build", args) @@ -672,7 +715,7 @@ func (b *Builder) runLane(ctx context.Context, l *lane, repository string, m del return } l.digests[t.Artifact] = built.Digest - if built.Reference != "" && !candidate { + if built.Reference != "" && (stored(t.Kind) || !candidate) { l.references[t.Artifact] = built.Reference } } diff --git a/modules/mesh-delivery-builder/internal/build/run_test.go b/modules/mesh-delivery-builder/internal/build/run_test.go index 6da1ebe..940e4fe 100644 --- a/modules/mesh-delivery-builder/internal/build/run_test.go +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -23,6 +23,8 @@ type buildModules struct { steps *fakeSteps digests map[string]string // `/` -> digest; the default is `digest` fails map[string]string // `/` -> the reason the step fails with + // noRef names `/` whose bundle or archive build answers no reference (its push not answered). + noRef map[string]bool // hold, when set before the builds start, blocks every call until it is closed; entered is sent to as a call arrives. // conflicts, when set, is a file a fetch with a base reports in conflict; noMerge makes it answer neither a merge nor a conflict. conflicts string @@ -32,7 +34,7 @@ type buildModules struct { } func newBuildModules() *buildModules { - return &buildModules{steps: &fakeSteps{steps: map[string]delivery.Step{}}, digests: map[string]string{}, fails: map[string]string{}} + return &buildModules{steps: &fakeSteps{steps: map[string]delivery.Step{}}, digests: map[string]string{}, fails: map[string]string{}, noRef: map[string]bool{}} } func (b *buildModules) CallOn(node, key string, body any) (json.RawMessage, error) { @@ -69,6 +71,9 @@ func (b *buildModules) CallOn(node, key string, body any) (json.RawMessage, erro } if tool == "build_image_build" && args["publish"] == true { step.Result = json.RawMessage(`{"digest":"` + d + `","reference":"registry.example/m/img@` + d + `"}`) + } else if tool != "build_image_build" && !b.noRef[node+"/"+fmt.Sprint(args["artifact"])] { + // A bundle or an archive is pushed to the artifact store and its reference answered (ADR 0498 clause 1). + step.Result = json.RawMessage(`{"digest":"` + d + `","reference":"artifact-store://m/` + fmt.Sprint(args["artifact"]) + `/blobs/` + d + `"}`) } else { step.Result = json.RawMessage(`{"digest":"` + d + `"}`) } @@ -173,8 +178,8 @@ func TestABundleAndAnArchiveAreBuiltOnTwoNodesAndDoneWhenTheDigestsAgree(t *test t.Fatalf("build = %+v, want done by novox.builder with three artifacts", v) } for i, want := range []string{"files", "tool", "web"} { - if a := v.Artifacts[i]; a.Name != want || a.Digest != digest || a.Reference != "" { - t.Errorf("artifact %d = %+v, want %s with the digest and no reference (ADR 0478 clause 6)", i, a, want) + if a := v.Artifacts[i]; a.Name != want || a.Digest != digest || a.Reference != "artifact-store://m/"+want+"/blobs/"+digest { + t.Errorf("artifact %d = %+v, want %s with the digest and its artifact-store reference (ADR 0498 clause 1)", i, a, want) } } if v.Artifacts[0].Kind != "archive" || v.Artifacts[1].Kind != "bundle" {