diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index ae7627a1..44f6426a 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -232,8 +232,8 @@ func (f following) hearMerge(ctx context.Context, m link.SourceMoved, now time.T fmt.Print(nothingReadsIt(repository, m)) return nil } - // **A module the merge adds is asked for now** (novox/hq issue 1617), not when the batch is cut. - askNewModulesAtHearing(ctx, inv, m, entries, read, facts.from) + // A module the merge adds is registered from the builder's first done key of it, which the builder's write wakes + // (novox/hq ADR 0501 §6): nothing is asked here. // **Its delivery group's word, before the plans are held** (ADR 0306 decision 2): a slow answer holds up no walk. own := ownPath(touches) waits, holding, err := groupWaitHolding(ctx, inv, m, deliverySeatHeld(entries)) diff --git a/cmd/mesh-controller/build_source.go b/cmd/mesh-controller/build_source.go index 0c3d84ea..3520c8fd 100644 --- a/cmd/mesh-controller/build_source.go +++ b/cmd/mesh-controller/build_source.go @@ -697,37 +697,6 @@ func sourceWords(repository, seat string) string { return buildSource{Repository: repository, Seat: seat}.String() } -// firstBuildWarrant is what the controller checked before it asks the first build of the modules a merge adds to a -// repository nothing is built from: the merge was made on the operator's Approve of its head (approvedAnnounced). It -// travels in the context of that ask alone, in this process, so no verb carries one. -type firstBuildWarrant struct { - repository string // owner/name on the git seat - paths map[string]bool - why string -} - -type firstBuildWarrantKey struct{} - -// withFirstBuildWarrant is ctx carrying the warrant for asking the first builds of paths in repository. -func withFirstBuildWarrant(ctx context.Context, repository string, paths []string, why string) context.Context { - w := &firstBuildWarrant{repository: repository, paths: map[string]bool{}, why: why} - for _, p := range paths { - w.paths[strings.Trim(p, "/")] = true - } - return context.WithValue(ctx, firstBuildWarrantKey{}, w) -} - -func firstBuildWarrantIn(ctx context.Context) *firstBuildWarrant { - w, _ := ctx.Value(firstBuildWarrantKey{}).(*firstBuildWarrant) - return w -} - -// covers says the warrant is for exactly this ask: the repository on the git seat, and a module path the merge holds. -func (w *firstBuildWarrant) covers(source buildSource, path string) bool { - return w != nil && source.Seat == gitSeat && strings.EqualFold(strings.Trim(source.Repository, "/"), w.repository) && - w.paths[strings.Trim(path, "/")] -} - // verbMayAsk refuses, for a call through a verb, a build of a repository the catalogue builds no module from // (novox/hq ADR 0266): the build node would run what an agent wrote, and its outcome could never be // registered anyway. url is the repository as it is cloned. At the terminal anything may be asked. @@ -735,11 +704,6 @@ func verbMayAsk(ctx context.Context, source buildSource, url, path string) error if startedAtTheTerminal() { return nil } - // **Or the first build of a module a merge adds on the operator's Approve** (novox/hq ADR 0432, issue 1146): the - // controller's own ask, carrying the warrant it checked, for that repository and the module paths the merge holds. - if firstBuildWarrantIn(ctx).covers(source, path) { - return nil - } open, err := openStores(ctx) if err != nil { return err diff --git a/cmd/mesh-controller/builder_on_the_bus_test.go b/cmd/mesh-controller/builder_on_the_bus_test.go new file mode 100644 index 00000000..c20a6d6a --- /dev/null +++ b/cmd/mesh-controller/builder_on_the_bus_test.go @@ -0,0 +1,131 @@ +//go:build integration + +package main + +import ( + "context" + "sync" + "testing" + + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" + "github.com/nats-io/nats.go/jetstream" + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/testbus" +) + +// builderOnTheBus is the builder's and the indexer's buckets on a test's own bus, which the controller's connection for +// the moment (servingBus) reaches: what a take-in from a key reads (R4e, issue 1851). +type builderOnTheBus struct { + js *broker.JetStream + builds, modules, manifests jetstream.KeyValue +} + +func aBuilderOnTheBus(t *testing.T) builderOnTheBus { + t.Helper() + js, err := broker.Dial(testbus.URL(t)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(js.Close) + api, err := jetstream.New(js.Conn()) + if err != nil { + t.Fatal(err) + } + ctx := t.Context() + b := builderOnTheBus{js: js} + for _, c := range []struct { + into *jetstream.KeyValue + bucket string + history uint8 + }{ + {&b.builds, sdkdelivery.BuilderBuildsBucket, sdkdelivery.BuilderBuildsHistory}, + {&b.modules, sdkdelivery.ModulesBucket, sdkdelivery.ModulesHistory}, + {&b.manifests, sdkdelivery.ManifestsBucket, 1}, + } { + kv, err := api.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: c.bucket, History: c.history}) + if err != nil { + t.Fatal(err) + } + *c.into = kv + } + servingBus.Store(js) + t.Cleanup(func() { servingBus.Store(nil) }) + withTrunkList(t, true) + return b +} + +// indexed writes the indexer's trunk value of module at commit, naming fp, and its manifest: a module of one archive. +func (b builderOnTheBus) indexed(t *testing.T, module, version, repository, path, commit, fp string) string { + t.Helper() + keys, trunk := aKeyFor(module, version, fp, repository, path, commit) + raw := keys.manifests[trunk.Manifest] + value, err := sdkdelivery.EncodeManifest(sdkdelivery.Manifest{Module: module, JSON: raw}) + if err != nil { + t.Fatal(err) + } + if _, err := b.manifests.Put(context.Background(), trunk.Manifest, value); err != nil { + t.Fatal(err) + } + trunk.FP = "src1:" + fp[len("src2:"):] + encoded, err := sdkdelivery.EncodeModule(trunk) + if err != nil { + t.Fatal(err) + } + if _, err := b.modules.Put(context.Background(), module, encoded); err != nil { + t.Fatal(err) + } + return trunk.Manifest +} + +// built writes the builder's key of module at fp: done, its one archive named with a reference, or in another state. +func (b builderOnTheBus) built(t *testing.T, module, version, fp, manifest, state string) { + t.Helper() + keys, _ := aKeyFor(module, version, fp, "", "", "") + v := keys.keys[module].Build + v.Manifest = manifest + if state != sdkdelivery.BuildDone { + v = sdkdelivery.BuilderBuild{State: state, By: "ace"} + if state == sdkdelivery.BuildFailed { + v.Error = "digests differ: ace sha256:1, g14 sha256:2" + } + } + value, err := sdkdelivery.EncodeBuilderBuild(v) + if err != nil { + t.Fatal(err) + } + key, err := sdkdelivery.BuilderBuildKey(module, fp) + if err != nil { + t.Fatal(err) + } + if _, err := b.builds.Put(context.Background(), key, value); err != nil { + t.Fatal(err) + } +} + +// noBuildAsked fails the test if anything asks build-agent for a build (ADR 0501 §6, ADR 0503 §6). +func noBuildAsked(t *testing.T) { + t.Helper() + was := askABuild + askABuild = func(_ context.Context, _ buildSource, path, _ string) (string, error) { + t.Errorf("a build was asked of build-agent, at %q", path) + return "", context.Canceled + } + t.Cleanup(func() { askABuild = was }) +} + +// asksWithPaths records every build a merge asks for, as repository, path and ref. The merge window's goroutine and +// the test's own may ask at once, so the record is written under a lock (issue 1543). +func asksWithPaths(t *testing.T) *[][3]string { + t.Helper() + var asked [][3]string + var mu sync.Mutex + was := askABuild + askABuild = func(_ context.Context, source buildSource, path, ref string) (string, error) { + mu.Lock() + defer mu.Unlock() + asked = append(asked, [3]string{source.Repository, path, ref}) + return "b-" + path, nil + } + t.Cleanup(func() { askABuild = was }) + return &asked +} diff --git a/cmd/mesh-controller/first_build_ask_integration_test.go b/cmd/mesh-controller/first_build_ask_integration_test.go deleted file mode 100644 index 68d84fdd..00000000 --- a/cmd/mesh-controller/first_build_ask_integration_test.go +++ /dev/null @@ -1,84 +0,0 @@ -//go:build integration - -package main - -import ( - "context" - "errors" - "testing" - "time" - - "git.novox.be/novox/mesh-sdk/go/asks" - "git.novox.be/novox/mesh-sdk/go/catalogue" - "github.com/novox/mesh-controller/internal/broker" - "github.com/novox/mesh-controller/internal/identity" - "github.com/novox/mesh-controller/internal/link" -) - -// aForgeAndABuildSeat gives a test what a real ask of a build needs: the forge holding the git seat, assigned to anchor, so -// the clone URL composes; the identity store; the build seat's queue on a real bus; and the environment of the serving controller, which is -// never the controller's terminal (a verb's, in effect: startedAtTheTerminal is false). -func aForgeAndABuildSeat(t *testing.T, open *stores) *broker.JetStream { - t.Helper() - js, _ := aBuildQueue(t) - // The ask signs what it sends with the controller's identity, as the serving controller does. - identity.ForTest(t) - forge, err := catalogue.ParseManifest([]byte(`{"module":"gitea","version":"1", - "claims":[{"name":"git","scope":"mesh"}],"provides":[{"name":"git","scope":"mesh"}], - "serves":{"git":{"scheme":"http","port":3000}}}`)) - if err != nil { - t.Fatal(err) - } - register(t, open, forge) - if _, err := assign(t.Context(), open, "anchor", "gitea"); err != nil { - t.Fatal(err) - } - t.Setenv(servedVar, "1") - return js -} - -// 1146/AC9, through the real ask (askABuild, buildOneAsked, verbMayAsk) and a real bus: the first build of a module a -// merge adds on the operator's Approve is queued on the build seat; the same ask without that warrant, and the warrant's -// ask for a path the merge does not hold, are refused by the guard of a build through a verb. -func TestTheApprovedFirstBuildPassesTheGuardOfAVerbAndIsQueued(t *testing.T) { - const head, commit = "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1", "0177ea7a00000000" - open := aMesh(t) - ctx := t.Context() - js := aForgeAndABuildSeat(t, open) - - a := aDeliveryMergeOf(t, head, map[string]string{"novox/mesh-delivery@a1a1a1a1a1a1": head}) - wasKept, wasAsk := keptDeliveryMerge, routerAskOf - keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { - return a.merge, true, nil - } - routerAskOf = func(context.Context, catalogueReader, string, string) (*asks.Ask, *asks.Warrant, error) { - return &a.ask, &a.warrant, nil - } - t.Cleanup(func() { keptDeliveryMerge, routerAskOf = wasKept, wasAsk }) - - // Without the warrant the guard refuses, whatever the repository holds. - _, err := askABuild(ctx, buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "", "main") - var policy *heldAtTheTerminal - if !errors.As(err, &policy) { - t.Fatalf("1146/AC9: a first build asked without the warrant was not refused: %v", err) - } - // And a path the merge does not hold is refused on the warrant. - warranted := withFirstBuildWarrant(ctx, "novox/mesh-delivery", []string{""}, "the Approve") - if _, err := askABuild(warranted, buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "other", "main"); !errors.As(err, &policy) { - t.Fatalf("1146/AC9: the warrant covered a path the merge does not hold: %v", err) - } - - m := link.SourceMoved{Owner: "novox", Repo: "mesh-delivery", Base: "main", HeadSHA: head, Commit: commit, Number: 7, - MergedAt: time.Now().Add(-time.Hour).UTC().Format(time.RFC3339Nano), Paths: []string{"module.json", "main.go"}, - ModuleDirs: []string{"."}, ModuleDirsSaid: true} - if err := (following{open: open}).SourceMoved(ctx, m); err != nil { - t.Fatal(err) - } - queue, err := link.ReadQueue(ctx, js, link.TheBuildMachine) - if err != nil { - t.Fatal(err) - } - if len(queue.Asks) != 1 { - t.Fatalf("1146/AC9: the build queue holds %+v, want the one first build", queue) - } -} diff --git a/cmd/mesh-controller/first_build_warrant_test.go b/cmd/mesh-controller/first_build_warrant_test.go deleted file mode 100644 index e72b3965..00000000 --- a/cmd/mesh-controller/first_build_warrant_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package main - -import "testing" - -// 1146/AC9: the warrant the controller checked covers exactly the repository on the git seat and the module paths the -// merge holds; nothing else passes verbMayAsk on it. -func TestTheFirstBuildWarrantCoversOnlyItsRepositoryAndItsModulePaths(t *testing.T) { - ctx := withFirstBuildWarrant(t.Context(), "novox/mesh-delivery", []string{"", "view"}, "the Approve") - w := firstBuildWarrantIn(ctx) - for _, c := range []struct { - source buildSource - path string - covers bool - }{ - {buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "", true}, - {buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "/view/", true}, - {buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "other", false}, - {buildSource{Repository: "agent/mesh-delivery", Seat: "git"}, "", false}, - {buildSource{Repository: "novox/mesh-delivery"}, "", false}, - {buildSource{Repository: "novox/mesh-delivery", Seat: "elsewhere"}, "", false}, - } { - if got := w.covers(c.source, c.path); got != c.covers { - t.Errorf("1146/AC9: %v at %q covered = %v, want %v", c.source, c.path, got, c.covers) - } - } - if firstBuildWarrantIn(t.Context()).covers(buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, "") { - t.Error("1146/AC9: a context with no warrant covers an ask") - } -} diff --git a/cmd/mesh-controller/first_builds.go b/cmd/mesh-controller/first_builds.go index fb05302b..c6f6322c 100644 --- a/cmd/mesh-controller/first_builds.go +++ b/cmd/mesh-controller/first_builds.go @@ -47,7 +47,7 @@ func (f following) firstBuilds(ctx context.Context, m link.SourceMoved, entries case len(added) == 0: return false, nil } - if err := askFirstBuilds(ctx, inv, m, now, added, why); err != nil { + if err := askFirstBuilds(ctx, f.open, m, now, added, why); err != nil { return true, notNow(err) } return true, nil @@ -68,27 +68,35 @@ func keepFirstMerge(ctx context.Context, inv *inventory.Inventory, m link.Source return err } -// askFirstBuilds keeps the merge, asks the builds, and marks the merge as no longer waiting for them. -func askFirstBuilds(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved, now time.Time, added []string, +// askFirstBuilds keeps the merge as answered and registers its modules from the builder's done keys (novox/hq ADR 0501 +// §3, §6): nothing asks their first builds. The merge kept is the registration's warrant (approvedMove reads it by the +// commit the indexer's trunk value names); a module whose key is not done yet is registered when the key's write wakes +// the take-in (new_modules.go). +func askFirstBuilds(ctx context.Context, open *stores, m link.SourceMoved, now time.Time, added []string, why string) error { + inv := open.inventory repository := m.Owner + "/" + m.Repo if err := keepFirstMerge(ctx, inv, m, now, false); err != nil { return err } - fmt.Printf("%s merged into %s (%.8s) on the operator's Approve (%s): the mesh builds nothing from it yet, so the "+ - "first build of %s is asked\n", repository, m.Base, m.Commit, why, strings.Join(added, ", ")) - // The ask carries the warrant just checked, for this repository and these module paths alone: the guard of a build - // through a verb (verbMayAsk) lets exactly that through. - paths := make([]string, 0, len(added)) - for _, dir := range added { - if dir == "." { - dir = "" - } - paths = append(paths, dir) + fmt.Printf("%s merged into %s (%.8s) on the operator's Approve (%s): the mesh builds nothing from it yet, so %s "+ + "are registered from the builder's done keys, asking no build\n", repository, m.Base, m.Commit, why, + strings.Join(added, ", ")) + // Answered: it no longer waits, so the word's arrival again does nothing twice. + if err := inv.AnswerMerge(ctx, repository, m.Commit, inventory.WaitingRecord, false); err != nil { + return err } - askBuilds(withFirstBuildWarrant(ctx, repository, paths, why), inv, m, buildSource{Repository: repository, Seat: gitSeat}, added) - // Answered: it no longer waits, so the word's arrival again asks nothing twice. - return inv.AnswerMerge(ctx, repository, m.Commit, inventory.WaitingRecord, false) + modules, err := newModulesOf(ctx, repository) + switch { + case err != nil: + // Said, not failed: the merge is answered, and the look at every module when a controller starts, or the next + // write of the builder or the indexer for one of them, registers them. + fmt.Printf(" the indexer's modules of %s cannot be read now (%v): they are registered when the builder's next "+ + "write of one wakes the take-in\n", repository, err) + case len(modules) > 0: + go takeNewModules(context.WithoutCancel(ctx), open, modules...) + } + return nil } // resumeFirstBuilds runs firstBuilds for the first merges waiting for the word mesh-delivery just announced for a pull @@ -126,7 +134,7 @@ func (f following) resumeFirstBuilds(ctx context.Context, d deliveryMerge) error case err != nil: return notNow(err) case len(added) > 0: - if err := askFirstBuilds(ctx, inv, k.SourceMoved, time.Now().UTC(), added, why); err != nil { + if err := askFirstBuilds(ctx, f.open, k.SourceMoved, time.Now().UTC(), added, why); err != nil { return notNow(err) } } @@ -174,7 +182,9 @@ func firstBuildsOf(ctx context.Context, inv catalogueReader, m link.SourceMoved, } // heldFirstBuild says a first build was refused because its manifest uses a word the running build-agent does not read, -// and its merge is kept waiting for a build-agent that reads it (novox/hq ADR 0482 §5, issue 1173). +// and its merge is kept waiting for a build-agent that reads it (novox/hq ADR 0482 §5, issue 1173). Read from a merge kept +// before R4e, never written since: nothing asks a first build (ADR 0501 §6), and a controller that starts answers such a +// merge and registers its modules from their keys (answerHeldMerges). type heldFirstBuild struct { Module string `json:"module"` Path string `json:"path"` @@ -182,52 +192,6 @@ type heldFirstBuild struct { At time.Time `json:"at"` } -// judgeRefusedFirstBuild is the hold a first build's outcome puts its merge under: every module the merge holds already, -// and this one, unless it is among them; nil unless the build failed on a word of its manifest the running build-agent -// does not know, and was asked for a first merge, answered by the record of merges no plan answers or put back to waiting -// by an earlier hold (a second refusal in one merge is held too, review of issue 1173). -func judgeRefusedFirstBuild(result link.BuildResult, req inventory.BuildRequest, merge inventory.BatchedMerge) []heldFirstBuild { - word := refusedWord(result.Failed) - if word == "" || req.For != "merge" { - return nil - } - var held []heldFirstBuild - switch k := announcedOf(merge); { - case merge.Plan == inventory.WaitingRecord: - // Answered when its builds were asked: a hold from before (a build asked again) is over, and starts anew. - case merge.Plan == "" && len(k.Held) > 0: - held = k.Held - default: - return nil - } - this := heldFirstBuild{Module: req.Name(), Path: strings.Trim(req.Path, "/"), Word: word, At: time.Now().UTC()} - for i, h := range held { - if strings.Trim(h.Path, "/") == this.Path { - out := append([]heldFirstBuild(nil), held...) - out[i] = this - return out - } - } - return append(append([]heldFirstBuild(nil), held...), this) -} - -// heldDirsOf is the directories a held merge asks again: its held modules' own, each once, "." for the repository's root. -func heldDirsOf(k keptMerge) []string { - var dirs []string - seen := map[string]bool{} - for _, h := range k.Held { - dir := strings.Trim(h.Path, "/") - if dir == "" { - dir = "." - } - if !seen[dir] { - seen[dir] = true - dirs = append(dirs, dir) - } - } - return dirs -} - // unknownWord is how the strict reading of a manifest says a word is not one it knows: encoding/json's own words, which // the build-agent's answer carries whole. The build's answer has no typed field for it (Failed is the builder's words). var unknownWord = regexp.MustCompile(`json: unknown field "([^"]+)"`) @@ -240,55 +204,12 @@ func refusedWord(failed string) string { return "" } -// holdRefusedFirstBuild keeps a first build's merge waiting when the build was refused for a word the running build-agent -// does not read (novox/hq ADR 0482 §5, issue 1173): no failed build is recorded, nothing expires the merge, and it is -// named by a condition until a delivery of build-agent asks the build again. Answers whether the outcome was dealt with. -func holdRefusedFirstBuild(ctx context.Context, inv *inventory.Inventory, result link.BuildResult) (bool, error) { - if result.Failed == "" || refusedWord(result.Failed) == "" { - return false, nil - } - req, found, err := inv.BuildRequestByID(ctx, result.ID) - if err != nil || !found { - return false, err - } - merge, found, err := inv.MergeOf(ctx, req.Repository, req.Commit) - if err != nil || !found { - return false, err - } - held := judgeRefusedFirstBuild(result, req, merge) - if len(held) == 0 { - return false, nil - } - k := announcedOf(merge) - k.Held = held - event, err := json.Marshal(k) - if err != nil { - return false, err - } - if err := inv.HoldMerge(ctx, merge.Repository, merge.Commit, event); err != nil { - return false, err - } - this := held[len(held)-1] - for _, h := range held { - if h.Path == strings.Trim(req.Path, "/") { - this = h - } - } - fmt.Printf("%s: the first build of %s was refused for the word %q, which the running build-agent does not read; the "+ - "merge %.8s of %s is kept waiting for a build-agent that reads it (holding %s), and the build is asked again when "+ - "one is delivered\n", result.ID, this.Module, this.Word, merge.Commit, merge.Repository, heldNames(held)) - return true, nil -} - // probeFirstBuildsID is the probe that reads the first merges waiting for mesh-delivery's word. const probeFirstBuildsID = "D18" // kindFirstBuildUnasked is what a first merge waiting too long for mesh-delivery's word raises. const kindFirstBuildUnasked = "first-build-unasked" -// kindFirstBuildHeld is what a first merge held for a build-agent that reads its manifest raises. -const kindFirstBuildHeld = "first-build-held" - // firstBuildWaitBound is how long a first merge waits for mesh-delivery's announcement of the operator's Approve before // it is said: a bound in the sense of ADR 0426 §3, not a delay. Nothing waits on it: the word's arrival asks the builds // at once, and the merge is said, never dropped, once it is past this (issue 1146). @@ -300,10 +221,6 @@ func firstBuildObservations(waiting []inventory.BatchedMerge, now time.Time) []c var out []conditions.Observation for _, w := range waiting { k := announcedOf(w) - if len(k.Held) > 0 { - out = append(out, heldFirstBuildObservation(k)) - continue - } if !k.FirstBuild || now.Sub(w.Heard) <= firstBuildWaitBound { continue } @@ -318,108 +235,43 @@ func firstBuildObservations(waiting []inventory.BatchedMerge, now time.Time) []c Headline: fmt.Sprintf("The first build of %s is not asked: no Approve was announced", repository), Explanation: fmt.Sprintf("The merge %.8s into %s adds modules the mesh builds nothing from yet. It registers them "+ "only when mesh-delivery announces that it merged the pull request on your Approve of that head, and no such "+ - "announcement has come in %s. The merge is kept, and the builds are asked the moment the announcement is kept.", + "announcement has come in %s. The merge is kept, and the modules are registered from the builder's keys the moment "+ + "the announcement is kept.", k.Commit, repository, for_), - Resolved: "mesh-delivery announces the merge, or a hand `build` registers the modules", + Resolved: "mesh-delivery announces the merge, or the operator registers the modules at the controller's terminal", }) } return out } -// heldNames is the modules a merge holds, each with the word it waits for: "watchman (key-ttl), view (per-machine)". -func heldNames(held []heldFirstBuild) string { - names := make([]string, 0, len(held)) - for _, h := range held { - names = append(names, fmt.Sprintf("%s (%q)", h.Module, h.Word)) - } - return strings.Join(names, ", ") -} - -// heldFirstBuildObservation names a first merge held for a build-agent that reads its manifests: at once, since nothing -// is lost and nothing times out, but the operator can see what the merge waits for. One condition names every module held. -func heldFirstBuildObservation(k keptMerge) conditions.Observation { - repository := k.Owner + "/" + k.Repo - modules := make([]string, 0, len(k.Held)) - for _, h := range k.Held { - modules = append(modules, h.Module) - } - held := heldNames(k.Held) - return conditions.Observation{Scope: conditions.ScopeMerge, ID: fmt.Sprintf("%s_%.8s", repository, k.Commit), - Kind: kindFirstBuildHeld, Severity: conditions.Warning, - Summary: fmt.Sprintf("the merge %.8s of %s adds modules whose manifests use words the running build-agent does not "+ - "read: %s; the merge waits for a build-agent that reads them", k.Commit, repository, held), - Said: fmt.Sprintf("%s at %s waits for a build-agent that reads the words of %s", repository, k.Commit, held), - Headline: fmt.Sprintf("The first build of %s waits for a newer build-agent", strings.Join(modules, ", ")), - Explanation: fmt.Sprintf("The merge %.8s into %s adds modules whose manifests use words the build-agent now "+ - "running does not know: %s. The merge is kept, not failed: their first builds are asked again when a delivery "+ - "of build-agent ends.", k.Commit, repository, held), - Resolved: "a delivery of build-agent ends and the first builds register the modules, or a hand `build` does", - } -} - // probeFirstBuilds is D18: no first merge waits past its bound for mesh-delivery's word unsaid. func probeFirstBuilds(ctx context.Context, d *doctor) ([]conditions.Observation, error) { waiting, err := d.open.inventory.WaitingMerges(ctx) if err != nil { return nil, err } - return firstBuildObservations(waiting, time.Now().UTC()), nil + now := time.Now().UTC() + return append(firstBuildObservations(waiting, now), newModuleObservations(now)...), nil } -// askHeldFirstBuildsAgain asks again the first build of each first merge held for a build-agent that reads its manifest, -// when the plan that ended delivered build-agent (novox/hq issue 1173): once for each end, since the ask answers the merge -// and a refusal that comes again holds it anew. Answers how many merges it asked for. -func askHeldFirstBuildsAgain(ctx context.Context, open *stores, p inventory.Plan) int { - if !deliversBuildAgent(p) { - return 0 - } - actingOnMerges.Lock() - defer actingOnMerges.Unlock() - inv := open.inventory +// answerHeldMerges answers every merge a controller of before held for a build-agent that reads its new modules' +// manifests (issue 1173): nothing asks their first builds any more, and the look at every module when this controller +// starts registers them from their keys (ADR 0501 §6, §7). +func answerHeldMerges(ctx context.Context, inv *inventory.Inventory) { waiting, err := inv.WaitingMerges(ctx) if err != nil { - fmt.Printf("%s delivered build-agent, and the first merges held for it could not be read: %v\n", p.ID, err) - return 0 + fmt.Printf("the merges held for a newer build-agent could not be read: %v\n", err) + return } - asked := 0 for _, w := range waiting { - k := announcedOf(w) - if len(k.Held) == 0 { - continue - } - // Its own held modules, not the rule of a first merge: a module of the merge that passed is registered by now, - // and the repository no longer counts as one the mesh builds nothing from (review of issue 1173). - why, err := approvedAnnounced(ctx, inv, k.Owner, k.Repo, keptMerge{SourceMoved: k.SourceMoved}) - if err != nil { - fmt.Printf("%s delivered build-agent, and the merge %.8s of %s held for it asks no build now (%v): it stays "+ - "waiting\n", p.ID, w.Commit, w.Repository, err) - continue - } - dirs := heldDirsOf(k) - fmt.Printf("%s delivered build-agent: the first builds of %s are asked again\n", p.ID, heldNames(k.Held)) - paths := make([]string, 0, len(dirs)) - for _, dir := range dirs { - if dir == "." { - dir = "" + if k := announcedOf(w); len(k.Held) > 0 { + if err := inv.AnswerMerge(ctx, w.Repository, w.Commit, inventory.WaitingRecord, false); err != nil { + fmt.Printf("the merge %.8s of %s held for a newer build-agent could not be answered: %v\n", w.Commit, + w.Repository, err) + continue } - paths = append(paths, dir) + fmt.Printf("the merge %.8s of %s was held for a newer build-agent; its modules are registered from the "+ + "builder's keys instead\n", w.Commit, w.Repository) } - repository := k.Owner + "/" + k.Repo - askBuilds(withFirstBuildWarrant(ctx, repository, paths, why), inv, k.SourceMoved, - buildSource{Repository: repository, Seat: gitSeat}, dirs) - // Answered: it no longer waits, so this end asks nothing twice; a refusal that comes again holds it anew. - if err := inv.AnswerMerge(ctx, w.Repository, w.Commit, inventory.WaitingRecord, false); err != nil { - fmt.Printf(" and the merge could not be marked answered: %v\n", err) - continue - } - asked++ } - return asked -} - -// deliversBuildAgent says a delivery plan ended delivered with build-agent built in it: the reader a held first build -// waits for is the one now running. -func deliversBuildAgent(p inventory.Plan) bool { - m := p.Modules["build-agent"] - return p.WalkSaid == inventory.WalkEnded && p.Outcome() == inventory.WalkDelivered && m != nil && m.State == "built" } diff --git a/cmd/mesh-controller/first_builds_held_integration_test.go b/cmd/mesh-controller/first_builds_held_integration_test.go deleted file mode 100644 index fb69d5e2..00000000 --- a/cmd/mesh-controller/first_builds_held_integration_test.go +++ /dev/null @@ -1,221 +0,0 @@ -//go:build integration - -package main - -import ( - "context" - "fmt" - "strings" - "testing" - "time" - - "git.novox.be/novox/mesh-sdk/go/asks" - "github.com/novox/mesh-controller/internal/inventory" - "github.com/novox/mesh-controller/internal/link" -) - -// 1173/AC1, 1173/AC3 whole against the store: a first merge's build refused for a word the running build-agent does not -// read keeps the merge waiting with no failed build recorded; a delivery plan that does not deliver build-agent asks -// nothing; the one that ends delivering it asks the first build again once, and its pass registers the module. -func TestAHeldFirstMergeIsAskedAgainWhenABuildAgentIsDelivered(t *testing.T) { - const head, commit = "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1", "0177ea7a00000000" - open := aMesh(t) - ctx := t.Context() - inv := open.inventory - - a := aDeliveryMergeOf(t, head, map[string]string{"novox/mesh-delivery@a1a1a1a1a1a1": head}) - wasKept, wasAsk, realAsk := keptDeliveryMerge, routerAskOf, askABuild - keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { - return a.merge, true, nil - } - routerAskOf = func(context.Context, catalogueReader, string, string) (*asks.Ask, *asks.Warrant, error) { - return &a.ask, &a.warrant, nil - } - var asked []string - askABuild = func(_ context.Context, source buildSource, path, ref string) (string, error) { - id := "build-" + string(rune('1'+len(asked))) - asked = append(asked, id) - return id, nil - } - t.Cleanup(func() { keptDeliveryMerge, routerAskOf, askABuild = wasKept, wasAsk, realAsk }) - - m := link.SourceMoved{Owner: "novox", Repo: "mesh-delivery", Base: "main", HeadSHA: head, Commit: commit, Number: 7, - MergedAt: time.Now().Add(-time.Hour).UTC().Format(time.RFC3339Nano), Paths: []string{"module.json", "main.go"}, - ModuleDirs: []string{"."}, ModuleDirsSaid: true} - if err := (following{open: open}).SourceMoved(ctx, m); err != nil { - t.Fatal(err) - } - if len(asked) != 1 { - t.Fatalf("1173/AC3: the first merge asked %v, want the one first build", asked) - } - - // The running build-agent refuses the manifest's word. - refused := onTrunk("build-1", "novox/mesh-delivery", "git", "", nil) - refused.Failed, refused.Manifest, refused.Commit = refusedKeyTTL, nil, commit - if err := (builds{inv: inv, open: open}).Built(ctx, refused); err != nil { - t.Fatal(err) - } - if _, recorded, _ := inv.BuildByID(ctx, "build-1"); recorded { - t.Fatal("1173/AC1: a refused first build was recorded as a failed build") - } - waiting, err := inv.WaitingMerges(ctx) - if err != nil || len(waiting) != 1 || len(announcedOf(waiting[0]).Held) == 0 { - t.Fatalf("1173/AC1: the merges waiting are %+v (%v), want the first merge, held", waiting, err) - } - if obs := firstBuildObservations(waiting, time.Now()); len(obs) != 1 || obs[0].Kind != kindFirstBuildHeld { - t.Fatalf("1173/AC2: the held merge is named by %+v", obs) - } - - // A plan that ended without delivering build-agent asks nothing; one that delivered it asks once. - other := inventory.Plan{ID: "p1", State: inventory.PlanDone, WalkSaid: inventory.WalkEnded, - Modules: map[string]*inventory.PlanModule{"mesh-delivery": {State: "built"}}} - if n := askHeldFirstBuildsAgain(ctx, open, other); n != 0 || len(asked) != 1 { - t.Fatalf("1173/AC3: a plan that delivered no build-agent asked %d builds again (%v)", n, asked) - } - delivered := inventory.Plan{ID: "p2", State: inventory.PlanDone, WalkSaid: inventory.WalkEnded, - Modules: map[string]*inventory.PlanModule{"build-agent": {State: "built"}}} - if n := askHeldFirstBuildsAgain(ctx, open, delivered); n != 1 || len(asked) != 2 { - t.Fatalf("1173/AC3: the plan that delivered build-agent asked %d builds again (%v), want one", n, asked) - } - if n := askHeldFirstBuildsAgain(ctx, open, delivered); n != 0 || len(asked) != 2 { - t.Fatalf("1173/AC3: the same end asked again: %d (%v)", n, asked) - } - - // Its pass registers the module. - pass := onTrunk("build-2", "novox/mesh-delivery", "git", "", map[string]any{"module": "mesh-delivery-watchman", "version": "1"}) - pass.Commit = commit - if err := (builds{inv: inv, open: open}).Built(ctx, pass); err != nil { - t.Fatal(err) - } - if _, known := inventoryHolds(t, inv, "mesh-delivery-watchman"); !known { - t.Fatal("1173/AC3: the module is not registered after the first build passed") - } -} - -// aTwoModuleFirstMerge is a first merge into novox/mesh-delivery adding the modules watchman and view, made on the -// operator's Approve of its head, whose first builds are asked; asked says the build id asked for each path, in order. -func aTwoModuleFirstMerge(t *testing.T, open *stores, commit string) *[][2]string { - t.Helper() - const head = "b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2" - a := aDeliveryMergeOf(t, head, map[string]string{"novox/mesh-delivery@b2b2b2b2b2b2": head}) - wasKept, wasAsk, realAsk := keptDeliveryMerge, routerAskOf, askABuild - keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { - return a.merge, true, nil - } - routerAskOf = func(context.Context, catalogueReader, string, string) (*asks.Ask, *asks.Warrant, error) { - return &a.ask, &a.warrant, nil - } - asked := &[][2]string{} - askABuild = func(_ context.Context, _ buildSource, path, _ string) (string, error) { - id := fmt.Sprintf("build-%d", len(*asked)+1) - *asked = append(*asked, [2]string{path, id}) - return id, nil - } - t.Cleanup(func() { keptDeliveryMerge, routerAskOf, askABuild = wasKept, wasAsk, realAsk }) - m := link.SourceMoved{Owner: "novox", Repo: "mesh-delivery", Base: "main", HeadSHA: head, Commit: commit, Number: 7, - MergedAt: time.Now().Add(-time.Hour).UTC().Format(time.RFC3339Nano), - Paths: []string{"view/module.json", "view/main.go", "watchman/module.json", "watchman/main.go"}, - ModuleDirs: []string{"view", "watchman"}, ModuleDirsSaid: true} - if err := (following{open: open}).SourceMoved(t.Context(), m); err != nil { - t.Fatal(err) - } - if len(*asked) != 2 { - t.Fatalf("the first merge asked %v, want the two first builds", *asked) - } - return asked -} - -// idOf is the build asked last for path. -func idOf(t *testing.T, asked [][2]string, path string) string { - t.Helper() - for i := len(asked) - 1; i >= 0; i-- { - if asked[i][0] == path { - return asked[i][1] - } - } - t.Fatalf("no build asked for %s: %v", path, asked) - return "" -} - -// built hands the controller a build's outcome: refused for the word, or passing as module. -func built(t *testing.T, open *stores, id, path, commit, refusedFor, module string) { - t.Helper() - r := onTrunk(id, "novox/mesh-delivery", "git", path, map[string]any{"module": module, "version": "1"}) - r.Commit = commit - if refusedFor != "" { - r.Failed, r.Manifest = `this is not a module manifest: json: unknown field "`+refusedFor+`"`, nil - } - if err := (builds{inv: open.inventory, open: open}).Built(t.Context(), r); err != nil { - t.Fatal(err) - } -} - -var buildAgentDelivered = inventory.Plan{ID: "p-ba", State: inventory.PlanDone, WalkSaid: inventory.WalkEnded, - Modules: map[string]*inventory.PlanModule{"build-agent": {State: "built"}}} - -// 1173/AC3 (review round 1, item 1): in a first merge of two modules where view passes and watchman is refused, the -// held watchman is asked again when build-agent is delivered, though the repository builds view by then. -func TestAHeldModuleIsAskedAgainAfterItsSiblingRegistered(t *testing.T) { - const commit = "0177ea7b00000000" - open := aMesh(t) - ctx := t.Context() - asked := aTwoModuleFirstMerge(t, open, commit) - - built(t, open, idOf(t, *asked, "view"), "view", commit, "", "mesh-delivery-view") - if _, known := inventoryHolds(t, open.inventory, "mesh-delivery-view"); !known { - t.Fatal("view is not registered after its first build passed") - } - refused := idOf(t, *asked, "watchman") - built(t, open, refused, "watchman", commit, "key-ttl", "") - if _, recorded, _ := open.inventory.BuildByID(ctx, refused); recorded { - t.Fatal("1173/AC1: the refused first build of watchman was recorded as a failed build") - } - waiting, err := open.inventory.WaitingMerges(ctx) - if err != nil || len(waiting) != 1 || len(announcedOf(waiting[0]).Held) != 1 { - t.Fatalf("1173/AC1: the merges waiting are %+v (%v), want the first merge, holding watchman", waiting, err) - } - - if n := askHeldFirstBuildsAgain(ctx, open, buildAgentDelivered); n != 1 || len(*asked) != 3 || (*asked)[2][0] != "watchman" { - t.Fatalf("1173/AC3: the delivery of build-agent asked %d merges again (%v), want watchman's first build alone", n, *asked) - } - built(t, open, idOf(t, *asked, "watchman"), "watchman", commit, "", "mesh-delivery-watchman") - if _, known := inventoryHolds(t, open.inventory, "mesh-delivery-watchman"); !known { - t.Fatal("1173/AC3: watchman is not registered after the first build asked again passed") - } -} - -// 1173/AC1, AC2, AC3 (review round 1, item 2): in a first merge of two modules both refused, neither is recorded as a -// failed build, the condition names both, and the delivery of build-agent asks both again. -func TestTwoRefusedModulesOfOneFirstMergeAreBothHeld(t *testing.T) { - const commit = "0177ea7c00000000" - open := aMesh(t) - ctx := t.Context() - asked := aTwoModuleFirstMerge(t, open, commit) - - for _, path := range []string{"watchman", "view"} { - id := idOf(t, *asked, path) - built(t, open, id, path, commit, "key-ttl", "") - if _, recorded, _ := open.inventory.BuildByID(ctx, id); recorded { - t.Fatalf("1173/AC1: the refused first build of %s was recorded as a failed build", path) - } - } - waiting, err := open.inventory.WaitingMerges(ctx) - if err != nil || len(waiting) != 1 || len(announcedOf(waiting[0]).Held) != 2 { - t.Fatalf("1173/AC1: the merges waiting are %+v (%v), want the first merge, holding both", waiting, err) - } - obs := firstBuildObservations(waiting, time.Now()) - if len(obs) != 1 || !strings.Contains(obs[0].Summary, "watchman") || !strings.Contains(obs[0].Summary, "view") { - t.Fatalf("1173/AC2: the held merge is named by %+v, want one condition naming both", obs) - } - - if n := askHeldFirstBuildsAgain(ctx, open, buildAgentDelivered); n != 1 || len(*asked) != 4 { - t.Fatalf("1173/AC3: the delivery of build-agent asked %d merges again (%v), want both first builds", n, *asked) - } - built(t, open, idOf(t, *asked, "watchman"), "watchman", commit, "", "mesh-delivery-watchman") - built(t, open, idOf(t, *asked, "view"), "view", commit, "", "mesh-delivery-view") - for _, module := range []string{"mesh-delivery-watchman", "mesh-delivery-view"} { - if _, known := inventoryHolds(t, open.inventory, module); !known { - t.Fatalf("1173/AC3: %s is not registered after the first build asked again passed", module) - } - } -} diff --git a/cmd/mesh-controller/first_builds_held_test.go b/cmd/mesh-controller/first_builds_held_test.go deleted file mode 100644 index 96110263..00000000 --- a/cmd/mesh-controller/first_builds_held_test.go +++ /dev/null @@ -1,141 +0,0 @@ -package main - -import ( - "encoding/json" - "strings" - "testing" - "time" - - "github.com/novox/mesh-controller/internal/conditions" - "github.com/novox/mesh-controller/internal/inventory" - "github.com/novox/mesh-controller/internal/link" -) - -const refusedKeyTTL = `this is not a module manifest: a state is either a name or {name, history, ttl-seconds, per-machine}: ` + - `json: unknown field "key-ttl"` - -// aFirstMergeAnswered is a first merge as askFirstBuilds leaves it: answered by the record of merges no plan answers. -func aFirstMergeAnswered() (inventory.BuildRequest, inventory.BatchedMerge) { - req := inventory.BuildRequest{ID: "build-1", Repository: "novox/mesh-delivery", Seat: "git", Path: "watchman", Ref: "main", - Commit: "0177ea7a00", For: "merge"} - event, _ := json.Marshal(keptMerge{SourceMoved: link.SourceMoved{Owner: "novox", Repo: "mesh-delivery", Base: "main", - Commit: "0177ea7a00", Number: 3, HeadSHA: "a1a1a1a1a1a1"}, FirstBuild: true}) - return req, inventory.BatchedMerge{Repository: "novox/mesh-delivery", Branch: "main", Commit: "0177ea7a00", - Plan: inventory.WaitingRecord, Event: event} -} - -// 1173/AC1: a first build refused for a word its manifest uses and the running build-agent does not know holds its merge. -func TestAFirstBuildRefusedForAnUnknownWordHoldsItsMerge(t *testing.T) { - req, merge := aFirstMergeAnswered() - held := judgeRefusedFirstBuild(link.BuildResult{ID: "build-1", Failed: refusedKeyTTL}, req, merge) - if len(held) != 1 || held[0].Word != "key-ttl" || held[0].Module != "watchman" || held[0].Path != "watchman" { - t.Fatalf("1173/AC1: a first build refused for \"key-ttl\" is judged %+v, want a hold on that word for watchman", held) - } -} - -// 1173/AC2: a held first merge is named at once by one condition: the merge, the module, the word and what it waits for. -func TestAHeldFirstMergeIsNamedByACondition(t *testing.T) { - now := time.Date(2026, 10, 22, 14, 0, 0, 0, time.UTC) - _, merge := aFirstMergeAnswered() - var k keptMerge - _ = json.Unmarshal(merge.Event, &k) - k.Held = []heldFirstBuild{{Module: "watchman", Path: "watchman", Word: "key-ttl", At: now}, - {Module: "view", Path: "view", Word: "per-machine", At: now}} - merge.Event, _ = json.Marshal(k) - merge.Heard = now.Add(-time.Second) - got := firstBuildObservations([]inventory.BatchedMerge{merge}, now) - if len(got) != 1 { - t.Fatalf("1173/AC2: %d conditions for a held first merge, want one: %+v", len(got), got) - } - o := got[0] - if o.Kind != kindFirstBuildHeld || o.Scope != conditions.ScopeMerge { - t.Fatalf("1173/AC2: the condition is %+v", o) - } - for _, want := range []string{"0177ea7a", "watchman", "key-ttl", "view", "per-machine", "build-agent"} { - if !strings.Contains(o.Summary, want) { - t.Fatalf("1173/AC2: the summary %q does not name %q", o.Summary, want) - } - } -} - -// 1173/AC4: a first build that fails for any other reason is not held. -func TestAFirstBuildFailingForAnyOtherReasonIsNotHeld(t *testing.T) { - req, merge := aFirstMergeAnswered() - if judgeRefusedFirstBuild(link.BuildResult{Failed: refusedKeyTTL}, req, merge) == nil { - t.Fatal("1173/AC4: the control, a refusal for an unknown word, is not held") - } - for _, c := range []struct { - what string - failed string - change func(*inventory.BuildRequest, *inventory.BatchedMerge) - }{ - {what: "a compile error", failed: "main.go:3: undefined: foo"}, - {what: "no failure", failed: ""}, - {what: "a missing field", failed: `json: cannot unmarshal string into Go struct field`}, - {what: "a build a person asked", failed: refusedKeyTTL, change: func(r *inventory.BuildRequest, _ *inventory.BatchedMerge) { r.For = "build" }}, - {what: "a merge a plan answers", failed: refusedKeyTTL, change: func(_ *inventory.BuildRequest, m *inventory.BatchedMerge) { m.Plan = "plan-7" }}, - {what: "a waiting merge held for nothing", failed: refusedKeyTTL, change: func(_ *inventory.BuildRequest, m *inventory.BatchedMerge) { m.Plan = "" }}, - } { - r, m := req, merge - if c.change != nil { - c.change(&r, &m) - } - if held := judgeRefusedFirstBuild(link.BuildResult{Failed: c.failed}, r, m); held != nil { - t.Fatalf("1173/AC4: %s is held: %+v", c.what, held) - } - } -} - -// 1173/AC1, AC2 (review round 1, item 2): a second refusal in a merge already held is held too, its module added to the -// hold, so no failed build is recorded and the condition names both. -func TestASecondRefusalInAHeldMergeIsAddedToTheHold(t *testing.T) { - req, merge := aFirstMergeAnswered() - first := judgeRefusedFirstBuild(link.BuildResult{Failed: refusedKeyTTL}, req, merge) - k := announcedOf(merge) - k.Held = first - merge.Event, _ = json.Marshal(k) - merge.Plan = "" // HoldMerge put it back to waiting - req.ID, req.Path = "build-2", "view" - held := judgeRefusedFirstBuild(link.BuildResult{Failed: `json: unknown field "per-machine"`}, req, merge) - if len(held) != 2 || held[0].Module != "watchman" || held[1].Module != "view" || held[1].Word != "per-machine" { - t.Fatalf("1173/AC1: the second refusal in a held merge is judged %+v, want watchman and view held", held) - } - // The same module refused again is held once. - k.Held = held - merge.Event, _ = json.Marshal(k) - if again := judgeRefusedFirstBuild(link.BuildResult{Failed: refusedKeyTTL}, req, merge); len(again) != 2 { - t.Fatalf("1173/AC1: view refused again is held as %+v, want it once", again) - } -} - -// 1173/AC3 (review round 1, item 1): a held merge is asked again for its own held modules' directories, not by the rule of -// a first merge, which finds nothing once another module of the repository is registered. -func TestAHeldMergeIsAskedAgainForItsHeldModulesAlone(t *testing.T) { - k := keptMerge{Held: []heldFirstBuild{{Module: "watchman", Path: "watchman"}, {Module: "mesh-delivery", Path: ""}, - {Module: "watchman", Path: "/watchman/"}}} - if got := heldDirsOf(k); strings.Join(got, ",") != "watchman,." { - t.Fatalf("1173/AC3: a held merge asks again %v, want watchman and the root once each", got) - } -} - -// 1173/AC3 (the part that needs no store): only a plan that ended delivered with build-agent built in it is the end a held -// first build waits for. -func TestOnlyADeliveredPlanOfBuildAgentIsWhatAHeldFirstBuildWaitsFor(t *testing.T) { - plan := func(state, walk string, modules map[string]*inventory.PlanModule) inventory.Plan { - return inventory.Plan{ID: "p", State: state, WalkSaid: walk, Modules: modules} - } - built := map[string]*inventory.PlanModule{"build-agent": {State: "built"}} - if !deliversBuildAgent(plan(inventory.PlanDone, inventory.WalkEnded, built)) { - t.Fatal("1173/AC3: a plan that delivered build-agent is not the end a held first build waits for") - } - for what, p := range map[string]inventory.Plan{ - "a plan still running": plan(inventory.PlanBuilding, "", built), - "a plan that failed": plan(inventory.PlanFailed, inventory.WalkEnded, built), - "a failed build-agent": plan(inventory.PlanDone, inventory.WalkEnded, map[string]*inventory.PlanModule{"build-agent": {State: "failed"}}), - "another module's plan": plan(inventory.PlanDone, inventory.WalkEnded, map[string]*inventory.PlanModule{"mailu": {State: "built"}}), - } { - if deliversBuildAgent(p) { - t.Fatalf("1173/AC3: %s is the end a held first build waits for", what) - } - } -} diff --git a/cmd/mesh-controller/first_builds_integration_test.go b/cmd/mesh-controller/first_builds_integration_test.go index e82d7284..bab2559b 100644 --- a/cmd/mesh-controller/first_builds_integration_test.go +++ b/cmd/mesh-controller/first_builds_integration_test.go @@ -5,71 +5,48 @@ package main import ( "context" "encoding/json" - "slices" - "sync" "testing" "time" "github.com/nats-io/nats.go/jetstream" "git.novox.be/novox/mesh-sdk/go/catalogue" + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" - "github.com/novox/mesh-controller/internal/testbus" ) -// 1146/AC7, whole against a real bus and the store, in both orders of the two events (novox/hq ADR 0432, issue 1146): -// mesh-delivery #7 merged on the operator's Approve into a repository the mesh builds nothing from. The forge's merge, -// heard as the controller hears it (SourceMoved), and mesh-delivery's word, published on the subject the controller -// consumes with the router's record of the ask in its bucket, come in either order; each way the first build of the -// module the merge commit holds is asked exactly once, its outcome registers the module, and it is in the catalogue. -// Without the Approve in the record nothing is asked. Store-backed: skips without one. -func TestAMergeOnTheApproveIntoANewRepositoryAsksTheFirstBuildAndItsOutcomeRegistersTheModule(t *testing.T) { +// 1146/AC7 on keys (ADR 0501 §3, §6; issue 1851, A5), whole against a real bus and the store, in both orders of the two +// events: mesh-delivery #7 merged on the operator's Approve into a repository the mesh builds nothing from. The forge's +// merge, heard as the controller hears it (SourceMoved), and mesh-delivery's word, published on the subject the +// controller consumes with the router's record of the ask in its bucket, come in either order; each way the module the +// merge commit holds is registered from the builder's done key of it, and nothing asks a build. Without the Approve in +// the record it is not registered. Store-backed: skips without one. +func TestAMergeOnTheApproveIntoANewRepositoryRegistersItsModuleFromItsKey(t *testing.T) { const head, commit = "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1", "0177ea7a00000000" for _, c := range []struct { - what string - wordFirst bool - approved bool // the router's record holds the operator's Approve of the merged head - wantRegiter bool + what string + wordFirst bool + approved bool // the router's record holds the operator's Approve of the merged head + wantRegister bool }{ - {what: "the forge's merge before mesh-delivery's word", approved: true, wantRegiter: true}, - {what: "mesh-delivery's word before the forge's merge", wordFirst: true, approved: true, wantRegiter: true}, + {what: "the forge's merge before mesh-delivery's word", approved: true, wantRegister: true}, + {what: "mesh-delivery's word before the forge's merge", wordFirst: true, approved: true, wantRegister: true}, {what: "a word whose record holds no Approve", wordFirst: true}, } { t.Run(c.what, func(t *testing.T) { open := aMesh(t) ctx := t.Context() inv := open.inventory - // The real ask, guard and queue included: only its answers are kept, so the build's outcome can be matched. - aForgeAndABuildSeat(t, open) - // Written by the controller's merge path on its own goroutine, read by the test: under one lock. - var mu sync.Mutex - var asks [][3]string - var ids []string - asked := func() [][3]string { mu.Lock(); defer mu.Unlock(); return slices.Clone(asks) } - realAsk := askABuild - askABuild = func(c context.Context, source buildSource, path, ref string) (string, error) { - id, err := realAsk(c, source, path, ref) - if err == nil { - mu.Lock() - asks = append(asks, [3]string{source.Repository, path, ref}) - ids = append(ids, id) - mu.Unlock() - } - return id, err - } - t.Cleanup(func() { askABuild = realAsk }) + noBuildAsked(t) + b := aBuilderOnTheBus(t) + js := b.js if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "operator-channel", Version: "1", DefinesSeats: []catalogue.SeatDeclaration{{Name: broker.AsksSeat, Records: []string{"asks"}}}}, inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/operator-channel", BuiltFrom: "c0ffee"}); err != nil { t.Fatal(err) } - js, err := broker.Dial(testbus.URL(t)) - if err != nil { - t.Fatal(err) - } - t.Cleanup(js.Close) if err := broker.AssertMeshStreams(js); err != nil { t.Fatal(err) } @@ -91,6 +68,10 @@ func TestAMergeOnTheApproveIntoANewRepositoryAsksTheFirstBuildAndItsOutcomeRegis t.Fatal(err) } } + // The indexer's value and the builder's done key of the module the merge commit holds. + fp := fpOf('9') + manifest := b.indexed(t, "mesh-delivery-view", "1", "novox/mesh-delivery", ".", commit, fp) + b.built(t, "mesh-delivery-view", "1", fp, manifest, sdkdelivery.BuildDone) following := following{open: open} server := link.ConnectNats(js, nil, noListener{}) @@ -104,8 +85,6 @@ func TestAMergeOnTheApproveIntoANewRepositoryAsksTheFirstBuildAndItsOutcomeRegis done := make(chan struct{}) go func() { defer close(done); _ = server.Serve(serving) }() t.Cleanup(func() { stop(); <-done }) - servingBus.Store(js) - t.Cleanup(func() { servingBus.Store(nil) }) word := func() { t.Helper() @@ -133,37 +112,27 @@ func TestAMergeOnTheApproveIntoANewRepositoryAsksTheFirstBuildAndItsOutcomeRegis forge() } else { forge() - if len(asked()) != 0 { - t.Fatalf("1146/AC7: the merge asked %v before mesh-delivery's word was kept", asked()) + time.Sleep(300 * time.Millisecond) + if _, known := inventoryHolds(t, inv, "mesh-delivery-view"); known { + t.Fatal("1146/AC7: registered before mesh-delivery's word was kept") } word() } - if !c.wantRegiter { - if len(asked()) != 0 { - t.Fatalf("1146/AC7: a merge whose record holds no Approve asked %v", asked()) + if !c.wantRegister { + time.Sleep(time.Second) + if _, known := inventoryHolds(t, inv, "mesh-delivery-view"); known { + t.Fatal("1146/AC7: a merge whose record holds no Approve registered its module") } return } - want := [3]string{"novox/mesh-delivery", "", "main"} - becomes(t, 10*time.Second, "the first build asked", func() bool { return len(asked()) >= 1 }) - if got := asked(); len(got) != 1 || got[0] != want { - t.Fatalf("1146/AC7: the merge asked %v, want %v once", got, want) - } - mu.Lock() - first := ids[0] - mu.Unlock() - build := onTrunk(first, "novox/mesh-delivery", "git", "", map[string]any{"module": "mesh-delivery-view", "version": "1"}) - build.Commit = commit - if _, _, err := takeIn(ctx, inv, build); err != nil { - t.Fatalf("1146/AC7: the outcome of the first build was refused: %v", err) - } + becomes(t, 10*time.Second, "the module registered", func() bool { + _, known := inventoryHolds(t, inv, "mesh-delivery-view") + return known + }) src, err := inv.SourceOf(ctx, "mesh-delivery-view") if err != nil || src.Repository != "novox/mesh-delivery" { t.Fatalf("1146/AC7: the module is built from %q (%v)", src.Repository, err) } - if _, known := inventoryHolds(t, inv, "mesh-delivery-view"); !known { - t.Fatal("1146/AC7: the module is not in the catalogue") - } }) } } diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index 7f204dac..d3f380b2 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -408,13 +408,6 @@ func (b builds) Built(ctx context.Context, result link.BuildResult) error { fmt.Printf("%s: a dry run of %s on %s, not taken in\n", result.ID, result.Repository, result.Ref) return nil } - // **A first build refused for a word the running build-agent does not read waits for one that does** (novox/hq ADR - // 0482 §5, issue 1173): its merge is kept, named, and never failed; no failed build is recorded. - if held, err := holdRefusedFirstBuild(ctx, b.inv, result); err != nil { - return err - } else if held { - return nil - } manifest, _, err := takeIn(ctx, b.inv, result) // The assignments pending on this build, made or ended (novox/hq issue 325), said in the daemon's log // after what became of the build. diff --git a/cmd/mesh-controller/new_module_asked_once_test.go b/cmd/mesh-controller/new_module_asked_once_test.go deleted file mode 100644 index d2dc87fa..00000000 --- a/cmd/mesh-controller/new_module_asked_once_test.go +++ /dev/null @@ -1,37 +0,0 @@ -package main - -import ( - "reflect" - "testing" - "time" - - "github.com/novox/mesh-controller/internal/inventory" -) - -// 1617/AC2 (unit): a new module whose first build a merge's hearing asked is not asked again when its batch is cut, -// and one whose ask failed, was never handed over or is long lost is. -func TestAModuleWhoseBuildIsComingIsNotAskedAgain(t *testing.T) { - now := time.Date(2026, 10, 21, 12, 0, 0, 0, time.UTC) - source := buildSource{Repository: "http://forge/novox/mesh-catalog.git", Seat: "git"} - asked := func(path string, mod func(*inventory.RequestOutcome)) inventory.RequestOutcome { - r := inventory.RequestOutcome{BuildRequest: inventory.BuildRequest{ID: "b-" + path, Repository: source.Repository, - Seat: "git", Path: path, For: "merge", At: now.Add(-3 * time.Minute)}} - if mod != nil { - mod(&r) - } - return r - } - requests := []inventory.RequestOutcome{ - asked("modules/go", nil), - asked("modules/failed", func(r *inventory.RequestOutcome) { r.Heard, r.Failed = true, "no module.json" }), - asked("modules/notasked", func(r *inventory.RequestOutcome) { r.NotAsked = "the bus is silent" }), - asked("modules/lost", func(r *inventory.RequestOutcome) { r.At = now.Add(-2 * time.Hour) }), - asked("modules/byhand", func(r *inventory.RequestOutcome) { r.For = "build" }), - } - got := unaskedDirs([]string{"modules/go", "modules/failed", "modules/notasked", "modules/lost", "modules/byhand", - "modules/nodejs"}, requests, source, now) - want := []string{"modules/failed", "modules/notasked", "modules/lost", "modules/byhand", "modules/nodejs"} - if !reflect.DeepEqual(got, want) { - t.Fatalf("asked again %v, want %v", got, want) - } -} diff --git a/cmd/mesh-controller/new_module_on_merge_test.go b/cmd/mesh-controller/new_module_on_merge_test.go index a5b38109..eb693721 100644 --- a/cmd/mesh-controller/new_module_on_merge_test.go +++ b/cmd/mesh-controller/new_module_on_merge_test.go @@ -4,134 +4,109 @@ package main import ( "context" - "sync" + "strings" "testing" "time" "git.novox.be/novox/mesh-sdk/go/catalogue" + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" ) -// asksWithPaths records every build a merge asks for, as repository, path and ref. The merge window's goroutine and -// the test's own may ask at once, so the record is written under a lock (issue 1543). -func asksWithPaths(t *testing.T) *[][3]string { - t.Helper() - var asked [][3]string - var mu sync.Mutex - was := askABuild - askABuild = func(_ context.Context, source buildSource, path, ref string) (string, error) { - mu.Lock() - defer mu.Unlock() - asked = append(asked, [3]string{source.Repository, path, ref}) - return "b-" + path, nil - } - t.Cleanup(func() { askABuild = was }) - return &asked -} - -// The live case of 2026-10-07 (novox/hq issue 300), replayed: a merge of the catalogue adds one module's -// directory and touches nothing else the mesh holds. Its delivery plan said "builds new: …, sent nowhere"; -// the merge said it changed nothing any module the mesh holds is built from, and built nothing, so the -// module was never registered and `assign` refused it. The merge asks for its build, at the branch merged -// into, from the repository as the mesh spells it — and opens no plan, since nothing it holds moved. -func TestAMergeBuildsTheNewModuleItAdds(t *testing.T) { +// ADR 0501 §6 (issue 1851, A5): the live case of 2026-10-07 (novox/hq issue 300), on keys. A merge of the catalogue adds +// one module's directory and touches nothing else the mesh holds: nothing asks its first build, it opens no plan, and +// the module is registered from the builder's first done key of it, assigned nowhere, with no build request written. +func TestAMergeAddingAModuleEndsRegisteredFromItsKeyAndAsksNoBuild(t *testing.T) { open := aMesh(t) ctx := t.Context() inv := open.inventory - asked := asksWithPaths(t) + noBuildAsked(t) + b := aBuilderOnTheBus(t) if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "networkmanager", Version: "1"}, inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/networkmanager", Ref: "main", BuiltFrom: "c0", Head: "c0"}); err != nil { t.Fatal(err) } - m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "3da80a4b00", + watching, stop := context.WithCancel(context.Background()) + t.Cleanup(stop) + watchNewModules(watching, open) + + const commit = "3da80a4b00000000" + m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: commit, Paths: []string{"modules/systemd-resolved/module.json", "modules/systemd-resolved/cmd/main.go"}, ModuleDirs: []string{"modules/systemd-resolved"}, ModuleDirsSaid: true} if err := (following{open: open}).SourceMoved(ctx, m); err != nil { t.Fatal(err) } - want := [3]string{"novox/mesh-catalog", "modules/systemd-resolved", "main"} - if len(*asked) != 1 || (*asked)[0] != want { - t.Fatalf("the merge adding modules/systemd-resolved asked for %v, not %v", *asked, want) - } if plans, err := inv.OpenPlans(ctx); err != nil || len(plans) != 0 { t.Fatalf("a merge moving nothing the mesh holds opened a plan: %+v %v", plans, err) } + // The indexer writes the module's trunk value; the builder builds it, then its key is done. + fp := fpOf('7') + manifest := b.indexed(t, "systemd-resolved", "1", "novox/mesh-catalog", "modules/systemd-resolved", commit, fp) + b.built(t, "systemd-resolved", "1", fp, manifest, sdkdelivery.BuildBuilding) + time.Sleep(300 * time.Millisecond) + if _, known := inventoryHolds(t, inv, "systemd-resolved"); known { + t.Fatal("0501 §1: registered from a key still building") + } + if obs := newModuleObservations(time.Now().Add(time.Hour)); len(obs) != 1 || obs[0].Kind != kindNewModuleWaits { + t.Fatalf("0501 §6: the new module waiting on its key past its bound is named by %+v", obs) + } + b.built(t, "systemd-resolved", "1", fp, manifest, sdkdelivery.BuildDone) + becomes(t, 10*time.Second, "the new module registered", func() bool { + _, known := inventoryHolds(t, inv, "systemd-resolved") + return known + }) + if obs := newModuleObservations(time.Now().Add(time.Hour)); len(obs) != 0 { + t.Fatalf("a registered module is still named waiting: %+v", obs) + } + requests, err := inv.RequestsNamed(ctx, inventory.BuildRequest{Repository: "novox/mesh-catalog", + Path: "modules/systemd-resolved"}.Name()) + if err != nil || len(requests) != 0 { + t.Fatalf("0501 §6: a build request was written: %+v (%v)", requests, err) + } } -// A merge that changes a held module and adds a new one does both: the held one walks its plan, the new -// one is asked for beside it. A merge adding nothing builds nothing new. -func TestAMergeBuildsItsNewModuleBesideItsPlan(t *testing.T) { +// ADR 0501 §7: a new module whose manifest uses a word this controller does not read is named waiting at once, never +// failed, and registered from the same key, with no second build, when a controller that reads it looks. +func TestANewModuleWithAnUnreadWordWaitsForTheControllerThatReadsIt(t *testing.T) { open := aMesh(t) ctx := t.Context() inv := open.inventory - asked := asksWithPaths(t) - if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "app", Version: "1"}, - inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/app", Ref: "main", + noBuildAsked(t) + b := aBuilderOnTheBus(t) + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "networkmanager", Version: "1"}, + inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/networkmanager", Ref: "main", BuiltFrom: "c0", Head: "c0"}); err != nil { t.Fatal(err) } - m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "c1aaaaaaaa", - Paths: []string{"modules/app/index.ts", "modules/fresh/module.json"}, - ModuleDirs: []string{"modules/app", "modules/fresh"}, ModuleDirsSaid: true} - if err := (following{open: open}).SourceMoved(ctx, m); err != nil { + fp := fpOf('8') + raw := `{"module":"watchman","version":"1","not-a-word-yet":"5m","build":{"artifacts":[{"name":"a","kind":"archive","from":"files"}]},` + + `"resources":[{"id":"a","type":"archive","path":"/opt/watchman","artifact":"a"}]}` + digest := sdkdelivery.ManifestDigest([]byte(raw)) + value, _ := sdkdelivery.EncodeManifest(sdkdelivery.Manifest{Module: "watchman", JSON: raw}) + if _, err := b.manifests.Put(ctx, digest, value); err != nil { t.Fatal(err) } - paths := map[string]bool{} - for _, a := range *asked { - paths[a[1]] = true - } - if !paths["modules/fresh"] || !paths["modules/app"] || len(*asked) != 2 { - t.Fatalf("asked %v; want the new module and the plan's first tier", *asked) - } - plans, err := inv.OpenPlans(ctx) - if err != nil || len(plans) != 1 || plans[0].Modules["fresh"] != nil || plans[0].Modules["app"] == nil { - t.Fatalf("the plan should walk app alone: %+v %v", plans, err) - } - - *asked = nil - m2 := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "c2aaaaaaaa", - Paths: []string{"README.md"}, ModuleDirsSaid: true} - if err := (following{open: open}).SourceMoved(ctx, m2); err != nil { + trunk, _ := sdkdelivery.EncodeModule(sdkdelivery.Module{FP: "src1:" + fp[5:], FP2: fp, Manifest: digest, + Repository: "novox/mesh-catalog", Commit: "4444444400000000", Path: "modules/watchman"}) + if _, err := b.modules.Put(ctx, "watchman", trunk); err != nil { t.Fatal(err) } - if len(*asked) != 0 { - t.Fatalf("a merge adding no module asked for %v", *asked) + b.built(t, "watchman", "1", fp, digest, sdkdelivery.BuildDone) + takeNewModules(ctx, open, "watchman") + if _, known := inventoryHolds(t, inv, "watchman"); known { + t.Fatal("0501 §7: a manifest with an unread word was registered") } -} - -// 1617/AC1 (integration): a merge adding a module to a repository the mesh builds modules from, heard while a batch of -// that repository is still assembling (its window running), asks the module's first build when it is heard, so the -// module is registered while the batch waits; cutting the batch asks it no second time (1617/AC2). -func TestAMergeAddingAModuleAsksItsBuildBeforeItsBatchIsCut(t *testing.T) { - open := windowed(t) - groupsWaiting(t, "pr/c1") - asked := asksWithPaths(t) - catalog := func(commit string, made time.Time, paths ...string) link.SourceMoved { - return link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: commit, - Head: "pr/" + commit, MergedAt: made.Format(time.RFC3339Nano), Paths: paths} + obs := newModuleObservations(time.Now()) + if len(obs) != 1 || obs[0].Kind != kindNewModuleWaits || !strings.Contains(obs[0].Summary, `"not-a-word-yet"`) { + t.Fatalf("0501 §7: the module waiting for a reader of its word is named by %+v", obs) } - hear(t, open, catalog("c1", t0, "modules/app/index.ts"), t0) - if len(*asked) != 0 { - t.Fatalf("a merge adding no module asked %v", *asked) - } - hear(t, open, lone(catalog("c2", t0.Add(10*time.Second), "modules/go/packages.json", "modules/go/module.json")), t0.Add(10*time.Second)) - if ws, bs := walks(t, open); len(ws) != 0 || len(bs) != 1 { - t.Fatalf("the batch should still be assembling: %d walk(s), %d batch(es)", len(ws), len(bs)) - } - want := [3]string{"novox/mesh-catalog", "modules/go", "main"} - if len(*asked) != 1 || (*asked)[0] != want { - t.Fatalf("heard with its batch assembling, the merge asked %v, not %v", *asked, want) - } - cutAt(t, open, t0.Add(10*time.Second+90*time.Second)) - n := 0 - for _, a := range *asked { - if a[1] == "modules/go" { - n++ + builds, _ := inv.Builds(ctx, "watchman", 5) + for _, b := range builds { + if !b.Worked() { + t.Fatalf("0501 §7: a failed build was recorded: %+v", b) } } - if n != 1 { - t.Fatalf("modules/go was asked %d times: %v", n, *asked) - } } diff --git a/cmd/mesh-controller/new_modules.go b/cmd/mesh-controller/new_modules.go new file mode 100644 index 00000000..75871383 --- /dev/null +++ b/cmd/mesh-controller/new_modules.go @@ -0,0 +1,251 @@ +package main + +import ( + "context" + "errors" + "fmt" + "slices" + "sort" + "strings" + "sync" + "time" + + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" + "github.com/novox/mesh-controller/internal/builderkeys" + "github.com/novox/mesh-controller/internal/conditions" + "github.com/novox/mesh-controller/internal/inventory" +) + +// A new module is registered from its first done key (novox/hq ADR 0501 §1, §6; issue 1851, A5). +// +// Nothing asks a new module's first build: the builder builds every fingerprint the indexer writes, a new module's +// included (ADR 0429 §5). The controller registers a module the catalogue does not hold from the builder's done key of the +// fingerprint the indexer's trunk value names, under the same rules of who may register as before (ADR 0501 §3): from a +// repository the catalogue builds another module from, or on the operator's Approve of the merge the trunk value names +// (a first merge, kept with mesh-delivery's word), or at the terminal (ADR 0501 §4). It is assigned nowhere. +// +// **No timer** (ADR 0426): a write of the builder or the indexer wakes it for that module (builderkeys.Watch), a first +// merge's word wakes it for that repository, and a controller that starts looks at every module once (ADR 0501 §7: a +// manifest the controller before could not read is registered when the controller that reads it runs). + +// newModulesMu keeps one take-in of new modules at a time in this process. +var newModulesMu sync.Mutex + +// newModuleWait is a new module the controller could not register yet, and why: what the first-builds condition names. +type newModuleWait struct { + Module, Repository, Commit, Why string + // Word is the manifest word this controller does not read, when that is why (ADR 0501 §7). + Word string + Since time.Time +} + +// newModulesWaiting are the new modules waiting, by module, in this process: a restart looks at every module again. +var newModulesWaiting = struct { + sync.Mutex + by map[string]newModuleWait +}{by: map[string]newModuleWait{}} + +// takeNewModules looks at the modules the indexer holds that the catalogue does not, all of them or those named, and +// registers each whose done key may register it. Each outcome is said once: a module still waiting for the same reason +// is not said again. +func takeNewModules(ctx context.Context, open *stores, only ...string) { + newModulesMu.Lock() + defer newModulesMu.Unlock() + keys, closeKeys, err := openBuilderKeys() + if err != nil { + fmt.Printf("new modules: the builder's keys cannot be read: %v\n", err) + return + } + defer closeKeys() + takeNewModulesWith(ctx, open.inventory, keys, only...) +} + +// takeNewModulesWith is takeNewModules on the buckets handed in. +func takeNewModulesWith(ctx context.Context, inv *inventory.Inventory, keys newModuleReads, only ...string) { + entries, err := inv.Catalogued(ctx) + if err != nil { + fmt.Printf("new modules: the catalogue cannot be read: %v\n", err) + return + } + held := map[string]bool{} + for _, e := range entries { + held[e.Manifest.Module] = true + } + values, unread, err := keys.Modules(ctx) + if err != nil { + fmt.Printf("new modules: the indexer's modules cannot be read: %v\n", err) + return + } + for module, err := range unread { + if !held[module] && (len(only) == 0 || slices.Contains(only, module)) { + sayNewModuleWait(newModuleWait{Module: module, Why: "the indexer's value of it cannot be read: " + err.Error()}) + } + } + names := make([]string, 0, len(values)) + for module := range values { + if !held[module] && (len(only) == 0 || slices.Contains(only, module)) { + names = append(names, module) + } + } + sort.Strings(names) + for _, module := range names { + v := values[module] + _, _, err := takeInKey(ctx, inv, keys, keyTake{Module: module, Trunk: v, Ask: registrationAsk{Merge: v.Commit}}) + switch { + case err == nil: + forgetNewModuleWait(module) + fmt.Printf("%s is new in %s at %.8s: registered from the builder's done key, assigned nowhere\n", module, + v.Repository, v.Commit) + case errors.Is(err, errKeyWaits): + w := newModuleWait{Module: module, Repository: v.Repository, Commit: v.Commit, Why: err.Error()} + if word := wordIn(err.Error()); word != "" { + w.Word = word + } + sayNewModuleWait(w) + default: + sayNewModuleWait(newModuleWait{Module: module, Repository: v.Repository, Commit: v.Commit, + Why: "not registered: " + err.Error()}) + } + } +} + +// newModuleReads is what the take-in of new modules reads of the builder and the indexer. +type newModuleReads interface { + keyReads + Modules(ctx context.Context) (map[string]sdkdelivery.Module, map[string]error, error) +} + +var _ newModuleReads = builderkeys.Reader{} + +// wordIn is the manifest word a take-in's words say this controller does not read. +func wordIn(said string) string { + _, rest, ok := strings.Cut(said, "uses the word \"") + if !ok { + return "" + } + word, _, _ := strings.Cut(rest, "\"") + return word +} + +// sayNewModuleWait keeps a new module waiting and says it, once for each reason. +func sayNewModuleWait(w newModuleWait) { + newModulesWaiting.Lock() + defer newModulesWaiting.Unlock() + was, known := newModulesWaiting.by[w.Module] + if known && was.Why == w.Why && was.Commit == w.Commit { + return + } + w.Since = time.Now().UTC() + if known && was.Commit == w.Commit { + w.Since = was.Since + } + newModulesWaiting.by[w.Module] = w + fmt.Printf("%s is new at %.8s and not registered yet: %s\n", w.Module, w.Commit, firstLine(w.Why)) +} + +func forgetNewModuleWait(module string) { + newModulesWaiting.Lock() + defer newModulesWaiting.Unlock() + delete(newModulesWaiting.by, module) +} + +// kindNewModuleWaits is what a new module waiting for this controller's reader of its manifest, or waiting past its +// bound, raises (ADR 0501 §6, §7: the first-builds condition reads the key, not a build request). +const kindNewModuleWaits = "new-module-waits" + +// newModuleWaitBound is how long a new module waits for its key before it is said: the builder builds a fingerprint in +// minutes. Not a delay: its key's write registers it the moment it is done. +const newModuleWaitBound = 30 * time.Minute + +// newModuleObservations names the new modules waiting: one whose manifest uses a word this controller does not read at +// once (nothing is lost, but the operator sees what waits for which controller), any other past the bound. +func newModuleObservations(now time.Time) []conditions.Observation { + newModulesWaiting.Lock() + defer newModulesWaiting.Unlock() + var out []conditions.Observation + for _, w := range newModulesWaiting.by { + if w.Word == "" && now.Sub(w.Since) <= newModuleWaitBound { + continue + } + head := fmt.Sprintf("%s is not registered: %s", w.Module, firstLine(w.Why)) + resolved := "its key is done and its take-in registers it" + if w.Word != "" { + head = fmt.Sprintf("%s waits for a controller that reads the manifest word %q", w.Module, w.Word) + resolved = "a controller that reads the word runs and registers it from the same key, with no second build" + } + out = append(out, conditions.Observation{Scope: conditions.ScopeMerge, + ID: fmt.Sprintf("%s_%s", w.Module, short(w.Commit)), Kind: kindNewModuleWaits, Severity: conditions.Warning, + Summary: head, + Said: fmt.Sprintf("%s at %s waits since %s: %s", w.Module, w.Commit, w.Since.Format(time.RFC3339), w.Why), + Headline: fmt.Sprintf("The new module %s is not registered yet", w.Module), + Explanation: fmt.Sprintf("The merge %.8s of %s adds %s. The mesh registers it from the builder's done key of its "+ + "fingerprint, and has not yet: %s", w.Commit, w.Repository, w.Module, w.Why), + Resolved: resolved, + }) + } + sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID }) + return out +} + +// watchNewModules wakes the take-in of new modules for each module the builder or the indexer writes, until ctx ends, and +// looks at every module once now. A bucket not there yet is said: the look at start and a first merge's word still work. +func watchNewModules(ctx context.Context, open *stores) { + go func() { + answerHeldMerges(ctx, open.inventory) + takeNewModules(ctx, open) + }() + keys, closeKeys, err := openBuilderKeys() + if err != nil { + fmt.Printf("new modules: the builder's keys are not watched: %v\n", err) + return + } + if err := builderkeys.Watch(ctx, keys.JS, func(module string) { wakeNewModule(ctx, open, module) }); err != nil { + closeKeys() + fmt.Printf("new modules: the builder's keys are not watched: %v\n", err) + return + } + go func() { + <-ctx.Done() + closeKeys() + }() +} + +// wakeNewModule takes module in when the catalogue does not hold it: never on the watch's own goroutine, which must keep +// draining. +func wakeNewModule(ctx context.Context, open *stores, module string) { + go func() { + entries, err := open.inventory.Catalogued(ctx) + if err != nil { + return + } + for _, e := range entries { + if e.Manifest.Module == module { + // Known: a plan waiting on its key is woken by the plans' own look. + askToLook() + return + } + } + takeNewModules(ctx, open, module) + }() +} + +// newModulesOf are the modules the indexer holds in a repository: what a first merge's word wakes. +func newModulesOf(ctx context.Context, repository string) ([]string, error) { + keys, closeKeys, err := openBuilderKeys() + if err != nil { + return nil, err + } + defer closeKeys() + values, _, err := keys.Modules(ctx) + if err != nil { + return nil, err + } + var out []string + for module, v := range values { + if strings.EqualFold(v.Repository, repository) { + out = append(out, module) + } + } + sort.Strings(out) + return out, nil +} diff --git a/cmd/mesh-controller/new_modules_test.go b/cmd/mesh-controller/new_modules_test.go new file mode 100644 index 00000000..cfa845ac --- /dev/null +++ b/cmd/mesh-controller/new_modules_test.go @@ -0,0 +1,33 @@ +package main + +import ( + "strings" + "testing" + "time" +) + +// ADR 0501 §6, §7 (issue 1851, A5b): the condition of a new module reads its key's take-in: one waiting for a controller +// that reads its manifest's word is named at once, one waiting for its key only past the bound, each reason said once. +func TestANewModuleWaitingIsNamedByWhatItWaitsFor(t *testing.T) { + t.Cleanup(func() { + newModulesWaiting.Lock() + newModulesWaiting.by = map[string]newModuleWait{} + newModulesWaiting.Unlock() + }) + word := `waits: watchman's manifest uses the word "key-ttl", which this controller does not read: it is registered when a controller that reads it runs` + sayNewModuleWait(newModuleWait{Module: "watchman", Commit: "c1", Why: word, Word: wordIn(word)}) + sayNewModuleWait(newModuleWait{Module: "view", Commit: "c1", Why: "waits: the build of view at aaaa is building on ace"}) + now := time.Now() + obs := newModuleObservations(now) + if len(obs) != 1 || !strings.Contains(obs[0].Summary, `"key-ttl"`) || obs[0].Kind != kindNewModuleWaits { + t.Fatalf("at once, only the unread word is named: %+v", obs) + } + if obs := newModuleObservations(now.Add(newModuleWaitBound + time.Minute)); len(obs) != 2 { + t.Fatalf("past the bound, both are named: %+v", obs) + } + forgetNewModuleWait("watchman") + forgetNewModuleWait("view") + if obs := newModuleObservations(now.Add(time.Hour)); len(obs) != 0 { + t.Fatalf("registered modules are still named: %+v", obs) + } +} diff --git a/cmd/mesh-controller/plan_looks.go b/cmd/mesh-controller/plan_looks.go index c29e4065..855d6e02 100644 --- a/cmd/mesh-controller/plan_looks.go +++ b/cmd/mesh-controller/plan_looks.go @@ -209,10 +209,6 @@ func servingPlanSaved(ctx context.Context, bus link.Bus, open *stores) func(*inv keepBuildingKey(ctx, p) if p.WalkSaid == inventory.WalkEnded { go forgetChecksOfWalk(context.WithoutCancel(ctx), open.inventory, p) - // A first build refused for a word only a newer build-agent reads is asked again (novox/hq issue 1173). - if deliversBuildAgent(p) { - go askHeldFirstBuildsAgain(context.WithoutCancel(ctx), open, p) - } } } } diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 9f38cd91..e3997d88 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -286,6 +286,9 @@ func serve(ctx context.Context) (err error) { go sweepingDeclarationFiles(ctx, inv, bus.Conn) // And the key of each plan in its build phase, which holds the build seat's checks while a delivery builds (ADR 0483 clause 2). go keepingPlansBuilding(ctx, inv, bus.Conn) + // And the modules the catalogue does not hold yet, registered from the builder's first done key of each, woken by + // the builder's and the indexer's writes (novox/hq ADR 0501 §6, issue 1851): nothing asks a first build. + watchNewModules(ctx, open) // Every call carries the lease's epoch, and its record is written only under the lease (novox/hq // to-be 45 §6). link.Calls.UnderLease(func() (uint64, error) { return theLease.epoch(ctx) }) diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index da962d91..07a3c964 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -371,12 +371,17 @@ func movesOfMerge(ctx context.Context, inv *inventory.Inventory, m link.SourceMo } // **A new module is built and registered, and sent nowhere** (novox/hq issue 300): the delivery plan // says so, and assigning it is a person's act, which needs it registered first. - built := askNewModules(ctx, inv, m, from, added) + // Nothing asks its first build (novox/hq ADR 0501 §6): the builder builds every fingerprint the indexer writes, and + // the controller registers the module from its first done key (new_modules.go). + var built []string + if len(from) > 0 { + built = added + } moved := append(append([]inventory.Entry{}, touched...), packaging...) if len(moved) == 0 { if len(built) > 0 { fmt.Printf("%s/%s merged into %s (%.8s); it changed no module the mesh holds, and adds %s: "+ - "built, registered when the build lands, and sent nowhere\n", m.Owner, m.Repo, m.Base, m.Commit, + "registered from the builder's done key of each, and sent nowhere\n", m.Owner, m.Repo, m.Base, m.Commit, strings.Join(built, ", ")) return nil, nil } @@ -399,107 +404,6 @@ func movesOfMerge(ctx context.Context, inv *inventory.Inventory, m link.SourceMo return names, nil } -// askNewModules asks the build seat for every module a merge adds to the repository — a directory holding a -// module.json that no module of the mesh is known from (touchedBy's `added`) — at the branch merged into, -// not waited for: the build's take-in registers it, as a hand `build` does, and sends it nowhere, since no -// machine is assigned it (novox/hq issue 300). Before this a merge said "it changed nothing any module -// the mesh holds is built from", the module was never registered, and `assign` refused it as unknown. -// -// The repository is spelled, and found on the seat, as the modules already built from it are; `from` is -// those of them this merge moves, so a merge read as history, or of a branch nothing follows, builds no -// new module either. Outside the plan: the plan walks modules the catalogue holds, and a new one has no -// machine to send to and nothing standing on it. What could not be asked is said, and left to `build`. -// Returns the directories asked for. -func askNewModules(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved, from []inventory.Entry, - added []string) []string { - if len(added) == 0 || len(from) == 0 { - return nil - } - source := buildSource{Repository: from[0].Source.Repository, Seat: from[0].Source.Seat} - // Asked once (novox/hq issue 1617): a module is asked when its merge is heard, and again here when the batch is cut. - var requests []inventory.RequestOutcome - for _, dir := range added { - if got, err := inv.RequestsNamed(ctx, inventory.BuildRequest{Repository: source.Repository, Path: dir}.Name()); err == nil { - requests = append(requests, got...) - } - } - return askBuilds(ctx, inv, m, source, unaskedDirs(added, requests, source, time.Now())) -} - -// askedInFlightFor is how long a merge's build request stands as the build coming for its module: past it, a lost one -// is asked again. -const askedInFlightFor = time.Hour - -// unaskedDirs are the directories of a new module no build of a merge is on its way for: a kept request of the same -// repository and directory, asked for a merge and handed over, whose outcome has not come and is not given up on, -// is the build coming (novox/hq issue 1617). -func unaskedDirs(added []string, requests []inventory.RequestOutcome, source buildSource, now time.Time) []string { - var out []string - for _, dir := range added { - path := dir - if path == "." { - path = "" - } - coming := false - for _, r := range requests { - if r.For == "merge" && r.Repository == source.Repository && strings.Trim(r.Path, "/") == path && - r.NotAsked == "" && r.OutcomeUnknown == "" && !r.Heard && now.Sub(r.At) < askedInFlightFor { - coming = true - break - } - } - if !coming { - out = append(out, dir) - } - } - return out -} - -// askNewModulesAtHearing asks the first build of the modules a merge adds to a repository the mesh builds modules from, -// when the merge is heard, not when its batch is cut (novox/hq issue 1617): a batch waits out its window and the walk -// before it, and a module added by a merge delivered meanwhile could not be assigned. Outside the batch and its plan. -func askNewModulesAtHearing(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved, entries []inventory.Entry, - read map[string][]inventory.ReadRepository, from []inventory.Entry) { - if len(from) == 0 { - return - } - _, added, _ := touchedBy(from, entries, m, read) - if asked := askNewModules(ctx, inv, m, from, added); len(asked) > 0 { - fmt.Printf("%s/%s merged into %s (%.8s) adds %s: the first build is asked now, before its batch is cut\n", - m.Owner, m.Repo, m.Base, m.Commit, strings.Join(asked, ", ")) - } -} - -// askBuilds asks the first build of each directory of a merge's repository, as askNewModules does, from the source given. -func askBuilds(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved, source buildSource, - added []string) []string { - var asked []string - for _, dir := range added { - path := dir - if path == "." { - path = "" - } - id, err := askABuild(ctx, source, path, m.Base) - // Kept, asked or not, so `assign` can tell a build in flight, or a merge that could not ask for one, - // from a module nobody ever heard of (novox/hq issue 325). - request := inventory.BuildRequest{ID: id, Repository: source.Repository, Seat: source.Seat, Path: path, - Ref: m.Base, Commit: m.Commit, For: "merge"} - if err != nil { - request.ID = notAskedID(m.Commit, dir) - request.NotAsked = err.Error() - recordBuildRequest(ctx, inv, request) - fmt.Printf(" %s is a new module in %s/%s and could not be built: %v — a hand `build` of it "+ - "asks again\n", dir, m.Owner, m.Repo, err) - continue - } - recordBuildRequest(ctx, inv, request) - fmt.Printf(" %s is a new module in %s/%s: build %s asked at %s; registered when it lands, assigned "+ - "nowhere\n", dir, m.Owner, m.Repo, id, m.Base) - asked = append(asked, dir) - } - return asked -} - // notAskedID is the id a merge's build that could not be asked is kept under: a key the bus takes, so a module at the // repository's root (".") is kept as one in a folder is. func notAskedID(commit, dir string) string { diff --git a/internal/builderkeys/builderkeys.go b/internal/builderkeys/builderkeys.go index b931defd..036ba35b 100644 --- a/internal/builderkeys/builderkeys.go +++ b/internal/builderkeys/builderkeys.go @@ -158,6 +158,39 @@ func (r Reader) ModuleAt(ctx context.Context, module, commit string) (delivery.M return pickAt(module, commit, values) } +// Modules is every module the indexer holds a trunk value of, by module: the value it holds now. A value that does not +// decode is left out and said in unread. +func (r Reader) Modules(ctx context.Context) (values map[string]delivery.Module, unread map[string]error, err error) { + kv, err := r.bucket(ctx, delivery.ModulesBucket) + if err != nil || kv == nil { + return nil, nil, err + } + keys, err := kv.Keys(ctx) + if errors.Is(err, jetstream.ErrNoKeysFound) { + return nil, nil, nil + } + if err != nil { + return nil, nil, fmt.Errorf("listing the indexer's modules: %w", err) + } + values, unread = map[string]delivery.Module{}, map[string]error{} + for _, key := range keys { + e, err := kv.Get(ctx, key) + if errors.Is(err, jetstream.ErrKeyNotFound) { + continue + } + if err != nil { + return nil, nil, fmt.Errorf("the indexer's value of %s: %w", key, err) + } + m, err := delivery.DecodeModule(e.Value()) + if err != nil { + unread[key] = err + continue + } + values[key] = m + } + return values, unread, nil +} + // pickAt is, of a key's values oldest first, the newest whose commit is commit (any, with commit empty); values that // do not decode are passed over, and one of them is said when nothing else is found. func pickAt(module, commit string, values [][]byte) (delivery.Module, error) { diff --git a/internal/builderkeys/builderkeys_integration_test.go b/internal/builderkeys/builderkeys_integration_test.go index 80352ad1..57cfdadf 100644 --- a/internal/builderkeys/builderkeys_integration_test.go +++ b/internal/builderkeys/builderkeys_integration_test.go @@ -124,6 +124,11 @@ func TestTheControllerReadsTheBuildersKeysAndTheIndexersValues(t *testing.T) { if _, err := r.ModuleAt(ctx, "x", "9999999999"); !errors.Is(err, ErrNoValue) { t.Fatalf("a commit the indexer never wrote: %v", err) } + put(t, modules, "broken", []byte("{not json")) + all, unread, err := r.Modules(ctx) + if err != nil || len(all) != 1 || all["x"].Commit != "3333333333" || unread["broken"] == nil { + t.Fatalf("every module's value now: %+v %v %v", all, unread, err) + } } // The controller's account reaches the builder's and the indexer's buckets: it is granted the JetStream API whole.