From bbc690539b4c468e4889ff62b4080e300ed10106 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 10:19:26 +0200 Subject: [PATCH] Build an image on one node through mesh-build-image; only a trunk key publishes (issue 1705) --- .../mesh-delivery-builder/integration_test.go | 80 +++++++++++ .../catalogue-a67f0196-imaged.module.json | 14 ++ .../internal/build/claim.go | 8 +- .../internal/build/claim_test.go | 16 +++ .../internal/build/plan.go | 15 +- .../internal/build/plan_test.go | 16 ++- .../internal/build/run.go | 81 +++++++++-- .../internal/build/run_test.go | 129 +++++++++++++++++- .../mesh-delivery-builder/manifest_test.go | 6 +- modules/mesh-delivery-builder/module.json | 3 + 10 files changed, 346 insertions(+), 22 deletions(-) create mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-imaged.module.json diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go index 7affcfc..0236e64 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go @@ -57,6 +57,7 @@ type fakeBuildModules struct { mu sync.Mutex calls map[string]int // by `.` fetches []map[string]any + builds []map[string]any // the arguments of every build call } func (f *fakeBuildModules) count(tool string) int { @@ -88,6 +89,10 @@ func (f *fakeBuildModules) serve(t *testing.T, nc *nats.Conn, module string) { f.mu.Lock() f.fetches = append(f.fetches, args) f.mu.Unlock() + } else { + f.mu.Lock() + f.builds = append(f.builds, args) + f.mu.Unlock() } id := fmt.Sprintf("s%d", f.n.Add(1)) key, _ := delivery.StepKey(node, id) @@ -116,6 +121,9 @@ func (f *fakeBuildModules) serve(t *testing.T, nc *nats.Conn, module string) { step.State, step.Result = delivery.StepPassed, json.RawMessage(`{"workspace":"w`+id+`","commit":"`+commit+`"`+merge+`}`) case f.failBuild.Load() && node == "n1": step.State, step.Reason = delivery.StepFailed, fmt.Sprintf("go build of %v: boom", args["artifact"]) + case tool == "build_image_build" && args["publish"] == true: // a trunk image is pushed: the digest and the reference + d := digestOf(fmt.Sprint(args["artifact"])) + step.State, step.Result = delivery.StepPassed, json.RawMessage(`{"digest":"`+d+`","reference":"registry.example/imaged/server@`+d+`"}`) default: step.State, step.Result = delivery.StepPassed, json.RawMessage(`{"digest":"`+digestOf(fmt.Sprint(args["artifact"]))+`"}`) } @@ -218,6 +226,8 @@ func startBed(t *testing.T) *bed { b.fake = &fakeBuildModules{js: js, ctx: ctx, calls: map[string]int{}} b.fake.serve(t, ctl, "mesh-build-go") b.fake.serve(t, ctl, "mesh-build-node") + b.fake.serve(t, ctl, "mesh-build-image") + b.setHealth(t, "n1", "mesh-build-image", delivery.HealthOK) // the image build module runs on n1 only b.report(t, "mesh-build-go", "n1", "go", "go1.27.1", "1") b.report(t, "mesh-build-go", "n2", "go", "go1.27.1", "1") b.report(t, "mesh-build-node", "n1", "typescript", "tsc5", "1") @@ -582,3 +592,73 @@ func TestBuilderBuildsACandidatesFingerprintFromTheHeadMergedOntoTheTrunk(t *tes t.Errorf("fetches = %v, want one of the candidate's head with its ref and the trunk commit", b.fake.fetches) } } + +// TestAnImageIsBuiltOnOneNodeAndOnlyATrunkKeyPublishes: modules.imaged (one image artifact) is built on n1, where +// mesh-build-image runs: a trunk record's key is done with the digest and the reference, its build call carrying publish; a +// candidate's key for the same module is done with no reference, its build call carrying no publish (ADR 0487 clause 12). +func TestAnImageIsBuiltOnOneNodeAndOnlyATrunkKeyPublishes(t *testing.T) { // 1705 + b := startBed(t) + digest := b.moduleCommit(t, "imaged", "catalogue-a67f0196-imaged.module.json", "modules/imaged", nil, commit) + b.start(t, "n1", nil) + v := b.await(t, "imaged", isState(delivery.BuildDone)) + if len(v.Artifacts) != 1 || v.Artifacts[0].Digest != digestOf("server") || !strings.HasSuffix(v.Artifacts[0].Reference, "@"+digestOf("server")) { + t.Fatalf("trunk build = %+v, want the digest and the reference", v) + } + if got := b.fake.count("mesh-build-image.build_image_build"); got != 1 { + t.Errorf("build_image_build called %d times, want once on one node", got) + } + b.fake.mu.Lock() + if len(b.fake.builds) != 1 || b.fake.builds[0]["publish"] != true { + t.Errorf("builds = %v, want one carrying publish", b.fake.builds) + } + b.fake.builds = nil + b.fake.mu.Unlock() + + // A candidate: the trunk's record moves to another fingerprint, the candidate carries fp. + other := "src2:" + strings.Repeat("e", 64) + data, err := delivery.EncodeModule(delivery.Module{Repository: "novox/mesh-catalog", Commit: commit, FP: "src1:" + strings.Repeat("f", 64), FP2: other, + Manifest: digest, Path: "modules/imaged"}) + if err != nil { + t.Fatal(err) + } + if err := b.builds.Delete(b.ctx, mustKey(t, "imaged")); err != nil { + t.Fatal(err) + } + if _, err := b.modules.Put(b.ctx, "imaged", data); err != nil { + t.Fatal(err) + } + const head = "1111111111111111111111111111111111111111" + pulls := []delivery.CandidatePull{{Head: delivery.Head{Repository: "novox/mesh-catalog", Number: 7, Head: head}, Modules: []string{"imaged"}, + Fps: map[string]delivery.CandidateModule{"imaged": {FP2: fp, Manifest: digest}}}} + hash, _ := delivery.HeadsHash([]delivery.Head{pulls[0].Head}) + cand, err := delivery.EncodeCandidate(delivery.Candidate{V: delivery.Version, Group: "feat/x", HeadsHash: hash, Pulls: pulls, FPSet: delivery.FingerprintSet(pulls)}) + if err != nil { + t.Fatal(err) + } + ckey, _ := delivery.CandidatesKey("feat/x") + if _, err := b.candidates.Put(b.ctx, ckey, cand); err != nil { + t.Fatal(err) + } + v = b.await(t, "imaged", func(v delivery.BuilderBuild) bool { + return v.State == delivery.BuildDone && len(v.Artifacts) == 1 && v.By != "" + }) + // The trunk's new fingerprint is built too, and published; the candidate's is built and not. + for deadline := time.Now().Add(10 * time.Second); time.Now().Before(deadline) && b.fake.count("mesh-build-image.build_image_build") < 3; time.Sleep(50 * time.Millisecond) { + } + b.fake.mu.Lock() + defer b.fake.mu.Unlock() + if v.Artifacts[0].Reference != "" { + t.Errorf("candidate build = %+v, want no reference", v) + } + published, plain := 0, 0 + for _, a := range b.fake.builds { + if _, has := a["publish"]; has { + published++ + } else { + plain++ + } + } + if published != 1 || plain != 1 { + t.Errorf("builds = %v, want one trunk call with publish and one candidate call with none", b.fake.builds) + } +} diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-imaged.module.json b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-imaged.module.json new file mode 100644 index 0000000..eaf4606 --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-imaged.module.json @@ -0,0 +1,14 @@ +{ + "module": "imaged", + "version": "1", + "slug": "imaged", + "build": { + "artifacts": [ + { + "name": "server", + "kind": "image", + "from": "Dockerfile" + } + ] + } +} diff --git a/modules/mesh-delivery-builder/internal/build/claim.go b/modules/mesh-delivery-builder/internal/build/claim.go index 9ab0285..5acdf7d 100644 --- a/modules/mesh-delivery-builder/internal/build/claim.go +++ b/modules/mesh-delivery-builder/internal/build/claim.go @@ -63,6 +63,9 @@ type ClaimOptions struct { // qualifies now and the key may be built again (it is not held back after a failed second build, issue 1664). Nil means // none does. MoreNodes func(modules []string) bool + // ImageBuilder says whether a node runs a healthy image build module now: a key failed for NoImageBuilderPrefix is taken + // again only then. Nil means none does. + ImageBuilder func() bool } // Claim takes the build of module at fingerprint, or answers nil when it is not this builder's to build now: @@ -72,7 +75,8 @@ type ClaimOptions struct { // builder writes the new revision when the second build ends (ADR 0478 clause 4); // - 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 yet (RefusedPrefix, ADR 0478 clause 2) or for digests that differ (buildrun.DigestsDifferPrefix, clause 4), -// which wait for another fingerprint; +// which wait for another fingerprint, and one failed for want of an image builder (NoImageBuilderPrefix), which waits for +// another fingerprint or a node that runs the image build module (ClaimOptions.ImageBuilder); // - a key left queued or building by this builder is taken as it stands (ADR 0478 clause 5: a stopped builder takes its // own again when it starts), and so is one left by another builder when ClaimOptions.TakeOver (clause 6). func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string, opt ClaimOptions) (*Claim, error) { @@ -100,6 +104,8 @@ func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string return nil, nil case cur.State == delivery.BuildFailed && (strings.HasPrefix(cur.Error, RefusedPrefix) || strings.HasPrefix(cur.Error, buildrun.DigestsDifferPrefix)): return nil, nil + case cur.State == delivery.BuildFailed && strings.HasPrefix(cur.Error, NoImageBuilderPrefix) && (opt.ImageBuilder == nil || !opt.ImageBuilder()): + return nil, nil case cur.State == delivery.BuildFailed: return c.write(ctx, key, building, func(data []byte) (uint64, error) { return c.Store.Update(ctx, key, data, rev) }) case cur.By == c.By || opt.TakeOver: diff --git a/modules/mesh-delivery-builder/internal/build/claim_test.go b/modules/mesh-delivery-builder/internal/build/claim_test.go index a5b2cb9..cab71e3 100644 --- a/modules/mesh-delivery-builder/internal/build/claim_test.go +++ b/modules/mesh-delivery-builder/internal/build/claim_test.go @@ -155,6 +155,22 @@ func TestAFailedKeyIsTakenAgainAsANewRevisionExceptARefusedKind(t *testing.T) { } } +func TestAKeyFailedForNoImageBuilderIsTakenAgainOnlyWhenANodeHasTheModule(t *testing.T) { // 1705b + st := newMem() + key := seed(t, st, delivery.BuilderBuild{State: delivery.BuildFailed, By: "novox.a", Error: NoImageBuilderPrefix + " no node has mesh-build-image"}) + writes := st.writes + c := Claimer{Store: st, By: "novox.a"} + for _, opt := range []ClaimOptions{{}, {ImageBuilder: func() bool { return false }}} { + if got, err := c.Claim(context.Background(), "m", fp1, "", opt); err != nil || got != nil || st.writes != writes { + t.Errorf("claim = %v, %v, writes %d (was %d), want none and no write", got, err, st.writes, writes) + } + } + got, err := c.Claim(context.Background(), "m", fp1, hex64('a'), ClaimOptions{ImageBuilder: func() bool { return true }}) + if err != nil || got == nil || valueOf(t, st, key).State != delivery.BuildBuilding { + t.Errorf("claim = %v, %v, want a claim with the key building", got, err) + } +} + func TestFinishMovesTheKeyByCompareAndSetAndWritesOnlyADifferentValue(t *testing.T) { // 1607/AC3 st := newMem() c := Claimer{Store: st, By: "novox.a"} diff --git a/modules/mesh-delivery-builder/internal/build/plan.go b/modules/mesh-delivery-builder/internal/build/plan.go index edebb4e..dc1f6b7 100644 --- a/modules/mesh-delivery-builder/internal/build/plan.go +++ b/modules/mesh-delivery-builder/internal/build/plan.go @@ -15,6 +15,15 @@ import ( // a key is not built again as a new revision. const RefusedPrefix = "refused kind:" +// ImageBuildModule is the build module that builds an image artifact (novox/hq ADR 0487): buildah, rootless, on the nodes that +// run it. It has no toolchain language; the builder finds its nodes from the node-engine's have. +const ImageBuildModule = "mesh-build-image" + +// NoImageBuilderPrefix opens the error of a build key failed because no node runs ImageBuildModule (ADR 0487 clause 5): such a +// key is not built again as a new revision, only when a node reports the module (ClaimOptions.ImageBuilder) or the fingerprint +// changes. +const NoImageBuilderPrefix = "no image builder:" + // Task is one artifact to build: its name and kind in the manifest, the build module that builds it and the language // of the toolchain it is built with. type Task struct { @@ -45,7 +54,7 @@ func (p Plan) Modules() []string { // (the indexer gives an archive no language, so a fingerprint names no toolchain for it; ADR 0478 clause 1). func (p Plan) NeedsToolchain(module string) bool { for _, t := range p.Tasks { - if t.Module == module && t.Kind != catalogue.ArtifactArchive { + if t.Module == module && t.Kind != catalogue.ArtifactArchive && t.Kind != catalogue.ArtifactImage { return true } } @@ -61,7 +70,7 @@ func (r *Refused) Error() string { } // PlanOf is the plan of a manifest (ADR 0478 clauses 1 and 2): a Go bundle and an archive are built by the Go build -// module, a TypeScript bundle by the Node one, each in the order of the artifacts' names. A manifest with an image, +// module, an image by the image build module (ADR 0487), a TypeScript bundle by the Node one, each in the order of the artifacts' names. A manifest with an // upstream or package artifact, or a bundle of another language, is refused as a whole, naming the first such artifact // by name. A manifest with no artifact has an empty plan. func PlanOf(m catalogue.Manifest) (Plan, error) { @@ -79,6 +88,8 @@ func PlanOf(m catalogue.Manifest) (Plan, error) { p.Tasks = append(p.Tasks, Task{Artifact: a.Name, Kind: a.Kind, Module: buildrun.GoBuildModule, Language: "go"}) case a.Kind == catalogue.ArtifactBundle && a.Language == "typescript": p.Tasks = append(p.Tasks, Task{Artifact: a.Name, Kind: a.Kind, Module: buildrun.NodeBuildModule, Language: "typescript"}) + case a.Kind == catalogue.ArtifactImage: + p.Tasks = append(p.Tasks, Task{Artifact: a.Name, Kind: a.Kind, Module: ImageBuildModule}) case a.Kind == catalogue.ArtifactBundle: return Plan{}, &Refused{Artifact: a.Name, What: fmt.Sprintf("bundle of language %q", a.Language)} default: diff --git a/modules/mesh-delivery-builder/internal/build/plan_test.go b/modules/mesh-delivery-builder/internal/build/plan_test.go index 3760917..d6b18b2 100644 --- a/modules/mesh-delivery-builder/internal/build/plan_test.go +++ b/modules/mesh-delivery-builder/internal/build/plan_test.go @@ -41,12 +41,11 @@ func TestPlanRoutesEachKindToItsBuildModule(t *testing.T) { // 1607/AC2 } } -func TestPlanRefusesImageUpstreamAndPackageByName(t *testing.T) { // 1607/AC2, ADR 0478 clause 2 +func TestPlanRefusesUpstreamAndPackageByName(t *testing.T) { // 1607/AC2, ADR 0478 clause 2 for _, c := range []struct { artifact catalogue.Artifact kind string }{ - {catalogue.Artifact{Name: "img", Kind: "image"}, "image"}, {catalogue.Artifact{Name: "up", Kind: "upstream"}, "upstream"}, {catalogue.Artifact{Name: "pkg", Kind: "package", Language: "typescript"}, "package"}, } { @@ -74,3 +73,16 @@ func TestPlanRefusesABundleOfAnotherLanguageAndAModuleWithNoArtifact(t *testing. } func asRefused(err error, target **Refused) bool { return errors.As(err, target) } + +func TestPlanSendsAnImageToMeshBuildImageWithNoToolchain(t *testing.T) { // 1705a, ADR 0487 clause 5 + p, err := PlanOf(manifestOf(catalogue.Artifact{Name: "img", Kind: "image"}, catalogue.Artifact{Name: "files", Kind: "archive"})) + if err != nil || len(p.Tasks) != 2 { + t.Fatalf("plan %+v, error %v", p, err) + } + if p.Tasks[1].Artifact != "img" || p.Tasks[1].Module != ImageBuildModule { + t.Errorf("task = %+v, want the image on %s", p.Tasks[1], ImageBuildModule) + } + if p.NeedsToolchain(ImageBuildModule) { + t.Error("an image needs no toolchain") + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index 5f7782d..1c4cbf4 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -10,6 +10,7 @@ import ( "strings" "sync" + "git.novox.be/novox/mesh-sdk/go/catalogue" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -19,7 +20,7 @@ type ToolchainEntries interface { } // toolPrefix is the start of each build module's tool names: fetch and build. -var toolPrefix = map[string]string{buildrun.GoBuildModule: "build_go_", buildrun.NodeBuildModule: "build_node_"} +var toolPrefix = map[string]string{buildrun.GoBuildModule: "build_go_", buildrun.NodeBuildModule: "build_node_", ImageBuildModule: "build_image_"} // Builder builds, dark, every new-form fingerprint the indexer writes (novox/hq ADR 0453 clause 4, ADR 0478): it is // handed each modules value as the watch of the indexer's bucket sees it, and for each module with a fingerprint and an @@ -296,7 +297,7 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { // A second build that failed is not tried again until the nodes change (issue 1664). return (!failedBefore || epoch > failedAt) && b.moreNodes(modules) } - cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest, ClaimOptions{TakeOver: first, MoreNodes: moreNodes}) + cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest, ClaimOptions{TakeOver: first, MoreNodes: moreNodes, ImageBuilder: b.imageBuilderRuns}) if err != nil { b.logf("%s: %v", module, err) return @@ -360,6 +361,7 @@ type lane struct { module, node string tasks []Task digests map[string]string // artifact -> digest + references map[string]string // artifact -> the reference an image was pushed to (a trunk build only) err error } @@ -391,7 +393,17 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p entries := b.Toolchains.Entries(language) qualifies := func(node string) bool { return b.qualifies(node, bm) } var nodes []string - if plan.NeedsToolchain(bm) { + if bm == ImageBuildModule { + // An image is built on one node, chosen from the nodes whose image build module runs healthy (ADR 0487 clause 8): + // the module has no toolchain language, so no toolchain entry names its nodes, and a second build would only + // push the image again. + ns, err := buildrun.ChooseAnyNodes("image", b.imageEntries(), b.Places, qualifies, 1) + if err != nil { + giveUpLanes() + return failed("%s no node has %s, so the image cannot be built: the provision rootless-image-build is held by the module buildah; assign buildah and %s to a node (%v)", NoImageBuilderPrefix, ImageBuildModule, ImageBuildModule, err) + } + nodes = ns + } else if plan.NeedsToolchain(bm) { ns, tc, err := buildrun.ChooseNodes(language, entries, b.Places, qualifies, b.Nodes) if err != nil { giveUpLanes() @@ -413,7 +425,9 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p } nodes = ns } - if len(nodes) == 1 { + if bm == ImageBuildModule { + steps = append(steps, fmt.Sprintf("%s on %s", bm, nodes[0])) + } else if len(nodes) == 1 { oneNode = append(oneNode, bm) steps = append(steps, fmt.Sprintf("%s on one node, %s", bm, nodes[0])) } else { @@ -428,7 +442,7 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p } } for _, n := range nodes { - lanes = append(lanes, &lane{module: bm, node: n, tasks: tasks, digests: map[string]string{}}) + lanes = append(lanes, &lane{module: bm, node: n, tasks: tasks, digests: map[string]string{}, references: map[string]string{}}) } } var wg sync.WaitGroup @@ -458,7 +472,13 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p if err != nil { return failed("%s", err) } - artifacts = append(artifacts, delivery.BuiltArtifact{Name: t.Artifact, Kind: t.Kind, Digest: d}) + 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 { + built.Reference = r + } + } + artifacts = append(artifacts, built) } step := "built " + strings.Join(steps, "; ") if len(oneNode) > 0 { @@ -478,6 +498,39 @@ func (b *Builder) qualifies(node, bm string) bool { return b.Places.CapacityOf(node) > 0 && b.online(node) && (b.Health == nil || b.Health.Healthy(node, bm)) } +// imageEntries are the nodes an image may be built on, as toolchain entries for the chooser: every node the builder has a +// capacity for or a build module toolchain from. Whether the image build module runs there is qualifies', from the have. +func (b *Builder) imageEntries() []delivery.NodeToolchain { + seen := map[string]bool{} + var entries []delivery.NodeToolchain + add := func(n string) { + if !seen[n] { + seen[n] = true + entries = append(entries, delivery.NodeToolchain{Node: n}) + } + } + for n := range b.Places.Capacity { + add(n) + } + for _, language := range []string{"go", "typescript"} { + for _, e := range b.Toolchains.Entries(language) { + add(e.Node) + } + } + sort.Slice(entries, func(i, j int) bool { return entries[i].Node < entries[j].Node }) + return entries +} + +// imageBuilderRuns is whether a node can build an image now. +func (b *Builder) imageBuilderRuns() bool { + for _, e := range b.imageEntries() { + if b.qualifies(e.Node, ImageBuildModule) { + return true + } + } + return false +} + // moreNodes is whether any of these build modules has two nodes that qualify now. func (b *Builder) moreNodes(modules []string) bool { for _, bm := range modules { @@ -534,7 +587,8 @@ func (b *Builder) runLane(ctx context.Context, l *lane, repository string, m del } // A candidate is built from its head merged onto the trunk. A head that does not merge ends the fetch passed with the tree at // the trunk, and building that would store the trunk's build under the candidate's fingerprint. - if _, candidate := fetchArgs["base"]; candidate { + _, candidate := fetchArgs["base"] + if candidate { if len(fetched.Conflicts) > 0 { l.err = fmt.Errorf("the head does not merge into the trunk on %s: files in conflict: %s", l.node, strings.Join(fetched.Conflicts, ", ")) return @@ -545,18 +599,27 @@ func (b *Builder) runLane(ctx context.Context, l *lane, repository string, m del } } for _, t := range l.tasks { - raw, err := b.step(ctx, l.module, l.node, "build", map[string]any{"workspace": fetched.Workspace, "module": m.Path, "artifact": t.Artifact}) + 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). + args["publish"] = true + } + raw, err := b.step(ctx, l.module, l.node, "build", args) if err != nil { l.err = err return } var built struct { - Digest string `json:"digest"` + Digest string `json:"digest"` + Reference string `json:"reference"` } if err := json.Unmarshal(raw, &built); err != nil || built.Digest == "" { l.err = fmt.Errorf("the build of %s of %s on %s answered no digest: %s", t.Artifact, module, l.node, raw) return } l.digests[t.Artifact] = built.Digest + if built.Reference != "" && !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 73812c5..489f174 100644 --- a/modules/mesh-delivery-builder/internal/build/run_test.go +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -67,7 +67,11 @@ func (b *buildModules) CallOn(node, key string, body any) (json.RawMessage, erro if v, ok := b.digests[node+"/"+fmt.Sprint(args["artifact"])]; ok { d = v } - step.Result = json.RawMessage(`{"digest":"` + d + `"}`) + if tool == "build_image_build" && args["publish"] == true { + step.Result = json.RawMessage(`{"digest":"` + d + `","reference":"registry.example/m/img@` + d + `"}`) + } else { + step.Result = json.RawMessage(`{"digest":"` + d + `"}`) + } } sk, _ := delivery.StepKey(node, id) b.steps.mu.Lock() @@ -238,11 +242,126 @@ func TestAFailingStepFailsTheBuildWithItsReasonAndNodeAndIsTriedAgainAsANewRevis } } -func TestAModuleWithAnImageGetsARefusedKeyNeverBuiltAgain(t *testing.T) { // 1607/AC2, ADR 0478 clause 2 - r := newRig(t, archiveA, imageArt) +// imageHave marks mesh-build-image running and healthy on these nodes. +func imageHave(t *testing.T, nodes ...string) *buildrun.HaveSet { + t.Helper() + have := &buildrun.HaveSet{} + for _, n := range nodes { + have.Put(n+".mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) + have.Put(n+"."+ImageBuildModule, haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) + } + return have +} + +func TestAnImageGoesToOneNodeAndGetsOneDoneKeyWithItsReference(t *testing.T) { // 1705a, ADR 0487 clause 5 + r := newRig(t, imageArt) + r.b.Health = imageHave(t, "ace", "novox") // two nodes qualify; an image still builds on one v := r.consider(t, "modules/m") - if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, RefusedPrefix) || !strings.Contains(v.Error, "image") || !strings.Contains(v.Error, "build-agent") { - t.Fatalf("build = %+v, want failed 'refused kind: image' naming build-agent", v) + if v.State != delivery.BuildDone || len(v.Artifacts) != 1 || v.Artifacts[0].Kind != "image" || v.Artifacts[0].Digest != digest { + t.Fatalf("build = %+v, want done with one image artifact and its digest", v) + } + if v.Artifacts[0].Reference == "" { + t.Errorf("a trunk image has no reference: %+v", v.Artifacts[0]) + } + if strings.HasPrefix(v.Step, OneNodePrefix) || len(OneNodeModules(v)) != 0 { + t.Errorf("step = %q, want no one-node mark: an image is never built again on a second node", v.Step) + } + if n := len(r.bm.callsOf("build_image_fetch")); n != 1 { + t.Errorf("%d image fetches, want one", n) + } + writes := r.st.writes + r.consider(t, "modules/m") + if r.st.writes != writes || len(r.bm.callsOf("build_image_build")) != 1 { + t.Errorf("a done image key was built again") + } +} + +func TestAnImageIsBuiltOnlyWhereMeshBuildImageIsHealthy(t *testing.T) { // 1705a + r := newRig(t, imageArt) + r.b.Health = imageHave(t, "novox") + r.consider(t, "modules/m") + c := r.bm.callsOf("build_image_build") + if len(c) != 1 || c[0].node != "novox" { + t.Errorf("calls = %+v, want one on novox", c) + } +} + +func TestABundleAndAnImageOfOneModuleBuildOnTwoNodesAndOne(t *testing.T) { // 1705a + r := newRig(t, goBundle, imageArt) + r.b.Health = imageHave(t, "ace", "novox") + v := r.consider(t, "modules/m") + if v.State != delivery.BuildDone || len(v.Artifacts) != 2 { + t.Fatalf("build = %+v", v) + } + if n := len(r.bm.callsOf("build_go_build")); n != 2 { + t.Errorf("%d go builds, want 2", n) + } + if n := len(r.bm.callsOf("build_image_build")); n != 1 { + t.Errorf("%d image builds, want 1", n) + } + if modules := OneNodeModules(v); len(modules) != 0 { + t.Errorf("one-node modules = %v, want none", modules) + } +} + +func TestWithNoImageNodeTheKeyFailsNamingTheProvisionAndIsNotRetakenUntilANodeReportsTheModule(t *testing.T) { // 1705b, lead's decision + r := newRig(t, imageArt) + r.b.Health = imageHave(t) // mesh-build-image runs nowhere + v := r.consider(t, "modules/m") + if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, NoImageBuilderPrefix) || !strings.Contains(v.Error, "rootless-image-build") || !strings.Contains(v.Error, "buildah") || !strings.Contains(v.Error, ImageBuildModule) { + t.Fatalf("build = %+v, want failed naming rootless-image-build, buildah and mesh-build-image", v) + } + if len(r.bm.calls) != 0 { + t.Errorf("%d calls, want none", len(r.bm.calls)) + } + writes := r.st.writes + r.consider(t, "modules/m") + r.consider(t, "modules/m") + if r.st.writes != writes { + t.Errorf("the failed key was written again (%d writes, was %d)", r.st.writes, writes) + } + // A node reports the module: the key is taken again. + r.b.Health = imageHave(t, "ace") + v = r.consider(t, "modules/m") + if v.State != delivery.BuildDone { + t.Errorf("build = %+v, want done once a node reports mesh-build-image", v) + } +} + +func TestACandidatesImageCallCarriesNoPublishAndItsKeyNoReference(t *testing.T) { // 1705b, lead's decision + r := newRig(t, imageArt) + r.b.Health = imageHave(t, "ace") + 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 || len(v.Artifacts) != 1 || v.Artifacts[0].Reference != "" { + t.Fatalf("build = %+v, want done with no reference", v) + } + calls := r.bm.callsOf("build_image_build") + if len(calls) != 1 { + t.Fatalf("%d image builds, want 1", len(calls)) + } + if _, has := calls[0].body.(map[string]any)["publish"]; has { + t.Errorf("a candidate's image call carries publish: %+v", calls[0].body) + } +} + +func TestATrunkImageCallCarriesPublish(t *testing.T) { // 1705b + r := newRig(t, imageArt) + r.b.Health = imageHave(t, "ace") + r.consider(t, "modules/m") + calls := r.bm.callsOf("build_image_build") + if len(calls) != 1 || calls[0].body.(map[string]any)["publish"] != true { + t.Errorf("calls = %+v, want publish true", calls) + } +} + +func TestAnUpstreamArtifactIsStillRefusedKeyNeverBuiltAgain(t *testing.T) { // 1607/AC2, ADR 0478 clause 2 + r := newRig(t, archiveA, catalogue.Artifact{Name: "up", Kind: "upstream"}) + v := r.consider(t, "modules/m") + if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, RefusedPrefix) || !strings.Contains(v.Error, "upstream") || !strings.Contains(v.Error, "build-agent") { + t.Fatalf("build = %+v, want failed 'refused kind: upstream' naming build-agent", v) } if len(r.bm.calls) != 0 { t.Errorf("%d calls, want none for a refused kind", len(r.bm.calls)) diff --git a/modules/mesh-delivery-builder/manifest_test.go b/modules/mesh-delivery-builder/manifest_test.go index 70f4ca4..28d9afa 100644 --- a/modules/mesh-delivery-builder/manifest_test.go +++ b/modules/mesh-delivery-builder/manifest_test.go @@ -86,12 +86,12 @@ func TestManifestDeclarations(t *testing.T) { // 1607/AC1 if want := []state{{Name: "builds", History: 5}}; !reflect.DeepEqual(m.State, want) { // delivery.BuilderBuildsHistory t.Errorf("state = %+v, want %+v", m.State, want) } - if want := []string{"mesh-build-go.steps", "mesh-build-go.toolchains", "mesh-build-node.steps", "mesh-build-node.toolchains", + if want := []string{"mesh-build-go.steps", "mesh-build-go.toolchains", "mesh-build-image.steps", "mesh-build-node.steps", "mesh-build-node.toolchains", "mesh-delivery-indexer.candidates", "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules", "mesh-delivery-watchman.presence", "node-engine.have"}; !reflect.DeepEqual(sorted(m.Reads), want) { t.Errorf("reads = %v, want %v", m.Reads, want) } - if want := []string{"mesh-build-go.build_go_build", "mesh-build-go.build_go_fetch", "mesh-build-node.build_node_build", "mesh-build-node.build_node_fetch"}; !reflect.DeepEqual(sorted(m.Invokes), want) { - t.Errorf("invokes = %v, want only the build modules' fetch and build", m.Invokes) + if want := []string{"mesh-build-go.build_go_build", "mesh-build-go.build_go_fetch", "mesh-build-image.build_image_build", "mesh-build-image.build_image_fetch", "mesh-build-node.build_node_build", "mesh-build-node.build_node_fetch"}; !reflect.DeepEqual(sorted(m.Invokes), want) { + t.Errorf("invokes = %v, want only the build modules' fetch and build (go, image, node)", m.Invokes) } // 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) { diff --git a/modules/mesh-delivery-builder/module.json b/modules/mesh-delivery-builder/module.json index cb976f3..6d36190 100644 --- a/modules/mesh-delivery-builder/module.json +++ b/modules/mesh-delivery-builder/module.json @@ -12,6 +12,7 @@ "reads": [ "mesh-build-go.steps", "mesh-build-go.toolchains", + "mesh-build-image.steps", "mesh-build-node.steps", "mesh-build-node.toolchains", "mesh-delivery-indexer.candidates", @@ -23,6 +24,8 @@ "invokes": [ "mesh-build-go.build_go_build", "mesh-build-go.build_go_fetch", + "mesh-build-image.build_image_build", + "mesh-build-image.build_image_fetch", "mesh-build-node.build_node_build", "mesh-build-node.build_node_fetch" ],