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 4a03d309..d2dc3250 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 @@ -398,7 +398,7 @@ func startBed(t *testing.T) *bed { delivery.CheckerOwedBucket: delivery.CheckerOwedHistory, delivery.PreviewsBucket: delivery.PreviewsHistory, delivery.CandidatesBucket: delivery.CandidatesHistory, delivery.ModulesBucket: delivery.ModulesHistory, delivery.ManifestsBucket: delivery.ManifestsHistory, delivery.SourceBucket: delivery.SourceHistory, delivery.PresenceBucket: delivery.PresenceHistory, delivery.HaveBucket: delivery.HaveHistory, check.TriesBucket: 1, - delivery.TurnsBucket: delivery.TurnsHistory} { + delivery.TurnsBucket: delivery.TurnsHistory, delivery.BuilderBuildsBucket: 1} { kv, err := js.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: name, History: uint8(history)}) if err != nil { t.Fatal(err) @@ -578,12 +578,50 @@ func (b *bed) owed(t *testing.T, number int, head, state string) delivery.Owed { // TestCheckerJudgesACandidateAndOwesItsStatus: a candidate the indexer wrote, one pull request of mesh-delivery, is fetched // merged onto the trunk the gate names, digested and tested at both levels on the node, kept, previewed; the verdict passes // with the counts of both levels and the composition not run, and the status mesh/check is owed on the head as success. -func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300/AC5, 1445/AC1, 1929/AC1 +func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300/AC5, 1445/AC1, 1929/AC1, ADR 0603 §10 b := startBed(t) - key := b.candidate(t, "feat/x", pull(repo, 7, head1)) + // ADR 0603 §10: the candidate changes the module m, whose manifest the trunk holds; the builder has not built m's + // candidate fingerprint yet, so the check waits on the builder's key before it composes. + manifest := `{"module":"m"}` + digest := delivery.ManifestDigest([]byte(manifest)) + stored, err := delivery.EncodeManifest(delivery.Manifest{Module: "m", JSON: manifest}) + if err != nil { + t.Fatal(err) + } + mkey, _ := delivery.ManifestsKey(digest) + if _, err := b.buckets[delivery.ManifestsBucket].Put(b.ctx, mkey, stored); err != nil { + t.Fatal(err) + } + known, err := delivery.EncodeModule(delivery.Module{Repository: repo, Commit: trunk, Path: "modules/m", Manifest: digest, None2: "the fingerprint is computed with its chain"}) + if err != nil { + t.Fatal(err) + } + if _, err := b.buckets[delivery.ModulesBucket].Put(b.ctx, "m", known); err != nil { + t.Fatal(err) + } + fp := "src2:" + strings.Repeat("f", 64) + p := pull(repo, 7, head1) + p.Modules, p.Fps = []string{"m"}, map[string]delivery.CandidateModule{"m": {FP2: fp, Manifest: digest}} + key := b.candidate(t, "feat/x", p) // ADR 0593 §7, §9 (D12): n1 and n2 are placed; n1's engine answers compose-candidate on the bus, n2's never does. asked := b.engine(t, "n1", "n2") b.start(t, nil) + b.said(t, "waits on the builder's builds of m before it composes") + if got := asked(); len(got) != 0 { + t.Fatalf("the engines were asked %+v before m's build ended", got) + } + bkey, _ := delivery.BuilderBuildKey("m", fp) + building, _ := json.Marshal(delivery.BuilderBuild{V: 1, State: delivery.BuildBuilding}) + if _, err := b.buckets[delivery.BuilderBuildsBucket].Put(b.ctx, bkey, building); err != nil { + t.Fatal(err) + } + if v := b.verdict(t, key, func(delivery.Verdict) bool { return true }); v.State != delivery.VerdictRunning || len(asked()) != 0 { + t.Fatalf("verdict %s and %d asks while m builds: the check composes only once its build ended", v.State, len(asked())) + } + built, _ := json.Marshal(delivery.BuilderBuild{V: 1, State: delivery.BuildDone, Manifest: digest}) + if _, err := b.buckets[delivery.BuilderBuildsBucket].Put(b.ctx, bkey, built); err != nil { + t.Fatal(err) + } v := b.verdict(t, key, ended) if v.State != delivery.VerdictPass || v.Levels[delivery.LevelUnit].Run != 2 || v.Levels[delivery.LevelIntegration].Run != 2 || v.Trunk[repo] != trunk { t.Fatalf("verdict = %+v", v) @@ -592,6 +630,9 @@ func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300 !strings.Contains(c.Words, "n2: "+delivery.ComposeNoAnswer) || len(v.Machines) != 1 || v.Machines[0] != "n1" { t.Errorf("composition %+v machines %v", v.Composition, v.Machines) } + if v.Composition != nil && strings.Contains(v.Composition.Words, "did not end") { + t.Errorf("the composition says %q: m's build ended within the bound", v.Composition.Words) + } if got := asked(); len(got) == 0 || got[0].Candidate != key { t.Errorf("n1's engine was asked %+v, want the check's candidate %s", got, key) } 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 532f5fff..949f4376 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -8,6 +8,7 @@ import ( "sort" "strings" "sync" + "time" "github.com/nats-io/nats.go/jetstream" @@ -32,6 +33,10 @@ type judgeDeps struct { forge check.Forge announce check.Announcer builds check.BuildStates + // ends waits on the builder's builds of a candidate before the check composes (ADR 0603 §10); nil composes without waiting. + ends check.BuildEnds + // buildWait is the setting build-wait-bound; check.DefaultBuildWait when 0. + buildWait time.Duration // compositions makes a check's composition from the engines' answers (ADR 0593 §7); nil leaves it not run. compositions check.CompositionReader // nodes names every node with a record; nil hands no names. @@ -98,7 +103,7 @@ func newJudge(d judgeDeps) check.Judge { nodes = d.nodes } return &check.Examiner{Nodes: nodes, Compositions: d.compositions, Asks: d.asks, Toolchains: d.tools, - Kept: check.Kept{Store: d.kept}, Previews: d.previews, Modules: d.repos, Packages: d.repos, Published: d.published, Forge: d.forge, Runs: d.repos, Builds: d.builds, Announce: d.announce, Trunks: d.trunks, Manifests: d.repos, Read: d.repos, Predefined: d.predefined, Logf: d.logf} + Kept: check.Kept{Store: d.kept}, Previews: d.previews, Modules: d.repos, Packages: d.repos, Published: d.published, Forge: d.forge, Runs: d.repos, Builds: d.builds, BuildEnds: d.ends, BuildWait: d.buildWait, Announce: d.announce, Trunks: d.trunks, Manifests: d.repos, Read: d.repos, Predefined: d.predefined, Logf: d.logf} } // manifestKV reads a manifest by its digest. @@ -294,6 +299,60 @@ func (r buildReader) Build(ctx context.Context, module, fp2 string) (delivery.Bu return b, err == nil, err } +// buildEnds hears the builder's build keys of a candidate on the bus (mesh-delivery-builder_builds): a watch of exactly +// those keys gives each one's value now and every write the builder makes after, so the check waits on the part that owns +// the fact and polls nothing (novox/hq ADR 0603 §10). +type buildEnds struct{ kv jetstream.KeyValue } + +func (e buildEnds) Ended(ctx context.Context, builds map[string]string) (map[string]delivery.BuilderBuild, error) { + byKey := map[string]string{} + keys := make([]string, 0, len(builds)) + for module, fp2 := range builds { + key, err := delivery.BuilderBuildKey(module, fp2) + if err != nil { + return nil, err + } + byKey[key] = module + keys = append(keys, key) + } + ended := map[string]delivery.BuilderBuild{} + if len(keys) == 0 { + return ended, nil + } + w, err := e.kv.WatchFiltered(ctx, keys) + if err != nil { + return nil, err + } + defer func() { _ = w.Stop() }() + for len(ended) < len(keys) { + select { + case <-ctx.Done(): + return ended, nil // the bound: the builds not ended are left out + case entry, open := <-w.Updates(): + if !open { + return ended, nil + } + if entry == nil || entry.Operation() != jetstream.KeyValuePut { + continue // the end of the values now, or a key deleted: it has not ended + } + module, ours := byKey[entry.Key()] + if !ours { + continue + } + b, err := delivery.DecodeBuilderBuild(entry.Value()) + if err != nil { + continue // not a build the check can read: it has not ended for the check + } + if b.State == delivery.BuildDone || b.State == delivery.BuildFailed { + ended[module] = b + } else { + delete(ended, module) // asked again (builder_build_again): it runs once more + } + } + } + return ended, nil +} + // ManifestDigests is the manifest digest of every module the indexer holds, by module: the catalogue the indexer's judge // judges a change's manifests in (novox/hq ADR 0578 §2). func (r *repoModules) ManifestDigests() map[string]string { 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 ef544bcb..8ca5d68c 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -166,14 +166,16 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro // The builder's builds say how the candidates' fingerprints stand (issue 1684); where its bucket is not there yet the line // says nothing of the builds, until the checker starts again. var builds check.BuildStates - if kv := process.OptionalBucket(ctx, js, delivery.BuilderBuildsBucket, "the line says nothing of the builds until the checker starts again", logger.Printf); kv != nil { - builds = buildReader{kvStore{kv}} + var ends check.BuildEnds + if kv := process.OptionalBucket(ctx, js, delivery.BuilderBuildsBucket, "the line says nothing of the builds, and a check composes without waiting on them, until the checker starts again", logger.Printf); kv != nil { + builds, ends = buildReader{kvStore{kv}}, buildEnds{kv} } // The check's composition is the checker's, from what each placed node's engine answers (ADR 0593 §7, §9). - answerBound, err := composeAnswerBound(getenv("MESH_CHECKER_SETTINGS")) + set, err := readSettings(getenv("MESH_CHECKER_SETTINGS")) if err != nil { return err } + answerBound := set.answer holdCallsFor(answerBound) verdicts := &manifestVerdicts{} compositions := check.EngineCompositions{ @@ -212,7 +214,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro ch.Judge = newJudge(judgeDeps{ asks: buildrun.Asker{Publisher: asks, Waiter: waiter, Bound: stepBound}, tools: tools, trunks: trunks, kept: kvStore{buckets[delivery.CheckerKeptBucket]}, previews: kvStore{buckets[delivery.PreviewsBucket]}, - repos: repos, published: published, builds: builds, forge: forgeReads{call: caller.Call}, logf: logger.Printf, + repos: repos, published: published, builds: builds, ends: ends, buildWait: set.buildWait, forge: forgeReads{call: caller.Call}, logf: logger.Printf, announce: announce, compositions: compositions, nodes: openNodeNames(ctx, js, logger.Printf), predefined: keptPredefined{process.AwaitBucket(ctx, js, conn, delivery.IndexerModule+"_"+delivery.IndexerPredefinedState, diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go index c3c44afd..19d895f3 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go @@ -10,38 +10,57 @@ import ( "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/process" + + "mesh-delivery-checker/internal/check" ) // The checker's own settings (its config, written by the node-engine into ${dir:state}/settings.json, named to the // process by MESH_CHECKER_SETTINGS): compose-answer-bound, the seconds a check waits for a node's engine to answer -// compose-candidate (novox/hq ADR 0593 §7, §9: 60 by default; the checker owns it). +// compose-candidate (novox/hq ADR 0593 §7, §9: 60 by default), and build-wait-bound, the seconds a check waits for the +// builder's builds of its candidate before it composes (ADR 0603 §10: 1800 by default). The checker owns both. -// composeAnswerBound reads compose-answer-bound from the settings file at path: delivery.ComposeAnswerBound where no -// file or no value is there; a value that is no number of seconds from 1 to 600 is refused, naming it. -func composeAnswerBound(path string) (time.Duration, error) { +// settings are the checker's settings as read. +type settings struct { + // answer is compose-answer-bound; buildWait is build-wait-bound. + answer, buildWait time.Duration +} + +// readSettings reads the settings file at path: each value's default where no file or no value is there; a value that is +// no whole number of seconds within its range is refused, naming it. +func readSettings(path string) (settings, error) { + out := settings{answer: delivery.ComposeAnswerBound, buildWait: check.DefaultBuildWait} if path == "" { - return delivery.ComposeAnswerBound, nil + return out, nil } raw, err := os.ReadFile(path) if errors.Is(err, fs.ErrNotExist) { - return delivery.ComposeAnswerBound, nil + return out, nil } if err != nil { - return 0, err + return settings{}, err } var s struct { - Bound *float64 `json:"compose-answer-bound"` + Answer *float64 `json:"compose-answer-bound"` + BuildWait *float64 `json:"build-wait-bound"` } if err := json.Unmarshal(raw, &s); err != nil { - return 0, fmt.Errorf("%s is no settings file: %w", path, err) + return settings{}, fmt.Errorf("%s is no settings file: %w", path, err) } - if s.Bound == nil { - return delivery.ComposeAnswerBound, nil + for _, v := range []struct { + name string + value *float64 + min, max float64 + into *time.Duration + }{{"compose-answer-bound", s.Answer, 1, 600, &out.answer}, {"build-wait-bound", s.BuildWait, 60, 7200, &out.buildWait}} { + if v.value == nil { + continue + } + if *v.value < v.min || *v.value > v.max || *v.value != float64(int(*v.value)) { + return settings{}, fmt.Errorf("%s is %v: a whole number of seconds from %v to %v", v.name, *v.value, v.min, v.max) + } + *v.into = time.Duration(*v.value) * time.Second } - if *s.Bound < 1 || *s.Bound > 600 || *s.Bound != float64(int(*s.Bound)) { - return 0, fmt.Errorf("compose-answer-bound is %v: a whole number of seconds from 1 to 600", *s.Bound) - } - return time.Duration(*s.Bound) * time.Second, nil + return out, nil } // holdCallsFor makes a bus call wait the compose-answer-bound (novox/hq ADR 0593 §7), not the SDK's CallTimeout of 30 s: 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 index ad9a9863..02e1cc14 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings_test.go @@ -8,19 +8,25 @@ import ( "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/process" + + "mesh-delivery-checker/internal/check" ) // ADR 0593 §7: the answer bound is the checker's setting compose-answer-bound, 60 s when the file or the value is -// absent; a value that is no whole number of seconds from 1 to 600 stops the start, naming it. +// absent; a value that is no whole number of seconds from 1 to 600 stops the start, naming it. ADR 0603 §10: the wait for +// the candidate's builds is build-wait-bound, 30 minutes when absent, from 60 to 7200 seconds. func TestTheAnswerBoundIsTheCheckersSetting(t *testing.T) { dir := t.TempDir() for _, row := range []struct { - body string - want time.Duration - err bool - }{{"", delivery.ComposeAnswerBound, false}, {`{}`, delivery.ComposeAnswerBound, false}, - {`{"compose-answer-bound": 90}`, 90 * time.Second, false}, {`{"compose-answer-bound": 0}`, 0, true}, - {`{"compose-answer-bound": 1.5}`, 0, true}} { + body string + want time.Duration + err bool + buildWait time.Duration + }{{"", delivery.ComposeAnswerBound, false, check.DefaultBuildWait}, {`{}`, delivery.ComposeAnswerBound, false, check.DefaultBuildWait}, + {`{"compose-answer-bound": 90}`, 90 * time.Second, false, check.DefaultBuildWait}, {`{"compose-answer-bound": 0}`, 0, true, 0}, + {`{"compose-answer-bound": 1.5}`, 0, true, 0}, + {`{"compose-answer-bound": 60, "build-wait-bound": 600}`, 60 * time.Second, false, 10 * time.Minute}, + {`{"build-wait-bound": 30}`, 0, true, 0}} { path := filepath.Join(dir, "settings.json") os.Remove(path) if row.body != "" { @@ -28,9 +34,10 @@ func TestTheAnswerBoundIsTheCheckersSetting(t *testing.T) { t.Fatal(err) } } - got, err := composeAnswerBound(path) - if (err != nil) != row.err || got != row.want { - t.Errorf("%q: %s, %v", row.body, got, err) + set, err := readSettings(path) + got := set.answer + if (err != nil) != row.err || got != row.want || set.buildWait != row.buildWait { + t.Errorf("%q: %s, %s, %v", row.body, got, set.buildWait, err) } // Issue 1126: the SDK's call gives up at CallTimeout (30 s), so the checker's calls must wait the bound read // (an answer at 31 s is heard under 60 s), and the bound never shortens the SDK's own. diff --git a/modules/mesh-delivery-checker/files/settings.json b/modules/mesh-delivery-checker/files/settings.json index b9a19671..c6d811e8 100644 --- a/modules/mesh-delivery-checker/files/settings.json +++ b/modules/mesh-delivery-checker/files/settings.json @@ -1,3 +1,4 @@ { - "compose-answer-bound": 60 + "compose-answer-bound": 60, + "build-wait-bound": 1800 } diff --git a/modules/mesh-delivery-checker/internal/check/build_change.go b/modules/mesh-delivery-checker/internal/check/build_change.go index 7065c080..d22339e4 100644 --- a/modules/mesh-delivery-checker/internal/check/build_change.go +++ b/modules/mesh-delivery-checker/internal/check/build_change.go @@ -13,56 +13,99 @@ import ( "git.novox.be/novox/mesh-delivery/sdk/graph" ) -// A change to a build module or a base (novox/hq ADR 0603 §10), as the check can say it. What the check runs is its own -// tests: a candidate Go build module judges its own change (runner workspace, ADR 0535); the check builds no module's -// artifacts, nor a changed base before what stands on it. A base's fingerprint enters its dependents' fingerprints (the -// indexer), so they are other builds once it changes, and the build module's recipe enters the fingerprint of every module -// it builds, so each is built again once the agreed toolchain moves (ADR 0603 §3). The composition composes them from their -// manifests, and the verdict names the changed modules this concerns, as warnings of its composition: words for the -// operator, no part decides on them. +// What the check builds of a candidate before it composes (novox/hq ADR 0603 §10). The builder builds every fingerprint the +// indexer's candidate carries, as a key of its own (builds..), with the build modules that run; the +// check adds no build path of its own and waits on those keys (awaitBuilds), so the node-engines compose the candidate's +// modules with their builds. It does not wait on two kinds of changed module, and names them in its composition's warnings: +// +// - one built by a build module the change alters (the recipe of mesh-build-go or mesh-build-node enters the fingerprint +// of every module it builds, ADR 0530 §1): its candidate build is the running build module's, and the check cannot run +// the candidate's build module to build it (it runs it only for its own digest and tests, ADR 0535 §1), so it is not +// composed with a build of the changed build module; it is built again once the agreed toolchain moves (ADR 0603 §3). +// The build module's own build is waited on, as any other: the running build module builds its new source. +// - one that stands on a module the change alters (build.on): its candidate fingerprint names the base's trunk +// fingerprint, and no build module builds on another module's artifact (ADR 0487 §1), so its build stands on the +// trunk's base; the base's own build is waited on. +// +// A module the trunk holds no record of yet is not built by the builder until it is merged, so it is not waited on either. -// buildChangeWarnings are the composition warnings of a change that alters the Go build module, or a module other changed -// modules are built on: one line each, naming those modules, sorted. None for a change that alters neither, or when the -// manifests cannot be read (nothing is said that was not read). -func (st *state) buildChangeWarnings(ctx context.Context) []string { +// buildsOfTheCheck are the builds the check waits on before it composes (module to fingerprint) and the words for the +// changed modules it composes without waiting on their build, one line each, sorted. A manifest that cannot be read leaves +// every build waited on and nothing said of build modules or bases (nothing is said that was not read). +func (st *state) buildsOfTheCheck(ctx context.Context) (map[string]string, []string) { + wait := map[string]string{} changed := map[string]catalogue.Manifest{} + readable := true for _, p := range st.c.Pulls { for module, cm := range p.Fps { - if cm.FP2 == "" || cm.Manifest == "" || st.j.Read == nil { + if cm.FP2 == "" || cm.Manifest == "" { + continue // the builder builds no fingerprint of it (candidates.go: none of the new form, or no manifest) + } + wait[module] = cm.FP2 + if !readable || st.j.Read == nil { + readable = false continue } m, err := st.j.Read.Manifest(ctx, cm.Manifest) if err != nil { - st.j.logf("%s: the manifest of %s cannot be read, so the modules built with a changed build module or base are not named: %v", st.c.Key, module, err) - return nil + st.j.logf("%s: the manifest of %s cannot be read, so the modules built with a changed build module or base are not named and every build is waited on: %v", st.c.Key, module, err) + readable = false + continue } changed[module] = m } } - var out []string - for _, t := range st.targets { - if t.module != buildrun.GoBuildModule || !changesBuildGo(t.pull.Repository, t.changed) { + var words []string + if st.j.Runs != nil { + var unheld []string + for _, module := range slices.Sorted(maps.Keys(wait)) { + if st.j.Runs.CommitOf(module) == "" { + unheld = append(unheld, module) + delete(wait, module) + } + } + if len(unheld) > 0 { + words = append(words, fmt.Sprintf("%s: the trunk holds no record of it yet, so the builder builds it once it is merged and the check composes it without a build (ADR 0603 §10)", + strings.Join(unheld, ", "))) + } + } + if !readable { + return wait, words + } + for _, bm := range slices.Sorted(maps.Keys(buildrun.LanguageOf)) { + if _, alters := changed[bm]; !alters { continue } var built []string for module, m := range changed { - if m.Build != nil && slices.ContainsFunc(m.Build.Artifacts, func(a catalogue.Artifact) bool { return a.Language == "go" }) { + if _, isBuildModule := buildrun.LanguageOf[module]; !isBuildModule && buildsWith(m, buildrun.LanguageOf[bm]) { built = append(built, module) + delete(wait, module) } } slices.Sort(built) if len(built) > 0 { - out = append(out, fmt.Sprintf("the change alters the build module %s: %s, built with it, are composed from their manifests, not built by the check, and built again once the agreed toolchain moves (ADR 0603 §3, §10)", - buildrun.GoBuildModule, strings.Join(built, ", "))) + words = append(words, fmt.Sprintf("the change alters the build module %s: %s, which it builds, are built by the running %s, not the changed one, which the check cannot run, so the check does not wait on their builds; they are built again once the agreed toolchain moves (ADR 0603 §3, §10)", + bm, strings.Join(built, ", "), bm)) } - break } g := graph.OfManifests(changed) for _, base := range slices.Sorted(maps.Keys(changed)) { if on := g.Dependents(base); len(on) > 0 { - out = append(out, fmt.Sprintf("the change alters %s, a base of %s: their fingerprints name its fingerprint, so they are composed from their manifests, not built by the check (ADR 0603 §10)", - base, strings.Join(on, ", "))) + for _, module := range on { + delete(wait, module) + } + words = append(words, fmt.Sprintf("the change alters %s, a base of %s: their builds stand on the trunk's %s, since no build module builds on a candidate's base, so the check waits on the build of %s alone (ADR 0603 §10)", + base, strings.Join(on, ", "), base, base)) } } - return out + return wait, words +} + +// buildsWith says whether a manifest has a bundle in the language: the artifacts whose fingerprint names that language's +// toolchain and its build module's recipe (an archive is packed with no toolchain, ADR 0478 clause 1). +func buildsWith(m catalogue.Manifest, language string) bool { + return m.Build != nil && slices.ContainsFunc(m.Build.Artifacts, func(a catalogue.Artifact) bool { + return a.Kind == catalogue.ArtifactBundle && a.Language == language + }) } diff --git a/modules/mesh-delivery-checker/internal/check/builds.go b/modules/mesh-delivery-checker/internal/check/builds.go index f70115fb..68e51c7c 100644 --- a/modules/mesh-delivery-checker/internal/check/builds.go +++ b/modules/mesh-delivery-checker/internal/check/builds.go @@ -2,7 +2,12 @@ package check import ( "context" + "fmt" + "maps" + "slices" "sort" + "strings" + "time" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -64,3 +69,67 @@ func (j *Examiner) buildNotes(ctx context.Context, c *Case) []BuildNote { } return notes } + +// DefaultBuildWait is how long a check waits for the candidate's builds to end before it composes (the setting +// build-wait-bound, novox/hq ADR 0603 §10): past it the check composes without the builds not ended and names them. +const DefaultBuildWait = 30 * time.Minute + +// BuildEnds waits on the builder's build keys (builds..), hearing each key's changes on the bus as the +// builder writes them: the builder owns the fact, and nothing is polled. +type BuildEnds interface { + // Ended waits until the build of every module at its fingerprint (module to fingerprint) is done or failed, or ctx ends, + // and answers the builds that ended, by module. An error is only builds that cannot be heard at all. + Ended(ctx context.Context, builds map[string]string) (map[string]delivery.BuilderBuild, error) +} + +func (j *Examiner) buildWait() time.Duration { + if j.BuildWait > 0 { + return j.BuildWait + } + return DefaultBuildWait +} + +// awaitBuilds waits for the candidate's builds before the check composes (ADR 0603 §10): the check builds what the builder +// builds of the candidate and composes with it. A failed build fails the check, naming the module and the builder's words; +// a build not ended within the bound is named, and the check composes without it. It answers the words of the composition's +// warnings: what the check composes without waiting on its build, and why. +func (st *state) awaitBuilds(ctx context.Context, v *delivery.Verdict) []string { + wait, words := st.buildsOfTheCheck(ctx) + if len(wait) == 0 { + return words + } + modules := slices.Sorted(maps.Keys(wait)) + if st.j.BuildEnds == nil { + return append(words, fmt.Sprintf("the builds of %s cannot be heard, so the check composes without waiting on them (ADR 0603 §10)", strings.Join(modules, ", "))) + } + bound := st.j.buildWait() + waits := fmt.Sprintf("waits on the builder's builds of %s before it composes, at most %s (ADR 0603 §10)", strings.Join(modules, ", "), bound) + st.c.note(waits) + st.j.logf("%s %s", st.c.Key, waits) + at, cancel := context.WithTimeout(ctx, bound) + ended, err := st.j.BuildEnds.Ended(at, wait) + cancel() + if err != nil { + st.j.logf("%s: the builds of %s cannot be heard: %v", st.c.Key, strings.Join(modules, ", "), err) + return append(words, fmt.Sprintf("the builds of %s cannot be heard (%v), so the check composes without waiting on them (ADR 0603 §10)", strings.Join(modules, ", "), err)) + } + var late []string + for _, module := range modules { + b, done := ended[module] + switch { + case !done: + late = append(late, module) + case b.State == delivery.BuildFailed: + f := delivery.FailedFirst{Test: "build: " + module, Words: fmt.Sprintf("the candidate's build at %s failed (builder_build_again builds it again, then mesh-delivery.recheck asks the check again): %s", wait[module], b.Error)} + if v.State != delivery.VerdictFail { + v.State, v.Failed = delivery.VerdictFail, f + continue + } + addFailure(v, f) + } + } + if len(late) > 0 && ctx.Err() == nil { + words = append(words, fmt.Sprintf("the builds of %s did not end within the build-wait-bound of %s, so the check composes without them (ADR 0603 §10)", strings.Join(late, ", "), bound)) + } + return words +} diff --git a/modules/mesh-delivery-checker/internal/check/composition_test.go b/modules/mesh-delivery-checker/internal/check/composition_test.go index c8dc90c3..53f42d07 100644 --- a/modules/mesh-delivery-checker/internal/check/composition_test.go +++ b/modules/mesh-delivery-checker/internal/check/composition_test.go @@ -104,41 +104,122 @@ type fixedTrunks struct{ heads map[string]string } func (f fixedTrunks) Heads() map[string]string { return f.heads } -// ADR 0603 §10: a change that alters the Go build module, or a module others stand on, names in the composition's warnings -// (and so the check's line) the changed modules built with it, from their manifests; a change that alters neither says nothing. +// fakeEnds answers the builder's builds of a candidate as fakeBuilds holds them, recording the builds waited on; the +// composition must not have been asked yet when it is (ADR 0603 §10: the check composes with the builds). +type fakeEnds struct { + builds fakeBuilds + asked map[string]string + composed *fakeCompositions + early bool +} + +func (f *fakeEnds) Ended(_ context.Context, builds map[string]string) (map[string]delivery.BuilderBuild, error) { + f.asked = builds + f.early = f.composed != nil && len(f.composed.asked) > 0 + out := map[string]delivery.BuilderBuild{} + for module, fp := range builds { + if b, ok := f.builds[module+" "+fp]; ok && (b.State == delivery.BuildDone || b.State == delivery.BuildFailed) { + out[module] = b + } + } + return out, nil +} + +// knownRuns are the modules the trunk holds a record of (the indexer's modules): any commit. +type knownRuns []string + +func (k knownRuns) CommitOf(module string) string { + if slices.Contains(k, module) { + return trunkA + } + return "" +} + +// ADR 0603 §10: before it composes, the check waits on the builder's builds of the candidate's changed modules and composes +// with them; a failed build fails the check naming it, and one not ended within the bound is named. It does not wait on a +// module built by a build module the change alters, nor on one standing on a base the change alters, nor on one the trunk +// holds no record of, and names each in the composition's warnings (and so the check's line); a change that alters none of +// these says nothing. func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { dig := func(c string) string { return strings.Repeat(c, 64) } - goBuild := &catalogue.Build{Artifacts: []catalogue.Artifact{{Name: "bin", Language: "go"}}} + fp := "src2:" + dig("f") + goBuild := &catalogue.Build{Artifacts: []catalogue.Artifact{{Name: "bin", Kind: catalogue.ArtifactBundle, Language: "go"}}} + tsBuild := &catalogue.Build{Artifacts: []catalogue.Artifact{{Name: "app", Kind: catalogue.ArtifactBundle, Language: "typescript"}}} + done := delivery.BuilderBuild{State: delivery.BuildDone} for _, row := range []struct { name string - changed []string manifest map[string]catalogue.Manifest + builds map[string]delivery.BuilderBuild // by module, at fp; nil leaves the check without the builds' ends + runs knownRuns // nil: the trunk holds every module want []string + waited string // the modules waited on, sorted + failed string // the failure's test, when the check fails }{ - {name: "the build module changes", changed: []string{buildGoFile}, - manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}, "mail": {Module: "mail", Build: goBuild}, "docs": {Module: "docs"}}, - want: []string{"mesh-build-go: mail, web, built with it"}}, - {name: "a base changes", changed: []string{"README.md"}, + {name: "the Go build module changes", + manifest: map[string]catalogue.Manifest{"mesh-build-go": {Module: "mesh-build-go", Build: goBuild}, "web": {Module: "web", Build: goBuild}, + "mail": {Module: "mail", Build: goBuild}, "docs": {Module: "docs"}}, + builds: map[string]delivery.BuilderBuild{"mesh-build-go": done, "docs": done}, + want: []string{"the build module mesh-build-go: mail, web, which it builds"}, waited: "docs, mesh-build-go"}, + {name: "the Node build module changes", + manifest: map[string]catalogue.Manifest{"mesh-build-node": {Module: "mesh-build-node", Build: goBuild}, "panel": {Module: "panel", Build: tsBuild}, + "web": {Module: "web", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"mesh-build-node": done, "web": done}, + want: []string{"the build module mesh-build-node: panel, which it builds"}, waited: "mesh-build-node, web"}, + {name: "a base changes", manifest: map[string]catalogue.Manifest{"image": {Module: "image"}, "web": {Module: "web", Build: &catalogue.Build{On: []catalogue.BuildsOn{{Arg: "BASE", Module: "image", Artifact: "img"}}}}}, - want: []string{"alters image, a base of web"}}, - {name: "neither changes", changed: []string{"README.md"}, - manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}}, + builds: map[string]delivery.BuilderBuild{"image": done}, + want: []string{"alters image, a base of web", "waits on the build of image alone"}, waited: "image"}, + {name: "neither changes", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": done}, waited: "web"}, + {name: "a build fails", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}, "mail": {Module: "mail", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildFailed, Error: "go build: undefined: x"}, "mail": done}, + waited: "mail, web", failed: "build: web"}, + {name: "a build does not end within the bound", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildBuilding}}, + want: []string{"the builds of web did not end within the build-wait-bound"}, waited: "web"}, + {name: "a module the trunk does not hold", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}, "new": {Module: "new", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": done}, runs: knownRuns{"web"}, + want: []string{"new: the trunk holds no record of it yet"}, waited: "web"}, + {name: "the builds cannot be heard", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, + want: []string{"the builds of web cannot be heard"}}, } { b := candidateBench(t, true) - b.fetch.Changed = row.changed - b.ex.Compositions = &fakeCompositions{v: delivery.CompositionVerdict{Group: "feat/x", Heads: "0123456789ab", State: delivery.CompositionPass, SDK: "x", Machines: []string{"g14"}}} + comps := &fakeCompositions{v: delivery.CompositionVerdict{Group: "feat/x", Heads: "0123456789ab", State: delivery.CompositionPass, SDK: "x", Machines: []string{"g14"}}} + b.ex.Compositions = comps + var ends *fakeEnds + if row.builds != nil { + ends = &fakeEnds{builds: fakeBuilds{}, composed: comps} + for module, build := range row.builds { + ends.builds[module+" "+fp] = build + } + b.ex.BuildEnds = ends + } + if row.runs != nil { + b.ex.Runs = row.runs + } read, fps := fixedManifests{}, map[string]delivery.CandidateModule{} for module, m := range row.manifest { d := dig(string(rune('a' + len(read)))) - read[d], fps[module] = m, delivery.CandidateModule{FP2: "src2:" + dig("f"), Manifest: d} + read[d], fps[module] = m, delivery.CandidateModule{FP2: fp, Manifest: d} } b.ex.Read = read p := pullOf(catalogueRepository, 9, head1) p.Fps, p.Modules = fps, slices.Sorted(maps.Keys(fps)) v := b.judge(t, caseOf(t, p)).Verdict - if v.State != delivery.VerdictPass || v.Composition == nil { - t.Fatalf("%s: %+v", row.name, v) + if v.Composition == nil || len(comps.asked) != 1 { + t.Fatalf("%s: composed %d times: %+v", row.name, len(comps.asked), v) + } + if ends != nil { + if got := strings.Join(slices.Sorted(maps.Keys(ends.asked)), ", "); got != row.waited || ends.early { + t.Errorf("%s: waited on %q (after the composition: %v), want %q before it composes", row.name, got, ends.early, row.waited) + } + } + switch { + case row.failed != "" && (v.State != delivery.VerdictFail || v.Failed.Test != row.failed || !strings.Contains(v.Failed.Words, "undefined: x")): + t.Errorf("%s: the verdict is %s failing %+v, want it failed naming %s and the builder's words", row.name, v.State, v.Failed, row.failed) + case row.failed == "" && v.State != delivery.VerdictPass: + t.Errorf("%s: %+v", row.name, v) } words := compositionWords(v.Composition, v.Took.Composition) for _, w := range row.want { @@ -147,7 +228,7 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { } } if row.want == nil && v.Composition.Words != "" { - t.Errorf("%s: warned %q though neither a build module nor a base changed", row.name, v.Composition.Words) + t.Errorf("%s: warned %q though the check waited on every build of the change", row.name, v.Composition.Words) } } } diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index f15c9429..cb77a6e5 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -102,6 +102,11 @@ type Examiner struct { // Builds says how the build of a fingerprint the candidate carries stands (the builder's builds bucket); nil says nothing // of the builds. Builds BuildStates + // BuildEnds waits on the builder's builds of the candidate before the check composes (ADR 0603 §10); nil composes + // without waiting, said in the composition's warnings. + BuildEnds BuildEnds + // BuildWait is how long the check waits for those builds (the setting build-wait-bound); DefaultBuildWait when 0. + BuildWait time.Duration // Announce says a preview was written under the key (EventPreviewWritten); nil says nothing. Announce Announcer // Packages names, of a repository, the paths (repository-relative) its TypeScript package artifacts are built from, from @@ -280,6 +285,7 @@ func (st *state) composed(ctx context.Context, v *delivery.Verdict) { strings.Join(st.unpreviewed, ", ") + " (its head does not merge or its fetch did not answer), so the change cannot be composed"} return } + words := st.awaitBuilds(ctx, v) // ADR 0603 §10: the check composes with the candidate's builds comp, err := st.j.Compositions.Compose(ctx, st.c) if err != nil { // Said in the verdict, never left as a composition not yet run (review finding 4). @@ -287,7 +293,7 @@ func (st *state) composed(ctx context.Context, v *delivery.Verdict) { v.Composition = &delivery.Composition{State: delivery.CompositionNotRun, Words: "not made: " + err.Error()} return } - comp.Warnings = append(comp.Warnings, st.buildChangeWarnings(ctx)...) // ADR 0603 §10 + comp.Warnings = append(comp.Warnings, words...) // ADR 0603 §10 withComposition(v, comp) if v.Composition != nil && v.Composition.State != delivery.CompositionNotRun { v.Composition.FPSet = st.c.FPSet // a carry keeps it while the fingerprints stay the same (ADR 0548 §5) diff --git a/modules/mesh-delivery-checker/manifest_test.go b/modules/mesh-delivery-checker/manifest_test.go index 7d995a3d..6a379ebe 100644 --- a/modules/mesh-delivery-checker/manifest_test.go +++ b/modules/mesh-delivery-checker/manifest_test.go @@ -174,13 +174,16 @@ func TestManifestProcess(t *testing.T) { // 1678/AC1 if len(env) != 2 || env["MESH_BROKER_FILE"] != "${dir:state}/broker" || env["MESH_CHECKER_SETTINGS"] != "${dir:state}/settings.json" { t.Errorf("env = %v, want the broker file and the settings file", p["env"]) } - // Its settings hold its answer bound alone (ADR 0593 §7: compose-answer-bound, 60 s, the checker's), never a capacity - // of its own (ADR 0525 §4). + // Its settings hold its answer bound (ADR 0593 §7: compose-answer-bound, 60 s, the checker's) and its wait for the + // candidate's builds (ADR 0603 §10: build-wait-bound, 1800 s), never a capacity of its own (ADR 0525 §4). for _, r := range m.Resources { - if r["id"] == "settings" && !reflect.DeepEqual(r["takes-config"], []any{"compose-answer-bound"}) { - t.Errorf("the settings resource %v takes more than compose-answer-bound", r) + if r["id"] == "settings" && !reflect.DeepEqual(r["takes-config"], []any{"compose-answer-bound", "build-wait-bound"}) { + t.Errorf("the settings resource %v takes other than compose-answer-bound and build-wait-bound", r) } } + if c, ok := m.Config["build-wait-bound"]; !ok || c.Default != float64(1800) || c.Trusted { + t.Errorf("build-wait-bound = %+v, want 1800 by default, not trusted", c) + } if c, ok := m.Config["compose-answer-bound"]; !ok || c.Default != float64(60) || c.Trusted { t.Errorf("compose-answer-bound = %+v, want 60 by default, not trusted", c) } diff --git a/modules/mesh-delivery-checker/module.json b/modules/mesh-delivery-checker/module.json index 767884cb..537551f1 100644 --- a/modules/mesh-delivery-checker/module.json +++ b/modules/mesh-delivery-checker/module.json @@ -81,6 +81,14 @@ "maximum": 600, "trusted": false, "default": 60 + }, + "build-wait-bound": { + "type": "number", + "description": "seconds a check waits for the builder's builds of its candidate to end before it composes with them (novox/hq ADR 0603 §10); a build failed fails the check, naming it, and a build not ended by then is named in the verdict and the check composes without it.", + "minimum": 60, + "maximum": 7200, + "trusted": false, + "default": 1800 } }, "resources": [ @@ -106,7 +114,8 @@ "mode": "0600", "owner": "mesh-delivery-checker", "takes-config": [ - "compose-answer-bound" + "compose-answer-bound", + "build-wait-bound" ], "merge": "json", "template": "files/settings.json"