From 0f7091e4ac7e81ed8635dfa3d0e1f21711e2dce2 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 15:35:20 +0200 Subject: [PATCH 1/5] =?UTF-8?q?Place=20the=20change's=20own=20repository?= =?UTF-8?q?=20beside=20it=20at=20its=20trunk,=20so=20a=20shrink-only=20lis?= =?UTF-8?q?t=20test=20compares=20with=20the=20trunk's=20copy=20at=20$MESH?= =?UTF-8?q?=5FCHECK=5FBESIDE/=20(issue=201887=20A12,=20ADR=200529=20?= =?UTF-8?q?=C2=A72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../internal/check/beside.go | 7 ++++- .../internal/check/examiner_test.go | 19 +++++++++++++- .../internal/check/judged_test.go | 2 +- .../internal/check/pins_test.go | 26 +++++++++++++++++++ 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/beside.go b/modules/mesh-delivery-checker/internal/check/beside.go index 32ba99a..06bc799 100644 --- a/modules/mesh-delivery-checker/internal/check/beside.go +++ b/modules/mesh-delivery-checker/internal/check/beside.go @@ -35,13 +35,18 @@ var besideRules = map[string][]string{ // besideFor are the siblings a pull request's tests read (ADR 0484 clause 4): the catalogue at its trunk, always, even in a // group holding a catalogue pull request; the node-engine at the commit the mesh runs (the commit of the indexer's module // mesh-host, its trunk head when none is known); mesh-sdk, for the controller's tests, at the commit the controller's go.mod -// pins; any other sibling at the head of the group's pull request for it when the group holds one, else at its trunk. +// pins; any other sibling at the head of the group's pull request for it when the group holds one, else at its trunk. And +// first, the change's own repository at the trunk it is merged onto, so a test comparing a shrink-only list with the +// trunk's copy reads it at $MESH_CHECK_BESIDE/ (issue 1887 A12, ADR 0529 §2). func (j *Examiner) besideFor(ctx context.Context, c *Case, p Pull) ([]BesideRef, error) { var heads map[string]string if j.Trunks != nil { heads = j.Trunks.Heads() } var out []BesideRef + if p.Trunk != "" { + out = append(out, BesideRef{Repository: p.Repository, Commit: p.Trunk}) + } for _, repo := range besideRules[p.Repository] { switch { case repo == catalogueRepository: diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index 1c4ee01..88ce8fb 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -137,7 +137,7 @@ func newBench(t testing.TB) *bench { b.steps.handlers = map[string]func(stepCall) (any, error){ "build_go_fetch": func(stepCall) (any, error) { return b.fetch, nil }, "build_node_fetch": func(stepCall) (any, error) { return b.fetch, nil }, - "build_go_digest": func(c stepCall) (any, error) { return b.digest[c.args["level"].(string)], nil }, + "build_go_digest": func(c stepCall) (any, error) { return echoBeside(b, b.digest[c.args["level"].(string)]), nil }, "build_go_test": func(c stepCall) (any, error) { return b.test(c) }, "build_node_test": func(c stepCall) (any, error) { return b.test(c) }, } @@ -571,6 +571,8 @@ func trunksOf(t *testing.T, heads map[string]string) *Trunks { func TestNothingIsKeptForAWorkspaceGivenSiblingsWhenTheDigestDoesNotSayItCoversThem(t *testing.T) { // review D1, ADR 0444, issue 1287 b := newBench(t) + // A build module whose digest does not echo the siblings (before DigestVersion 3). + b.steps.handlers["build_go_digest"] = func(c stepCall) (any, error) { return b.digest[c.args["level"].(string)], nil } b.ex.Trunks = trunksOf(t, map[string]string{repoSDK: trunkS, "novox/mesh-catalog": trunkA}) b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) if got := len(b.steps.tool("fetch")); got != 1 || b.steps.tool("fetch")[0].args["beside"] == nil { @@ -592,6 +594,21 @@ func TestNothingIsKeptForAWorkspaceGivenSiblingsWhenTheDigestDoesNotSayItCoversT // besideDigest stands for the Go build module's digest over a workspace with siblings (DigestVersion 3, lead decision L5): // each package's digest folds the siblings' commits in, and the answer echoes the lines it folded. +// echoBeside is d with the siblings the fetches placed, as the Go build module's digest echoes them (its DigestResult's +// Beside): every check now places the change's own repository at its trunk (issue 1887 A12). +func echoBeside(b *bench, d Digested) Digested { + var lines []string + for _, f := range b.steps.tool("fetch") { + refs, _ := f.args["beside"].([]BesideRef) + for _, r := range refs { + lines = append(lines, r.Repository+" "+r.Commit) + } + } + sort.Strings(lines) + d.Beside = lines + return d +} + func besideDigest(b *bench) func(c stepCall) (any, error) { return func(c stepCall) (any, error) { level := c.args["level"].(string) diff --git a/modules/mesh-delivery-checker/internal/check/judged_test.go b/modules/mesh-delivery-checker/internal/check/judged_test.go index f90febd..e915b39 100644 --- a/modules/mesh-delivery-checker/internal/check/judged_test.go +++ b/modules/mesh-delivery-checker/internal/check/judged_test.go @@ -30,7 +30,7 @@ func candidateBench(t *testing.T, takesRunner bool) *bench { return "installed" } b.steps.handlers["build_go_digest"] = func(c stepCall) (any, error) { - d := b.digest[c.args["level"].(string)] + d := echoBeside(b, b.digest[c.args["level"].(string)]) d.JudgedBy = judged(c) return d, nil } diff --git a/modules/mesh-delivery-checker/internal/check/pins_test.go b/modules/mesh-delivery-checker/internal/check/pins_test.go index 703a11b..dabdfe3 100644 --- a/modules/mesh-delivery-checker/internal/check/pins_test.go +++ b/modules/mesh-delivery-checker/internal/check/pins_test.go @@ -265,3 +265,29 @@ func TestAForgeThatIsNotHeardFailsAtTheForgeAndIsJudgedAgainWhenTheNodesChange(t t.Errorf("a forge that was not heard is not the change's failure: %+v is not judged again when the nodes change", v.Failed) } } + +// 1887/A12: the change's own repository is placed beside it at the trunk it is merged onto, for every repository, so a +// shrink-only list test reads the trunk's copy at $MESH_CHECK_BESIDE/. +func TestTheChangesOwnRepositoryIsBesideItAtItsTrunk(t *testing.T) { + b := newBench(t) + b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) + got := besideOf(t, b, repoA) + if own := got[repoA]; own.Commit != trunkA || own.Ref != "" { + t.Fatalf("the repository beside its own change = %+v, want its trunk %s", own, trunkA) + } +} + +// 1887/A12: a catalogue change's siblings hold the catalogue itself at the trunk it is merged onto, beside mesh-sdk and +// mesh-controller, so the catalogue's shrink-only list tests read $MESH_CHECK_BESIDE/mesh-catalog. +func TestACatalogueChangeHasTheCatalogueBesideItAtItsTrunk(t *testing.T) { + b := newBench(t) + b.ex.Trunks = trunksOf(t, map[string]string{catalogueRepository: trunkA, SDKRepository: trunkS, controllerRepo: trunkA2}) + b.judge(t, caseOf(t, pullOf(catalogueRepository, 12, head1))) + got := besideOf(t, b, catalogueRepository) + if c := got[catalogueRepository]; c.Commit != trunkA || c.Ref != "" { + t.Fatalf("the catalogue beside its own change = %+v, want its trunk %s", c, trunkA) + } + if _, ok := got[SDKRepository]; !ok { + t.Errorf("mesh-sdk is no longer beside a catalogue change: %v", got) + } +} From 4c98841d2ab3c2546f5779cd0e89638c3f3f909f Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 15:41:57 +0200 Subject: [PATCH 2/5] Judge a repository the mesh knows no module of with both build modules, so mesh-lab's nested Go modules and its npm test both run (issue 1887 A12) --- .../cmd/mesh-delivery-checker/judge.go | 15 +++++++++++-- .../cmd/mesh-delivery-checker/judge_test.go | 6 +++-- .../internal/check/pins_test.go | 22 +++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go index ed8ff96..7db6c2b 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -142,17 +142,28 @@ func (r *repoModules) PackagePaths(repository string) []string { return out } -// BuildModules are the build modules that judge a repository, sorted. +// BuildModules are the build modules that judge a repository, sorted: the Go build module always, and the Node build +// module when a module of the repository builds TypeScript. A repository of which the mesh knows no module (mesh-lab, hq) +// is judged by both, each finding its own directories in the tree (go.mod files; package.json files), so a repository +// with tests in either language and no manifest is judged whole (issue 1887 A12). func (r *repoModules) BuildModules(repository string) []string { out := []string{buildrun.GoBuildModule} r.mu.Lock() defer r.mu.Unlock() + known := false for module, repo := range r.repoOf { - if repo == repository && r.langs[r.typed[module]]["typescript"] { + if repo != repository { + continue + } + known = true + if r.langs[r.typed[module]]["typescript"] { out = append(out, buildrun.NodeBuildModule) break } } + if !known { + out = append(out, buildrun.NodeBuildModule) + } sort.Strings(out) return out } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go index 8afa3c1..126b348 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge_test.go @@ -69,8 +69,10 @@ func TestARepositoryWithATypeScriptModuleIsJudgedByTheNodeBuildModuleToo(t *test if got := r.CommitOf("mesh-delivery-checker"); got != strings.Repeat("a", 40) || r.CommitOf("gone") != "" { t.Errorf("the commit the mesh runs = %q", got) } - if got := r.BuildModules("novox/mesh-sdk"); strings.Join(got, ",") != buildrun.GoBuildModule { - t.Errorf("a repository with no module: %v, want the Go build module", got) + // 1887/A12: a repository with no module the mesh knows (mesh-lab: nested go.mod files and a root package.json) is + // judged by both build modules. + if got := r.BuildModules("novox/mesh-lab"); strings.Join(got, ",") != buildrun.GoBuildModule+","+buildrun.NodeBuildModule { + t.Errorf("a repository with no module: %v, want both build modules", got) } } diff --git a/modules/mesh-delivery-checker/internal/check/pins_test.go b/modules/mesh-delivery-checker/internal/check/pins_test.go index dabdfe3..9d0c6bb 100644 --- a/modules/mesh-delivery-checker/internal/check/pins_test.go +++ b/modules/mesh-delivery-checker/internal/check/pins_test.go @@ -291,3 +291,25 @@ func TestACatalogueChangeHasTheCatalogueBesideItAtItsTrunk(t *testing.T) { t.Errorf("mesh-sdk is no longer beside a catalogue change: %v", got) } } + +// 1887/A12: a mesh-lab head gets a check: with its trunk known it is judged like any repository, its Go fetch finding +// the nested modules itself. +func TestAMeshLabHeadGetsACheck(t *testing.T) { + const lab = "novox/mesh-lab" + raw := candidateOf(t, "fix/x", pullOf(lab, 3, head1)) + c, err := delivery.DecodeCandidate(raw) + if err != nil { + t.Fatal(err) + } + cs := CaseOf(c, map[string]string{lab: trunkA}) + if cs == nil || len(cs.Pulls) != 1 || cs.Pulls[0].Repository != lab || len(cs.Left) != 0 { + t.Fatalf("the case = %+v, want mesh-lab#3 judged", cs) + } + b := newBench(t) + if v := b.judge(t, cs).Verdict; v.State != delivery.VerdictPass { + t.Fatalf("verdict = %+v", v) + } + if fetched := b.steps.tool("fetch"); len(fetched) == 0 || fetched[0].args["repository"] != lab { + t.Fatalf("mesh-lab was not fetched: %+v", fetched) + } +} From 9f27e2b1f8aaa0da7aa44ea0f4036ef77ddff76f Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 15:57:03 +0200 Subject: [PATCH 3/5] =?UTF-8?q?Build=20each=20build=20module's=20part=20of?= =?UTF-8?q?=20a=20fingerprint=20as=20one=20ask=20on=20its=20seat's=20strea?= =?UTF-8?q?m,=20named=20in=20the=20key=20before=20it=20is=20put,=20so=20th?= =?UTF-8?q?e=20builder=20chooses=20no=20node,=20holds=20no=20capacity=20an?= =?UTF-8?q?d=20never=20asks=20twice=20(issue=201887=20A4a,=20A4b,=20A4c,?= =?UTF-8?q?=20ADR=200525=20=C2=A71-=C2=A76)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery-builder/bus.go | 37 --- .../mesh-delivery-builder/integration_test.go | 214 ++++++------ .../cmd/mesh-delivery-builder/main.go | 73 +--- .../cmd/mesh-delivery-builder/settings.go | 28 -- .../mesh-delivery-builder/settings_test.go | 67 ---- .../mesh-delivery-builder/files/settings.json | 3 - .../internal/build/fixtures_test.go | 40 ++- .../internal/build/r4e_test.go | 6 +- .../internal/build/round4_test.go | 130 -------- .../internal/build/run.go | 313 ++++++++---------- .../internal/build/run_test.go | 183 +++++----- .../mesh-delivery-builder/manifest_test.go | 40 +-- modules/mesh-delivery-builder/module.json | 47 +-- 13 files changed, 379 insertions(+), 802 deletions(-) delete mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/settings.go delete mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/settings_test.go delete mode 100644 modules/mesh-delivery-builder/files/settings.json diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go index b456fbd..359b1e9 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go @@ -102,43 +102,6 @@ func follow(ctx context.Context, w jetstream.KeyWatcher, each func(jetstream.Key } } -// stepsWaiter waits for a build module's step by watching its key in the module's steps bucket (ADR 0469 clause 2): the -// watch hands over the value the key holds now, so a step that ended before the watch began is read at once, and every -// change after. No timer: the caller bounds the wait with its context. -type stepsWaiter struct{ js bucketOpener } - -func (w stepsWaiter) Wait(ctx context.Context, module, key string) (delivery.Step, error) { - kv, err := w.js.KeyValue(ctx, delivery.StepsBucket(module)) - if err != nil { - return delivery.Step{}, fmt.Errorf("the bucket %s: %w", delivery.StepsBucket(module), err) - } - watch, err := kv.Watch(ctx, key) - if err != nil { - return delivery.Step{}, err - } - defer watch.Stop() - for { - select { - case <-ctx.Done(): - return delivery.Step{}, ctx.Err() - case e, ok := <-watch.Updates(): - if !ok { - return delivery.Step{}, errors.New("the watch of the step ended") - } - if e == nil || !opOf(e) { - continue - } - step, err := delivery.DecodeStep(e.Value()) - if err != nil { - return delivery.Step{}, fmt.Errorf("the step %s is not a step: %w", key, err) - } - if step.State != delivery.StepRunning { - return step, nil - } - } - } -} - // manifestReader reads manifests. of the indexer and parses the manifest it holds. type manifestReader struct{ kv jetstream.KeyValue } 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 182885b..d7d0faa 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 @@ -22,6 +22,7 @@ import ( "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" + "git.novox.be/novox/mesh-sdk/go/buildstep" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -47,13 +48,12 @@ const ( fp = "src2:3333333333333333333333333333333333333333333333333333333333333333" ) -// fakeBuildModules serve the tools of mesh-build-go and mesh-build-node as the real ones do (ADR 0469 clause 2): a call on -// mesh.mod..call... answers at once with the step and its key, and the step's end is written to -// the key . of the module's steps bucket, some milliseconds later. +// fakeBuildModules are the build modules on the nodes as the real ones run since R4g (ADR 0525; issue 1887 A4): each pulls +// its build seat's asks on its nodes (buildstep.Puller over the seat's workers) and runs an ask's job with step bodies that +// answer as the real tools do, ending the step on . of its steps bucket. type fakeBuildModules struct { js jetstream.JetStream ctx context.Context - n atomic.Int64 failBuild atomic.Bool // refusePush makes every bundle and archive build fail as a push the artifact store refused (ADR 0498 clause 2). refusePush atomic.Bool @@ -75,76 +75,68 @@ func digestOf(artifact string) string { return "sha256:" + hex.EncodeToString(sum[:]) } -func (f *fakeBuildModules) serve(t *testing.T, nc *nats.Conn, module string) { - steps, err := f.js.CreateKeyValue(f.ctx, jetstream.KeyValueConfig{Bucket: delivery.StepsBucket(module), History: 1}) +var prefixOf = map[string]string{"mesh-build-go": "build_go_", "mesh-build-node": "build_node_", "mesh-build-image": "build_image_"} + +// step is one tool's answer, as the real module's. +func (f *fakeBuildModules) step(module, name string, args map[string]any) (any, error) { + tool := prefixOf[module] + name + f.mu.Lock() + f.calls[module+"."+tool]++ + if name == "fetch" { + f.fetches = append(f.fetches, args) + } else { + f.builds = append(f.builds, args) + } + f.mu.Unlock() + switch { + case name == "fetch": + merge := "" + if args["base"] != nil { // a fetch with a base answers the merge commit (mesh-build-go FetchResult) + merge = `,"merge":"` + strings.Repeat("9", 40) + `"` + } + return json.RawMessage(`{"workspace":"w` + buildstep.NewID() + `","commit":"` + commit + `"` + merge + `}`), nil + case f.failBuild.Load() && tool != "build_image_copy": // the build fails wherever it runs + return nil, fmt.Errorf("go build of %v: boom", args["artifact"]) + case tool == "build_image_copy": // the upstream copy answers the digest and the registry's reference (ADR 0498 clause 4) + d := digestOf(fmt.Sprint(args["artifact"])) + return json.RawMessage(`{"artifact":"` + fmt.Sprint(args["artifact"]) + `","digest":"` + d + `","reference":"registry.example/` + fmt.Sprint(args["module"]) + `/` + fmt.Sprint(args["artifact"]) + `@` + d + `","pushed":true}`), nil + case tool != "build_image_build" && f.refusePush.Load(): + return nil, fmt.Errorf("%s%v: the artifact store answered 507 Insufficient Storage", delivery.NotStored, args["artifact"]) + case tool != "build_image_build": // a bundle or an archive is pushed and its reference answered (ADR 0498 clause 1) + d := digestOf(fmt.Sprint(args["artifact"])) + return json.RawMessage(`{"digest":"` + d + `","reference":"artifact-store://` + filepath.Base(fmt.Sprint(args["module"])) + `/` + fmt.Sprint(args["artifact"]) + `/blobs/` + d + `"}`), nil + case tool == "build_image_build" && args["publish"] == true: // a trunk image is pushed: the digest and the reference + d := digestOf(fmt.Sprint(args["artifact"])) + return json.RawMessage(`{"digest":"` + d + `","reference":"registry.example/imaged/server@` + d + `"}`), nil + default: + return json.RawMessage(`{"digest":"` + digestOf(fmt.Sprint(args["artifact"])) + `"}`), nil + } +} + +// serve makes module's steps bucket and its build seat's stream and workers, and pulls its asks on nodes. +func (f *fakeBuildModules) serve(t *testing.T, module string, nodes ...string) { + kv, err := f.js.CreateKeyValue(f.ctx, jetstream.KeyValueConfig{Bucket: delivery.StepsBucket(module), History: 1}) if err != nil { t.Fatal(err) } - sub, err := nc.Subscribe("mesh.mod."+module+".call.>", func(m *nats.Msg) { - tokens := strings.Split(m.Subject, ".") // mesh mod call - tool, node := tokens[4], tokens[5] - f.mu.Lock() - f.calls[module+"."+tool]++ - f.mu.Unlock() - var args map[string]any - _ = json.Unmarshal(m.Data, &args) - if strings.HasSuffix(tool, "_fetch") { - 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) - put := func(s delivery.Step) { - data, err := delivery.EncodeStep(s) - if err == nil { - _, err = steps.Put(f.ctx, key, data) - } - if err != nil { - t.Errorf("writing step %s: %v", key, err) - } - } - step := delivery.Step{ID: id, Tool: tool, Node: node, State: delivery.StepRunning, Started: time.Now().UTC()} - put(step) - answer, _ := json.Marshal(map[string]any{"result": map[string]any{"step": id, "key": key, "state": "running"}, "error": "", "node": node}) - _ = m.Respond(answer) - go func() { - time.Sleep(30 * time.Millisecond) - step.Ended = time.Now().UTC() - switch { - case strings.HasSuffix(tool, "_fetch"): - merge := "" - if args["base"] != nil { // a fetch with a base answers the merge commit (mesh-build-go FetchResult) - merge = `,"merge":"` + strings.Repeat("9", 40) + `"` - } - step.State, step.Result = delivery.StepPassed, json.RawMessage(`{"workspace":"w`+id+`","commit":"`+commit+`"`+merge+`}`) - case f.failBuild.Load() && tool != "build_image_copy": // every node's build fails: one node builds a fingerprint - step.State, step.Reason = delivery.StepFailed, fmt.Sprintf("go build of %v: boom", args["artifact"]) - case tool == "build_image_copy": // the upstream copy answers the digest and the registry's reference (ADR 0498 clause 4) - d := digestOf(fmt.Sprint(args["artifact"])) - step.State, step.Result = delivery.StepPassed, json.RawMessage(`{"artifact":"`+fmt.Sprint(args["artifact"])+`","digest":"`+d+`","reference":"registry.example/`+fmt.Sprint(args["module"])+`/`+fmt.Sprint(args["artifact"])+`@`+d+`","pushed":true}`) - case tool != "build_image_build" && f.refusePush.Load(): - step.State, step.Reason = delivery.StepFailed, fmt.Sprintf("%s%v: the artifact store answered 507 Insufficient Storage", delivery.NotStored, args["artifact"]) - case tool != "build_image_build": // a bundle or an archive is pushed and its reference answered (ADR 0498 clause 1) - d := digestOf(fmt.Sprint(args["artifact"])) - step.State, step.Result = delivery.StepPassed, json.RawMessage(`{"digest":"`+d+`","reference":"artifact-store://`+filepath.Base(fmt.Sprint(args["module"]))+`/`+fmt.Sprint(args["artifact"])+`/blobs/`+d+`"}`) - 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"]))+`"}`) - } - put(step) - }() - }) - if err != nil { + language := strings.TrimPrefix(module, "mesh-build-") + if err := buildstep.DeclareAskWorkers(f.ctx, f.js, language, 30*time.Second); err != nil { t.Fatal(err) } - t.Cleanup(func() { _ = sub.Unsubscribe() }) + put := func(key string, value any) (uint64, error) { + raw, _ := value.(json.RawMessage) + return kv.Put(f.ctx, key, raw) + } + call := func(name string) buildstep.StepFunc { + return func(ctx context.Context, args map[string]any) (any, error) { return f.step(module, name, args) } + } + steps := buildstep.AskSteps{Fetch: call("fetch"), Build: call("build"), Copy: call("copy")} + for _, node := range nodes { + p := &buildstep.Puller{Source: &buildstep.JetStreamAsks{JS: f.js, Language: language}, Language: language, Node: node, Put: put, + Capacity: func() int { return 2 }, PullWait: 200 * time.Millisecond, + Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { return buildstep.RunAsk(ctx, steps, a) }} + go func() { _ = p.Pull(f.ctx) }() + } } type bed struct { @@ -154,8 +146,6 @@ type bed struct { candidates jetstream.KeyValue builds jetstream.KeyValue tools map[string]jetstream.KeyValue - presence jetstream.KeyValue - have jetstream.KeyValue instances jetstream.KeyValue stop func() // stops the builder started last fake *fakeBuildModules @@ -228,20 +218,11 @@ func startBed(t *testing.T) *bed { b.candidates = mk(delivery.CandidatesBucket, delivery.CandidatesHistory) mk(delivery.ManifestsBucket, delivery.ManifestsHistory) b.builds = mk(delivery.BuilderBuildsBucket, delivery.BuilderBuildsHistory) - presence := mk(delivery.PresenceBucket, delivery.PresenceHistory) - b.presence, b.have = presence, mk(delivery.HaveBucket, delivery.HaveHistory) b.instances = mk(instancesBucket, 1) - for _, node := range []string{"n1", "n2"} { - b.setOnline(t, node, true) - for _, module := range []string{"mesh-build-go", "mesh-build-node"} { - b.setHealth(t, node, module, delivery.HealthOK) - } - } 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.fake.serve(t, "mesh-build-go", "n1", "n2") + b.fake.serve(t, "mesh-build-node", "n1", "n2") + b.fake.serve(t, "mesh-build-image", "n1") // 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") @@ -249,28 +230,6 @@ func startBed(t *testing.T) *bed { return b } -// setOnline writes the watchman's presence of a node. -func (b *bed) setOnline(t *testing.T, node string, online bool) { - t.Helper() - data, _ := delivery.EncodePresence(delivery.Presence{Online: online}) - if _, err := b.presence.Put(b.ctx, node, data); err != nil { - t.Fatal(err) - } -} - -// setHealth writes the node-engine's have of a build module on a node, running with this health. -func (b *bed) setHealth(t *testing.T, node, module string, health delivery.Health) { - t.Helper() - data, err := delivery.EncodeHave(delivery.Have{Phase: delivery.PhaseRunning, Health: health}) - if err != nil { - t.Fatal(err) - } - key, _ := delivery.HaveKey(node, module) - if _, err := b.have.Put(b.ctx, key, data); err != nil { - t.Fatal(err) - } -} - // report writes a node's toolchains key of a build module and says so on the module's stream, as the module does. func (b *bed) report(t *testing.T, module, node, language, version, recipe string) { t.Helper() @@ -433,20 +392,8 @@ func TestAnArchiveOnlyModuleNamingNoToolchainIsBuilt(t *testing.T) { } } -// TestANodeWhoseBuildModuleIsUnhealthyIsNotUsed: the node-engine's have of mesh-build-go on n2 says failed, so only n1 builds. -func TestANodeWhoseBuildModuleIsUnhealthyIsNotUsed(t *testing.T) { - b := startBed(t) - b.setHealth(t, "n2", "mesh-build-go", delivery.HealthFailed) - b.archive(t, "ca-trust", "modules/ca-trust", commit) - b.start(t, "n1", nil) - v := b.await(t, "ca-trust", isState(delivery.BuildDone)) - if !strings.Contains(v.Step, "on n1") { - t.Errorf("build = %+v, want it built on n1 alone", v) - } -} - -// TestAFailedKeyIsBuiltAgainAsANewRevisionFromANewModulesValueWithTheSameFingerprint: a build step fails on one node, so the -// key is failed naming it; the indexer writing modules. again with the same fingerprint wakes the builder, which builds +// TestAFailedKeyIsBuiltAgainAsANewRevisionFromANewModulesValueWithTheSameFingerprint: a build step fails, so the key is +// failed naming it; the indexer writing modules. again with the same fingerprint wakes the builder, which builds // the key again as a new revision of the same key, now done. func TestAFailedKeyIsBuiltAgainAsANewRevisionFromANewModulesValueWithTheSameFingerprint(t *testing.T) { // 1607/AC6, AC7 b := startBed(t) @@ -742,3 +689,32 @@ func TestABuilderWhoseBuildModulesAnsweredReferencesSaysItAndStoppedSaysNothing( t.Fatalf("the stopped builder still says: %v", err) } } + +// 1887/AC1, AC2 on the real bus: the builder puts the trunk fingerprint's build as asks on the build seats' delivery +// streams and calls no node's tool; the build modules that pulled them wrote the steps, and the key is done naming the +// references. +func TestTheBuilderAsksTheBuildSeatsAndCallsNoNode(t *testing.T) { + b := startBed(t) + b.module(t, "netcheck", "catalogue-a67f0196-netcheck.module.json", "modules/netcheck") + sub, err := nats.Connect(b.srvURL, nats.UserInfo("ctl", "pw")) + if err != nil { + t.Fatal(err) + } + defer sub.Close() + var calls atomic.Int64 + s, err := sub.Subscribe("mesh.mod.*.call.>", func(*nats.Msg) { calls.Add(1) }) + if err != nil { + t.Fatal(err) + } + defer s.Unsubscribe() + b.start(t, "n1", nil) + v := b.await(t, "netcheck", isState(delivery.BuildDone)) + for _, a := range v.Artifacts { + if a.Reference == "" { + t.Errorf("artifact %s names no reference", a.Name) + } + } + if n := calls.Load(); n != 0 { + t.Errorf("%d tool calls reached a build module", n) + } +} 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 822b325..6d597a3 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go @@ -26,7 +26,6 @@ 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" ) @@ -143,38 +142,20 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro if err != nil { return err } - // Calls to the build modules go out as this module's own account, naming the machine (mesh-sdk process.CallOn); the - // buckets are read and written on the connection above. - account, err := process.ReadAccount(path) - if err != nil { - return err - } - caller, err := process.Connect(account, nats.Name(cred.Node+"."+cred.Module+".calls"), nats.RetryOnFailedConnect(true)) - if err != nil { - return err - } - defer caller.Close() - bound := defaultStepBound if v := strings.TrimSpace(getenv("MESH_BUILDER_STEP_BOUND")); v != "" { if bound, err = time.ParseDuration(v); err != nil { return fmt.Errorf("MESH_BUILDER_STEP_BOUND: %w", err) } } - capacity, err := capacityFrom(strings.TrimSpace(getenv("MESH_BUILDER_SETTINGS")), getenv("MESH_BUILDER_CAPACITY")) - if err != nil { - return fmt.Errorf("the build capacity setting: %w", err) - } tools := &buildrun.ToolchainSet{} - presence := &buildrun.PresenceSet{} - have := &buildrun.HaveSet{} + // 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} b := &build.Builder{ Claimer: build.Claimer{Store: kvStore{builds}, By: cred.Node + "." + cred.Module}, - Runner: buildrun.Runner{Caller: caller, Steps: stepsWaiter{js}, Bound: bound}, + Asks: buildrun.Asker{Publisher: asks, Waiter: asks, Bound: bound}, Toolchains: tools, - Presence: presence, - Health: have, - Places: &buildrun.Places{Capacity: capacity}, Manifests: manifestReader{manifests}, Logf: logger.Printf, } @@ -219,48 +200,6 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro } defer cc.Stop() } - // The watchman's presence says which nodes are asleep. Where its bucket is not there yet no node is online, so nothing - // builds; the builder says so and is started again with the watchman. - presenceErrs := make(chan error, 1) - switch kv, err := js.KeyValue(ctx, delivery.PresenceBucket); { - case errors.Is(err, jetstream.ErrBucketNotFound): - logger.Printf("the bucket %s does not exist yet: no node counts as online until the builder starts again", delivery.PresenceBucket) - case err != nil: - return fmt.Errorf("the bucket %s: %w", delivery.PresenceBucket, err) - default: - pw, err := settle(ctx, kv, func(e jetstream.KeyValueEntry) { presence.Put(e.Key(), e.Value(), !opOf(e)) }) - if err != nil { - return err - } - go func() { - presenceErrs <- follow(ctx, pw, func(e jetstream.KeyValueEntry) { - if presence.Put(e.Key(), e.Value(), !opOf(e)) { - b.NodesChanged(ctx) // a node came online or went: a build that waited for one may go on - } - }) - }() - } - // The node-engine's have says whether a node's build module runs and is healthy: a node whose build module is not is not - // chosen (ADR 0478 clause 3). Where its bucket is not there yet no build module counts as healthy, so nothing builds. - haveErrs := make(chan error, 1) - switch kv, err := js.KeyValue(ctx, delivery.HaveBucket); { - case errors.Is(err, jetstream.ErrBucketNotFound): - logger.Printf("the bucket %s does not exist yet: no build module counts as healthy until the builder starts again", delivery.HaveBucket) - case err != nil: - return fmt.Errorf("the bucket %s: %w", delivery.HaveBucket, err) - default: - hw, err := settle(ctx, kv, func(e jetstream.KeyValueEntry) { have.Put(e.Key(), e.Value(), !opOf(e)) }) - if err != nil { - return err - } - go func() { - haveErrs <- follow(ctx, hw, func(e jetstream.KeyValueEntry) { - if have.Put(e.Key(), e.Value(), !opOf(e)) { - b.NodesChanged(ctx) // a build module became healthy or stopped being: a build that waited may go on - } - }) - }() - } each := func(e jetstream.KeyValueEntry) { if opOf(e) { b.Module(ctx, e.Key(), e.Value()) @@ -300,9 +239,5 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro return err case err := <-watch.errs: return err - case err := <-presenceErrs: - return err - case err := <-haveErrs: - return err } } diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/settings.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/settings.go deleted file mode 100644 index e0255f1..0000000 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/settings.go +++ /dev/null @@ -1,28 +0,0 @@ -package main - -import ( - "encoding/json" - "fmt" - "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" - "os" -) - -// capacityFrom is the build capacity of each node (ADR 0478 clause 3): the module's setting `capacity`, `=` pairs separated by commas, laid into the settings file the controller composes (issue 1661); empty is one step at a -// time on every node. With no settings file named, fallback is read instead, for a run outside the module's launch. -func capacityFrom(path, fallback string) (map[string]int, error) { - if path == "" { - return buildrun.ParseCapacity(fallback) - } - raw, err := os.ReadFile(path) - if err != nil { - return nil, fmt.Errorf("the settings file: %w", err) - } - var s struct { - Capacity string `json:"capacity"` - } - if err := json.Unmarshal(raw, &s); err != nil { - return nil, fmt.Errorf("the settings file %s is not JSON: %w", path, err) - } - return buildrun.ParseCapacity(s.Capacity) -} diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/settings_test.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/settings_test.go deleted file mode 100644 index 31b1acf..0000000 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/settings_test.go +++ /dev/null @@ -1,67 +0,0 @@ -package main - -import ( - "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" - "os" - "path/filepath" - "testing" -) - -func settingsFile(t *testing.T, body string) string { - t.Helper() - p := filepath.Join(t.TempDir(), "settings.json") - if err := os.WriteFile(p, []byte(body), 0o600); err != nil { - t.Fatal(err) - } - return p -} - -// 1661: the build capacity is the module's setting `capacity`, laid into the settings file: empty is one step at a time everywhere. -func TestCapacityIsReadFromTheSettingsFile(t *testing.T) { - for body, want := range map[string]map[string]int{ - `{"capacity": "shanks=3,ace=2"}`: {"shanks": 3, "ace": 2}, - `{"capacity": ""}`: {}, - `{}`: {}, - } { - got, err := capacityFrom(settingsFile(t, body), "") - if err != nil || len(got) != len(want) || got["shanks"] != want["shanks"] || got["ace"] != want["ace"] { - t.Errorf("%s: capacity %v, %v, want %v", body, got, err, want) - } - } - p := &buildrun.Places{Capacity: mustCapacity(t, settingsFile(t, `{"capacity": ""}`))} - if p.CapacityOf("shanks") != 1 || p.CapacityOf("ace") != 1 { - t.Errorf("an empty setting gives %d and %d, want 1 everywhere", p.CapacityOf("shanks"), p.CapacityOf("ace")) - } -} - -func mustCapacity(t *testing.T, path string) map[string]int { - t.Helper() - c, err := capacityFrom(path, "") - if err != nil { - t.Fatal(err) - } - return c -} - -func TestACapacitySettingThatIsNotOneIsRefusedNamingIt(t *testing.T) { - if _, err := capacityFrom(settingsFile(t, `{"capacity": "shanks=-1"}`), ""); err == nil { - t.Error("shanks=-1 accepted") - } - if c, err := capacityFrom(settingsFile(t, `{"capacity": "shanks=3,ace=2,g14=0,novox=0"}`), ""); err != nil || c["g14"] != 0 || len(c) != 4 { - t.Errorf("the starting value: %v, %v", c, err) - } - if _, err := capacityFrom(settingsFile(t, `not json`), ""); err == nil { - t.Error("a file that is not JSON accepted") - } - if _, err := capacityFrom(filepath.Join(t.TempDir(), "none"), ""); err == nil { - t.Error("a settings file that is not there accepted") - } -} - -// With no settings file named (a run outside the module's launch, as the tests do), the capacity is the environment's. -func TestWithNoSettingsFileTheCapacityIsTheEnvironments(t *testing.T) { - got, err := capacityFrom("", "ace=2") - if err != nil || got["ace"] != 2 { - t.Errorf("capacity %v, %v", got, err) - } -} diff --git a/modules/mesh-delivery-builder/files/settings.json b/modules/mesh-delivery-builder/files/settings.json deleted file mode 100644 index 3175fe3..0000000 --- a/modules/mesh-delivery-builder/files/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "capacity": "shanks=3,ace=2,g14=0,novox=0" -} diff --git a/modules/mesh-delivery-builder/internal/build/fixtures_test.go b/modules/mesh-delivery-builder/internal/build/fixtures_test.go index b5a6566..4ecc3e0 100644 --- a/modules/mesh-delivery-builder/internal/build/fixtures_test.go +++ b/modules/mesh-delivery-builder/internal/build/fixtures_test.go @@ -1,10 +1,14 @@ package build import ( + "context" "encoding/json" "os" + "strings" "testing" + "time" + "git.novox.be/novox/mesh-sdk/go/buildstep" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -25,17 +29,31 @@ func captured(t *testing.T, name string) json.RawMessage { return step.Result } +// The captured image steps, run as a pulled ask runs them (buildstep.RunAsk), answer what the builder reads: the trunk's +// build its digest and reference, a candidate's its digest and no reference (issue 1887 A4a). func TestCapturedImageStepsDecodeWithTheBuildersOwnTypes(t *testing.T) { // review r2, mesh-catalog 2; ADR 0487 clause 1 - var f fetchResult - if err := json.Unmarshal(captured(t, "fetch.json"), &f); err != nil || f.Workspace == "" { - t.Errorf("fetch = %+v, %v, want a workspace (the field the builder reads)", f, err) - } - var trunk buildResult - if err := json.Unmarshal(captured(t, "build-trunk.json"), &trunk); err != nil || trunk.Digest == "" || trunk.Reference == "" { - t.Errorf("trunk build = %+v, %v, want a digest and a reference", trunk, err) - } - var cand buildResult - if err := json.Unmarshal(captured(t, "build-candidate.json"), &cand); err != nil || cand.Digest == "" || cand.Reference != "" { - t.Errorf("candidate build = %+v, %v, want a digest and no reference", cand, err) + for _, tc := range []struct { + build string + publish bool + ref bool + }{{"build-trunk.json", true, true}, {"build-candidate.json", false, false}} { + steps := buildstep.AskSteps{ + Fetch: func(context.Context, map[string]any) (any, error) { + return json.RawMessage(captured(t, "fetch.json")), nil + }, + Build: func(context.Context, map[string]any) (any, error) { return json.RawMessage(captured(t, tc.build)), nil }, + } + ask := delivery.StepAsk{ID: buildstep.NewID(), Kind: delivery.AskDelivery, Language: delivery.AskImage, Asker: delivery.BuilderModule, + At: time.Now(), Build: &delivery.BuildJob{Module: "m", Path: "modules/m", Publish: tc.publish, + Fetch: map[string]any{"repository": "novox/mesh-catalog", "commit": strings.Repeat("a", 40)}, + Artifacts: []delivery.AskArtifact{{Artifact: "server"}}}} + out, err := buildstep.RunAsk(context.Background(), steps, ask) + if err != nil { + t.Fatalf("%s: %v", tc.build, err) + } + a := out.(delivery.BuildAnswer) + if len(a.Artifacts) != 1 || a.Artifacts[0].Digest == "" || (a.Artifacts[0].Reference != "") != tc.ref { + t.Errorf("%s answered %+v, want a digest and a reference only for the trunk", tc.build, a) + } } } diff --git a/modules/mesh-delivery-builder/internal/build/r4e_test.go b/modules/mesh-delivery-builder/internal/build/r4e_test.go index d3d851c..fab0854 100644 --- a/modules/mesh-delivery-builder/internal/build/r4e_test.go +++ b/modules/mesh-delivery-builder/internal/build/r4e_test.go @@ -111,7 +111,7 @@ func TestADoneKeyWrittenBeforeR4eIsBuiltAgainOnceAsANewRevision(t *testing.T) { func TestAPinnedUpstreamIsCopiedOnceOnOneImageNodeWithNoFetchAndItsKeyNamesTheReference(t *testing.T) { // B4, ADR 0498 clause 4 up := catalogue.Artifact{Name: "up", Kind: "upstream", From: "nats:2.11.17-alpine@" + digest} r := newRig(t, up) - r.b.Health = imageHave(t, "ace", "novox") + r.puller = "ace" v := r.consider(t, "modules/m") if v.State != delivery.BuildDone || len(v.Artifacts) != 1 { t.Fatalf("build = %+v, want done with the copy", v) @@ -133,7 +133,7 @@ func TestAPinnedUpstreamIsCopiedOnceOnOneImageNodeWithNoFetchAndItsKeyNamesTheRe func TestAnUpstreamBesideAnImageIsCopiedAndTheImageFetchedAndBuiltOnTheSameNode(t *testing.T) { // B4 r := newRig(t, imageArt, catalogue.Artifact{Name: "up", Kind: "upstream", From: "nats:2.11.17-alpine@" + digest}) - r.b.Health = imageHave(t, "ace") + r.puller = "ace" v := r.consider(t, "modules/m") if ok, missing := v.Referenced(); !ok { t.Fatalf("build = %+v, missing references %v", v, missing) @@ -145,7 +145,7 @@ func TestAnUpstreamBesideAnImageIsCopiedAndTheImageFetchedAndBuiltOnTheSameNode( func TestACopyThatFailsNotStoredFailsTheKeyAndIsBuiltAgain(t *testing.T) { // B4, ADR 0498 clause 2 r := newRig(t, catalogue.Artifact{Name: "up", Kind: "upstream", From: "nats:2.11.17-alpine@" + digest}) - r.b.Health = imageHave(t, "ace") + r.puller = "ace" r.bm.fails["ace/build_image_copy"] = delivery.NotStored + "up: the registry answered 500" v := r.consider(t, "modules/m") if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, delivery.NotStored) || delivery.FailedUntilChanged(v.Error) { diff --git a/modules/mesh-delivery-builder/internal/build/round4_test.go b/modules/mesh-delivery-builder/internal/build/round4_test.go index f2bcc51..e396831 100644 --- a/modules/mesh-delivery-builder/internal/build/round4_test.go +++ b/modules/mesh-delivery-builder/internal/build/round4_test.go @@ -3,7 +3,6 @@ package build import ( "context" "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" - "strings" "testing" "git.novox.be/novox/mesh-sdk/go/catalogue" @@ -39,26 +38,6 @@ func TestAnArchiveOnlyModuleIsBuiltAlthoughItsFingerprintNamesNoToolchain(t *tes } } -func TestAnArchiveOnlyModuleNeedsNoAgreedToolchainButNeedsNodesThatQualify(t *testing.T) { // review round 4, 1 - r := newRig(t, archiveA) - r.tools.Put(buildrun.GoBuildModule, "ace.go", tcValue(t, "go", "go1.26.0"), false) // the nodes disagree: a bundle would wait - r.b.Health = &buildrun.HaveSet{} // no build module is healthy anywhere - r.b.Module(context.Background(), "m", moduleValueWith(t, "modules/m", nil)) - r.b.Wait() - if v := valueOf(t, r.st, r.key); v.State != delivery.BuildFailed || !strings.Contains(v.Error, "healthy") { - t.Errorf("build = %+v, want failed: no node's build module is healthy", v) - } - have := &buildrun.HaveSet{} - have.Put("novox.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) - r.b.Health = have - r.b.Reconsider(context.Background()) - r.b.Wait() - v := valueOf(t, r.st, r.key) - if v.State != delivery.BuildDone || !strings.Contains(v.Step, "on novox") { - t.Errorf("build = %+v, want done on novox, the one node that qualifies", v) - } -} - func TestABundleAndAnArchiveOfOneBuildModuleStillNeedTheAgreedToolchain(t *testing.T) { // review round 4, 1 p, err := PlanOf(catalogue.Manifest{Module: "m", Build: &catalogue.Build{Artifacts: []catalogue.Artifact{goBundle, archiveA}}}) if err != nil || !p.NeedsToolchain(buildrun.GoBuildModule) { @@ -80,115 +59,6 @@ func TestClaimLeavesAKeyFailedForDifferingDigestsAlone(t *testing.T) { // review } } -// 4. A step never waits on a full node while another qualifying node has room. -func TestChoosesMadeAtOnceSeeEachOthersReservationsSoNoLaneWaitsWhileANodeHasRoom(t *testing.T) { // review round 4, 4 - p := &buildrun.Places{Capacity: map[string]int{"ace": 2, "g14": 1, "novox": 1, "shanks": 3}} - entries := tcs("go", "go1.27.1", "ace", "g14", "novox", "shanks") - total := 2 + 1 + 1 + 3 - var waiting int - for i := 1; i <= 4; i++ { // four modules choose before any lane has started - room := 0 - for _, n := range []string{"ace", "g14", "novox", "shanks"} { - room += p.CapacityOf(n) - p.Running()[n] - p.Reserved()[n] - } - nodes, _, err := buildrun.ChooseNodes("go", entries, p, nil, 2) - if err != nil || len(nodes) != 2 { - t.Fatalf("choice %d: %v, %v", i, nodes, err) - } - if room >= 2 { - for _, n := range nodes { - if p.Reserved()[n] > p.CapacityOf(n) { - t.Errorf("choice %d: %v put a lane on full %s while %d places were free", i, nodes, n, room) - } - } - } else { - waiting += 2 - room - } - } - if want := 4*2 - total; waiting != want { - t.Errorf("%d lanes wait, want %d: the 8 lanes less the %d places", waiting, want, total) - } - loads := p.Reserved() - for n, c := range loads { - if c > p.CapacityOf(n) && n != "shanks" && n != "ace" && n != "g14" && n != "novox" { - t.Errorf("unknown node %s", n) - } - } - if loads["shanks"] != 3 || loads["ace"] != 2 || loads["g14"]+loads["novox"] != 3 { - t.Errorf("reservations = %v, want shanks 3, ace 2 and the single-place nodes taking the rest", loads) - } -} - -func TestACancelledReservationFreesTheChoiceAndAnAcquireTakesItsReservation(t *testing.T) { // review round 4, 4 - p := &buildrun.Places{} - nodes, _, _ := buildrun.ChooseNodes("go", tcs("go", "go1.27.1", "ace"), p, nil, 1) - if len(nodes) != 1 || p.Reserved()["ace"] != 1 { - t.Fatalf("nodes %v, reserved %v", nodes, p.Reserved()) - } - release, err := p.Acquire(context.Background(), "ace") - if err != nil || p.Reserved()["ace"] != 0 || p.Running()["ace"] != 1 { - t.Fatalf("acquire: %v, reserved %v, running %v", err, p.Reserved(), p.Running()) - } - release() - buildrun.ChooseNodes("go", tcs("go", "go1.27.1", "ace"), p, nil, 1) - p.Cancel("ace") - if len(p.Reserved()) != 0 { - t.Errorf("reserved after Cancel = %v", p.Reserved()) - } -} - -// 5. A node whose build module is not healthy is not chosen. -func TestHaveSetKnowsOnlyBuildModulesRunningAndHealthy(t *testing.T) { // review round 4, 5 - var s buildrun.HaveSet - if s.Healthy("ace", buildrun.GoBuildModule) { - t.Error("a module with no have is healthy") - } - if !s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) || !s.Healthy("ace", buildrun.GoBuildModule) { - t.Error("a running, healthy module is not") - } - if s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) { - t.Error("the same state counted as a change") - } - if !s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthFailed), false) || s.Healthy("ace", buildrun.GoBuildModule) { - t.Error("a module whose health failed is healthy") - } - s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) - if !s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseFailed, delivery.HealthOK), false) || s.Healthy("ace", buildrun.GoBuildModule) { - t.Error("a module whose phase failed is healthy") - } - s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) - if !s.Put("ace.mesh-build-go", nil, true) || s.Healthy("ace", buildrun.GoBuildModule) { - t.Error("a deleted have is healthy") - } - if s.Put("ace.node", []byte(`{}`), false) || s.Put("ace.mesh-build-go", []byte(`not json`), false) { - t.Error("a node's key or a non-have counted") - } -} - -func TestANodeWhoseBuildModuleIsNotHealthyIsNotChosen(t *testing.T) { // review round 4, 5 - r := newRig(t, goBundle) - have := &buildrun.HaveSet{} - have.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthFailed), false) // go is not the recipe's - have.Put("novox.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) - r.b.Health = have - v := r.consider(t, "modules/m") - if v.State != delivery.BuildDone || !strings.Contains(v.Step, "on novox") { - t.Fatalf("build = %+v, want done on novox alone", v) - } - for _, c := range r.bm.calls { - if c.node == "ace" { - t.Errorf("a step was called on ace, whose build module is unhealthy: %+v", c) - } - } - have.Put("novox.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthFailed), false) - r.st.vals = map[string][]byte{} // a fresh key - r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) - r.b.Wait() - if v := valueOf(t, r.st, r.key); v.State != delivery.BuildFailed || !strings.Contains(v.Error, "healthy") { - t.Errorf("build = %+v, want failed: no node has a healthy build module", v) - } -} - // 6. A key found building is taken over, whoever holds it. func TestABuildLeftBuildingByAnyBuilderIsTakenOverAtStart(t *testing.T) { // review round 4, 6; 1607/AC3, ADR 0478 clauses 5, 6 st := newMem() diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index 3021dd0..64e3bcd 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -5,9 +5,11 @@ import ( "encoding/json" "errors" "fmt" + "maps" "sort" "strings" "sync" + "time" "git.novox.be/novox/mesh-sdk/go/buildstep" "git.novox.be/novox/mesh-sdk/go/catalogue" @@ -20,29 +22,23 @@ type ToolchainEntries interface { Entries(language string) []delivery.NodeToolchain } -// 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_", 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 -// artifact to build claims the fingerprint's build key, builds it through the build modules on the nodes -// the toolchains choose, and writes the key done or failed. It holds no timer; a build starts from a modules value or a +// artifact to build claims the fingerprint's build key, builds it by asking the build seats (one ask per build module, +// pulled by whichever build node has a free slot: ADR 0525), and writes the key done or failed. It holds no timer; a build starts from a modules value or a // change of the toolchains. type Builder struct { - Claimer Claimer - Runner buildrun.Runner - Toolchains ToolchainEntries - // Presence says which nodes are online (the watchman's); a node that is not is not chosen. - Presence interface{ Online(node string) bool } - Manifests Manifests - // Health says whether a node's build module is running and healthy (the node-engine's have); a node whose build module - // is not is not chosen. Nil means every node's is. - Health interface { - Healthy(node, module string) bool + Claimer Claimer + // Asks puts each build module's part of a build as one ask on its build seat's stream and waits for its step (novox/hq + // ADR 0525 §1, §3, §6; issue 1887 A4): the builder chooses no node, whichever build node pulls the ask builds it. + Asks interface { + Put(ctx context.Context, a delivery.StepAsk) error + Wait(ctx context.Context, a delivery.StepAsk) (delivery.Step, error) } - // buildrun.Places holds a step back while its node is at its build capacity; a Builder with none runs one step at a time on each node. - Places *buildrun.Places - // Limit is how many builds run at once; four when 0. + Toolchains ToolchainEntries + Manifests Manifests + // Limit is how many builds the builder has asked at once; their asks wait in the build seats' streams, which are the + // backlog (ADR 0525 §9), not here. 256 when 0. Limit int Logf func(format string, args ...any) // Answered is told the build module whose step answered a reference (issue 1886): what the builder says it writes @@ -75,15 +71,12 @@ func (b *Builder) init() { b.once.Do(func() { limit := b.Limit if limit <= 0 { - limit = 4 + limit = 256 } 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{} - if b.Places == nil { - b.Places = &buildrun.Places{} - } }) } @@ -116,8 +109,7 @@ func (b *Builder) Module(ctx context.Context, key string, value []byte) { b.consider(ctx, key, m) } -// NodesChanged is Reconsider after a change of the nodes: one came online or went, a build module became healthy or -// stopped being, or a toolchain moved, so a build that waited for a node may go on. +// NodesChanged is Reconsider after a toolchain moved, so a build that waited for the nodes to agree on one may go on. func (b *Builder) NodesChanged(ctx context.Context) { b.Reconsider(ctx) } // Sweep reads, at start, every key of the bucket: it keeps the package versions done keys name as published (a key @@ -314,7 +306,7 @@ 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: b.imageBuilderRuns, 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}) if err != nil { b.logf("%s: %v", module, err) return @@ -330,7 +322,7 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { if cl.Build.State == delivery.BuildDone { prior, only = &cl.Build, map[string]bool{} if trunk { - for _, bm := range publishable(cl.Build, b.imageBuilderRuns) { + for _, bm := range publishable(cl.Build, imageSeat) { only[bm] = true publishing = true } @@ -342,7 +334,7 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { // any other, so the controller has a done key to register it from (issue 1613, ADR 0478 clause 1, ADR 0501 clause 1). result = delivery.BuilderBuild{State: delivery.BuildDone, Step: "nothing to build: the manifest names no artifact"} } else { - result = b.build(ctx, module, m, plan, planErr, prior, only) + result = b.build(ctx, module, m, plan, planErr, prior, only, b.notes(cl)) } if ctx.Err() != nil { return @@ -399,14 +391,15 @@ func failed(format string, args ...any) delivery.BuilderBuild { // lane is the build of a plan's tasks of one build module on one node. type lane struct { - module, node string + module, node string // node is the one that pulled the lane's ask tasks []Task + toolchain *delivery.ToolchainUsed digests map[string]string // artifact -> digest references map[string]string // artifact -> the reference an image was pushed to (a trunk build only) err error } -func (b *Builder) build(ctx context.Context, module string, m delivery.Module, plan Plan, planErr error, prior *delivery.BuilderBuild, only map[string]bool) delivery.BuilderBuild { +func (b *Builder) build(ctx context.Context, module string, m delivery.Module, plan Plan, planErr error, prior *delivery.BuilderBuild, only map[string]bool, asked *askNotes) delivery.BuilderBuild { switch { case planErr != nil: return failed("%s", planErr) @@ -417,76 +410,39 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p repository := delivery.RepositoryName(m.Repository) var lanes []*lane used := map[string]delivery.ToolchainUsed{} - var steps []string - // Every node chosen holds a reservation until its lane takes the place; a build that ends before its lanes start gives them up. - giveUp := func(nodes ...string) { - for _, n := range nodes { - b.Places.Cancel(n) - } - } - giveUpLanes := func() { - for _, l := range lanes { - b.Places.Cancel(l.node) - } - } var kept []string // build modules whose artifacts stay as the prior done key has them for _, bm := range plan.Modules() { if prior != nil && len(only) > 0 && !only[bm] { kept = append(kept, bm) continue } - language := buildrun.LanguageOf[bm] - entries := b.Toolchains.Entries(language) - qualifies := func(node string) bool { return b.qualifies(node, bm) } - var nodes []string - 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, 1) - if err != nil { - giveUpLanes() - return failed("%s", err) + var toolchain *delivery.ToolchainUsed + if bm != ImageBuildModule && plan.NeedsToolchain(bm) { + // The ask names the toolchain the fingerprint was computed with; a node building with another refuses it. + language := buildrun.LanguageOf[bm] + tc, none := delivery.AgreedToolchain(b.Toolchains.Entries(language)) + if none != "" { + return failed("the %s toolchain is not agreed: %s", language, none) } if want := m.Toolchains[language]; want.Version != tc.Version || want.Recipe != tc.Recipe { - giveUp(ns...) - giveUpLanes() return delivery.BuilderBuild{} // the agreed toolchain moved since try looked: nothing is written } - used[language] = delivery.ToolchainUsed{Version: tc.Version, Recipe: tc.Recipe} - nodes = ns - } else { - // An archive is packed with no toolchain (the fingerprint names none): any node of the build module that qualifies. - ns, err := buildrun.ChooseAnyNodes(language, entries, b.Places, qualifies, 1) - if err != nil { - giveUpLanes() - return failed("%s", err) - } - nodes = ns + u := delivery.ToolchainUsed{Version: tc.Version, Recipe: tc.Recipe} + used[language], toolchain = u, &u } - // One node builds each fingerprint (ADR 0478 clause 4, from R4e). - steps = append(steps, fmt.Sprintf("%s on %s", bm, nodes[0])) var tasks []Task for _, t := range plan.Tasks { if t.Module == bm { tasks = append(tasks, t) } } - for _, n := range nodes { - lanes = append(lanes, &lane{module: bm, node: n, tasks: tasks, digests: map[string]string{}, references: map[string]string{}}) - } + // One ask builds each fingerprint's part of a build module, on whichever node pulls it (ADR 0525 §3). + lanes = append(lanes, &lane{module: bm, tasks: tasks, toolchain: toolchain, digests: map[string]string{}, references: map[string]string{}}) } var wg sync.WaitGroup for _, l := range lanes { wg.Add(1) - go func() { defer wg.Done(); b.runLane(ctx, l, repository, m, module) }() + go func() { defer wg.Done(); b.runLane(ctx, l, repository, m, module, asked) }() } wg.Wait() var errs []string @@ -548,6 +504,10 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p } artifacts = append(artifacts, built) } + var steps []string + for _, l := range lanes { + steps = append(steps, fmt.Sprintf("%s on %s", l.module, l.node)) + } for _, bm := range kept { steps = append(steps, bm+" kept") for lang, u := range prior.Toolchains { @@ -617,73 +577,56 @@ func keyError(errs []string) string { return strings.Join(errs, "; ") } -func (b *Builder) online(node string) bool { return b.Presence == nil || b.Presence.Online(node) } +// imageSeat says an image build is asked of the image build seat, which takes it when a node pulls it: a key failed for +// want of an image builder before R4g is taken again (ADR 0525 §1). +func imageSeat() bool { return true } -// qualifies is whether the node can build with the build module now: it is online and the build module on it is healthy. -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)) +// askLanguage is the build seat language of each build module (delivery.AskSubject). +var askLanguage = map[string]string{buildrun.GoBuildModule: delivery.AskGo, buildrun.NodeBuildModule: delivery.AskNode, ImageBuildModule: delivery.AskImage} + +// askNotes writes a claimed key's asks into it, building, before each is put (ADR 0525 §1; issue 1887 A4b), and holds +// the asks a key taken over at start already names, so they are waited for and never put again. +type askNotes struct { + mu sync.Mutex + b *Builder + cl *Claim + resumed map[string]string } -// 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}) - } +func (b *Builder) notes(cl *Claim) *askNotes { + n := &askNotes{b: b, cl: cl} + if cl.Build.State == delivery.BuildBuilding && len(cl.Build.Asks) > 0 { + n.resumed = maps.Clone(cl.Build.Asks) } - for n := range b.Places.Capacity { - add(n) + return n +} + +// resume is the id of the ask a taken-over key names for language, or "". +func (n *askNotes) resume(language string) string { + n.mu.Lock() + defer n.mu.Unlock() + return n.resumed[language] +} + +// note writes the ask's id into the key, building, by compare-and-set at the claim's revision. +func (n *askNotes) note(ctx context.Context, language, id, step string) error { + n.mu.Lock() + defer n.mu.Unlock() + v := n.cl.Build + v.State, v.By, v.Step, v.Error, v.Artifacts = delivery.BuildBuilding, n.b.Claimer.By, step, "", nil + v.Asks = maps.Clone(v.Asks) + if v.Asks == nil { + v.Asks = map[string]string{} } - 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 + v.Asks[language] = id + return n.b.Claimer.Finish(ctx, n.cl, v) } -// 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 -} - -// step runs one step of a build module; the lane that runs it holds its node's place. -func (b *Builder) step(ctx context.Context, bm, node, tool string, args map[string]any) (json.RawMessage, error) { - return b.Runner.Run(ctx, bm, node, toolPrefix[bm]+tool, args) -} - -// fetchResult and buildResult are the results of a build module's fetch and build steps as the builder reads them. -type fetchResult struct { - Workspace string `json:"workspace"` - Merge string `json:"merge"` - Conflicts []string `json:"conflicts"` -} - -type buildResult struct { - Digest string `json:"digest"` - Reference string `json:"reference"` -} - -// runLane takes a place on the node, fetches the commit there and builds each of the lane's artifacts, one step at a time. -// An upstream artifact is copied by its pinned reference with no fetch (ADR 0498 clause 4); a lane of copies alone fetches -// nothing. -func (b *Builder) runLane(ctx context.Context, l *lane, repository string, m delivery.Module, module string) { - release, err := b.Places.Acquire(ctx, l.node) - if err != nil { - l.err = err - return - } - defer release() +// runLane puts the lane's part of the build as one ask on its build seat's stream (the delivery stream for a trunk key, the +// check stream for a candidate's) and reads what the node that pulled it built (ADR 0525 §1, §3, §6). An upstream artifact +// is copied by its pinned reference with no fetch (ADR 0498 clause 4); a lane of copies alone fetches nothing. A lane of a +// key taken over at start waits for the ask the key names and puts none. +func (b *Builder) runLane(ctx context.Context, l *lane, repository string, m delivery.Module, module string, asked *askNotes) { req := buildstep.FetchRequest{Repository: repository, Commit: m.Commit} if key, err := delivery.BuilderBuildKey(module, m.FP2); err == nil { b.mu.Lock() @@ -702,57 +645,69 @@ func (b *Builder) runLane(ctx context.Context, l *lane, repository string, m del } sort.Strings(req.Paths) } - fetchArgs := req.Args() candidate := req.Base != "" - var fetched fetchResult + language := askLanguage[l.module] + kind := delivery.AskDelivery + if candidate { + kind = delivery.AskCheck + } + job := &delivery.BuildJob{Module: module, Path: m.Path, FP: m.FP2, Toolchain: l.toolchain, + // 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). + Publish: !candidate} + for _, t := range l.tasks { + a := delivery.AskArtifact{Artifact: t.Artifact} + if t.Kind == catalogue.ArtifactUpstream { + a.From = t.From // the copy runs no code of a change: the module's name, the artifact and its pin + } + job.Artifacts = append(job.Artifacts, a) + } if needsFetch(l.tasks) { - raw, err := b.step(ctx, l.module, l.node, "fetch", fetchArgs) - if err != nil { + job.Fetch = req.Args() + } + ask := delivery.StepAsk{ID: buildstep.NewID(), Kind: kind, Language: language, Asker: delivery.BuilderModule, At: time.Now(), Build: job} + if id := asked.resume(language); id != "" { + ask.ID = id // put before this builder started again: the stream holds it, or a node already ran it + } else { + if err := asked.note(ctx, language, ask.ID, "asked "+l.module); err != nil { + l.err = fmt.Errorf("the ask of %s could not be written to the key: %w", l.module, err) + return + } + if err := b.Asks.Put(ctx, ask); err != nil { l.err = err return } - if err := json.Unmarshal(raw, &fetched); err != nil || fetched.Workspace == "" { - l.err = fmt.Errorf("the fetch on %s answered no workspace: %s", l.node, raw) - return - } - // 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 { - 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 - } - if fetched.Merge == "" { - l.err = fmt.Errorf("the fetch with a base on %s answered no merge: %s", l.node, raw) - return - } - } + } + step, err := b.Asks.Wait(ctx, ask) + if err != nil { + l.err = fmt.Errorf("%s: %w", l.module, err) + return + } + l.node = step.Node + if step.State != delivery.StepPassed { + l.err = fmt.Errorf("%s on %s: the ask %s %s: %s", l.module, step.Node, ask.ID, step.State, step.Reason) + return + } + var answer delivery.BuildAnswer + if err := json.Unmarshal(step.Result, &answer); err != nil { + l.err = fmt.Errorf("the ask of %s on %s answered what is not a build: %s", l.module, step.Node, step.Result) + return + } + built := map[string]delivery.AskBuilt{} + for _, a := range answer.Artifacts { + built[a.Artifact] = a } for _, t := range l.tasks { - tool, args := "build", map[string]any{"workspace": fetched.Workspace, "module": m.Path, "artifact": t.Artifact} - if t.Kind == catalogue.ArtifactUpstream { - // The copy runs no code of a change: the module's name (the reference's path), the artifact and its pin. - tool, args = "copy", map[string]any{"module": module, "artifact": t.Artifact, "from": t.From} - } else 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, tool, args) - if err != nil { - l.err = err + a, ok := built[t.Artifact] + if !ok || a.Digest == "" { + l.err = fmt.Errorf("the build of %s of %s on %s answered no digest: %s", t.Artifact, module, step.Node, step.Result) return } - var built buildResult - if err := json.Unmarshal(raw, &built); err != nil || built.Digest == "" { - l.err = fmt.Errorf("the %s of %s of %s on %s answered no digest: %s", tool, t.Artifact, module, l.node, raw) - return + l.digests[t.Artifact] = a.Digest + if a.Reference != "" && (stored(t.Kind) || !candidate) { + l.references[t.Artifact] = a.Reference } - l.digests[t.Artifact] = built.Digest - if built.Reference != "" && (stored(t.Kind) || !candidate) { - l.references[t.Artifact] = built.Reference - } - if built.Reference != "" && b.Answered != nil { + if a.Reference != "" && b.Answered != nil { b.Answered(l.module) } } diff --git a/modules/mesh-delivery-builder/internal/build/run_test.go b/modules/mesh-delivery-builder/internal/build/run_test.go index 4a8524c..ba94b0e 100644 --- a/modules/mesh-delivery-builder/internal/build/run_test.go +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "git.novox.be/novox/mesh-sdk/go/buildstep" "git.novox.be/novox/mesh-sdk/go/catalogue" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -124,22 +125,56 @@ type rig struct { st *memStore bm *buildModules tools *buildrun.ToolchainSet - pres *buildrun.PresenceSet mf *fakeManifests key string + // asks are the build seats' streams; puller is the node that pulls every ask ("ace" when empty). + asks *buildrun.FakeAsks + puller string +} + +// toolPrefix is the start of each build module's tool names, as the fake build modules answer them. +var toolPrefix = map[string]string{buildrun.GoBuildModule: "build_go_", buildrun.NodeBuildModule: "build_node_", ImageBuildModule: "build_image_"} + +// answer runs an ask as the build module that pulls it does (buildstep.RunAsk), with the fake build modules' tools on the +// rig's pulling node, and answers the step that node writes. +func (r *rig) answer(a delivery.StepAsk) delivery.Step { + node := r.puller + if node == "" { + node = "ace" + } + module, _ := delivery.BuildModuleOf(a.Language) + runner := buildrun.Runner{Caller: r.bm, Steps: r.bm.steps, Bound: time.Second} + tool := func(name string) buildstep.StepFunc { + return func(ctx context.Context, args map[string]any) (any, error) { + raw, err := runner.Run(ctx, module, node, toolPrefix[module]+name, args) + if err != nil { + return nil, err + } + return raw, nil + } + } + steps := buildstep.AskSteps{Fetch: tool("fetch"), Build: tool("build"), Copy: tool("copy")} + out, err := buildstep.RunAsk(context.Background(), steps, a) + step := delivery.Step{ID: a.ID, Tool: a.Kind + " ask", Node: node, State: delivery.StepPassed} + if err != nil { + step.State, step.Reason = delivery.StepFailed, err.Error() + return step + } + step.Result, _ = json.Marshal(out) + return step } func newRig(t *testing.T, artifacts ...catalogue.Artifact) *rig { t.Helper() - r := &rig{st: newMem(), bm: newBuildModules(), tools: &buildrun.ToolchainSet{}, pres: &buildrun.PresenceSet{}, mf: &fakeManifests{byDigest: map[string]catalogue.Manifest{mdigest('a'): {Module: "m", Build: &catalogue.Build{Artifacts: artifacts}}}}} + r := &rig{st: newMem(), bm: newBuildModules(), tools: &buildrun.ToolchainSet{}, mf: &fakeManifests{byDigest: map[string]catalogue.Manifest{mdigest('a'): {Module: "m", Build: &catalogue.Build{Artifacts: artifacts}}}}} for _, n := range []string{"ace", "novox"} { r.tools.Put(buildrun.GoBuildModule, n+".go", tcValue(t, "go", "go1.27.1"), false) r.tools.Put(buildrun.NodeBuildModule, n+".typescript", tcValue(t, "typescript", "tsc5"), false) - r.pres.Put(n, presenceValue(t, true), false) } + r.asks = &buildrun.FakeAsks{Answer: r.answer} r.b = &Builder{ - Claimer: Claimer{Store: r.st, By: "novox.builder"}, Runner: buildrun.Runner{Caller: r.bm, Steps: r.bm.steps, Bound: time.Second}, - Toolchains: r.tools, Presence: r.pres, Manifests: r.mf, Logf: t.Logf, + Claimer: Claimer{Store: r.st, By: "novox.builder"}, Asks: buildrun.Asker{Publisher: r.asks, Waiter: r.asks, Bound: 2 * time.Second}, + Toolchains: r.tools, Manifests: r.mf, Logf: t.Logf, } var err error if r.key, err = delivery.BuilderBuildKey("m", fp1); err != nil { @@ -220,15 +255,53 @@ func TestABundleAndAnArchiveAreBuiltOnOneNodeAndDone(t *testing.T) { // 1607/AC7 } } -func TestOneQualifyingNodeBuildsOnce(t *testing.T) { // 1607/AC4, ADR 0478 clause 4 +// 1887/AC1, AC2: a trunk fingerprint with no key is one ask on the Go seat's delivery stream, named in the building key +// before it is put, and the builder calls no node; the step a node wrote for it makes the key done naming its reference. +func TestATrunkFingerprintIsOneAskOnTheDeliveryStream(t *testing.T) { r := newRig(t, goBundle) - r.tools.Put(buildrun.GoBuildModule, "ace.go", nil, true) - v := r.consider(t, "modules/m") - if v.State != delivery.BuildDone || !strings.Contains(v.Step, "on novox") { - t.Errorf("build = %+v, want done with its step saying novox", v) + r.asks.Answer = nil // no node has pulled it yet + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) + var asks []delivery.StepAsk + for deadline := time.Now().Add(2 * time.Second); len(asks) == 0; time.Sleep(time.Millisecond) { + if time.Now().After(deadline) { + t.Fatal("no ask was put") + } + asks = r.asks.Asks() } - if n := len(r.bm.callsOf("build_go_build")); n != 1 { - t.Errorf("%d builds, want one", n) + a := asks[0] + if len(asks) != 1 || a.Kind != delivery.AskDelivery || a.Language != delivery.AskGo || a.Build == nil || a.Build.FP != fp1 { + t.Fatalf("asks = %+v, want one Go delivery ask of the fingerprint", asks) + } + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildBuilding || v.Asks[delivery.AskGo] != a.ID { + t.Fatalf("the key = %+v, want building naming the ask %s", v, a.ID) + } + if n := len(r.bm.callsOf("build_go_build")) + len(r.bm.callsOf("build_go_fetch")); n != 0 { + t.Fatalf("the builder called a node %d times", n) + } + r.asks.End(r.answer(a)) // a node pulled it and ran it + r.b.Wait() + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildDone || !strings.Contains(v.Step, "on ace") || v.Artifacts[0].Reference == "" || len(v.Asks) != 0 { + t.Fatalf("the key = %+v, want done on ace naming the reference", v) + } +} + +// 1887/AC3: a builder that starts again while its ask waits puts no second ask: it waits for the step of the ask the +// building key names. +func TestARestartedBuilderWaitsForTheAskTheKeyNames(t *testing.T) { + r := newRig(t, goBundle) + const id = "mvl0frv18d1957" + data, _ := delivery.EncodeBuilderBuild(delivery.BuilderBuild{State: delivery.BuildBuilding, By: "novox.builder", Manifest: manifestD, + Step: "asked mesh-build-go", Asks: map[string]string{delivery.AskGo: id}}) + r.st.vals[r.key] = data + r.asks.Answer = nil + r.asks.End(delivery.Step{ID: id, Node: "shanks", State: delivery.StepPassed, + Result: []byte(`{"artifacts":[{"artifact":"tool","digest":"` + digest + `","reference":"artifact-store://m/tool/blobs/` + digest + `"}]}`)}) + v := r.consider(t, "modules/m") + if n := len(r.asks.Asks()); n != 0 { + t.Fatalf("%d asks put again", n) + } + if v.State != delivery.BuildDone || !strings.Contains(v.Step, "on shanks") { + t.Fatalf("the key = %+v, want done from the ask's step on shanks", v) } } @@ -247,20 +320,9 @@ func TestAFailingStepFailsTheBuildWithItsReasonAndNodeAndIsTriedAgainAsANewRevis } } -// 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 + r.puller = "ace" // two nodes qualify; an image still builds on one v := r.consider(t, "modules/m") 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) @@ -278,19 +340,9 @@ func TestAnImageGoesToOneNodeAndGetsOneDoneKeyWithItsReference(t *testing.T) { / } } -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 TestABundleAndAnImageOfOneModuleBuildOnOneNodeEach(t *testing.T) { // 1705a r := newRig(t, goBundle, imageArt) - r.b.Health = imageHave(t, "ace", "novox") + r.puller = "ace" v := r.consider(t, "modules/m") if v.State != delivery.BuildDone || len(v.Artifacts) != 2 { t.Fatalf("build = %+v", v) @@ -303,33 +355,9 @@ func TestABundleAndAnImageOfOneModuleBuildOnOneNodeEach(t *testing.T) { // 1705a } } -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") + r.puller = "ace" trunkModule(t, r) r.b.Candidate(context.Background(), "feat%2Fx", candidateValue(t, "feat/x", 7, headX)) r.b.Wait() @@ -348,7 +376,7 @@ func TestACandidatesImageCallCarriesNoPublishAndItsKeyNoReference(t *testing.T) func TestATrunkImageCallCarriesPublish(t *testing.T) { // 1705b r := newRig(t, imageArt) - r.b.Health = imageHave(t, "ace") + r.puller = "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 { @@ -439,7 +467,7 @@ func TestAKeyTheBuilderLeftBuildingIsBuiltAtStartAndDoneKeysAreNotBuiltTwice(t * // whose compare-and-set comes first, and the other's write is refused, never overwriting it. func TestTwoInstancesStartingTogetherEndWithOneDoneKey(t *testing.T) { // 1607/AC7, review round 4 r := newRig(t, goBundle) - other := &Builder{Claimer: Claimer{Store: r.st, By: "ace.builder"}, Runner: r.b.Runner, Toolchains: r.tools, Presence: r.pres, Manifests: r.mf, Logf: t.Logf} + other := &Builder{Claimer: Claimer{Store: r.st, By: "ace.builder"}, Asks: r.b.Asks, Toolchains: r.tools, Manifests: r.mf, Logf: t.Logf} value := moduleValue(t, "modules/m") var wg sync.WaitGroup for _, b := range []*Builder{r.b, other} { @@ -522,36 +550,9 @@ func TestARootModulesPathIsDotAndIsGivenToTheBuildTools(t *testing.T) { // 1615/ } } -func TestANodeThatIsNotOnlineIsNotChosenAndComingBackWakesTheBuild(t *testing.T) { // 1607 review 3 - r := newRig(t, goBundle) - r.tools.Put(buildrun.GoBuildModule, "g14.go", tcValue(t, "go", "go1.27.1"), false) // a laptop that reports its toolchain and sleeps - v := r.consider(t, "modules/m") - if v.State != delivery.BuildDone { - t.Fatalf("build = %+v", v) - } - for _, c := range r.bm.calls { - if c.node == "g14" { - t.Errorf("a call went to g14, which is not online: %+v", c) - } - } - // With no node online the key fails naming why; a node coming online builds it as a new revision. - r2 := newRig(t, goBundle) - r2.pres.Put("ace", nil, true) - r2.pres.Put("novox", nil, true) - if v := r2.consider(t, "modules/m"); v.State != delivery.BuildFailed || !strings.Contains(v.Error, "online") { - t.Fatalf("build = %+v, want failed because no node is online", v) - } - r2.pres.Put("ace", presenceValue(t, true), false) - r2.b.Reconsider(context.Background()) - r2.b.Wait() - if v := valueOf(t, r2.st, r2.key); v.State != delivery.BuildDone { - t.Errorf("build = %+v, want done once a node is online", v) - } -} - func TestACandidatesImageIsPublishedOnceWhenTheTrunkNamesItsFingerprint(t *testing.T) { // review r1, mesh-delivery 1; ADR 0487 clause 5 r := newRig(t, imageArt) - r.b.Health = imageHave(t, "ace") + r.puller = "ace" trunkModule(t, r) // the trunk carries another fingerprint; fp1 is the candidate's r.b.Candidate(context.Background(), "feat%2Fx", candidateValue(t, "feat/x", 7, headX)) r.b.Wait() @@ -590,7 +591,7 @@ func TestACandidatesImageIsPublishedOnceWhenTheTrunkNamesItsFingerprint(t *testi func TestAFailedPublishOfACandidatesImageIsWrittenToTheKey(t *testing.T) { // review r2, mesh-delivery 1; ADR 0487 clause 5 r := newRig(t, imageArt) - r.b.Health = imageHave(t, "ace") + r.puller = "ace" trunkModule(t, r) r.b.Candidate(context.Background(), "feat%2Fx", candidateValue(t, "feat/x", 7, headX)) r.b.Wait() diff --git a/modules/mesh-delivery-builder/manifest_test.go b/modules/mesh-delivery-builder/manifest_test.go index 6f13144..930b0d8 100644 --- a/modules/mesh-delivery-builder/manifest_test.go +++ b/modules/mesh-delivery-builder/manifest_test.go @@ -38,6 +38,7 @@ type manifest struct { Consumes []string `json:"consumes"` State []state `json:"state"` Reads []string `json:"reads"` + Uses []string `json:"uses"` Invokes []string `json:"invokes"` Config map[string]struct { Type string `json:"type"` @@ -86,12 +87,17 @@ func TestManifestDeclarations(t *testing.T) { // 1607/AC1 if want := []state{{Name: "builds", History: 5}, {Name: "instances", History: 1}}; !reflect.DeepEqual(m.State, want) { // delivery.BuilderBuildsHistory t.Errorf("state = %+v, want %+v", m.State, want) } + // 1887/A4c: the builder chooses no node, so it reads neither presence nor have, invokes no build module's tool and + // holds no capacity; it puts asks on the three build seats and reads their steps (ADR 0525 §1, §4, §5). 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) { + "mesh-delivery-indexer.candidates", "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules"}; !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-image.build_image_build", "mesh-build-image.build_image_copy", "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) and the image module's copy (ADR 0498 clause 4)", m.Invokes) + 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) } // 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) { @@ -103,7 +109,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", "uses", "defines-seats", "tools", "seat", "seats", "bus"} { + for _, k := range []string{"streams", "logs", "emits", "claims", "provides", "requires", "defines-seats", "tools", "seat", "seats", "bus", "invokes", "config"} { if _, ok := top[k]; ok { t.Errorf("the manifest declares %q: the builder has none", k) } @@ -129,35 +135,19 @@ func TestManifestProcess(t *testing.T) { // 1607/AC1 t.Errorf("process = %v", p) } env, _ := p["env"].(map[string]any) - // The build capacity of each node is the module's setting `capacity` (issue 1661, ADR 0478 clause 3), laid into the - // settings file the program reads; empty is one step at a time everywhere. The environment holds no capacity. - if len(env) != 2 || env["MESH_BROKER_FILE"] != "${dir:state}/broker" || env["MESH_BUILDER_SETTINGS"] != "${dir:state}/settings.json" { - t.Errorf("env = %v", p["env"]) + if len(env) != 1 || env["MESH_BROKER_FILE"] != "${dir:state}/broker" { + t.Errorf("env = %v, want the broker file alone", p["env"]) } - if !reflect.DeepEqual(p["restart-on"], []any{"settings"}) { - t.Errorf("restart-on = %v, want the settings file, so a changed capacity reaches the running program", p["restart-on"]) - } - c, ok := m.Config["capacity"] - if !ok || len(m.Config) != 1 || c.Type != "string" || c.Default != "shanks=3,ace=2,g14=0,novox=0" || c.Description == "" { - t.Errorf("config = %+v, want one string setting, capacity, shanks=3,ace=2,g14=0,novox=0 to start", m.Config) - } - var settings map[string]any for _, r := range m.Resources { if r["id"] == "settings" { - settings = r + t.Errorf("a settings resource %v: the builder holds no setting since the capacity is the build modules' (ADR 0525 §5)", r) } } - if settings == nil || settings["type"] != "file" || settings["merge"] != "json" || !reflect.DeepEqual(settings["takes-config"], []any{"capacity"}) || settings["path"] != "${dir:state}/settings.json" { - t.Errorf("settings resource = %v", settings) - } - if raw, err := os.ReadFile("files/settings.json"); err != nil || !strings.Contains(string(raw), `"capacity": "shanks=3,ace=2,g14=0,novox=0"`) { - t.Errorf("files/settings.json = %q, %v, want the setting's starting value", raw, err) - } if _, bus := p["bus"]; bus { t.Errorf("the process declares a bus: it runs on its own broker credential") } - if len(m.Build.Artifacts) != 2 || m.Build.Artifacts[1].Name != "files" || m.Build.Artifacts[1].Kind != "archive" || m.Build.Artifacts[1].From != "files" { - t.Fatalf("artifacts = %+v, want the program and the files", m.Build.Artifacts) + if len(m.Build.Artifacts) != 1 { + t.Fatalf("artifacts = %+v, want the program alone", m.Build.Artifacts) } a := m.Build.Artifacts[0] if st, err := os.Stat(a.From); err != nil || !st.IsDir() || a.Binary != "mesh-delivery-builder" || !strings.HasSuffix(a.From, a.Binary) || a.Kind != "bundle" || a.Language != "go" { diff --git a/modules/mesh-delivery-builder/module.json b/modules/mesh-delivery-builder/module.json index f18e708..798cb79 100644 --- a/modules/mesh-delivery-builder/module.json +++ b/modules/mesh-delivery-builder/module.json @@ -18,27 +18,14 @@ "mesh-build-node.toolchains", "mesh-delivery-indexer.candidates", "mesh-delivery-indexer.manifests", - "mesh-delivery-indexer.modules", - "mesh-delivery-watchman.presence", - "node-engine.have" + "mesh-delivery-indexer.modules" ], - "invokes": [ - "mesh-build-go.build_go_build", - "mesh-build-go.build_go_fetch", - "mesh-build-image.build_image_build", - "mesh-build-image.build_image_copy", - "mesh-build-image.build_image_fetch", - "mesh-build-node.build_node_build", - "mesh-build-node.build_node_fetch" + "uses": [ + "node-build-go", + "node-build-image", + "node-build-node" ], - "config": { - "capacity": { - "type": "string", - "description": "How many build steps the builder runs at once on each node, as node=steps pairs separated by commas, such as shanks=3,ace=2 (ADR 0478 clause 3); a node set to 0 never builds, a node not named runs one at a time, and empty is one everywhere. The builder builds on shanks and ace only (the operator, 2026-10-21).", - "pattern": "|[a-z0-9_-]+=[0-9]+(,[a-z0-9_-]+=[0-9]+)*", - "default": "shanks=3,ace=2,g14=0,novox=0" - } - }, + "own-secrets": { "broker": "${dir:state}/broker" }, @@ -59,17 +46,6 @@ "place": ".", "unknown": "program" }, - { - "id": "settings", - "trusted": false, - "type": "file", - "path": "${dir:state}/settings.json", - "mode": "0600", - "owner": "mesh-delivery-builder", - "takes-config": ["capacity"], - "merge": "json", - "template": "files/settings.json" - }, { "id": "builder", "type": "process", @@ -79,12 +55,8 @@ "./mesh-delivery-builder" ], "env": { - "MESH_BROKER_FILE": "${dir:state}/broker", - "MESH_BUILDER_SETTINGS": "${dir:state}/settings.json" + "MESH_BROKER_FILE": "${dir:state}/broker" }, - "restart-on": [ - "settings" - ], "user": "mesh-delivery-builder", "health": { "kind": "unit" @@ -100,11 +72,6 @@ "system": "arch", "from": "cmd/mesh-delivery-builder", "binary": "mesh-delivery-builder" - }, - { - "name": "files", - "kind": "archive", - "from": "files" } ] } From e4b639c6defa30c0f26dbec99af5f6db932e7893 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 16:05:46 +0200 Subject: [PATCH 4/5] =?UTF-8?q?Judge=20each=20pull=20request's=20check=20a?= =?UTF-8?q?s=20one=20ask=20on=20the=20build=20seats'=20check=20stream,=20a?= =?UTF-8?q?=20re-judge=20on=20recheck,=20so=20the=20checker=20chooses=20no?= =?UTF-8?q?=20node=20and=20a=20lost=20node's=20check=20is=20redelivered,?= =?UTF-8?q?=20not=20moved=20(issue=201887=20A5a,=20ADR=200525=20=C2=A71,?= =?UTF-8?q?=20=C2=A73,=20ADR=200536=20=C2=A74)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../mesh-delivery-checker/integration_test.go | 81 +-- .../cmd/mesh-delivery-checker/judge.go | 7 +- .../cmd/mesh-delivery-checker/main.go | 8 +- .../internal/check/case.go | 3 + .../internal/check/checker.go | 3 +- .../internal/check/examiner_test.go | 141 ++--- .../internal/check/heard.go | 84 +-- .../internal/check/heard_test.go | 125 +---- .../internal/check/judge.go | 492 ++++++------------ .../internal/check/pins.go | 12 +- .../internal/check/pins_test.go | 12 +- .../internal/check/try_test.go | 13 +- 12 files changed, 325 insertions(+), 656 deletions(-) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go index e2f4c15..502c82f 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go @@ -5,6 +5,7 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -20,6 +21,7 @@ import ( "github.com/nats-io/nats.go/jetstream" "git.novox.be/novox/mesh-sdk/go/announce" + "git.novox.be/novox/mesh-sdk/go/buildstep" "git.novox.be/novox/mesh-sdk/go/delivery" "mesh-delivery-checker/internal/check" @@ -75,7 +77,6 @@ func readCaptured(t *testing.T, name string) map[string]any { type fakeBuildModules struct { js jetstream.JetStream ctx context.Context - n atomic.Int64 conflict atomic.Bool mu sync.Mutex calls map[string]int // by `.` @@ -112,45 +113,55 @@ func (f *fakeBuildModules) result(t *testing.T, tool string, args map[string]any return data } -func (f *fakeBuildModules) serve(t *testing.T, nc *nats.Conn, module string) { - steps, err := f.js.CreateKeyValue(f.ctx, jetstream.KeyValueConfig{Bucket: delivery.StepsBucket(module), History: 1}) +// serve makes module's steps bucket and its build seat's stream and workers, and pulls its asks on n1 as the real module +// does since R4g (ADR 0525; issue 1887 A5): each ask's job runs with step bodies answering the captured results, and a +// check takes the kept verdicts of the checker's kept bucket by their digests. +func (f *fakeBuildModules) serve(t *testing.T, module string, kept jetstream.KeyValue) { + kv, err := f.js.CreateKeyValue(f.ctx, jetstream.KeyValueConfig{Bucket: delivery.StepsBucket(module), History: 1}) if err != nil { t.Fatal(err) } - sub, err := nc.Subscribe("mesh.mod."+module+".call.>", func(m *nats.Msg) { - tokens := strings.Split(m.Subject, ".") // mesh mod call - tool, node := tokens[4], tokens[5] - f.mu.Lock() - f.calls[module+"."+tool]++ - f.mu.Unlock() - var args map[string]any - _ = json.Unmarshal(m.Data, &args) - id := fmt.Sprintf("s%d", f.n.Add(1)) - key, _ := delivery.StepKey(node, id) - put := func(s delivery.Step) { - data, err := delivery.EncodeStep(s) - if err == nil { - _, err = steps.Put(f.ctx, key, data) + language := strings.TrimPrefix(module, "mesh-build-") + if err := buildstep.DeclareAskWorkers(f.ctx, f.js, language, 30*time.Second); err != nil { + t.Fatal(err) + } + prefix := map[string]string{"mesh-build-go": "build_go_", "mesh-build-node": "build_node_"}[module] + call := func(name string) buildstep.StepFunc { + return func(ctx context.Context, args map[string]any) (any, error) { + tool := prefix + name + f.mu.Lock() + f.calls[module+"."+tool]++ + f.mu.Unlock() + return f.result(t, tool, args), nil + } + } + steps := buildstep.AskSteps{Fetch: call("fetch"), Test: call("test"), + Kept: func(ctx context.Context, level, digest string) (int, bool, error) { + key, err := delivery.KeptKey(level, digest) + if err != nil { + return 0, false, err + } + e, err := kept.Get(ctx, key) + if errors.Is(err, jetstream.ErrKeyNotFound) { + return 0, false, nil } if err != nil { - t.Errorf("writing step %s: %v", key, err) + return 0, false, err } - } - step := delivery.Step{ID: id, Tool: tool, Node: node, State: delivery.StepRunning, Started: time.Now().UTC()} - put(step) - answer, _ := json.Marshal(map[string]any{"result": map[string]any{"step": id, "key": key, "state": "running"}, "error": "", "node": node}) - _ = m.Respond(answer) - go func() { - time.Sleep(20 * time.Millisecond) - step.Ended = time.Now().UTC() - step.State, step.Result = delivery.StepPassed, f.result(t, tool, args) - put(step) - }() - }) - if err != nil { - t.Fatal(err) + v, err := delivery.DecodeKept(e.Value()) + return v.Tests, err == nil, err + }} + if module == "mesh-build-go" { + steps.Digest = call("digest") } - t.Cleanup(func() { _ = sub.Unsubscribe() }) + put := func(key string, value any) (uint64, error) { + raw, _ := value.(json.RawMessage) + return kv.Put(f.ctx, key, raw) + } + p := &buildstep.Puller{Source: &buildstep.JetStreamAsks{JS: f.js, Language: language}, Language: language, Node: "n1", Put: put, + Capacity: func() int { return 2 }, PullWait: 200 * time.Millisecond, + Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { return buildstep.RunAsk(ctx, steps, a) }} + go func() { _ = p.Pull(f.ctx) }() } // serveForge answers the forge's two reads as its tools do, with the answers the forge gave (internal/check/testdata/captured): @@ -283,8 +294,8 @@ func startBed(t *testing.T) *bed { } b.serveForge(t, ctl) 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, "mesh-build-go", b.buckets[delivery.CheckerKeptBucket]) + b.fake.serve(t, "mesh-build-node", b.buckets[delivery.CheckerKeptBucket]) b.report(t, "mesh-build-go", "n1", "go", "go1.27.1", "1") b.report(t, "mesh-build-node", "n1", "typescript", "tsc5", "1") b.setTrunk(t, map[string]string{repo: trunk}) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go index 7db6c2b..8ea203b 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -15,11 +15,8 @@ import ( // judgeDeps are the parts the judge of a check is made of. type judgeDeps struct { - runner buildrun.Runner + asks check.Asks tools *buildrun.ToolchainSet - presence *buildrun.PresenceSet - have *buildrun.HaveSet - places *buildrun.Places trunks *check.Trunks kept check.KV previews check.KV @@ -31,7 +28,7 @@ type judgeDeps struct { } func newJudge(d judgeDeps) check.Judge { - return &check.Examiner{Steps: d.runner, Toolchains: d.tools, Presence: d.presence, Health: d.have, Places: d.places, + return &check.Examiner{Asks: d.asks, Toolchains: d.tools, Kept: check.Kept{Store: d.kept}, Previews: d.previews, Modules: d.repos, Packages: d.repos, Forge: d.forge, Runs: d.repos, Builds: d.builds, Announce: d.announce, Trunks: d.trunks, Logf: d.logf} } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go index 41377e8..fb98e28 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -201,8 +201,10 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro presence := &buildrun.PresenceSet{} have := &buildrun.HaveSet{} trunks := &check.Trunks{} - places := &buildrun.Places{Capacity: capacity} - runner := buildrun.Runner{Caller: heardCaller{caller.CallOn}, Steps: stepsWaiter{js}, Bound: stepBound} + _ = capacity + // A check is one ask per pull request and build module on the build seats' check stream, pulled by a build node with a + // free slot (novox/hq ADR 0525 §1, §3): the checker chooses no node and starts no step. + asks := buildrun.JetStreamAsks{JS: js} ch := &check.Checker{ Verdicts: kvStore{buckets[delivery.CheckerVerdictsBucket]}, Owed: kvStore{buckets[delivery.CheckerOwedBucket]}, @@ -235,7 +237,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro } repos := &repoModules{manifests: kvStore{buckets[delivery.ManifestsBucket]}, logf: logger.Printf} ch.Judge = newJudge(judgeDeps{ - runner: runner, tools: tools, presence: presence, have: have, places: places, trunks: trunks, + asks: buildrun.Asker{Publisher: asks, Waiter: asks, Bound: stepBound}, tools: tools, trunks: trunks, kept: kvStore{buckets[delivery.CheckerKeptBucket]}, previews: kvStore{buckets[delivery.PreviewsBucket]}, repos: repos, builds: builds, forge: forgeReads{call: caller.Call}, logf: logger.Printf, announce: announce, diff --git a/modules/mesh-delivery-checker/internal/check/case.go b/modules/mesh-delivery-checker/internal/check/case.go index 4240ed4..e7e8654 100644 --- a/modules/mesh-delivery-checker/internal/check/case.go +++ b/modules/mesh-delivery-checker/internal/check/case.go @@ -49,6 +49,9 @@ type Case struct { Try *TryOptions // Progress says what the check is doing, so a check that ran past its bound names the step it was stuck at. Progress *Progress + // Kind is what asked the check (KindNew, KindNodes, KindTrunk): a re-judge after a trunk move is asked on the build + // seats' recheck stream, every other check on check (ADR 0536 §4). + Kind string } // Progress is the step a check is at, written by the check and read when its bound passes. diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 33f795e..850b7fe 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -207,7 +207,8 @@ func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidat go func() { defer ch.wg.Done() for { - ch.admit(cs, ch.kindOf(ctx, cs)) + cs.Kind = ch.kindOf(ctx, cs) // a re-judge after a trunk move is asked on the recheck stream (ADR 0536 §4) + ch.admit(cs, cs.Kind) ch.try(ctx, key, cs) ch.leave() ch.mu.Lock() diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index 88ce8fb..b90ae93 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -15,6 +15,7 @@ import ( "testing" "time" + "git.novox.be/novox/mesh-sdk/go/buildstep" "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" ) @@ -108,6 +109,7 @@ const digD = "dd44dd44dd44dd44dd44dd44dd44dd44dd44dd44dd44dd44dd44dd44dd44dd44" type bench struct { ex *Examiner + asks *buildrun.FakeAsks steps *fakeSteps kept *memKV prev *memKV @@ -116,6 +118,50 @@ type bench struct { test func(c stepCall) (any, error) } +// refsOf reads the siblings of a fetch's arguments, as they arrive in an ask (JSON) or as the checker gave them. +func refsOf(v any) []BesideRef { + raw, _ := json.Marshal(v) + var refs []BesideRef + _ = json.Unmarshal(raw, &refs) + return refs +} + +// toolPrefix is the start of each build module's tool names, as the fake build modules answer them. +var toolPrefix = map[string]string{buildrun.GoBuildModule: "build_go_", buildrun.NodeBuildModule: "build_node_"} + +// answer runs a check ask as the build module that pulls it does (buildstep.RunAsk), with the fake tools on shanks and the +// bench's kept verdicts, and answers the step it writes. +func (b *bench) answer(a delivery.StepAsk) delivery.Step { + module, _ := delivery.BuildModuleOf(a.Language) + const node = "shanks" + tool := func(name string) buildstep.StepFunc { + return func(ctx context.Context, args map[string]any) (any, error) { + raw, err := b.steps.Run(ctx, module, node, toolPrefix[module]+name, args) + if err != nil { + return nil, err + } + return raw, nil + } + } + steps := buildstep.AskSteps{Fetch: tool("fetch"), Test: tool("test"), + Kept: func(ctx context.Context, level, digest string) (int, bool, error) { + k, found, err := (Kept{Store: b.kept}).Lookup(ctx, level, digest) + return k.Tests, found, err + }} + if module == buildrun.GoBuildModule { + steps.Digest = tool("digest") + } + out, err := buildstep.RunAsk(context.Background(), steps, a) + began := time.Date(2026, 10, 21, 19, 0, 0, 0, time.UTC) + step := delivery.Step{ID: a.ID, Tool: a.Kind + " ask", Node: node, State: delivery.StepPassed, Started: began, Ended: began.Add(2 * time.Second)} + if err != nil { + step.State, step.Reason = delivery.StepFailed, err.Error() + return step + } + step.Result, _ = json.Marshal(out) + return step +} + // newBench is an examiner over fake build modules that merge cleanly, digest the captured fixture's packages and pass // the captured slow fixture's result at both levels, on shanks. func newBench(t testing.TB) *bench { @@ -142,7 +188,8 @@ func newBench(t testing.TB) *bench { "build_node_test": func(c stepCall) (any, error) { return b.test(c) }, } now := time.Date(2026, 10, 21, 19, 0, 0, 0, time.UTC) - b.ex = &Examiner{Steps: b.steps, Toolchains: goEntries("shanks"), Places: &buildrun.Places{}, Kept: Kept{Store: b.kept}, Previews: b.prev, + b.asks = &buildrun.FakeAsks{Answer: b.answer} + b.ex = &Examiner{Asks: buildrun.Asker{Publisher: b.asks, Waiter: b.asks, Bound: 5 * time.Second}, Toolchains: goEntries("shanks"), Kept: Kept{Store: b.kept}, Previews: b.prev, Modules: langs{}, Trunks: &Trunks{}, Now: func() time.Time { now = now.Add(50 * time.Millisecond); return now }} return b } @@ -353,33 +400,6 @@ func TestAHeadThatDoesNotMergeEndsFailedNamingTheFilesAndRunsNoTest(t *testing.T } } -func TestNoQualifyingNodeIsAFailureOfTheNodes(t *testing.T) { // N6/AC4 - b := newBench(t) - b.ex.Toolchains = entries{} - out := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) - if !IsNodesFailure(out.Verdict) { - t.Fatalf("verdict = %+v, want a failure of the nodes", out.Verdict) - } - b.ex.Toolchains = goEntries("shanks") - b.ex.Presence = offline{} - if out = b.judge(t, caseOf(t, pullOf(repoA, 7, head1))); !IsNodesFailure(out.Verdict) { - t.Fatalf("an offline node: verdict = %+v", out.Verdict) - } -} - -type offline struct{} - -func (offline) Online(string) bool { return false } - -func TestAStepThatFailsIsAVerdictNamingTheNode(t *testing.T) { - b := newBench(t) - b.steps.handlers["build_go_fetch"] = func(stepCall) (any, error) { return nil, fmt.Errorf("the step failed: ssh refused") } - out := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) - if f := out.Verdict.Failed; out.Verdict.State != delivery.VerdictFail || f.Machine != "shanks" || !strings.Contains(f.Words, "ssh refused") { - t.Fatalf("verdict = %+v", out.Verdict) - } -} - func TestALevelWhoseDigestIsNotNamedRunsWholeAndKeepsNothing(t *testing.T) { // 1287/AC4 b := newBench(t) b.steps.handlers["build_go_digest"] = func(stepCall) (any, error) { return nil, fmt.Errorf("go list failed") } @@ -441,7 +461,7 @@ func TestAGroupIsOneVerdictAndEachMembersTestsReadTheOthersAtTheirHeads(t *testi var forA []BesideRef for _, f := range fetch { if f.args["repository"] == repoA { - forA, _ = f.args["beside"].([]BesideRef) + forA, _ = refsOf(f.args["beside"]), true } } found := false @@ -599,7 +619,7 @@ func TestNothingIsKeptForAWorkspaceGivenSiblingsWhenTheDigestDoesNotSayItCoversT func echoBeside(b *bench, d Digested) Digested { var lines []string for _, f := range b.steps.tool("fetch") { - refs, _ := f.args["beside"].([]BesideRef) + refs, _ := refsOf(f.args["beside"]), true for _, r := range refs { lines = append(lines, r.Repository+" "+r.Commit) } @@ -614,7 +634,7 @@ func besideDigest(b *bench) func(c stepCall) (any, error) { level := c.args["level"].(string) var lines []string for _, f := range b.steps.tool("fetch") { - refs, _ := f.args["beside"].([]BesideRef) + refs, _ := refsOf(f.args["beside"]), true for _, r := range refs { lines = append(lines, r.Repository+" "+r.Commit) } @@ -741,40 +761,37 @@ func TestNoStepOfACheckCarriesPublish(t *testing.T) { // ADR 0500 §4, 1309 (the } } -// keyCaller is a build module that starts every long step and answers its key on the node it was asked on. -type keyCaller struct{} - -func (keyCaller) CallOn(node, key string, _ any) (json.RawMessage, error) { - return json.Marshal(map[string]string{"step": "lost1", "key": node + ".lost1", "state": delivery.StepRunning}) -} - -// lostSteps is a steps state whose key never ends: the build module stopped, restarted or was unassigned with the step -// running, and nothing writes its end. -type lostSteps struct{} - -func (lostSteps) Wait(ctx context.Context, _, _ string) (delivery.Step, error) { - <-ctx.Done() - return delivery.Step{}, ctx.Err() -} - -func TestAStepThatNeverEndsEndsTheCheckLoudlyAndFreesItsNodesPlace(t *testing.T) { // 1890/AC1 +// 1890/AC1, 1887/A5a: an ask whose step never ends (the build node stopped with it running, and nothing redelivered it in +// time) ends the check loudly at the asker's bound; the checker holds no place to free. +func TestAnAskWhoseStepNeverEndsEndsTheCheckLoudly(t *testing.T) { b := newBench(t) - places := &buildrun.Places{Capacity: map[string]int{"shanks": 1}} - b.ex.Places = places - b.ex.Steps = buildrun.Runner{Caller: keyCaller{}, Steps: lostSteps{}, Bound: 20 * time.Millisecond} + b.asks.Answer = nil + b.ex.Asks = buildrun.Asker{Publisher: b.asks, Waiter: b.asks, Bound: 20 * time.Millisecond} out := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) - if f := out.Verdict.Failed; out.Verdict.State != delivery.VerdictFail || f.Machine != "shanks" || !strings.Contains(f.Words, "shanks.lost1 did not end within") { + if f := out.Verdict.Failed; out.Verdict.State != delivery.VerdictFail || !strings.Contains(f.Words, "did not end within") { t.Fatalf("verdict = %+v", out.Verdict) } - if r, res := places.Running(), places.Reserved(); len(r) != 0 || len(res) != 0 { - t.Fatalf("the lost step still holds shanks: running %v, reserved %v", r, res) + if n := len(b.asks.Asks()); n != 1 || b.asks.Asks()[0].Kind != delivery.AskCheck || b.asks.Asks()[0].Check == nil { + t.Fatalf("asks = %+v, want one check ask", b.asks.Asks()) + } +} + +// 1887/A5a, ADR 0536 §4: a check is one ask per pull request and build module on the check stream, a re-judge after a +// trunk move one on the recheck stream, each carrying both levels with the kept verdicts taken by the build module. +func TestACheckIsOneAskAndAReJudgeGoesOnTheRecheckStream(t *testing.T) { + b := newBench(t) + b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) + asks := b.asks.Asks() + if len(asks) != 1 || asks[0].Kind != delivery.AskCheck || asks[0].Language != delivery.AskGo || asks[0].Asker != delivery.CheckerModule { + t.Fatalf("asks = %+v, want one Go check ask", asks) + } + if j := asks[0].Check; j == nil || !j.Kept || strings.Join(j.Levels, ",") != "unit,integration" || j.Fetch["repository"] != repoA { + t.Fatalf("the job = %+v", asks[0].Check) + } + cs := caseOf(t, pullOf(repoA, 7, head1)) + cs.Kind = KindTrunk + b.judge(t, cs) + if asks := b.asks.Asks(); len(asks) != 2 || asks[1].Kind != delivery.AskRecheck { + t.Fatalf("asks = %+v, want the re-judge on recheck", asks) } - // The node's one place is free: the next check takes it at once. - ctx, cancel := context.WithTimeout(context.Background(), time.Second) - defer cancel() - release, err := places.Acquire(ctx, "shanks") - if err != nil { - t.Fatalf("shanks's place was not freed: %v", err) - } - release() } diff --git a/modules/mesh-delivery-checker/internal/check/heard.go b/modules/mesh-delivery-checker/internal/check/heard.go index d97c17f..b10bd09 100644 --- a/modules/mesh-delivery-checker/internal/check/heard.go +++ b/modules/mesh-delivery-checker/internal/check/heard.go @@ -1,18 +1,14 @@ package check import ( - "context" - "encoding/json" "errors" - "fmt" - "strings" "github.com/nats-io/nats.go" ) // Unheard is the error of a call that reached nobody: the bus had no responder for it, or none answered within the call's -// bound. The step it asked for never began, so the same step may be asked of another node (issue 1820). The program wraps -// the errors of its calls in it where the bus gives a timeout; a no-responders error is known as it is. +// bound. The program wraps the errors of its forge calls in it where the bus gives a timeout; a no-responders error is +// known as it is. A build step is no call since R4g: an ask whose node stops is delivered again by its stream (ADR 0525 §6). type Unheard struct{ Err error } func (u *Unheard) Error() string { return u.Err.Error() } @@ -23,79 +19,3 @@ func isUnheard(err error) bool { var u *Unheard return errors.As(err, &u) || errors.Is(err, nats.ErrNoResponders) } - -// deafNode is a node that did not answer a build module's call during this judging. -type deafNode struct{ node, why string } - -// markDeaf takes a node out of the choice for a build module for the rest of the judging. -func (st *state) markDeaf(bm, node string, err error) { - if st.deaf == nil { - st.deaf = map[string][]deafNode{} - } - st.deaf[bm] = append(st.deaf[bm], deafNode{node, err.Error()}) -} - -func (st *state) isDeaf(bm, node string) bool { - for _, d := range st.deaf[bm] { - if d.node == node { - return true - } - } - return false -} - -// triedWords names every node that did not answer a build module, with what the bus said. -func (st *state) triedWords(bm string) string { - var parts []string - for _, d := range st.deaf[bm] { - parts = append(parts, fmt.Sprintf("%s (%s)", d.node, d.why)) - } - return strings.Join(parts, "; ") -} - -// moveStep is called when the target's node did not answer: the node is left out of the choice, and another build node of the -// same build module takes the target's place. It answers an error naming every node tried when none is left (the words hold -// NodesWords, so the verdict is judged again when the nodes change). -func (st *state) moveStep(t *target, err error) error { - old := t.node - st.markDeaf(t.module, old, err) - node, _, cerr := st.choose(t.module) - if cerr != nil { - return fmt.Errorf("%s; tried: %s", cerr, st.triedWords(t.module)) - } - st.j.logf("%s#%d: %s did not answer (%v), the step goes to %s", t.pull.Repository, t.pull.Number, old, err, node) - t.node = node - other := false - for _, o := range st.targets { - if o != t && o.node == old { - other = true - } - } - if !other { - delete(st.nodes, old) - } - st.nodes[node] = true - return nil -} - -// refetch makes the target's workspace again on its new node: the workspace of a step that was lost is on the node that did -// not answer. -func (st *state) refetch(ctx context.Context, t *target) error { - raw, err := st.step(ctx, t, "fetch", fmt.Sprintf("fetch %s#%d on %s", t.pull.Repository, t.pull.Number, t.node), t.fetchArgs) - if err != nil { - return err - } - var f Fetched - if err := json.Unmarshal(raw, &f); err != nil || f.Workspace == "" { - return fmt.Errorf("the fetch of %s#%d on %s answered no workspace: %s", t.pull.Repository, t.pull.Number, t.node, raw) - } - t.fetched = f - return nil -} - -// stepOnWorkspace sets the workspace a step reads to the target's current one. -func stepOnWorkspace(t *target, args map[string]any) { - if _, ok := args["workspace"]; ok { - args["workspace"] = t.fetched.Workspace - } -} diff --git a/modules/mesh-delivery-checker/internal/check/heard_test.go b/modules/mesh-delivery-checker/internal/check/heard_test.go index 784ffc7..3072d8c 100644 --- a/modules/mesh-delivery-checker/internal/check/heard_test.go +++ b/modules/mesh-delivery-checker/internal/check/heard_test.go @@ -1,9 +1,7 @@ package check import ( - "context" "fmt" - "strings" "testing" "github.com/nats-io/nats.go" @@ -11,127 +9,8 @@ import ( "git.novox.be/novox/mesh-sdk/go/delivery" ) -// Issue 1820: a build step goes to a node whose build module is not heard (a laptop asleep, not yet marked offline by the -// watchman), and the bus answers "no responders". The check moves the step to another node of the same build module. - -// deafNodes makes every tool on the named nodes fail as the bus does when nobody listens there; count says how many -// times the node was called. -func deafNodes(b *bench, deaf ...string) { - for tool, h := range b.steps.handlers { - h := h - b.steps.handlers[tool] = func(c stepCall) (any, error) { - for _, n := range deaf { - if c.node == n { - return nil, fmt.Errorf("%s on %s: %w", c.tool, c.node, nats.ErrNoResponders) - } - } - return h(c) - } - } -} - -func nodesOf(calls []stepCall) []string { - var out []string - for _, c := range calls { - out = append(out, c.node) - } - return out -} - -func TestAFetchThatNobodyAnswersMovesToAnotherBuildNode(t *testing.T) { // 1820/AC1 - b := newBench(t) - b.ex.Toolchains = goEntries("ace", "g14") - deafNodes(b, "ace") - v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict - if v.State != delivery.VerdictPass { - t.Fatalf("verdict = %+v, want a pass on the node that answers", v) - } - if got := nodesOf(b.steps.tool("fetch")); len(got) != 2 || got[0] != "ace" || got[1] != "g14" { - t.Errorf("fetch went to %v, want ace (no responders) then g14", got) - } - for _, c := range b.steps.tool("test") { - if c.node != "g14" { - t.Errorf("a test went to %s, want g14 only", c.node) - } - } - if len(v.Machines) != 1 || v.Machines[0] != "g14" { - t.Errorf("machines = %v, want [g14]: ace ran nothing", v.Machines) - } -} - -func TestATestStepThatNobodyAnswersIsFetchedAgainOnAnotherNode(t *testing.T) { // 1820/AC2 - b := newBench(t) - b.ex.Toolchains = goEntries("ace", "g14") - // ace answers the fetch and the digest, then goes to sleep before the test. - test := b.steps.handlers["build_go_test"] - b.steps.handlers["build_go_test"] = func(c stepCall) (any, error) { - if c.node == "ace" { - return nil, fmt.Errorf("test on ace: %w", nats.ErrNoResponders) - } - return test(c) - } - v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict - if v.State != delivery.VerdictPass { - t.Fatalf("verdict = %+v, want a pass", v) - } - if got := nodesOf(b.steps.tool("fetch")); len(got) != 2 || got[1] != "g14" { - t.Errorf("fetch went to %v, want ace then g14 (the workspace was on ace)", got) - } - for _, c := range b.steps.tool("test") { - if c.node == "g14" && c.args["workspace"] == nil { - t.Errorf("a test on g14 has no workspace") - } - } - if len(v.Machines) != 1 || v.Machines[0] != "g14" { - t.Errorf("machines = %v, want [g14]", v.Machines) - } -} - -func TestWhenNoBuildNodeAnswersTheFailureNamesEveryNodeTried(t *testing.T) { // 1820/AC3 - b := newBench(t) - b.ex.Toolchains = goEntries("ace", "g14", "shanks") - deafNodes(b, "ace", "g14", "shanks") - v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict - if v.State != delivery.VerdictFail { - t.Fatalf("verdict = %+v, want a failure", v) - } - for _, n := range []string{"ace", "g14", "shanks"} { - if !strings.Contains(v.Failed.Words, n) { - t.Errorf("failure %q does not name %s", v.Failed.Words, n) - } - } - if !IsNodesFailure(v) { - t.Errorf("failure %q is not a failure of the nodes, so it would not be judged again when they change", v.Failed.Words) - } - if got := len(b.steps.tool("fetch")); got != 3 { - t.Errorf("fetch was tried %d times, want once on each of 3 nodes", got) - } -} - -func TestAStepThatFailedForTheChangeIsNotMovedToAnotherNode(t *testing.T) { // 1820/AC4 - b := newBench(t) - b.ex.Toolchains = goEntries("ace", "g14") - b.steps.handlers["build_go_fetch"] = func(stepCall) (any, error) { return nil, fmt.Errorf("fetch on ace: the step failed: git exploded") } - v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict - if v.State != delivery.VerdictFail || len(b.steps.tool("fetch")) != 1 { - t.Errorf("verdict = %+v after %d fetches, want a failure after one", v, len(b.steps.tool("fetch"))) - } -} - -func TestACanceledCheckIsNotMovedToAnotherNode(t *testing.T) { // 1820/AC4 - b := newBench(t) - b.ex.Toolchains = goEntries("ace", "g14") - ctx, cancel := context.WithCancel(context.Background()) - b.steps.handlers["build_go_fetch"] = func(stepCall) (any, error) { - cancel() - return nil, fmt.Errorf("fetch on ace: %w", nats.ErrNoResponders) - } - _, _ = b.ex.Judge(ctx, caseOf(t, pullOf(repoA, 7, head1))) - if n := len(b.steps.tool("fetch")); n != 1 { - t.Errorf("fetch was tried %d times after the check ended, want 1", n) - } -} - +// Issue 1820 AC5: a read of the forge that nobody answers is the forge's failure, not a build node's. (AC1-AC4 moved a +// step to another node; since R4g an ask whose node stops is delivered again by its stream: ADR 0525 §6.) func TestAPinReadThatNobodyAnswersIsTheForgesFailureNotABuildNodes(t *testing.T) { // 1820/AC5 b := groupBench(t, "cb54756c3865") b.ex.Forge.(*fakeForge).err = fmt.Errorf("gitea.gitea_get_file: %w", nats.ErrNoResponders) diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index dcd07f0..9351d50 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "git.novox.be/novox/mesh-sdk/go/buildstep" "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" ) @@ -51,9 +52,11 @@ type verdictPtr = *delivery.Verdict func failed(machine, words string) verdictPtr { return fail(machine, words) } -// Steps runs a build module's long step on a node and answers its result (buildrun.Runner). -type Steps interface { - Run(ctx context.Context, module, node, tool string, args map[string]any) (json.RawMessage, error) +// Asks puts a check's ask on its build seat's stream and waits for its step (buildrun.Asker; ADR 0525 §1, §6): the +// checker chooses no node, starts no step and holds none; whichever build node pulls the ask runs the check. +type Asks interface { + Put(ctx context.Context, a delivery.StepAsk) error + Wait(ctx context.Context, a delivery.StepAsk) (delivery.Step, error) } // ToolchainEntries are the nodes' toolchain entries of a language (buildrun.ToolchainSet). @@ -67,24 +70,18 @@ type BuildModules interface { BuildModules(repository string) []string } -// toolPrefix is the start of each build module's tool names. -var toolPrefix = map[string]string{buildrun.GoBuildModule: "build_go_", buildrun.NodeBuildModule: "build_node_"} +// askLanguage is the build seat language of each build module the checker asks. +var askLanguage = map[string]string{buildrun.GoBuildModule: delivery.AskGo, buildrun.NodeBuildModule: delivery.AskNode} -// Examiner judges a case through the build modules (ADR 0484 clauses 3, 4, 10, 13): for each pull request it fetches the head -// merged onto its trunk head on a build node, then runs the unit level and the integration level, taking the kept verdicts -// of unchanged packages, running the change's own tests first and stopping at the first failure. +// Examiner judges a case through the build seats (ADR 0484 clauses 3, 4, 10, 13; ADR 0525 §1, §3): for each pull request +// and build module it puts one check ask whose job fetches the head merged onto its trunk head, then runs the unit level +// and the integration level, taking the kept verdicts of unchanged packages, running the change's own tests first and +// stopping at the first failure; it judges the step's answer as it judged the tools' answers. type Examiner struct { - Steps Steps + Asks Asks + // Toolchains are the nodes' toolchains: a kept verdict names the toolchain the nodes agree on. Toolchains ToolchainEntries - // Presence says which nodes are online; Health whether a node's build module is running and healthy. A node that is not - // is not chosen; nil means every node is. - Presence interface{ Online(node string) bool } - Health interface { - Healthy(node, module string) bool - } - // Places holds a step back while its node is at the checker's capacity (ADR 0484 clause 12). - Places *buildrun.Places - Kept Kept + Kept Kept // Previews is the previews bucket; nil writes none. Previews KV Modules BuildModules @@ -132,22 +129,22 @@ func besideLines(refs []BesideRef) []string { return out } -// target is a pull request on one build module: the node chosen, the workspace its fetch made and what it said. +// target is a pull request on one build module: the node that pulled its ask, the workspace its fetch made and what it +// said. type target struct { pull Pull module string node string toolchain string fetched Fetched - // fetchArgs are the arguments of the target's fetch, so a workspace lost with its node is made again on another. + // fetchArgs are the arguments of the fetch its check ask runs. fetchArgs map[string]any // priority are the Go packages (import paths) or the Node module directories whose test files the merge changes. changed []string // beside are the lines ` ` of the siblings the workspace was given, sorted: its tests may read them, // so a verdict is kept or taken only under a digest that says it folded exactly these in (lead decision L5). beside []string - plans map[string]*goPlan // by level - // runner is RunnerWorkspace while the target's steps are asked to run the candidate (ADR 0535 §1), empty otherwise. + // runner is RunnerWorkspace when the target's ask runs the candidate's own build module (ADR 0535 §1), empty otherwise. runner string // judged is what the line says of the program that judged a change to the build module; empty for any other change. judged string @@ -177,14 +174,6 @@ func (t *target) stepArgs(args map[string]any) map[string]any { return args } -// goPlan is what a Go target has to run at one level: the digests, the packages without a kept verdict and the tests taken. -type goPlan struct { - digests map[string]string - todo []string - // all says the digest step gave nothing to key a kept verdict by, so the level runs whole and keeps nothing. - all bool -} - // acc collects a level's counts across targets. type acc struct { run, kept, skipped int @@ -200,7 +189,6 @@ type state struct { targets []*target levels map[string]*acc nodes map[string]bool - deaf map[string][]deafNode // by build module: the nodes that did not answer during this judging clone time.Duration slow []string } @@ -209,24 +197,9 @@ type state struct { // be judged at all. func (j *Examiner) Judge(ctx context.Context, c *Case) (Outcome, error) { st := &state{j: j, c: c, levels: map[string]*acc{delivery.LevelUnit: {}, delivery.LevelIntegration: {}}, nodes: map[string]bool{}} - if v := st.fetchAll(ctx); v != nil { + if v := st.judgeAll(ctx); v != nil { return st.finish(ctx, *v), nil } - if v := st.checkPins(ctx); v != nil { - return st.finish(ctx, *v), nil - } - for _, level := range []string{delivery.LevelUnit, delivery.LevelIntegration} { - if c.Try != nil && c.Try.Level != "" && c.Try.Level != level { - continue // a try of one level runs that level alone - } - for _, priority := range []bool{true, false} { - for _, t := range st.targets { - if v := st.runTarget(ctx, t, level, priority); v != nil { - return st.finish(ctx, *v), nil - } - } - } - } return st.finish(ctx, delivery.Verdict{State: delivery.VerdictPass}), nil } @@ -291,55 +264,18 @@ func fail(machine, words string) *delivery.Verdict { return &v } -// step runs one step of a build module on the node the target is on, holding a place there while it runs. A node that does -// not answer (the bus has no responder, or none answered) is left out and the step goes to another node of the same build -// module; a step after the fetch makes its workspace again there first (issue 1820). -func (st *state) step(ctx context.Context, t *target, tool, label string, args map[string]any) (json.RawMessage, error) { - for { - raw, err := st.stepOnce(ctx, t, tool, label, args) - if err == nil || ctx.Err() != nil || !isUnheard(err) { - return raw, err - } - if merr := st.moveStep(t, err); merr != nil { - return nil, merr - } - if tool != "fetch" { - if rerr := st.refetch(ctx, t); rerr != nil { - return nil, rerr - } - stepOnWorkspace(t, args) - } +// levels are the levels a case's asks run: both, or the one a try names. +func (st *state) levelsOf() []string { + if st.c.Try != nil && st.c.Try.Level != "" { + return []string{st.c.Try.Level} // a try of one level runs that level alone } + return []string{delivery.LevelUnit, delivery.LevelIntegration} } -func (st *state) stepOnce(ctx context.Context, t *target, tool, label string, args map[string]any) (json.RawMessage, error) { - st.c.Progress.Set(label, t.node) - release, err := st.j.Places.Acquire(ctx, t.node) - if err != nil { - return nil, err - } - defer release() - return st.j.Steps.Run(ctx, t.module, t.node, toolPrefix[t.module]+tool, args) -} - -// choose picks the build node for a pull request's repository on a build module. -func (st *state) choose(bm string) (node, toolchain string, err error) { - language := buildrun.LanguageOf[bm] - qualifies := func(node string) bool { - return !st.isDeaf(bm, node) && (st.j.Presence == nil || st.j.Presence.Online(node)) && (st.j.Health == nil || st.j.Health.Healthy(node, bm)) - } - nodes, tc, err := buildrun.ChooseNodes(language, st.j.Toolchains.Entries(language), st.j.Places, qualifies, 1) - if err != nil { - return "", "", err - } - return nodes[0], tc.Version, nil -} - -// fetchAll fetches every pull request's head merged onto its trunk on a build node of each build module the repository -// needs. A head that does not merge, or a step that fails, ends the check at once, running no test (ADR 0484 clause 13). -func (st *state) fetchAll(ctx context.Context) *delivery.Verdict { - began := st.j.now() - defer func() { st.clone = st.j.now().Sub(began) }() +// judgeAll puts one check ask per pull request and build module the repository needs, the mesh-sdk member first, and +// judges each answer. A head that does not merge, a pin that is not allowed or a level that fails ends the check at once +// (ADR 0484 clause 13). +func (st *state) judgeAll(ctx context.Context) *delivery.Verdict { // The mesh-sdk member first: whether its merge changes the TypeScript package's paths decides whether the other members' // Node fetches take the group's package (ADR 0500 §5). pulls := slices.Clone(st.c.Pulls) @@ -356,47 +292,32 @@ func (st *state) fetchAll(ctx context.Context) *delivery.Verdict { for _, p := range pulls { var merged []Fetched for _, bm := range st.j.Modules.BuildModules(p.Repository) { - node, tc, err := st.choose(bm) - if err != nil { - return fail(bm, err.Error()) + t := &target{pull: p, module: bm} + if tc, none := delivery.AgreedToolchain(st.j.Toolchains.Entries(buildrun.LanguageOf[bm])); none == "" { + t.toolchain = tc.Version } - t := &target{pull: p, module: bm, node: node, toolchain: tc, plans: map[string]*goPlan{}} - st.nodes[node] = true args := map[string]any{"repository": p.Repository, "commit": p.Head, "ref": refOf(p.Number), "base": p.Trunk} - t.fetchArgs = args if bm == buildrun.NodeBuildModule && groupPackage != nil && p.Repository != SDKRepository { args["package"] = groupPackage } if bm == buildrun.GoBuildModule { beside, err := st.j.besideFor(ctx, st.c, p) if err != nil { - return fail(forgeMachine(err, node), err.Error()+unheardWords(err)) + return fail(forgeMachine(err, bm), err.Error()+unheardWords(err)) } if len(beside) > 0 { args["beside"] = beside t.beside = besideLines(beside) } - } - raw, err := st.step(ctx, t, "fetch", fmt.Sprintf("fetch %s#%d on %s", p.Repository, p.Number, node), args) - node = t.node // the step may have moved to another node - if err != nil { - return fail(node, fmt.Sprintf("fetch of %s#%d: %v%s", p.Repository, p.Number, err, pinHint(st.c, err.Error()))) - } - if err := json.Unmarshal(raw, &t.fetched); err != nil || t.fetched.Workspace == "" { - return fail(node, fmt.Sprintf("the fetch of %s#%d answered no workspace: %s", p.Repository, p.Number, raw)) - } - if len(t.fetched.Conflicts) > 0 { - if st.c.Try == nil { - st.j.writePreview(ctx, p, append(merged, t.fetched)) + if p.Repository == catalogueRepository { + t.runner = RunnerWorkspace // used by the build module when the merge changes it (ADR 0535 §1) } - return fail(node, fmt.Sprintf("%s#%d does not merge into the trunk: files in conflict: %s", p.Repository, p.Number, strings.Join(t.fetched.Conflicts, ", "))) } - t.changed = t.fetched.Changed - if bm == buildrun.GoBuildModule && changesBuildGo(p.Repository, t.changed) { - t.runner = RunnerWorkspace - } - merged = append(merged, t.fetched) + t.fetchArgs = args st.targets = append(st.targets, t) + if v := st.ask(ctx, t, &merged); v != nil { + return v + } } if st.c.Try == nil { // a try owes the indexer no preview: no pull request holds its head st.j.writePreview(ctx, p, merged) @@ -408,6 +329,138 @@ func (st *state) fetchAll(ctx context.Context) *delivery.Verdict { return nil } +// ask puts the target's check ask, waits for the step of the node that pulled it and judges its answer: the fetch, the +// pins of its go.mod files, then each level's digest, kept verdicts and tests. +func (st *state) ask(ctx context.Context, t *target, merged *[]Fetched) *delivery.Verdict { + p := t.pull + job := &delivery.CheckJob{Fetch: t.fetchArgs, Levels: st.levelsOf(), Kept: true} + if t.runner != "" { + job.Runner, job.RunnerWhen = t.runner, buildGoDir + } + if st.c.Try != nil && st.c.Try.Run != "" { + job.Run, job.Kept = st.c.Try.Run, false // a try of some tests runs them alone: a partial run proves no package + } + kind := delivery.AskCheck + if st.c.Kind == KindTrunk { + kind = delivery.AskRecheck // a re-judge after a trunk move goes after every new head's check (ADR 0536 §4) + } + a := delivery.StepAsk{ID: buildstep.NewID(), Kind: kind, Language: askLanguage[t.module], Asker: delivery.CheckerModule, + At: st.j.now().UTC(), Check: job} + st.c.Progress.Set(fmt.Sprintf("the %s ask %s of %s#%d", kind, a.ID, p.Repository, p.Number), "") + if err := st.j.Asks.Put(ctx, a); err != nil { + return fail(t.module, fmt.Sprintf("the check of %s#%d could not be asked: %v", p.Repository, p.Number, err)) + } + step, err := st.j.Asks.Wait(ctx, a) + if err != nil { + if ctx.Err() != nil { + return fail(t.module, ctx.Err().Error()) + } + return fail(t.module, fmt.Sprintf("the check of %s#%d: %v", p.Repository, p.Number, err)) + } + // The fetch and the digests are what of the ask's time its levels' running did not take (Took.Clone). + took := time.Duration(0) + if !step.Started.IsZero() && step.Ended.After(step.Started) { + took = step.Ended.Sub(step.Started) + } + t.node = step.Node + st.nodes[step.Node] = true + st.c.Progress.Set(fmt.Sprintf("judge the check of %s#%d", p.Repository, p.Number), step.Node) + if step.State != delivery.StepPassed { + return fail(step.Node, fmt.Sprintf("the check of %s#%d %s: %s%s", p.Repository, p.Number, step.State, step.Reason, pinHint(st.c, step.Reason))) + } + var answer delivery.CheckAnswer + if err := json.Unmarshal(step.Result, &answer); err != nil { + return fail(step.Node, fmt.Sprintf("the check of %s#%d answered what is not a check: %s", p.Repository, p.Number, step.Result)) + } + if err := json.Unmarshal(answer.Fetch, &t.fetched); err != nil || t.fetched.Workspace == "" { + return fail(step.Node, fmt.Sprintf("the fetch of %s#%d answered no workspace: %s", p.Repository, p.Number, answer.Fetch)) + } + if len(t.fetched.Conflicts) > 0 { + if st.c.Try == nil { + st.j.writePreview(ctx, p, append(*merged, t.fetched)) + } + return fail(step.Node, fmt.Sprintf("%s#%d does not merge into the trunk: files in conflict: %s", p.Repository, p.Number, strings.Join(t.fetched.Conflicts, ", "))) + } + t.changed = t.fetched.Changed + if !changesBuildGo(p.Repository, t.changed) { + t.runner = "" // the build module used no runner: the merge does not change it + } + *merged = append(*merged, t.fetched) + if v := st.checkPinsOf(ctx, t); v != nil { + return v + } + ran := 0.0 + for _, la := range answer.Levels { + v, elapsed := st.level(ctx, t, la) + ran += elapsed + if v != nil { + st.clone += max(0, took-time.Duration(ran*float64(time.Second))) + return v + } + } + st.clone += max(0, took-time.Duration(ran*float64(time.Second))) + return nil +} + +// level judges one level of a check ask's answer as the tools' answers were judged: the digest keys the kept verdicts +// of the packages that passed, the kept ones taken are counted, and the first test result that did not pass is the +// verdict. It answers the level's running time (its tests' and setup's). +func (st *state) level(ctx context.Context, t *target, la delivery.LevelAnswer) (*delivery.Verdict, float64) { + level := la.Level + a := st.levels[level] + if a == nil { + return fail(t.node, fmt.Sprintf("the check of %s#%d answered the level %q, which is no level", t.pull.Repository, t.pull.Number, level)), 0 + } + var digests map[string]string + if t.module == buildrun.GoBuildModule && len(la.Digest) > 0 { + var d Digested + if err := json.Unmarshal(la.Digest, &d); err == nil { + digests = d.Packages + if t.runner != "" { + t.judgedBy(d.JudgedBy) // the installed module judged it: the runner is not used again + } + } + } + if t.runner != "" && la.Judged != "" { + t.judgedBy(la.Judged) // the candidate judged the level: nothing is kept (ADR 0535 §4) + } + for _, k := range la.Kept { + a.kept += k.Tests + } + elapsed := 0.0 + for _, raw := range la.Tests { + var res Tested + if err := json.Unmarshal(raw, &res); err != nil { + return fail(t.node, fmt.Sprintf("the %s level of %s#%d answered what is not a result: %v", level, t.pull.Repository, t.pull.Number, err)), elapsed + } + if t.runner != "" { + t.judgedBy(res.JudgedBy) + } + elapsed += res.Elapsed + res.Setup + if t.module == buildrun.NodeBuildModule { + if !res.NoTests { + a.run++ // a Node package is counted whole: its build module counts no tests (ADR 0484 clause 10) + } + } else { + a.run += res.Tests + a.skipped += res.Skipped + a.slowest = append(a.slowest, res.Slowest...) + } + a.elapsed += res.Elapsed + a.setup += res.Setup + a.over = a.over || res.OverBound + if len(digests) > 0 { + if _, err := st.j.Kept.KeepPasses(ctx, t.pull.Repository, level, t.toolchain, res, digests, st.j.now()); err != nil { + st.j.logf("%s#%d: a passing package could not be kept: %v", t.pull.Repository, t.pull.Number, err) + } + } + if !res.Passed { + return levelFailure(t, level, res), elapsed + } + } + return nil, elapsed +} + // changesPackage says whether the merge of a pull request changes a file inside the named paths of one of its repository's // TypeScript packages: a change of the manifest alone, or of files outside those paths, does not (ADR 0500 §5). func (j *Examiner) changesPackage(p Pull, merged []Fetched) bool { @@ -428,51 +481,6 @@ func (j *Examiner) changesPackage(p Pull, merged []Fetched) bool { return false } -// isTestFile says whether a repository-relative path is a test file of the languages the checker judges: Go's _test.go and -// the TypeScript module's .test. and .spec. files and test directories. -func isTestFile(p string) bool { - switch { - case strings.HasSuffix(p, "_test.go"): - return true - case strings.Contains(p, ".test.") || strings.Contains(p, ".spec."): - return true - } - for _, seg := range strings.Split(path.Dir(p), "/") { - if seg == "__tests__" || seg == "test" || seg == "tests" { - return true - } - } - return false -} - -// changesTestOf says whether the merge changes a test file of the Go package with the import path pkg: the directory of the -// file and the import path agree on their last segments, whichever holds the module's directory and path in front. -func changesTestOf(changed []string, pkg string) bool { - for _, f := range changed { - if !strings.HasSuffix(f, "_test.go") { - continue - } - dir := path.Dir(f) - if dir == pkg || strings.HasSuffix(pkg, "/"+dir) || strings.HasSuffix(dir, "/"+pkg) { - return true - } - } - return false -} - -// changesTestIn says whether the merge changes a test file inside the directory of a Node module. -func changesTestIn(changed []string, dir string) bool { - for _, f := range changed { - if !isTestFile(f) || strings.HasSuffix(f, "_test.go") { - continue - } - if dir == "." || dir == "" || strings.HasPrefix(f, strings.TrimSuffix(dir, "/")+"/") { - return true - } - } - return false -} - // failedName names a failure: its package and test. func failedName(f Failure) string { if f.Test == "" { @@ -514,161 +522,3 @@ func levelFailure(t *target, level string, res Tested) *delivery.Verdict { } return &v } - -// runTarget runs one level of a target: first the packages the change's own tests are in (priority), then the others. -func (st *state) runTarget(ctx context.Context, t *target, level string, priority bool) *delivery.Verdict { - if t.module == buildrun.NodeBuildModule { - return st.runNode(ctx, t, level, priority) - } - return st.runGo(ctx, t, level, priority) -} - -// A level's work is planned once for a target: the digest of each package, the kept verdicts taken, the packages left to run. -func (st *state) planGo(ctx context.Context, t *target, level string) *delivery.Verdict { - if t.plans[level] != nil { - return nil - } - plan := &goPlan{digests: map[string]string{}} - t.plans[level] = plan - if st.c.Try != nil && st.c.Try.Run != "" { - // A try of some tests runs them alone: a partial run proves no package, so nothing is taken or kept. - plan.all = true - return nil - } - raw, err := st.step(ctx, t, "digest", fmt.Sprintf("digest the %s level of %s#%d on %s", level, t.pull.Repository, t.pull.Number, t.node), - t.stepArgs(map[string]any{"workspace": t.fetched.Workspace, "level": level})) - var d Digested - if err == nil { - err = json.Unmarshal(raw, &d) - } - if ctx.Err() != nil { - return fail(t.node, ctx.Err().Error()) - } - if err == nil && t.runner != "" && t.judgedBy(d.JudgedBy) { - // The candidate's digest decides which verdicts would be taken: every package runs and nothing is kept (ADR 0535 §4). - plan.all = true - return nil - } - if err != nil || len(d.Packages) == 0 { - // The build module named no digest (a level it cannot digest here): the level runs whole and keeps nothing. - if err != nil { - st.j.logf("%s#%d: no digest of the %s level, so it runs whole: %v", t.pull.Repository, t.pull.Number, level, err) - } - plan.all = true - return nil - } - if !slices.Equal(d.Beside, t.beside) { - // The siblings the tests read are not all in the digest (a build module before DigestVersion 3, or another set): a - // package whose tests read one is never kept, nor taken from a verdict kept for other inputs (ADR 0444, L5). - st.j.logf("%s#%d: the %s level's digest covers the siblings %v, not %v, so it runs whole and keeps nothing", t.pull.Repository, t.pull.Number, level, d.Beside, t.beside) - plan.all = true - return nil - } - plan.digests = d.Packages - a := st.levels[level] - pkgs := make([]string, 0, len(d.Packages)) - for p := range d.Packages { - pkgs = append(pkgs, p) - } - sort.Strings(pkgs) - for _, p := range pkgs { - k, found, err := st.j.Kept.Lookup(ctx, level, d.Packages[p]) - if err != nil { - st.j.logf("the kept verdict of %s at %s cannot be read, so it runs: %v", p, level, err) - } - if found { - a.kept += k.Tests - continue - } - plan.todo = append(plan.todo, p) - } - return nil -} - -func (st *state) runGo(ctx context.Context, t *target, level string, priority bool) *delivery.Verdict { - if v := st.planGo(ctx, t, level); v != nil { - return v - } - plan := t.plans[level] - var run []string - switch { - case plan.all: - if priority { - return nil // nothing to put first: the level runs whole after - } - default: - for _, p := range plan.todo { - if changesTestOf(t.changed, p) == priority { - run = append(run, p) - } - } - if len(run) == 0 { - return nil - } - } - args := t.stepArgs(map[string]any{"workspace": t.fetched.Workspace, "level": level}) - if run != nil { - args["packages"] = run - } - if st.c.Try != nil && st.c.Try.Run != "" { - args["run"] = st.c.Try.Run // build_go_test's go test -run pattern - } - raw, err := st.step(ctx, t, "test", fmt.Sprintf("test the %s level of %s#%d on %s", level, t.pull.Repository, t.pull.Number, t.node), args) - if err != nil { - return fail(t.node, fmt.Sprintf("the %s level of %s#%d: %v", level, t.pull.Repository, t.pull.Number, err)) - } - var res Tested - if err := json.Unmarshal(raw, &res); err != nil { - return fail(t.node, fmt.Sprintf("the %s level of %s#%d answered what is not a result: %v", level, t.pull.Repository, t.pull.Number, err)) - } - if t.runner != "" { - t.judgedBy(res.JudgedBy) - } - a := st.levels[level] - a.run += res.Tests - a.skipped += res.Skipped - a.elapsed += res.Elapsed - a.setup += res.Setup - a.over = a.over || res.OverBound - a.slowest = append(a.slowest, res.Slowest...) - if len(plan.digests) > 0 { - if _, err := st.j.Kept.KeepPasses(ctx, t.pull.Repository, level, t.toolchain, res, plan.digests, st.j.now()); err != nil { - st.j.logf("%s#%d: a passing package could not be kept: %v", t.pull.Repository, t.pull.Number, err) - } - } - if !res.Passed { - return levelFailure(t, level, res) - } - return nil -} - -func (st *state) runNode(ctx context.Context, t *target, level string, priority bool) *delivery.Verdict { - for _, dir := range t.fetched.Modules { - if changesTestIn(t.changed, dir) != priority { - continue - } - raw, err := st.step(ctx, t, "test", fmt.Sprintf("test the %s level of %s#%d (%s) on %s", level, t.pull.Repository, t.pull.Number, dir, t.node), - map[string]any{"workspace": t.fetched.Workspace, "module": dir, "level": level}) - if err != nil { - return fail(t.node, fmt.Sprintf("the %s level of %s#%d (%s): %v", level, t.pull.Repository, t.pull.Number, dir, err)) - } - var res Tested - if err := json.Unmarshal(raw, &res); err != nil { - return fail(t.node, fmt.Sprintf("the %s level of %s#%d (%s) answered what is not a result: %v", level, t.pull.Repository, t.pull.Number, dir, err)) - } - if res.Module == "" { - res.Module = dir - } - a := st.levels[level] - if !res.NoTests { - a.run++ // a Node package is counted whole: its build module counts no tests (ADR 0484 clause 10) - } - a.elapsed += res.Elapsed - a.setup += res.Setup - a.over = a.over || res.OverBound - if !res.Passed { - return levelFailure(t, level, res) - } - } - return nil -} diff --git a/modules/mesh-delivery-checker/internal/check/pins.go b/modules/mesh-delivery-checker/internal/check/pins.go index f1e2937..c07a8c4 100644 --- a/modules/mesh-delivery-checker/internal/check/pins.go +++ b/modules/mesh-delivery-checker/internal/check/pins.go @@ -55,19 +55,19 @@ func SDKPin(goMod string) (commit string, ok bool) { return "", false } -// checkPins refuses a Go member whose go.mod names a mesh-sdk commit that is neither on mesh-sdk's trunk nor the head of the +// checkPinsOf refuses a Go member whose go.mod names a mesh-sdk commit that is neither on mesh-sdk's trunk nor the head of the // group's mesh-sdk pull request. It reads each go.mod the fetch named from the forge: the one the check builds, which is the // head's when the head holds the file and the trunk's when it does not, since a module added on the trunk after the branch was // cut is in the merge and not at the head (issue 1845). It answers nil when every pin is the group's. -func (st *state) checkPins(ctx context.Context) verdictPtr { +func (st *state) checkPinsOf(ctx context.Context, t *target) verdictPtr { if st.j.Forge == nil { return nil } sdk := st.c.member(SDKRepository) - for _, t := range st.targets { - if t.module != goModule || t.pull.Repository == SDKRepository { - continue - } + if t.module != goModule || t.pull.Repository == SDKRepository { + return nil + } + { for _, dir := range t.fetched.Modules { file := path.Join(dir, "go.mod") st.c.Progress.Set(fmt.Sprintf("read %s of %s#%d", file, t.pull.Repository, t.pull.Number), t.node) diff --git a/modules/mesh-delivery-checker/internal/check/pins_test.go b/modules/mesh-delivery-checker/internal/check/pins_test.go index 9d0c6bb..ee8f873 100644 --- a/modules/mesh-delivery-checker/internal/check/pins_test.go +++ b/modules/mesh-delivery-checker/internal/check/pins_test.go @@ -97,7 +97,7 @@ func TestAMemberPinnedToTheSDKMembersHeadPassesAndIsBuiltAgainstIt(t *testing.T) var besideSDK bool for _, f := range b.steps.tool("fetch") { if f.args["repository"] == repoA { - refs, _ := f.args["beside"].([]BesideRef) + refs, _ := refsOf(f.args["beside"]), true for _, r := range refs { besideSDK = besideSDK || (r.Repository == repoSDK && r.Commit == head2) } @@ -116,9 +116,8 @@ func TestAMemberPinnedToAnOlderHeadOfTheSDKMemberFailsNamingTheMemberAndTheCommi !strings.Contains(f.Words, "neither on mesh-sdk's trunk nor the group's head") || !strings.Contains(f.Words, short12(head2)) { t.Fatalf("verdict = %+v", v) } - if n := len(b.steps.tool("test")); n != 0 { - t.Errorf("%d test steps ran after a stale pin", n) - } + // Since R4g the levels run in the member's one check ask before its go.mod pins are judged (ADR 0525 §3); the verdict is + // still the pin's, whatever the levels said. } func TestAMemberPinnedToACommitOnTheTrunkPasses(t *testing.T) { // 1624/AC2, AC4 @@ -213,7 +212,7 @@ func besideOf(t *testing.T, b *bench, repository string) map[string]BesideRef { out := map[string]BesideRef{} for _, f := range b.steps.tool("fetch") { if f.args["repository"] == repository { - refs, _ := f.args["beside"].([]BesideRef) + refs, _ := refsOf(f.args["beside"]), true for _, r := range refs { out[r.Repository] = r } @@ -228,7 +227,8 @@ func TestTheSiblingsAreWhereTheTestsStateThem(t *testing.T) { // review D2, ADR b := newBench(t) b.ex.Trunks = trunksOf(t, map[string]string{controller: trunkA, catalogue: trunkS, host: trunkA2, SDKRepository: "9999999999999999999999999999999999999999"}) b.ex.Runs = runsAt{"mesh-host": hostRuns} - forge := &fakeForge{goMods: map[string]string{controller + " go.mod " + head1: capturedGoMod(t, "cb54756c3865")}, onTrunk: map[string]bool{"cb54756c3865": true}} + forge := &fakeForge{goMods: map[string]string{controller + " go.mod " + head1: capturedGoMod(t, "cb54756c3865"), + catalogue + " go.mod " + head2: "module git.novox.be/novox/mesh-catalog\n"}, onTrunk: map[string]bool{"cb54756c3865": true}} b.ex.Forge = forge // A group of the controller and the catalogue: the controller's tests read the catalogue's trunk, not the group's head. b.judge(t, caseOf(t, pullOf(controller, 7, head1), pullOf(catalogue, 12, head2))) diff --git a/modules/mesh-delivery-checker/internal/check/try_test.go b/modules/mesh-delivery-checker/internal/check/try_test.go index 65b89b5..5c9e5a0 100644 --- a/modules/mesh-delivery-checker/internal/check/try_test.go +++ b/modules/mesh-delivery-checker/internal/check/try_test.go @@ -153,18 +153,7 @@ func TestAFailingTryNamesWhatFailedAndAStepThatCannotRunIsAnError(t *testing.T) if got := readTry(t, kv, id); got.State != TryFail || len(got.Failed) != 1 || !strings.Contains(got.Failed[0], "TestThreeIsFour") { t.Errorf("try = %+v", got) } - // No build node: the try could not run, which is an error, not a failure of the change. - kv2 := newMemKV() - b2 := newBench(t) - b2.ex.Toolchains = entries{} - id2, _ := AskTry(memTries{kv2}, tryTrunks, TryAsk{Repository: repoA, Head: head1}, time.Now()) - tr2 := &Tryer{Tries: kv2, Judge: b2.ex} - raw, _, _, _ = kv2.Get(ctx, id2) - tr2.Put(ctx, id2, raw) - tr2.Wait() - if got := readTry(t, kv2, id2); got.State != TryError { - t.Errorf("try with no node = %+v, want error", got) - } + // No build node is no error since R4g: the try's ask waits in the check stream until a node pulls it (ADR 0525 §4). } func TestATryWaitsWhileADeliveryBuildsAndATryLeftRunningIsTakenOver(t *testing.T) { From d9d1e57e3a13e67ec7f099d91fe90dc479bcd246 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 16:09:34 +0200 Subject: [PATCH 5/5] =?UTF-8?q?Delete=20the=20checker's=20hold,=20its=20ow?= =?UTF-8?q?n=20capacity=20and=20places,=20its=20node=20moves=20and=20its?= =?UTF-8?q?=20read=20of=20the=20controller's=20plans-building,=20so=20a=20?= =?UTF-8?q?check=20waits=20only=20in=20the=20build=20seats'=20streams=20(i?= =?UTF-8?q?ssue=201887=20A5b,=20closes=201685;=20ADR=200525=20=C2=A74,=20?= =?UTF-8?q?=C2=A77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery-checker-tools/main.go | 32 +--- .../cmd/mesh-delivery-checker/bus.go | 37 ----- .../cmd/mesh-delivery-checker/heard.go | 26 ---- .../cmd/mesh-delivery-checker/heard_test.go | 30 ---- .../cmd/mesh-delivery-checker/hold.go | 101 ------------- .../cmd/mesh-delivery-checker/hold_test.go | 35 ----- .../mesh-delivery-checker/integration_test.go | 28 ---- .../cmd/mesh-delivery-checker/kept_test.go | 3 +- .../cmd/mesh-delivery-checker/main.go | 14 +- .../cmd/mesh-delivery-checker/settings.go | 29 ---- .../mesh-delivery-checker/settings_test.go | 68 --------- .../mesh-delivery-checker/files/settings.json | 3 - .../internal/check/checker.go | 115 +------------- .../internal/check/checker_test.go | 18 --- .../internal/check/gate_test.go | 20 ++- .../internal/check/hold.go | 7 - .../internal/check/hold_test.go | 38 ----- .../internal/check/rank_test.go | 140 ------------------ .../internal/check/read.go | 44 ++---- .../internal/check/read_test.go | 21 +-- .../internal/check/try.go | 12 +- .../internal/check/try_test.go | 11 +- .../mesh-delivery-checker/manifest_test.go | 40 ++--- modules/mesh-delivery-checker/module.json | 43 +----- 24 files changed, 76 insertions(+), 839 deletions(-) delete mode 100644 modules/mesh-delivery-checker/cmd/mesh-delivery-checker/heard.go delete mode 100644 modules/mesh-delivery-checker/cmd/mesh-delivery-checker/heard_test.go delete mode 100644 modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold.go delete mode 100644 modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold_test.go delete mode 100644 modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go delete mode 100644 modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings_test.go delete mode 100644 modules/mesh-delivery-checker/files/settings.json delete mode 100644 modules/mesh-delivery-checker/internal/check/hold.go delete mode 100644 modules/mesh-delivery-checker/internal/check/hold_test.go diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go index acac211..700988d 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go @@ -143,32 +143,16 @@ func Tools(r check.Reader) []stdio.Tool { return map[string]any{"previews": ps, "unreadable": bad}, err }}, {Name: check.StatusReadTool, - Description: "What holds the checker back and what runs: the capacity per node (the setting, the default one step per node), whether the controller's " + - "delivery plans in their build phase hold a key (no new check starts while one does), the steps in flight on the build modules by node, and the " + - "candidates without a verdict for their heads (a group of hq's pull requests alone is left out: merge-check.sh judges it), and the " + - "candidates whose check waits by kind (waiting_by_kind: new, nodes, and trunk, which start after the other two). A key a build " + - "module left running (started past the step bound, or its build module not running on its node) is listed apart under left_running " + - "with why, and not counted. The places the checker holds are in its process, not in a bucket, and the answer says so. Read-only.", + Description: "What runs: the steps in flight on the build modules by node, the candidates without a verdict for their heads, " + + "and the candidates whose check is owed by kind (waiting_by_kind: new, nodes, and trunk, a re-judge pulled after the other two). A key a " + + "build module left running (started past the step bound, or its build module not running on its node) is listed apart under " + + "left_running with why, and not counted. Nothing holds the checker and it has no capacity of its own: a check is an ask on the " + + "build seats' streams, whose backlog the delivery view shows (novox/hq ADR 0525). Read-only.", Input: map[string]any{"type": "object", "properties": map[string]any{}}, Run: func(map[string]any) (any, error) { return r.Status() }}, } } -func capacity() (string, error) { - path := os.Getenv("MESH_CHECKER_SETTINGS") - if path == "" { - return os.Getenv("MESH_CHECKER_CAPACITY"), nil - } - raw, err := os.ReadFile(path) - if err != nil { - return "", err - } - var s struct { - Capacity string `json:"capacity"` - } - return s.Capacity, json.Unmarshal(raw, &s) -} - // stepBound is the checker's step bound as the checker reads it (MESH_CHECKER_STEP_BOUND), its default when unset or not a // duration. func stepBound() time.Duration { @@ -180,9 +164,9 @@ func stepBound() time.Duration { func main() { r := check.Reader{VerdictState: read(stdio.State("verdicts")), KeptState: read(stdio.State("kept")), CandidateState: read(stdio.State("mesh-delivery-indexer.candidates")), - PreviewState: read(stdio.State("previews")), PlansState: read(stdio.State("mesh-controller.plans-building")), - StepStates: map[string]check.StateRead{"mesh-build-go": read(stdio.State("mesh-build-go.steps")), "mesh-build-node": read(stdio.State("mesh-build-node.steps"))}, - Capacity: capacity, HaveState: read(stdio.State("node-engine.have")), StepBound: stepBound()} + PreviewState: read(stdio.State("previews")), + StepStates: map[string]check.StateRead{"mesh-build-go": read(stdio.State("mesh-build-go.steps")), "mesh-build-node": read(stdio.State("mesh-build-node.steps"))}, + HaveState: read(stdio.State("node-engine.have")), StepBound: stepBound()} tries := stdio.State(check.TriesState) source := stdio.State("mesh-delivery-gate.source") trunks := func() (map[string]string, error) { diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/bus.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/bus.go index 6415687..448979d 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/bus.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/bus.go @@ -105,43 +105,6 @@ func follow(ctx context.Context, w jetstream.KeyWatcher, each func(jetstream.Key } } -// stepsWaiter waits for a build module's step by watching its key in the module's steps bucket (ADR 0469 clause 2): the -// watch hands over the value the key holds now, so a step that ended before the watch began is read at once, and every -// change after. No timer: the caller bounds the wait with its context. -type stepsWaiter struct{ js bucketOpener } - -func (w stepsWaiter) Wait(ctx context.Context, module, key string) (delivery.Step, error) { - kv, err := w.js.KeyValue(ctx, delivery.StepsBucket(module)) - if err != nil { - return delivery.Step{}, fmt.Errorf("the bucket %s: %w", delivery.StepsBucket(module), err) - } - watch, err := kv.Watch(ctx, key) - if err != nil { - return delivery.Step{}, err - } - defer watch.Stop() - for { - select { - case <-ctx.Done(): - return delivery.Step{}, ctx.Err() - case e, ok := <-watch.Updates(): - if !ok { - return delivery.Step{}, errors.New("the watch of the step ended") - } - if e == nil || !opOf(e) { - continue - } - step, err := delivery.DecodeStep(e.Value()) - if err != nil { - return delivery.Step{}, fmt.Errorf("the step %s is not a step: %w", key, err) - } - if step.State != delivery.StepRunning { - return step, nil - } - } - } -} - // manifestReader reads manifests. of the indexer and parses the manifest it holds. type manifestReader struct{ kv jetstream.KeyValue } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/heard.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/heard.go deleted file mode 100644 index e186fce..0000000 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/heard.go +++ /dev/null @@ -1,26 +0,0 @@ -package main - -import ( - "context" - "encoding/json" - "errors" - - "github.com/nats-io/nats.go" - - "mesh-delivery-checker/internal/check" -) - -// heardCaller is the build runner's caller with the calls that reached nobody marked: no responders, or none answering within -// the call's bound. The step never began there, so the check may give it to another node (issue 1820). A call that was -// answered, with a failure or not, is left as it is. -type heardCaller struct { - call func(node, key string, body any) (json.RawMessage, error) -} - -func (h heardCaller) CallOn(node, key string, body any) (json.RawMessage, error) { - raw, err := h.call(node, key, body) - if errors.Is(err, nats.ErrNoResponders) || errors.Is(err, nats.ErrTimeout) || errors.Is(err, context.DeadlineExceeded) { - return nil, &check.Unheard{Err: err} - } - return raw, err -} diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/heard_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/heard_test.go deleted file mode 100644 index d660385..0000000 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/heard_test.go +++ /dev/null @@ -1,30 +0,0 @@ -package main - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "testing" - - "github.com/nats-io/nats.go" - - "mesh-delivery-checker/internal/check" -) - -func TestACallThatReachedNobodyIsMarkedAndAnAnsweredFailureIsNot(t *testing.T) { // 1820/AC6 - for _, err := range []error{nats.ErrNoResponders, nats.ErrTimeout, context.DeadlineExceeded} { - h := heardCaller{call: func(string, string, any) (json.RawMessage, error) { return nil, fmt.Errorf("call: %w", err) }} - _, got := h.CallOn("g14", "m.t", nil) - var u *check.Unheard - if !errors.As(got, &u) { - t.Errorf("%v: %v is not marked unheard", err, got) - } - } - h := heardCaller{call: func(string, string, any) (json.RawMessage, error) { return nil, errors.New("the module refused") }} - _, got := h.CallOn("g14", "m.t", nil) - var u *check.Unheard - if errors.As(got, &u) { - t.Errorf("an answered failure %v is marked unheard", got) - } -} diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold.go deleted file mode 100644 index 1518bd7..0000000 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold.go +++ /dev/null @@ -1,101 +0,0 @@ -package main - -import ( - "context" - "errors" - "log" - "sync" - - "github.com/nats-io/nats.go/jetstream" - - "mesh-delivery-checker/internal/check" -) - -// holdWatch holds the checker back while a delivery builds (ADR 0484 clause 12): whether one does is the list of the keys of -// the controller's bucket of the delivery plans in their build phase, read each time a check is about to start. Its keys age -// out (a time to live, and no delete marker), which a watch never hears, so a watch cannot decide it (mesh-controller -// internal/link/holds.go lists the keys too); the watch is only the wake-up, for a plan that ended by a delete. The bucket -// is read as empty where it is absent (issue 1652 makes it) or cannot be read, saying so once. It may come into being after -// the checker started and the checker holds no timer (ADR 0437 §8), so each time a check is about to start and the bucket is -// not known yet, the checker looks for it. -type holdWatch struct { - ctx context.Context - js bucketOpener - keys func(ctx context.Context) ([]string, error) - changed func(ctx context.Context) // a plan key changed: the checks that waited may start - logger *log.Logger - errs chan error - mu sync.Mutex - watched bool - said bool -} - -func newHoldWatch(ctx context.Context, js bucketOpener, changed func(context.Context), logger *log.Logger) *holdWatch { - return &holdWatch{ctx: ctx, js: js, changed: changed, logger: logger, errs: make(chan error, 1)} -} - -// Held says whether a delivery builds now. -func (w *holdWatch) Held() bool { - w.ensure() - w.mu.Lock() - keys := w.keys - w.mu.Unlock() - if keys == nil { - return false - } - list, err := keys(w.ctx) - if err != nil { - w.say("the keys of %s cannot be listed, so it is read as empty: %v", check.PlansBuildingBucket, err) - return false - } - return len(list) > 0 -} - -func (w *holdWatch) say(format string, args ...any) { - w.mu.Lock() - defer w.mu.Unlock() - if !w.said { - w.said = true - w.logger.Printf(format, args...) - } -} - -// ensure finds the bucket if it is not known and exists, and starts the wake-up watch. -func (w *holdWatch) ensure() { - w.mu.Lock() - if w.watched || w.js == nil { - w.mu.Unlock() - return - } - w.mu.Unlock() - kv, err := w.js.KeyValue(w.ctx, check.PlansBuildingBucket) - switch { - case errors.Is(err, jetstream.ErrBucketNotFound): - w.say("the bucket %s does not exist yet: it is read as empty, so no check is held back", check.PlansBuildingBucket) - return - case err != nil: - w.say("the bucket %s cannot be read, so it is read as empty: %v", check.PlansBuildingBucket, err) - return - } - w.mu.Lock() - defer w.mu.Unlock() - if w.watched { - return - } - w.watched, w.said = true, false - w.keys = func(ctx context.Context) ([]string, error) { - list, err := kv.Keys(ctx) - if errors.Is(err, jetstream.ErrNoKeysFound) { - return nil, nil - } - return list, err - } - go func() { - wake, err := kv.WatchAll(w.ctx) - if err != nil { - w.errs <- err - return - } - w.errs <- follow(w.ctx, wake, func(jetstream.KeyValueEntry) { w.changed(w.ctx) }) - }() -} diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold_test.go deleted file mode 100644 index 3f15b31..0000000 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/hold_test.go +++ /dev/null @@ -1,35 +0,0 @@ -package main - -import ( - "context" - "errors" - "io" - "log" - "testing" -) - -// The controller's plans-building keys age out of the bucket (a time to live, no delete marker), which a watch never hears: -// whether a delivery builds is the list of the keys now, and the watch is only a wake-up (review D3). -func TestADeliveryHoldsWhileItsKeyIsListedAndNotWhenItAgedOutWithNoEvent(t *testing.T) { - listed := []string{"plan-1"} - w := &holdWatch{watched: true, logger: log.New(io.Discard, "", 0), keys: func(context.Context) ([]string, error) { return listed, nil }} - if !w.Held() { - t.Fatal("a plan key is listed and nothing holds") - } - listed = nil // the key aged out: no delete reached the watch - if w.Held() { - t.Error("the key is gone from the bucket and the checker is still held") - } -} - -func TestABucketThatCannotBeListedIsReadAsEmpty(t *testing.T) { - w := &holdWatch{watched: true, logger: log.New(io.Discard, "", 0), keys: func(context.Context) ([]string, error) { return []string{"plan-1"}, errors.New("the bus is down") }} - if w.Held() { - t.Error("an unreadable bucket held the checker") - } - var absent holdWatch - absent.logger = log.New(io.Discard, "", 0) - if absent.Held() { - t.Error("a watch with no bucket held the checker") - } -} diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go index 502c82f..632bbf1 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go @@ -561,34 +561,6 @@ func TestACheckLeftRunningIsTakenOverWhenTheCheckerStarts(t *testing.T) { // N6/ } } -// TestNoCheckStartsWhileADeliveryBuildsAndOneStartsWhenItEnds: the controller's bucket of the plans in their build phase -// holds a plan's key; the candidate waits, no step is called; the key deleted, the check starts and passes (ADR 0484 clause 12). -func TestNoCheckStartsWhileADeliveryBuildsAndOneStartsWhenItEnds(t *testing.T) { // N8/AC1 - b := startBed(t) - plans, err := b.js.CreateKeyValue(b.ctx, jetstream.KeyValueConfig{Bucket: "mesh-controller_plans-building", History: 1}) - if err != nil { - t.Fatal(err) - } - if _, err := plans.Put(b.ctx, "plan-1", []byte(`{"plan":"plan-1","since":"2026-10-21T19:00:00Z","seen":1}`)); err != nil { - t.Fatal(err) - } - key := b.candidate(t, "feat/x", pull(repo, 7, head1)) - b.start(t, nil) - time.Sleep(1500 * time.Millisecond) // a wait for nothing to happen: there is no event to wait on - if _, err := b.buckets[delivery.CheckerVerdictsBucket].Get(b.ctx, key); err == nil { - t.Fatal("a check started while a delivery builds") - } - if n := b.fake.count("mesh-build-go.build_go_fetch"); n != 0 { - t.Fatalf("%d fetches while a delivery builds", n) - } - if err := plans.Delete(b.ctx, "plan-1"); err != nil { - t.Fatal(err) - } - if v := b.verdict(t, key, ended); v.State != delivery.VerdictPass { - t.Errorf("verdict = %+v", v) - } -} - // TestAMemberNamingAMeshSDKCommitOffTheTrunkFailsNamingTheCommit: the go.mod the forge answers names a commit that main does not // hold; the verdict fails naming the member and the commit and no test runs (ADR 0484 clause 9). func TestAMemberNamingAMeshSDKCommitOffTheTrunkFailsNamingTheCommit(t *testing.T) { // 1624/AC2 diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/kept_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/kept_test.go index e19c92c..c2f5f1e 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/kept_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/kept_test.go @@ -37,7 +37,8 @@ func TestTheCheckerAnnouncesEveryStateItKeepsAndOwedReadable(t *testing.T) { // if !equal(manifest, announced) { t.Errorf("announced %v, the manifest keeps %v", announced, manifest) } - if want := []string{"owed", "previews", "verdicts"}; !equal(readable, want) { + // kept is read by the Go build module for a check ask (issue 1887 design limit 1). + if want := []string{"kept", "owed", "previews", "verdicts"}; !equal(readable, want) { t.Errorf("readable %v, want %v", readable, want) } } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go index fb98e28..776f7fb 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -114,7 +114,9 @@ func durationEnv(getenv func(string) string, name string, def time.Duration) (ti // keptStates is every state the checker keeps and which others may read (ADR 0495 §2-§3): owed by the forge module, // which writes mesh/check from it (ADR 0521), and by the forge bridge; previews by the indexer; verdicts by the gate. var keptStates = []announce.State{ - {Name: "kept"}, + // The kept verdicts are read by the Go build module, which takes them for a check ask's unchanged packages (issue 1887's + // design limit 1); the checker alone writes them (ADR 0444 clause 5). + {Name: "kept", Readable: true}, {Name: "owed", Readable: true}, {Name: "previews", Readable: true}, {Name: "tries"}, @@ -189,10 +191,6 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro if err != nil { return err } - capacity, err := capacityFrom(strings.TrimSpace(getenv("MESH_CHECKER_SETTINGS")), getenv("MESH_CHECKER_CAPACITY")) - if err != nil { - return fmt.Errorf("the capacity setting: %w", err) - } link := strings.TrimSpace(getenv("MESH_CHECKER_LINK")) if link == "" { link = defaultLink @@ -201,7 +199,6 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro presence := &buildrun.PresenceSet{} have := &buildrun.HaveSet{} trunks := &check.Trunks{} - _ = capacity // A check is one ask per pull request and build module on the build seats' check stream, pulled by a build node with a // free slot (novox/hq ADR 0525 §1, §3): the checker chooses no node and starts no step. asks := buildrun.JetStreamAsks{JS: js} @@ -226,9 +223,6 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro } // The tries (ADR 0501 §5) are judged by the same judge, held by the same deliveries. tryer := &check.Tryer{Tries: kvStore{buckets[check.TriesBucket]}, Bound: checkBound, Logf: logger.Printf} - holds := newHoldWatch(ctx, js, func(ctx context.Context) { ch.Reconsider(ctx); tryer.Reconsider(ctx) }, logger) - ch.Hold = holds.Held - tryer.Hold = holds.Held announce := func(ctx context.Context, event, key string) { body, _ := json.Marshal(map[string]string{"key": key}) if _, err := js.Publish(ctx, eventSubject(event), body); err != nil { @@ -379,7 +373,5 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro return err case err := <-watch.errs: return err - case err := <-holds.errs: - return err } } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go deleted file mode 100644 index d32d39f..0000000 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go +++ /dev/null @@ -1,29 +0,0 @@ -package main - -import ( - "encoding/json" - "fmt" - "os" - - "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" -) - -// capacityFrom is the build capacity of each node (ADR 0478 clause 3): the module's setting `capacity`, `=` pairs separated by commas, laid into the settings file the controller composes (issue 1661); empty is one step at a -// time on every node. With no settings file named, fallback is read instead, for a run outside the module's launch. -func capacityFrom(path, fallback string) (map[string]int, error) { - if path == "" { - return buildrun.ParseCapacity(fallback) - } - raw, err := os.ReadFile(path) - if err != nil { - return nil, fmt.Errorf("the settings file: %w", err) - } - var s struct { - Capacity string `json:"capacity"` - } - if err := json.Unmarshal(raw, &s); err != nil { - return nil, fmt.Errorf("the settings file %s is not JSON: %w", path, err) - } - return buildrun.ParseCapacity(s.Capacity) -} diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings_test.go deleted file mode 100644 index 8543fe7..0000000 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings_test.go +++ /dev/null @@ -1,68 +0,0 @@ -package main - -import ( - "os" - "path/filepath" - "testing" - - "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" -) - -func settingsFile(t *testing.T, body string) string { - t.Helper() - p := filepath.Join(t.TempDir(), "settings.json") - if err := os.WriteFile(p, []byte(body), 0o600); err != nil { - t.Fatal(err) - } - return p -} - -// ADR 0484 clause 12: the build capacity is the module's setting `capacity`, laid into the settings file: empty is one step at a time everywhere. -func TestCapacityIsReadFromTheSettingsFile(t *testing.T) { - for body, want := range map[string]map[string]int{ - `{"capacity": "shanks=3,ace=2"}`: {"shanks": 3, "ace": 2}, - `{"capacity": ""}`: {}, - `{}`: {}, - } { - got, err := capacityFrom(settingsFile(t, body), "") - if err != nil || len(got) != len(want) || got["shanks"] != want["shanks"] || got["ace"] != want["ace"] { - t.Errorf("%s: capacity %v, %v, want %v", body, got, err, want) - } - } - p := &buildrun.Places{Capacity: mustCapacity(t, settingsFile(t, `{"capacity": ""}`))} - if p.CapacityOf("shanks") != 1 || p.CapacityOf("ace") != 1 { - t.Errorf("an empty setting gives %d and %d, want 1 everywhere", p.CapacityOf("shanks"), p.CapacityOf("ace")) - } -} - -func mustCapacity(t *testing.T, path string) map[string]int { - t.Helper() - c, err := capacityFrom(path, "") - if err != nil { - t.Fatal(err) - } - return c -} - -func TestACapacitySettingThatIsNotOneIsRefusedNamingIt(t *testing.T) { - if _, err := capacityFrom(settingsFile(t, `{"capacity": "shanks=-1"}`), ""); err == nil { - t.Error("shanks=-1 accepted") - } - if c, err := capacityFrom(settingsFile(t, `{"capacity": "shanks=3,ace=2,g14=0,novox=0"}`), ""); err != nil || c["g14"] != 0 || len(c) != 4 { - t.Errorf("the starting value: %v, %v", c, err) - } - if _, err := capacityFrom(settingsFile(t, `not json`), ""); err == nil { - t.Error("a file that is not JSON accepted") - } - if _, err := capacityFrom(filepath.Join(t.TempDir(), "none"), ""); err == nil { - t.Error("a settings file that is not there accepted") - } -} - -// With no settings file named (a run outside the module's launch, as the tests do), the capacity is the environment's. -func TestWithNoSettingsFileTheCapacityIsTheEnvironments(t *testing.T) { - got, err := capacityFrom("", "ace=2") - if err != nil || got["ace"] != 2 { - t.Errorf("capacity %v, %v", got, err) - } -} diff --git a/modules/mesh-delivery-checker/files/settings.json b/modules/mesh-delivery-checker/files/settings.json deleted file mode 100644 index 1354ea4..0000000 --- a/modules/mesh-delivery-checker/files/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "capacity": "" -} diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 850b7fe..2b92cc3 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "slices" "strings" "sync" "time" @@ -69,10 +68,6 @@ type Checker struct { Trunks interface{ Heads() map[string]string } // Bound is how long one check may run; DefaultBound when 0. Bound time.Duration - // Limit is how many checks run at once; two when 0. A freed place goes to the first waiting check by rank (ADR 0536 §4). - Limit int - // Hold says whether a delivery builds, so no new check starts (ADR 0484 clause 12); nil never holds. - Hold func() bool // Link is the base of the address a status points at; the verdict key follows. Link string // Announce says a status was written (EventOwedWritten); nil says nothing. @@ -81,10 +76,6 @@ type Checker struct { Logf func(format string, args ...any) once sync.Once - limit int - running int // checks holding a place - waiting []*waiter // checks waiting for a place, in the order they asked - seq uint64 wg sync.WaitGroup mu sync.Mutex known map[string]delivery.Candidate // by candidates key @@ -97,11 +88,6 @@ type Checker struct { func (ch *Checker) init() { ch.once.Do(func() { - limit := ch.Limit - if limit <= 0 { - limit = 2 - } - ch.limit = limit ch.known, ch.inflight, ch.again, ch.tried = map[string]delivery.Candidate{}, map[string]bool{}, map[string]bool{}, map[string]bool{} ch.nodesFail = map[string]uint64{} }) @@ -190,10 +176,6 @@ func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidat if cs == nil { return // nothing to judge: hq's pull requests, or no trunk known } - if ch.Hold != nil && ch.Hold() { - ch.logf("%s: not started, a delivery builds", cs.Key) - return // started when the plan bucket empties (Reconsider) - } cs.Queued = ch.now() ch.mu.Lock() if ch.inflight[cs.Key] { @@ -207,10 +189,11 @@ func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidat go func() { defer ch.wg.Done() for { - cs.Kind = ch.kindOf(ctx, cs) // a re-judge after a trunk move is asked on the recheck stream (ADR 0536 §4) - ch.admit(cs, cs.Kind) + // Every check is asked at once: the build seats' streams hold the backlog, a new head's check and a check after a + // failure of the nodes on check, a re-judge after a trunk move on recheck, pulled after them (ADR 0525 §4, §9; + // ADR 0536 §4). The checker holds no place and no capacity of its own. + cs.Kind = ch.kindOf(ctx, cs) ch.try(ctx, key, cs) - ch.leave() ch.mu.Lock() more := ch.again[cs.Key] delete(ch.again, cs.Key) @@ -223,62 +206,19 @@ func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidat if next := CaseOf(ch.candidate(key, c), ch.heads()); next != nil { cs = next } - cs.Queued = ch.now() // asked again: it waits for a place by its rank like any other + cs.Queued = ch.now() } }() } -// The kinds of a check waiting for a place, by the rank they start in (ADR 0536 §4): a head never judged and a head asked -// again after a failure of the nodes first, then a head asked again only because its trunk moved. +// The kinds of a check (ADR 0536 §4): a head never judged and a head asked again after a failure of the nodes are asked on +// the build seats' check stream, a head asked again only because its trunk moved on the recheck stream, pulled after. const ( KindNew = "new" KindNodes = "nodes" KindTrunk = "trunk" ) -var kindRank = map[string]int{KindNew: 0, KindNodes: 0, KindTrunk: 1} - -// waiter is a check waiting for a place. -type waiter struct { - group, key, kind string - queued time.Time - seq uint64 - ready chan struct{} -} - -// Waiting names a check waiting for a place: its group, its verdict key, its kind and when it was asked. -type Waiting struct { - Group string `json:"group"` - Key string `json:"key"` - Kind string `json:"kind"` - Queued time.Time `json:"queued"` -} - -// Waiting are the checks waiting for a place, in the order they would start. -func (ch *Checker) Waiting() []Waiting { - ch.init() - ch.mu.Lock() - defer ch.mu.Unlock() - ws := slices.Clone(ch.waiting) - slices.SortStableFunc(ws, before) - out := make([]Waiting, len(ws)) - for i, w := range ws { - out[i] = Waiting{Group: w.group, Key: w.key, Kind: w.kind, Queued: w.queued} - } - return out -} - -// before orders waiting checks: the lower rank, then the one asked first, then the one that asked for a place first. -func before(a, b *waiter) int { - if ra, rb := kindRank[a.kind], kindRank[b.kind]; ra != rb { - return ra - rb - } - if c := a.queued.Compare(b.queued); c != 0 { - return c - } - return int(a.seq) - int(b.seq) -} - // kindOf is the kind of a case's check, read from its verdict key: none (or one left running) is a head never judged; a // failure of the nodes is asked again after the nodes changed; a verdict on other trunk heads is a re-judge after a trunk // move. A verdict that stands is ranked with the re-judges: its claim finds nothing to do. @@ -297,47 +237,6 @@ func (ch *Checker) kindOf(ctx context.Context, cs *Case) string { return KindTrunk } -// admit returns when the check holds a place: at once when one is free and no check waits, else when a check's end hands its -// place to this one as the first by rank. No timer: a check's end wakes the next (leave). -func (ch *Checker) admit(cs *Case, kind string) { - ch.mu.Lock() - if ch.running < ch.limit && len(ch.waiting) == 0 { - ch.running++ - ch.mu.Unlock() - return - } - ch.seq++ - w := &waiter{group: cs.Group, key: cs.Key, kind: kind, queued: cs.Queued, seq: ch.seq, ready: make(chan struct{})} - ch.waiting = append(ch.waiting, w) - ch.handOn() - ch.mu.Unlock() - <-w.ready -} - -// leave frees the check's place and hands it to the first waiting check by rank. -func (ch *Checker) leave() { - ch.mu.Lock() - ch.running-- - ch.handOn() - ch.mu.Unlock() -} - -// handOn gives each free place to the first waiting check by rank; it is called with mu held. -func (ch *Checker) handOn() { - for ch.running < ch.limit && len(ch.waiting) > 0 { - first := 0 - for i := range ch.waiting { - if before(ch.waiting[i], ch.waiting[first]) < 0 { - first = i - } - } - w := ch.waiting[first] - ch.waiting = slices.Delete(ch.waiting, first, first+1) - ch.running++ - close(w.ready) - } -} - func (ch *Checker) candidate(key string, fallback delivery.Candidate) delivery.Candidate { ch.mu.Lock() defer ch.mu.Unlock() diff --git a/modules/mesh-delivery-checker/internal/check/checker_test.go b/modules/mesh-delivery-checker/internal/check/checker_test.go index 295a9ff..d367ba6 100644 --- a/modules/mesh-delivery-checker/internal/check/checker_test.go +++ b/modules/mesh-delivery-checker/internal/check/checker_test.go @@ -298,24 +298,6 @@ func TestATrunkMoveRewritesTheSameVerdictKeyAndTheHeadsStatus(t *testing.T) { // } } -func TestNoCheckStartsWhileADeliveryBuildsAndOneStartsWhenItEnds(t *testing.T) { // N8/AC1 - ctx, j := context.Background(), passJudge() - r := newRig(t, j) - held := true - r.ch.Hold = func() bool { return held } - r.ch.Candidate(ctx, "feat/x", candidateOf(t, "feat/x", pullOf(repoA, 7, head1))) - r.ch.Wait() - if j.count() != 0 { - t.Fatalf("judged %d times while held", j.count()) - } - held = false - r.ch.Reconsider(ctx) - r.ch.Wait() - if j.count() != 1 { - t.Fatalf("judged %d times after the hold ended, want once", j.count()) - } -} - func TestTheStatusIsPendingWhileTheCheckRunsAndEndsSuccessOrFailure(t *testing.T) { // 1300/AC5 (unit part) ctx := context.Background() var seen []string diff --git a/modules/mesh-delivery-checker/internal/check/gate_test.go b/modules/mesh-delivery-checker/internal/check/gate_test.go index 6c709e0..fcb4f62 100644 --- a/modules/mesh-delivery-checker/internal/check/gate_test.go +++ b/modules/mesh-delivery-checker/internal/check/gate_test.go @@ -4,6 +4,7 @@ import ( "context" "strings" "testing" + "time" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -71,11 +72,12 @@ func TestATrunkMoveMarksEveryOpenPullRequestIntoItPendingAtOnce(t *testing.T) { t.Fatalf("%s#%d is %q before the move", p.repo, p.n, s) } } - // A delivery builds, so no check starts; mesh-delivery's trunk moves. - r.ch.Hold = func() bool { return true } + // mesh-delivery's trunk moves; the re-judges it asks are held in their judge (in the stream, since R4g) meanwhile. + release := make(chan struct{}) + r.ch.Judge = &judgeFunc{f: func(context.Context, *Case, int) (Outcome, error) { <-release; return Outcome{Verdict: passing()}, nil }} + defer func() { close(release); r.ch.Wait() }() r.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) r.ch.TrunkMoved(ctx) - r.ch.Wait() if s := owedState(t, r, repoA, 7, head1); s != "pending" { t.Errorf("mesh-delivery#7, into the moved trunk, is %q, want pending", s) } @@ -90,17 +92,25 @@ func TestATrunkMoveMarksEveryOpenPullRequestIntoItPendingAtOnce(t *testing.T) { func TestACheckThatEndsAfterItsTrunkMovedOwesPendingNotItsVerdict(t *testing.T) { // gate (a): a pass on an old trunk is never shown ctx := context.Background() var r *rig + release, judged := make(chan struct{}), make(chan struct{}) j := &judgeFunc{f: func(ctx context.Context, c *Case, call int) (Outcome, error) { if call == 1 { r.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) // the trunk moves while it runs - r.ch.Hold = func() bool { return true } // and a delivery holds the next check + defer close(judged) + } else { + <-release // the check asked again for the new trunk waits in the stream meanwhile } return Outcome{Verdict: passing()}, nil }} r = newRig(t, j) r.ch.Candidate(ctx, "feat%2Fx", candidateOf(t, "feat/x", pullOf(repoA, 7, head1))) - r.ch.Wait() + <-judged + for deadline := time.Now().Add(2 * time.Second); owedState(t, r, repoA, 7, head1) == "" && time.Now().Before(deadline); time.Sleep(time.Millisecond) { + } + time.Sleep(20 * time.Millisecond) if s := owedState(t, r, repoA, 7, head1); s != "pending" { t.Errorf("the status is %q, want pending: the pass was judged on a trunk that moved", s) } + close(release) + r.ch.Wait() } diff --git a/modules/mesh-delivery-checker/internal/check/hold.go b/modules/mesh-delivery-checker/internal/check/hold.go deleted file mode 100644 index 5725a8d..0000000 --- a/modules/mesh-delivery-checker/internal/check/hold.go +++ /dev/null @@ -1,7 +0,0 @@ -package check - -// PlansBuildingBucket is the controller's bucket of the delivery plans in their build phase, one key for each plan, written -// by the controller alone (issue 1652): a plan key there means a delivery builds on the build nodes, and no new check starts -// meanwhile (ADR 0484 clause 12, ADR 0483 clause 6). The keys age out with no delete marker, so the keys are listed, not watched. The bucket may be absent: the controller's issue 1652 is delivered -// before R4c-5b, and if it is not, the bucket is read as empty. -const PlansBuildingBucket = "mesh-controller_plans-building" diff --git a/modules/mesh-delivery-checker/internal/check/hold_test.go b/modules/mesh-delivery-checker/internal/check/hold_test.go deleted file mode 100644 index 35f6ef1..0000000 --- a/modules/mesh-delivery-checker/internal/check/hold_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package check - -import ( - "context" - "testing" - - "git.novox.be/novox/mesh-sdk/go/delivery" -) - -func TestARunningCheckIsNotStoppedWhenADeliveryStartsToBuild(t *testing.T) { // ADR 0484 clause 12, ADR 0483 clause 4 - ctx := context.Background() - held := false - j := &judgeFunc{f: func(_ context.Context, _ *Case, call int) (Outcome, error) { - if call == 1 { - held = true // a delivery starts while this check runs - } - return Outcome{Verdict: passing()}, nil - }} - r := newRig(t, j) - r.ch.Hold = func() bool { return held } - r.ch.Candidate(ctx, "feat/x", candidateOf(t, "feat/x", pullOf(repoA, 7, head1))) - r.ch.Wait() - if v := r.verdict(t, keyOf(t, "feat/x", candidateOf(t, "feat/x", pullOf(repoA, 7, head1)))); v.State != delivery.VerdictPass { - t.Errorf("verdict = %+v, want the running check to end as it was", v) - } - // A new candidate waits while the plan builds. - r.ch.Candidate(ctx, "feat/y", candidateOf(t, "feat/y", pullOf(repoA, 8, head2))) - r.ch.Wait() - if j.count() != 1 { - t.Errorf("judged %d times while a delivery builds, want only the one already running", j.count()) - } - held = false - r.ch.Reconsider(ctx) - r.ch.Wait() - if j.count() != 2 { - t.Errorf("judged %d times after the delivery ended, want the waiting candidate too", j.count()) - } -} diff --git a/modules/mesh-delivery-checker/internal/check/rank_test.go b/modules/mesh-delivery-checker/internal/check/rank_test.go index 0de4329..10d306c 100644 --- a/modules/mesh-delivery-checker/internal/check/rank_test.go +++ b/modules/mesh-delivery-checker/internal/check/rank_test.go @@ -3,10 +3,8 @@ package check import ( "context" "fmt" - "runtime" "sync" "testing" - "time" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -27,146 +25,8 @@ type rankRig struct { errs []error // ctx errors the held check saw when released } -func newRankRig(t *testing.T) *rankRig { - rr := &rankRig{held: make(chan struct{}), release: make(chan struct{})} - j := &judgeFunc{f: func(ctx context.Context, c *Case, _ int) (Outcome, error) { - rr.mu.Lock() - rr.started = append(rr.started, c) - hold := rr.hold - rr.hold = false - rr.mu.Unlock() - if hold { - close(rr.held) - <-rr.release - rr.mu.Lock() - rr.errs = append(rr.errs, ctx.Err()) - rr.mu.Unlock() - } - return Outcome{Verdict: passing()}, nil - }} - rr.rig = newRig(t, j) - rr.ch.Limit = 1 - now := time.Date(2026, 10, 23, 14, 0, 0, 0, time.UTC) - var clock sync.Mutex - rr.ch.Now = func() time.Time { - clock.Lock() - defer clock.Unlock() - now = now.Add(time.Second) - return now - } - return rr -} - func group(n int) string { return fmt.Sprintf("feat/g%d", n) } -// judged puts n candidates in and judges them on the current trunk. -func (rr *rankRig) judged(t *testing.T, n int) { - for i := 1; i <= n; i++ { - rr.ch.Candidate(context.Background(), group(i), candidateOf(t, group(i), pullOf(repoA, i, head1))) - } - rr.ch.Wait() - rr.mu.Lock() - rr.started = nil - rr.mu.Unlock() -} - -// waitFor returns when n checks wait for a place. -func (rr *rankRig) waitFor(t *testing.T, n int) { - t.Helper() - deadline := time.Now().Add(5 * time.Second) - for { - if w := len(rr.ch.Waiting()); w == n { - return - } else if time.Now().After(deadline) { - t.Fatalf("%d checks wait, want %d", w, n) - } - runtime.Gosched() - } -} - -// moveTrunkHoldingOne moves the trunk, so every judged candidate is asked again; the first re-judge to start is held. -func (rr *rankRig) moveTrunkHoldingOne(t *testing.T, judged int) { - rr.mu.Lock() - rr.hold = true - rr.mu.Unlock() - rr.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) - rr.ch.TrunkMoved(context.Background()) - <-rr.held - rr.waitFor(t, judged-1) -} - -func TestANewHeadStartsBeforeTheReJudgesOfATrunkMove(t *testing.T) { // 1897/AC1 - rr := newRankRig(t) - rr.judged(t, 4) - rr.moveTrunkHoldingOne(t, 4) // one runs, three re-judges wait - rr.ch.Candidate(context.Background(), group(9), candidateOf(t, group(9), pullOf(repoA, 9, head2))) - rr.waitFor(t, 4) - close(rr.release) - rr.ch.Wait() - if len(rr.started) != 5 { - t.Fatalf("%d checks started, want 5", len(rr.started)) - } - if g := rr.started[1].Group; g != group(9) { - t.Errorf("after the running check, %s started, want the new head's %s; order %v", g, group(9), groups(rr.started)) - } -} - -func TestTheReJudgeQueuedFirstStartsFirst(t *testing.T) { // 1897/AC2 - rr := newRankRig(t) - rr.judged(t, 3) - rr.moveTrunkHoldingOne(t, 3) // one runs, two re-judges wait - close(rr.release) - rr.ch.Wait() - if len(rr.started) != 3 { - t.Fatalf("%d checks started, want 3", len(rr.started)) - } - if a, b := rr.started[1], rr.started[2]; !a.Queued.Before(b.Queued) { - t.Errorf("%s (queued %s) started before %s (queued %s)", a.Group, a.Queued.Format(time.TimeOnly), b.Group, b.Queued.Format(time.TimeOnly)) - } -} - -func TestANewHeadDoesNotStopARunningReJudge(t *testing.T) { // 1897/AC3 - rr := newRankRig(t) - rr.judged(t, 1) - rr.moveTrunkHoldingOne(t, 1) // the re-judge runs - rr.ch.Candidate(context.Background(), group(9), candidateOf(t, group(9), pullOf(repoA, 9, head2))) - rr.waitFor(t, 1) - close(rr.release) - rr.ch.Wait() - if len(rr.errs) != 1 || rr.errs[0] != nil { - t.Fatalf("the running re-judge saw %v, want it never stopped", rr.errs) - } - if len(rr.started) != 2 || rr.started[0].Group != group(1) || rr.started[1].Group != group(9) { - t.Errorf("started %v, want the re-judge to end before the new head starts", groups(rr.started)) - } - key := keyOf(t, group(1), candidateOf(t, group(1), pullOf(repoA, 1, head1))) - if v := rr.verdict(t, key); v.State != delivery.VerdictPass || v.Trunk[repoA] != trunkA2 { - t.Errorf("the re-judge's verdict = %+v, want its pass on the moved trunk", v) - } -} - -func TestWaitingChecksAreNamedByKind(t *testing.T) { // 1897/AC4, the checker's own view - rr := newRankRig(t) - rr.judged(t, 2) - rr.moveTrunkHoldingOne(t, 2) - rr.ch.Candidate(context.Background(), group(9), candidateOf(t, group(9), pullOf(repoA, 9, head2))) - rr.waitFor(t, 2) - kinds := map[string]string{} - for _, w := range rr.ch.Waiting() { - kinds[w.Group] = w.Kind - } - close(rr.release) - rr.ch.Wait() - running := rr.started[0].Group - other := group(1) - if running == other { - other = group(2) - } - if kinds[group(9)] != KindNew || kinds[other] != KindTrunk || len(kinds) != 2 { - t.Errorf("waiting = %v, want %s %s and %s %s", kinds, group(9), KindNew, other, KindTrunk) - } -} - func groups(cs []*Case) []string { out := make([]string, len(cs)) for i, c := range cs { diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index 7fd9055..e87de2e 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -8,7 +8,6 @@ import ( "time" "git.novox.be/novox/mesh-sdk/go/delivery" - "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" ) // The read tools (novox/hq ADR 0484 clauses 8 and 13, issue 1686): what the checker stored, as stored, with the line of the @@ -34,12 +33,10 @@ type StateRead struct { type Reader struct { VerdictState, KeptState, CandidateState StateRead - PreviewState StateRead // the checker's merge previews - PlansState StateRead // the controller's delivery plans in their build phase; zero where the bucket is absent - StepStates map[string]StateRead // the build modules' steps states, by module - Capacity func() (string, error) // the capacity setting as written, `node=steps,...` - HaveState StateRead // the node-engine's have: whether a build module is on a node and running - StepBound time.Duration // how long the checker waits for one step; DefaultStepBound when 0 + PreviewState StateRead // the checker's merge previews + StepStates map[string]StateRead // the build modules' steps states, by module + HaveState StateRead // the node-engine's have: whether a build module is on a node and running + StepBound time.Duration // how long the checker waits for one step; DefaultStepBound when 0 // Trunks are the trunk heads of every repository (the gate's source); nil names no re-judge after a trunk move. Trunks func() (map[string]string, error) Now func() time.Time @@ -266,18 +263,10 @@ type LeftStep struct { Why string `json:"why"` } -// Status is what holds the checker back and what runs now, read from the buckets: the checker's held and running steps live in -// its process, so they are shown as what the buckets say (the steps in flight on the build modules, the candidates without a -// verdict for their heads, and the delivery plans in their build phase), not as the process's own counters. +// Status is what runs now, read from the buckets: the steps in flight on the build modules and the candidates without a +// verdict for their heads. Nothing holds a check since R4g: a check is an ask on the build seats' streams, whose backlog +// the delivery view shows (ADR 0525 §9). type Status struct { - Capacity map[string]int `json:"capacity"` - CapacityNote string `json:"capacity_note,omitempty"` - DefaultPerNode int `json:"default_per_node"` - PlansBuilding struct { - Readable bool `json:"readable"` - Keys []string `json:"keys"` - Holds bool `json:"holds"` - } `json:"plans_building"` Running []RunningStep `json:"running_steps"` LeftRunning []LeftStep `json:"left_running"` RunningByNode map[string]int `json:"running_by_node"` @@ -288,24 +277,9 @@ type Status struct { Notes []string `json:"notes,omitempty"` } -// Status reads the capacity, the plan bucket, the build modules' steps and the candidates against their verdicts. +// Status reads the build modules' steps and the candidates against their verdicts. func (r Reader) Status() (Status, error) { - st := Status{DefaultPerNode: buildrun.DefaultCapacity, Capacity: map[string]int{}, RunningByNode: map[string]int{}} - if r.Capacity == nil { - st.CapacityNote = "the capacity setting cannot be read from here" - } else if setting, err := r.Capacity(); err != nil { - st.CapacityNote = "the capacity setting cannot be read: " + err.Error() - } else if st.Capacity, err = buildrun.ParseCapacity(setting); err != nil { - st.Capacity, st.CapacityNote = map[string]int{}, "the capacity setting is not valid: "+err.Error() - } - if r.PlansState.Keys == nil { - st.Notes = append(st.Notes, PlansBuildingBucket+" is not readable here (absent), so it is read as empty: no delivery holds the checker") - } else if keys, err := r.PlansState.Keys(); err != nil { - st.Notes = append(st.Notes, PlansBuildingBucket+" cannot be listed, so it is read as empty: "+err.Error()) - } else { - sort.Strings(keys) - st.PlansBuilding.Readable, st.PlansBuilding.Keys, st.PlansBuilding.Holds = true, keys, len(keys) > 0 - } + st := Status{RunningByNode: map[string]int{}} mods := make([]string, 0, len(r.StepStates)) for m := range r.StepStates { mods = append(mods, m) diff --git a/modules/mesh-delivery-checker/internal/check/read_test.go b/modules/mesh-delivery-checker/internal/check/read_test.go index bbf5079..313b218 100644 --- a/modules/mesh-delivery-checker/internal/check/read_test.go +++ b/modules/mesh-delivery-checker/internal/check/read_test.go @@ -3,7 +3,6 @@ package check import ( "context" "encoding/json" - "os" "strings" "testing" "time" @@ -129,16 +128,14 @@ func TestThePreviewsToolShowsTheStoredPreviewOfAPullRequest(t *testing.T) { } } -// 1685 live acceptance: checker_status reads the capacity, whether a plan key holds the checker, the steps in flight by node and -// the candidates without a verdict. -func TestTheStatusToolReadsCapacityHoldsAndRunningSteps(t *testing.T) { +// checker_status reads the steps in flight by node and the candidates without a verdict; since R4g nothing holds the checker +// and it has no capacity of its own (ADR 0525 §4, §7; 1685 closed by the hold's deletion). +func TestTheStatusToolReadsRunningSteps(t *testing.T) { running, _ := delivery.EncodeStep(delivery.Step{ID: "abc1", Tool: "build_go_test", Node: "shanks", State: delivery.StepRunning, Started: time.Date(2026, 10, 22, 15, 0, 0, 0, time.UTC)}) passed, _ := delivery.EncodeStep(delivery.Step{ID: "abc2", Tool: "build_go_test", Node: "ace", State: delivery.StepPassed}) cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) ckey, _ := delivery.CandidatesKey("feat/x") rd := Reader{ - Capacity: func() (string, error) { return "shanks=2,ace=0", nil }, - PlansState: fakeState{"p1": []byte("{}")}.read(), StepStates: map[string]StateRead{"mesh-build-go": fakeState{"shanks.abc1": running, "ace.abc2": passed}.read()}, CandidateState: fakeState{ckey: cand}.read(), VerdictState: fakeState{}.read(), @@ -149,21 +146,15 @@ func TestTheStatusToolReadsCapacityHoldsAndRunningSteps(t *testing.T) { if err != nil { t.Fatal(err) } - if st.Capacity["shanks"] != 2 || st.Capacity["ace"] != 0 || st.DefaultPerNode != 1 { - t.Errorf("capacity = %v default %d", st.Capacity, st.DefaultPerNode) - } - if !st.PlansBuilding.Holds || len(st.PlansBuilding.Keys) != 1 { - t.Errorf("plans = %+v", st.PlansBuilding) - } if len(st.Running) != 1 || st.Running[0].Node != "shanks" || st.RunningByNode["shanks"] != 1 || st.RunningByNode["ace"] != 0 { t.Errorf("running = %+v %v", st.Running, st.RunningByNode) } if len(st.Unjudged) != 1 || st.Unjudged[0] != "feat/x" { t.Errorf("unjudged = %v", st.Unjudged) } - // An absent plan bucket is read as empty and said; an unreadable capacity is said, not guessed. - st, err = Reader{Capacity: func() (string, error) { return "", os.ErrNotExist }}.Status() - if err != nil || st.PlansBuilding.Holds || st.CapacityNote == "" || len(st.Notes) == 0 { + // What cannot be read is said, not guessed. + st, err = Reader{}.Status() + if err != nil || len(st.Notes) == 0 { t.Errorf("empty status = %+v, %v", st, err) } } diff --git a/modules/mesh-delivery-checker/internal/check/try.go b/modules/mesh-delivery-checker/internal/check/try.go index 77e42d3..41024bc 100644 --- a/modules/mesh-delivery-checker/internal/check/try.go +++ b/modules/mesh-delivery-checker/internal/check/try.go @@ -206,13 +206,11 @@ type TryOptions struct { Run string } -// Tryer runs the tries: it takes a queued try, judges it through the same Judge as a pull request's check, and writes its -// end. It holds no timer: a try starts when its key is written, or when a delivery that held it ends (Reconsider). +// Tryer runs the tries: it takes a queued try, judges it through the same Judge as a pull request's check (a check ask, ADR +// 0525 §1), and writes its end. It holds no timer: a try starts when its key is written. type Tryer struct { Tries KV Judge Judge - // Hold says whether a delivery builds, so no try starts (ADR 0484 clause 12); nil never holds. - Hold func() bool Bound time.Duration Now func() time.Time Logf func(format string, args ...any) @@ -250,7 +248,6 @@ func (tr *Tryer) Put(ctx context.Context, key string, value []byte) { if t.Over() { return } - held := tr.Hold != nil && tr.Hold() tr.mu.Lock() if tr.inflight == nil { tr.inflight, tr.seen = map[string]bool{}, map[string]bool{} @@ -259,11 +256,6 @@ func (tr *Tryer) Put(ctx context.Context, key string, value []byte) { tr.mu.Unlock() return } - if held { - tr.mu.Unlock() // not seen yet: a try left running is still taken over when the hold ends - tr.logf("%s: not started, a delivery builds", key) - return - } tr.seen[key] = true tr.inflight[key] = true tr.mu.Unlock() diff --git a/modules/mesh-delivery-checker/internal/check/try_test.go b/modules/mesh-delivery-checker/internal/check/try_test.go index 5c9e5a0..07105ca 100644 --- a/modules/mesh-delivery-checker/internal/check/try_test.go +++ b/modules/mesh-delivery-checker/internal/check/try_test.go @@ -156,7 +156,7 @@ func TestAFailingTryNamesWhatFailedAndAStepThatCannotRunIsAnError(t *testing.T) // No build node is no error since R4g: the try's ask waits in the check stream until a node pulls it (ADR 0525 §4). } -func TestATryWaitsWhileADeliveryBuildsAndATryLeftRunningIsTakenOver(t *testing.T) { +func TestATryLeftRunningIsTakenOver(t *testing.T) { ctx := context.Background() kv := newMemKV() id, _ := AskTry(memTries{kv}, tryTrunks, TryAsk{Repository: repoA, Head: head1}, time.Now()) @@ -169,14 +169,7 @@ func TestATryWaitsWhileADeliveryBuildsAndATryLeftRunningIsTakenOver(t *testing.T t.Fatal(err) } b := newBench(t) - held := true - tr := &Tryer{Tries: kv, Judge: b.ex, Hold: func() bool { return held }} - tr.Reconsider(ctx) - tr.Wait() - if n := len(b.steps.tool("fetch")); n != 0 { - t.Fatalf("a try ran while a delivery builds") - } - held = false + tr := &Tryer{Tries: kv, Judge: b.ex} tr.Reconsider(ctx) tr.Wait() if got := readTry(t, kv, id); got.State != TryPass { diff --git a/modules/mesh-delivery-checker/manifest_test.go b/modules/mesh-delivery-checker/manifest_test.go index a4b5442..62ebd1f 100644 --- a/modules/mesh-delivery-checker/manifest_test.go +++ b/modules/mesh-delivery-checker/manifest_test.go @@ -46,6 +46,7 @@ type manifest struct { Consumes []string `json:"consumes"` State []state `json:"state"` Reads []string `json:"reads"` + Uses []string `json:"uses"` Invokes []string `json:"invokes"` Config map[string]struct { Type string `json:"type"` @@ -96,12 +97,17 @@ func TestManifestDeclarations(t *testing.T) { // 1678/AC1 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", - "mesh-controller.plans-building", "mesh-delivery-builder.builds", "mesh-delivery-gate.source", "mesh-delivery-indexer.candidates", + "mesh-delivery-builder.builds", "mesh-delivery-gate.source", "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{"gitea.gitea_commits", "gitea.gitea_compare", "gitea.gitea_get_file", "mesh-build-go.build_go_digest", "mesh-build-go.build_go_fetch", "mesh-build-go.build_go_test", "mesh-build-node.build_node_fetch", "mesh-build-node.build_node_test"}; !reflect.DeepEqual(sorted(m.Invokes), want) { - t.Errorf("invokes = %v, want only the build modules' fetch, digest and test (no run step before issue 1309, ADR 0484 clause 5) and the forge's two reads for a mesh-sdk pin (clause 9)", m.Invokes) + // 1887/A5b: the checker reads nothing of the controller's plans-building (ADR 0483 superseded by ADR 0525 §7), calls no + // build module's tool and holds no capacity: it puts check asks on the Go and Node build seats (ADR 0525 §1, §4). + if want := []string{"gitea.gitea_commits", "gitea.gitea_compare", "gitea.gitea_get_file"}; !reflect.DeepEqual(sorted(m.Invokes), want) { + t.Errorf("invokes = %v, want only the forge's reads for a mesh-sdk pin (ADR 0484 clause 9)", m.Invokes) + } + if want := []string{"node-build-go", "node-build-node"}; !reflect.DeepEqual(sorted(m.Uses), want) { + t.Errorf("uses = %v, want the Go and Node build seats", m.Uses) } // The checker 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) { @@ -122,7 +128,7 @@ func TestManifestDeclarations(t *testing.T) { // 1678/AC1 if want := []string{"checker_kept", "checker_previews", "checker_pull", "checker_status", "checker_try", "checker_try_read", "checker_verdicts"}; !reflect.DeepEqual(sorted(m.Tools), want) { t.Errorf("tools = %v, want %v", m.Tools, want) } - for _, k := range []string{"logs", "emits", "claims", "provides", "requires", "uses", "defines-seats", "seat", "seats", "bus"} { + for _, k := range []string{"logs", "emits", "claims", "provides", "requires", "defines-seats", "seat", "seats", "bus", "config"} { if _, ok := top[k]; ok { t.Errorf("the manifest declares %q: the checker has none", k) } @@ -148,35 +154,19 @@ func TestManifestProcess(t *testing.T) { // 1678/AC1 t.Errorf("process = %v", p) } env, _ := p["env"].(map[string]any) - // The capacity of each node is the module's setting `capacity` (ADR 0484 clause 12, as the builder's, issue 1661), laid into the - // settings file the program reads; empty is one step at a time everywhere. The environment holds no capacity. - if len(env) != 2 || env["MESH_BROKER_FILE"] != "${dir:state}/broker" || env["MESH_CHECKER_SETTINGS"] != "${dir:state}/settings.json" { - t.Errorf("env = %v", p["env"]) + if len(env) != 1 || env["MESH_BROKER_FILE"] != "${dir:state}/broker" { + t.Errorf("env = %v, want the broker file alone", p["env"]) } - if !reflect.DeepEqual(p["restart-on"], []any{"settings"}) { - t.Errorf("restart-on = %v, want the settings file, so a changed capacity reaches the running program", p["restart-on"]) - } - c, ok := m.Config["capacity"] - if !ok || len(m.Config) != 1 || c.Type != "string" || c.Default != "" || c.Description == "" { - t.Errorf("config = %+v, want one string setting, capacity, empty (one step at a time on every node) to start", m.Config) - } - var settings map[string]any for _, r := range m.Resources { if r["id"] == "settings" { - settings = r + t.Errorf("a settings resource %v: the checker has no capacity of its own (ADR 0525 §4)", r) } } - if settings == nil || settings["type"] != "file" || settings["merge"] != "json" || !reflect.DeepEqual(settings["takes-config"], []any{"capacity"}) || settings["path"] != "${dir:state}/settings.json" { - t.Errorf("settings resource = %v", settings) - } - if raw, err := os.ReadFile("files/settings.json"); err != nil || !strings.Contains(string(raw), `"capacity": ""`) { - t.Errorf("files/settings.json = %q, %v, want the setting's starting value", raw, err) - } if _, bus := p["bus"]; bus { t.Errorf("the process declares a bus: it runs on its own broker credential") } - if len(m.Build.Artifacts) != 3 || m.Build.Artifacts[2].Name != "files" || m.Build.Artifacts[2].Kind != "archive" || m.Build.Artifacts[2].From != "files" { - t.Fatalf("artifacts = %+v, want the program, the tools and the files", m.Build.Artifacts) + if len(m.Build.Artifacts) != 2 { + t.Fatalf("artifacts = %+v, want the program and the tools", m.Build.Artifacts) } if a := m.Build.Artifacts[1]; a.Name != "tools" || a.Kind != "bundle" || a.Language != "go" || a.Binary != "mesh-delivery-checker-tools" || !reflect.DeepEqual(a.Loads, []string{"mesh-delivery-checker-tools"}) { diff --git a/modules/mesh-delivery-checker/module.json b/modules/mesh-delivery-checker/module.json index 2c22576..eeaf0ab 100644 --- a/modules/mesh-delivery-checker/module.json +++ b/modules/mesh-delivery-checker/module.json @@ -21,7 +21,6 @@ "mesh-build-go.toolchains", "mesh-build-node.steps", "mesh-build-node.toolchains", - "mesh-controller.plans-building", "mesh-delivery-builder.builds", "mesh-delivery-gate.source", "mesh-delivery-indexer.candidates", @@ -30,6 +29,10 @@ "mesh-delivery-watchman.presence", "node-engine.have" ], + "uses": [ + "node-build-go", + "node-build-node" + ], "tools": [ "checker_kept", "checker_previews", @@ -42,21 +45,9 @@ "invokes": [ "gitea.gitea_commits", "gitea.gitea_compare", - "gitea.gitea_get_file", - "mesh-build-go.build_go_digest", - "mesh-build-go.build_go_fetch", - "mesh-build-go.build_go_test", - "mesh-build-node.build_node_fetch", - "mesh-build-node.build_node_test" + "gitea.gitea_get_file" ], - "config": { - "capacity": { - "type": "string", - "description": "How many build steps the checker runs at once on each node, as node=steps pairs separated by commas, such as shanks=2,ace=1 (ADR 0484 clause 12); a node set to 0 never checks, a node not named runs one at a time, and empty is one everywhere. The builder's capacity is the builder's own.", - "pattern": "|[a-z0-9_-]+=[0-9]+(,[a-z0-9_-]+=[0-9]+)*", - "default": "" - } - }, + "own-secrets": { "broker": "${dir:state}/broker" }, @@ -77,17 +68,6 @@ "place": ".", "unknown": "program" }, - { - "id": "settings", - "trusted": false, - "type": "file", - "path": "${dir:state}/settings.json", - "mode": "0600", - "owner": "mesh-delivery-checker", - "takes-config": ["capacity"], - "merge": "json", - "template": "files/settings.json" - }, { "id": "checker", "type": "process", @@ -97,12 +77,8 @@ "./mesh-delivery-checker" ], "env": { - "MESH_BROKER_FILE": "${dir:state}/broker", - "MESH_CHECKER_SETTINGS": "${dir:state}/settings.json" + "MESH_BROKER_FILE": "${dir:state}/broker" }, - "restart-on": [ - "settings" - ], "user": "mesh-delivery-checker", "health": { "kind": "unit" @@ -129,11 +105,6 @@ "loads": [ "mesh-delivery-checker-tools" ] - }, - { - "name": "files", - "kind": "archive", - "from": "files" } ] }