From fae7710bed6c5e3f4720c781fc6654f586ac9517 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 00:47:15 +0200 Subject: [PATCH 1/4] Ask build-agent for every module until a running builder says it writes references (issue 1886) R4e's own delivery plan waited for ever in tier 1: the controller took every module but build-agent's own from a referenced builder key, and only R4e's builder and build modules, delivered by that same plan, write references. The builder now says so in its bucket of instances; until it does, a walk asks build-agent as before R4e, and an open walk waiting on a key the running builder cannot reference is judged again at the next look. --- .../builder_says_default_test.go | 9 + cmd/mesh-controller/builder_says_test.go | 166 ++++++++++++++++++ cmd/mesh-controller/new_modules.go | 37 +++- cmd/mesh-controller/plan_keys.go | 37 ++++ cmd/mesh-controller/plan_keys_test.go | 4 + cmd/mesh-controller/release_plan.go | 20 ++- internal/builderkeys/builderkeys.go | 97 ++++++++++ internal/builderkeys/says_integration_test.go | 54 ++++++ internal/builderkeys/says_test.go | 24 +++ 9 files changed, 444 insertions(+), 4 deletions(-) create mode 100644 cmd/mesh-controller/builder_says_default_test.go create mode 100644 cmd/mesh-controller/builder_says_test.go create mode 100644 internal/builderkeys/says_integration_test.go create mode 100644 internal/builderkeys/says_test.go diff --git a/cmd/mesh-controller/builder_says_default_test.go b/cmd/mesh-controller/builder_says_default_test.go new file mode 100644 index 00000000..744970e7 --- /dev/null +++ b/cmd/mesh-controller/builder_says_default_test.go @@ -0,0 +1,9 @@ +package main + +import "context" + +// A test's builder says it writes references, as R4e's does, unless the test says otherwise (issue 1886): no test reads +// the bus for it. +func init() { + builderSaysReferences = func(context.Context) (bool, error) { return true, nil } +} diff --git a/cmd/mesh-controller/builder_says_test.go b/cmd/mesh-controller/builder_says_test.go new file mode 100644 index 00000000..39644f16 --- /dev/null +++ b/cmd/mesh-controller/builder_says_test.go @@ -0,0 +1,166 @@ +package main + +import ( + "context" + "errors" + "strings" + "testing" + + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" + + "github.com/novox/mesh-controller/internal/builderkeys" + "github.com/novox/mesh-controller/internal/inventory" +) + +// theBuilderSays sets what the running builder says for one test. +func theBuilderSays(t *testing.T, says bool, err error) { + t.Helper() + was := builderSaysReferences + builderSaysReferences = func(context.Context) (bool, error) { return says, err } + t.Cleanup(func() { builderSaysReferences = was }) +} + +// askingBuildAgent records the modules build-agent is asked for, by path. +func askingBuildAgent(t *testing.T) *[]string { + t.Helper() + var asked []string + was := askABuild + askABuild = func(_ context.Context, _ buildSource, path, _ string) (string, error) { + asked = append(asked, path) + return "build-9", nil + } + t.Cleanup(func() { askABuild = was }) + return &asked +} + +// 1886/AC2: while no running builder says it writes references, a walk asks build-agent for a module of its tier, as +// before R4e, and takes nothing from the builder's key. +func TestAWalkAsksBuildAgentWhileTheBuilderDoesNotSayItWritesReferences(t *testing.T) { + theBuilderSays(t, false, nil) + asked := askingBuildAgent(t) + p, byName := aWalkOfOne("mesh-build-go") + askModule(t.Context(), p, "mesh-build-go", byName) + s := p.Modules["mesh-build-go"] + if s.Key || s.Build != "build-9" || s.State != "asked" || len(*asked) != 1 { + t.Fatalf("the module is not asked of build-agent: %+v, asked %v", s, *asked) + } +} + +// 1886/AC3: once a running builder says it writes references, a walk takes the module from the builder's key (ADR +// 0498 §8) and asks build-agent for nothing. +func TestAWalkTakesTheKeyOnceTheBuilderSaysItWritesReferences(t *testing.T) { + theBuilderSays(t, true, nil) + asked := askingBuildAgent(t) + p, byName := aWalkOfOne("mesh-build-go") + askModule(t.Context(), p, "mesh-build-go", byName) + if s := p.Modules["mesh-build-go"]; !s.Key || s.Build != "" || len(*asked) != 0 { + t.Fatalf("the module is not taken from its key: %+v, asked %v", s, *asked) + } +} + +// 1886/AC2: a say that cannot be read leaves the module unasked and says why, so the next look asks it: neither path is +// guessed. +func TestASayNotReadLeavesTheModuleUnasked(t *testing.T) { + theBuilderSays(t, false, errors.New("the bus did not answer")) + asked := askingBuildAgent(t) + p, byName := aWalkOfOne("mesh-build-go") + askModule(t.Context(), p, "mesh-build-go", byName) + s := p.Modules["mesh-build-go"] + if s.State != "" || s.Key || len(*asked) != 0 || !strings.Contains(s.Why, "the bus did not answer") { + t.Fatalf("a say not read: %+v, asked %v", s, *asked) + } + if got := unaskedOf(*p); len(got) != 1 { + t.Fatalf("the module is not left for the next look to ask: %v", got) + } +} + +// 1886/AC2: the one function every ask goes through lets build-agent build any module while the builder does not say +// it writes references, and refuses it again once it does. +func TestBuildAgentIsAskedForAnyModuleWhileTheBuilderDoesNotSay(t *testing.T) { + was := askedModuleOf + t.Cleanup(func() { askedModuleOf = was }) + askedModuleOf = func(_ context.Context, _ buildSource, path string) (string, error) { + return strings.TrimPrefix(path, "modules/"), nil + } + theBuilderSays(t, false, nil) + if err := onlyTheBuildersOwn(t.Context(), buildSource{Repository: "novox/mesh-catalog", Seat: "git"}, "modules/sudo"); err != nil { + t.Fatalf("an ask while the builder does not say is refused: %v", err) + } + theBuilderSays(t, true, nil) + if err := onlyTheBuildersOwn(t.Context(), buildSource{Repository: "novox/mesh-catalog", Seat: "git"}, "modules/sudo"); !errors.Is(err, errNotTheBuildersOwn) { + t.Fatalf("an ask once the builder says is not refused: %v", err) + } +} + +// 1886/AC4: an open walk that waits on a key the running builder cannot reference (done without a reference, as R4e's +// own walk did in plan-1792707174627079192) is judged again at the next look: the module is handed back unasked, and +// that look's ask goes to build-agent. With the builder saying it writes references, the same walk keeps waiting. +func TestAnOpenWalkWaitingOnAnUnreferencedKeyIsAskedOfBuildAgent(t *testing.T) { + t.Cleanup(func() { + keyTakes.Lock() + keyTakes.running, keyTakes.waits, keyTakes.taken = map[string]bool{}, map[string]string{}, map[string]bool{} + keyTakes.Unlock() + }) + fp := "src2:" + strings.Repeat("e", 64) + unreferenced := builderkeys.Key{Module: "mesh-build-go", Fingerprint: fp, Found: true, Revision: 1, + Build: sdkdelivery.BuilderBuild{State: sdkdelivery.BuildDone, Manifest: strings.Repeat("b", 64), + Artifacts: []sdkdelivery.BuiltArtifact{{Name: "tools", Kind: "bundle", Digest: "sha256:1"}}}} + walk := func() (*inventory.Plan, map[string]inventory.Entry, planKeysOf) { + theBuilderSays(t, true, nil) + p, byName := aWalkOfOne("mesh-build-go") + askModule(t.Context(), p, "mesh-build-go", byName) + keys := planKeysOf{values: map[string]sdkdelivery.Module{"mesh-build-go@c0ffee0123456789": {FP2: fp}}, + keys: map[string]builderkeys.Key{"mesh-build-go": unreferenced}} + return p, byName, keys + } + take := func(string, sdkdelivery.Module, string) { t.Fatal("an unreferenced key was taken in") } + records := recordsOf{builds: map[string]inventory.Build{}} + + // Said: it waits for the reference, as ADR 0498 §8 has it. + p, _, keys := walk() + if _, err := settleKeys(t.Context(), records, keys, p, p.Tiers[0], take); err != nil { + t.Fatal(err) + } + if s := p.Modules["mesh-build-go"]; s.State != "asked" || !s.Key || !strings.Contains(s.Why, "waits for the reference") { + t.Fatalf("with the builder saying, the walk does not wait for the reference: %+v", s) + } + + // Not said: handed back, and the next look asks build-agent. + p, byName, keys := walk() + keys.notSaid = true + theBuilderSays(t, false, nil) + asked := askingBuildAgent(t) + changed, err := settleKeys(t.Context(), records, keys, p, p.Tiers[0], take) + if err != nil || !changed { + t.Fatalf("the walk was not judged again: %v %v", changed, err) + } + s := p.Modules["mesh-build-go"] + if s.Key || s.State != "" || p.State != inventory.PlanBuilding || !strings.Contains(s.Why, "does not say") { + t.Fatalf("the module is not handed back to be asked of build-agent: %+v %s", s, p.State) + } + for _, m := range unaskedOf(*p) { + askModule(t.Context(), p, m, byName) + } + if s := p.Modules["mesh-build-go"]; s.Key || s.Build != "build-9" || len(*asked) != 1 { + t.Fatalf("the next look did not ask build-agent: %+v, asked %v", s, *asked) + } +} + +// 1886/AC4: a key the running builder failed while it does not say it writes references fails no walk: the module is +// asked of build-agent, whose build is the one delivered. +func TestAFailedKeyOfABuilderThatDoesNotSayFailsNoWalk(t *testing.T) { + theBuilderSays(t, true, nil) + p, byName := aWalkOfOne("mesh-build-go") + askModule(t.Context(), p, "mesh-build-go", byName) + _ = byName + fp := "src2:" + strings.Repeat("f", 64) + keys := planKeysOf{notSaid: true, values: map[string]sdkdelivery.Module{"mesh-build-go@c0ffee0123456789": {FP2: fp}}, + keys: map[string]builderkeys.Key{"mesh-build-go": {Module: "mesh-build-go", Fingerprint: fp, Found: true, + Build: sdkdelivery.BuilderBuild{State: sdkdelivery.BuildFailed, Error: "not stored: tools: no reference"}}}} + if _, err := settleKeys(t.Context(), recordsOf{}, keys, p, p.Tiers[0], func(string, sdkdelivery.Module, string) {}); err != nil { + t.Fatal(err) + } + if s := p.Modules["mesh-build-go"]; p.State != inventory.PlanBuilding || s.State != "" || s.Key { + t.Fatalf("a failed key of a builder that does not say failed the walk: %s %q %+v", p.State, p.Note, s) + } +} diff --git a/cmd/mesh-controller/new_modules.go b/cmd/mesh-controller/new_modules.go index aa870fe7..3fe4b5b6 100644 --- a/cmd/mesh-controller/new_modules.go +++ b/cmd/mesh-controller/new_modules.go @@ -11,6 +11,7 @@ import ( "time" sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" + "github.com/nats-io/nats.go/jetstream" "github.com/novox/mesh-controller/internal/builderkeys" "github.com/novox/mesh-controller/internal/conditions" "github.com/novox/mesh-controller/internal/inventory" @@ -209,7 +210,12 @@ func watchNewModules(ctx context.Context, open *stores) { 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 { + says := watchingSays{js: keys.JS} + says.watch(ctx) + if err := builderkeys.Watch(ctx, keys.JS, func(module string) { + says.watch(ctx) + wakeNewModule(ctx, open, module) + }); err != nil { closeKeys() fmt.Printf("new modules: the builder's keys are not watched: %v\n", err) return @@ -272,3 +278,32 @@ func newModulesOf(ctx context.Context, repository string) ([]string, error) { sort.Strings(out) return out, nil } + +// watchingSays watches the builder's bucket of instances once it is there (issue 1886): a change of what the builder says +// asks the look, which judges every open walk again. A builder from before R4e makes no such bucket; a write of the +// builder's keys asks again whether it is there now, so nothing waits on a timer. +type watchingSays struct { + js jetstream.JetStream + mu *sync.Mutex + on *bool +} + +func (w *watchingSays) watch(ctx context.Context) { + if w.mu == nil { + w.mu, w.on = &sync.Mutex{}, new(bool) + } + w.mu.Lock() + defer w.mu.Unlock() + if *w.on { + return + } + on, err := builderkeys.WatchSays(ctx, w.js, askToLook) + if err != nil { + fmt.Printf("plans: what the builder says it writes is not watched now: %v\n", err) + return + } + if on { + *w.on = true + askToLook() // what it says now may differ from what the last look read + } +} diff --git a/cmd/mesh-controller/plan_keys.go b/cmd/mesh-controller/plan_keys.go index 453bcbc2..5380e954 100644 --- a/cmd/mesh-controller/plan_keys.go +++ b/cmd/mesh-controller/plan_keys.go @@ -42,6 +42,20 @@ var buildersOwn = func(module string) bool { return buildAgentsOwn[module] } type planKeyReads interface { ModuleAt(ctx context.Context, module, commit string) (sdkdelivery.Module, error) Build(ctx context.Context, module, fp string) (builderkeys.Key, error) + // SaysReferences is whether a running builder says it writes references (builderkeys.Reader.SaysReferences). + SaysReferences(ctx context.Context) (bool, error) +} + +// builderSaysReferences is whether a running builder instance says, in its own bucket of instances, that the keys it +// writes name references (issue 1886). Until one does, a walk asks build-agent for every module, as before R4e: the +// keys the running builder writes could never be delivered from. A variable so a test needs no bus. +var builderSaysReferences = func(ctx context.Context) (bool, error) { + keys, closeKeys, err := openBuilderKeys() + if err != nil { + return false, err + } + defer closeKeys() + return keys.SaysReferences(ctx) } // openPlanKeys opens what a walk reads of the indexer and the builder. A variable so a test needs no bus. @@ -102,6 +116,18 @@ func settleKeys(ctx context.Context, inv keyRecords, keys planKeyReads, p *inven changed = true } } + says, err := keys.SaysReferences(ctx) + if err != nil { + return changed, fmt.Errorf("whether the builder writes references cannot be read: %w", err) + } + // **A walk never waits on a key the running builder cannot reference** (issue 1886): while no builder says it writes + // references, a module that waits on its key, or whose key failed, is handed back unasked, and the look asks it of + // build-agent. A done key with every reference is still taken. + toAgent := func(s *inventory.PlanModule, why string) { + s.State, s.Key, s.AskedAt, s.KeyAt, s.Build, s.Fingerprint = "", false, nil, "", "", "" + s.Why = "the builder does not say it writes references, so build-agent is asked (" + why + ")" + changed = true + } for _, m := range tier { s := p.Modules[m] if s == nil || !s.Key || s.State != "asked" { @@ -109,6 +135,10 @@ func settleKeys(ctx context.Context, inv keyRecords, keys planKeyReads, p *inven } trunk, err := keys.ModuleAt(ctx, m, s.KeyAt) if errors.Is(err, builderkeys.ErrNoValue) { + if !says { + toAgent(s, "the indexer holds no value of it") + continue + } set(s, fmt.Sprintf("waits for the indexer's value of %s at %s", m, orNow(s.KeyAt))) continue } @@ -120,6 +150,10 @@ func settleKeys(ctx context.Context, inv keyRecords, keys planKeyReads, p *inven if why == "" { why = "the indexer wrote no fingerprint of the new form" } + if !says { + toAgent(s, why) + continue + } set(s, fmt.Sprintf("waits: %s at %s has no fingerprint to take a build by: %s", m, orNow(s.KeyAt), why)) continue } @@ -131,6 +165,9 @@ func settleKeys(ctx context.Context, inv keyRecords, keys planKeyReads, p *inven return changed, err } switch { + case !says && (k.Failed() || !k.Done() || len(k.Unreferenced()) > 0): + toAgent(s, k.Says()) + continue case k.Failed(): // **A failed key fails the walk in the builder's own words** (`digests differ:`, `not stored:`, `not pinned:`, // `version taken:`), and a rebuild of it is a new revision the next walk takes. diff --git a/cmd/mesh-controller/plan_keys_test.go b/cmd/mesh-controller/plan_keys_test.go index 29190f32..1a04a48b 100644 --- a/cmd/mesh-controller/plan_keys_test.go +++ b/cmd/mesh-controller/plan_keys_test.go @@ -16,8 +16,12 @@ import ( type planKeysOf struct { values map[string]sdkdelivery.Module // by module + "@" + commit keys map[string]builderkeys.Key + // notSaid is a builder that does not say it writes references (issue 1886); the zero value says it. + notSaid bool } +func (k planKeysOf) SaysReferences(context.Context) (bool, error) { return !k.notSaid, nil } + func (k planKeysOf) ModuleAt(_ context.Context, module, commit string) (sdkdelivery.Module, error) { if v, ok := k.values[module+"@"+commit]; ok { return v, nil diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 1238d769..83024e40 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -457,6 +457,10 @@ func onlyTheBuildersOwn(ctx context.Context, source buildSource, path string) er if isBuildersOwn(module) { return nil } + // **While no running builder says it writes references, build-agent builds every module** (issue 1886). + if says, err := builderSaysReferences(ctx); err == nil && !says { + return nil + } if module == "" { module = fmt.Sprintf("%s at %q", source, orRoot(path)) } @@ -529,10 +533,20 @@ func askModule(ctx context.Context, p *inventory.Plan, name string, byName map[s } // **Every module but the builder's own is taken from the builder's done key, asking no build** (novox/hq ADR 0503 // §6, ADR 0501, ADR 0453 §5): the look settles it (settleFromKeys). + // **Only while a running builder says it writes references** (issue 1886): until then the keys it writes could never + // be delivered from, and the module is asked of build-agent as before R4e. A say not read leaves it unasked, said, + // for the next look. if !isBuildersOwn(name) { - sayBuilding(ctx, p.ID, now) - takeFromKey(p, state, e, now) - return + says, err := builderSaysReferences(ctx) + if err != nil { + state.Why = "waits: whether the builder writes references cannot be read: " + firstLine(err.Error()) + return + } + if says { + sayBuilding(ctx, p.ID, now) + takeFromKey(p, state, e, now) + return + } } source := buildSource{Repository: e.Source.Repository, Seat: e.Source.Seat} fmt.Printf(" tier %d: ", p.Tier) diff --git a/internal/builderkeys/builderkeys.go b/internal/builderkeys/builderkeys.go index 036ba35b..6fddd35e 100644 --- a/internal/builderkeys/builderkeys.go +++ b/internal/builderkeys/builderkeys.go @@ -9,6 +9,7 @@ package builderkeys import ( "context" + "encoding/json" "errors" "fmt" "strings" @@ -309,3 +310,99 @@ func firstLine(s string) string { line, _, _ := strings.Cut(strings.TrimSpace(s), "\n") return line } + +// The builder's say of what its keys hold (novox/hq issue 1886): each running builder instance keeps a key, named by its +// node, in the builder's own announced bucket of instances, saying what the keys it writes hold. An instance that writes +// references (ADR 0498 §1) says WritesReferences there; a builder from before R4e keeps no such key, and has no bucket. +// The bucket is the builder's and the shape is read by the controller alone, so neither is in mesh-sdk. +const ( + // BuilderInstancesBucket is the builder's bucket of its running instances, `_`. + BuilderInstancesBucket = delivery.BuilderModule + "_instances" + // WritesReferences is the word an instance says when every done key it writes names where its artifacts are. + WritesReferences = "references" +) + +// instanceSays is an instance's value in BuilderInstancesBucket: what the keys it writes hold. +type instanceSays struct { + Writes []string `json:"writes"` +} + +// saysReferences says whether any instance's value says it writes references. A value that does not decode says +// nothing. +func saysReferences(values [][]byte) bool { + for _, v := range values { + var says instanceSays + if json.Unmarshal(v, &says) != nil { + continue + } + for _, w := range says.Writes { + if w == WritesReferences { + return true + } + } + } + return false +} + +// SaysReferences says whether a running builder instance says, in its bucket of instances, that it writes references. No +// bucket (the builder from before R4e, which never made it) and no key say no; a bucket that cannot be read is an error. +func (r Reader) SaysReferences(ctx context.Context) (bool, error) { + kv, err := r.bucket(ctx, BuilderInstancesBucket) + if err != nil || kv == nil { + return false, err + } + keys, err := kv.Keys(ctx) + if errors.Is(err, jetstream.ErrNoKeysFound) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("listing the builder's instances: %w", err) + } + var values [][]byte + for _, key := range keys { + e, err := kv.Get(ctx, key) + if errors.Is(err, jetstream.ErrKeyNotFound) { + continue + } + if err != nil { + return false, fmt.Errorf("the builder's instance %s: %w", key, err) + } + values = append(values, e.Value()) + } + return saysReferences(values), nil +} + +// WatchSays calls wake for every change of the builder's bucket of instances from now on, until ctx ends: what makes the +// look judge an open walk again when the builder starts or stops saying it writes references (issue 1886), with no +// timer. It says false, and watches nothing, while the bucket is not there; the caller asks again when the builder +// writes a key, which a builder that makes the bucket does. +func WatchSays(ctx context.Context, js jetstream.JetStream, wake func()) (bool, error) { + kv, err := js.KeyValue(ctx, BuilderInstancesBucket) + if errors.Is(err, jetstream.ErrBucketNotFound) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("the bucket %s: %w", BuilderInstancesBucket, err) + } + w, err := kv.WatchAll(ctx, jetstream.UpdatesOnly()) + if err != nil { + return false, fmt.Errorf("watching %s: %w", BuilderInstancesBucket, err) + } + go func() { + defer w.Stop() + for { + select { + case <-ctx.Done(): + return + case e, ok := <-w.Updates(): + if !ok { + return + } + if e != nil { + wake() // a put and a delete both change what the builder says + } + } + } + }() + return true, nil +} diff --git a/internal/builderkeys/says_integration_test.go b/internal/builderkeys/says_integration_test.go new file mode 100644 index 00000000..205faf9b --- /dev/null +++ b/internal/builderkeys/says_integration_test.go @@ -0,0 +1,54 @@ +//go:build integration + +package builderkeys + +import ( + "context" + "testing" + "time" + + "github.com/nats-io/nats.go/jetstream" +) + +// 1886/AC1 and AC4 on a real bus: no bucket of instances says no and watches nothing; an instance's key saying +// references says yes, and its put and its delete each wake the watcher, with no timer. +func TestTheBuildersSayIsReadAndWatchedOnTheBus(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + js := aBus(t) + r := Reader{JS: js} + if says, err := r.SaysReferences(ctx); err != nil || says { + t.Fatalf("no bucket: %v %v", says, err) + } + if on, err := WatchSays(ctx, js, func() {}); err != nil || on { + t.Fatalf("no bucket is watched: %v %v", on, err) + } + kv, err := js.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: BuilderInstancesBucket, History: 1}) + if err != nil { + t.Fatal(err) + } + woken := make(chan struct{}, 4) + if on, err := WatchSays(ctx, js, func() { woken <- struct{}{} }); err != nil || !on { + t.Fatalf("the bucket is not watched: %v %v", on, err) + } + wake := func(what string) { + t.Helper() + select { + case <-woken: + case <-ctx.Done(): + t.Fatalf("%s woke nothing", what) + } + } + put(t, kv, "shanks", []byte(`{"writes":["references"]}`)) + wake("a put") + if says, err := r.SaysReferences(ctx); err != nil || !says { + t.Fatalf("an instance saying references: %v %v", says, err) + } + if err := kv.Delete(ctx, "shanks"); err != nil { + t.Fatal(err) + } + wake("a delete") + if says, err := r.SaysReferences(ctx); err != nil || says { + t.Fatalf("no instance left: %v %v", says, err) + } +} diff --git a/internal/builderkeys/says_test.go b/internal/builderkeys/says_test.go new file mode 100644 index 00000000..45218598 --- /dev/null +++ b/internal/builderkeys/says_test.go @@ -0,0 +1,24 @@ +package builderkeys + +import "testing" + +// 1886/AC1: a builder that keeps no say, or a say without the word, is a builder that does not write references; one +// instance that says it is enough, since it builds every key done without a reference again (ADR 0498 §8). +func TestTheBuilderSaysReferencesOnlyWhenAnInstanceSaysTheWord(t *testing.T) { + cases := []struct { + name string + values [][]byte + want bool + }{ + {"no instance", nil, false}, + {"an instance saying nothing", [][]byte{[]byte(`{"writes":[]}`)}, false}, + {"a value not read", [][]byte{[]byte(`{not json`)}, false}, + {"another word", [][]byte{[]byte(`{"writes":["digests"]}`)}, false}, + {"one instance says it", [][]byte{[]byte(`{"writes":["digests"]}`), []byte(`{"v":1,"writes":["references"]}`)}, true}, + } + for _, c := range cases { + if got := saysReferences(c.values); got != c.want { + t.Errorf("%s: says references %v, want %v", c.name, got, c.want) + } + } +} From f67342f0d86e2868524e5ee585a1c18eb97ac203 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 00:53:25 +0200 Subject: [PATCH 2/4] =?UTF-8?q?Ask=20build-agent=20for=20the=20build=20mod?= =?UTF-8?q?ules=20that=20write=20references,=20whatever=20the=20builder=20?= =?UTF-8?q?says=20(issue=201886,=20ADR=200522=20=C2=A76)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit They push and answer each reference, so a key they write themselves can never carry their own delivery; until R4g build-agent delivers them as it does the builder. --- cmd/mesh-controller/builder_says_test.go | 61 +++++++++++++++--------- cmd/mesh-controller/plan_keys.go | 7 ++- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/cmd/mesh-controller/builder_says_test.go b/cmd/mesh-controller/builder_says_test.go index 39644f16..1fc5c5ab 100644 --- a/cmd/mesh-controller/builder_says_test.go +++ b/cmd/mesh-controller/builder_says_test.go @@ -38,9 +38,9 @@ func askingBuildAgent(t *testing.T) *[]string { func TestAWalkAsksBuildAgentWhileTheBuilderDoesNotSayItWritesReferences(t *testing.T) { theBuilderSays(t, false, nil) asked := askingBuildAgent(t) - p, byName := aWalkOfOne("mesh-build-go") - askModule(t.Context(), p, "mesh-build-go", byName) - s := p.Modules["mesh-build-go"] + p, byName := aWalkOfOne("sudo") + askModule(t.Context(), p, "sudo", byName) + s := p.Modules["sudo"] if s.Key || s.Build != "build-9" || s.State != "asked" || len(*asked) != 1 { t.Fatalf("the module is not asked of build-agent: %+v, asked %v", s, *asked) } @@ -51,9 +51,9 @@ func TestAWalkAsksBuildAgentWhileTheBuilderDoesNotSayItWritesReferences(t *testi func TestAWalkTakesTheKeyOnceTheBuilderSaysItWritesReferences(t *testing.T) { theBuilderSays(t, true, nil) asked := askingBuildAgent(t) - p, byName := aWalkOfOne("mesh-build-go") - askModule(t.Context(), p, "mesh-build-go", byName) - if s := p.Modules["mesh-build-go"]; !s.Key || s.Build != "" || len(*asked) != 0 { + p, byName := aWalkOfOne("sudo") + askModule(t.Context(), p, "sudo", byName) + if s := p.Modules["sudo"]; !s.Key || s.Build != "" || len(*asked) != 0 { t.Fatalf("the module is not taken from its key: %+v, asked %v", s, *asked) } } @@ -63,9 +63,9 @@ func TestAWalkTakesTheKeyOnceTheBuilderSaysItWritesReferences(t *testing.T) { func TestASayNotReadLeavesTheModuleUnasked(t *testing.T) { theBuilderSays(t, false, errors.New("the bus did not answer")) asked := askingBuildAgent(t) - p, byName := aWalkOfOne("mesh-build-go") - askModule(t.Context(), p, "mesh-build-go", byName) - s := p.Modules["mesh-build-go"] + p, byName := aWalkOfOne("sudo") + askModule(t.Context(), p, "sudo", byName) + s := p.Modules["sudo"] if s.State != "" || s.Key || len(*asked) != 0 || !strings.Contains(s.Why, "the bus did not answer") { t.Fatalf("a say not read: %+v, asked %v", s, *asked) } @@ -102,15 +102,15 @@ func TestAnOpenWalkWaitingOnAnUnreferencedKeyIsAskedOfBuildAgent(t *testing.T) { keyTakes.Unlock() }) fp := "src2:" + strings.Repeat("e", 64) - unreferenced := builderkeys.Key{Module: "mesh-build-go", Fingerprint: fp, Found: true, Revision: 1, + unreferenced := builderkeys.Key{Module: "sudo", Fingerprint: fp, Found: true, Revision: 1, Build: sdkdelivery.BuilderBuild{State: sdkdelivery.BuildDone, Manifest: strings.Repeat("b", 64), Artifacts: []sdkdelivery.BuiltArtifact{{Name: "tools", Kind: "bundle", Digest: "sha256:1"}}}} walk := func() (*inventory.Plan, map[string]inventory.Entry, planKeysOf) { theBuilderSays(t, true, nil) - p, byName := aWalkOfOne("mesh-build-go") - askModule(t.Context(), p, "mesh-build-go", byName) - keys := planKeysOf{values: map[string]sdkdelivery.Module{"mesh-build-go@c0ffee0123456789": {FP2: fp}}, - keys: map[string]builderkeys.Key{"mesh-build-go": unreferenced}} + p, byName := aWalkOfOne("sudo") + askModule(t.Context(), p, "sudo", byName) + keys := planKeysOf{values: map[string]sdkdelivery.Module{"sudo@c0ffee0123456789": {FP2: fp}}, + keys: map[string]builderkeys.Key{"sudo": unreferenced}} return p, byName, keys } take := func(string, sdkdelivery.Module, string) { t.Fatal("an unreferenced key was taken in") } @@ -121,7 +121,7 @@ func TestAnOpenWalkWaitingOnAnUnreferencedKeyIsAskedOfBuildAgent(t *testing.T) { if _, err := settleKeys(t.Context(), records, keys, p, p.Tiers[0], take); err != nil { t.Fatal(err) } - if s := p.Modules["mesh-build-go"]; s.State != "asked" || !s.Key || !strings.Contains(s.Why, "waits for the reference") { + if s := p.Modules["sudo"]; s.State != "asked" || !s.Key || !strings.Contains(s.Why, "waits for the reference") { t.Fatalf("with the builder saying, the walk does not wait for the reference: %+v", s) } @@ -134,14 +134,14 @@ func TestAnOpenWalkWaitingOnAnUnreferencedKeyIsAskedOfBuildAgent(t *testing.T) { if err != nil || !changed { t.Fatalf("the walk was not judged again: %v %v", changed, err) } - s := p.Modules["mesh-build-go"] + s := p.Modules["sudo"] if s.Key || s.State != "" || p.State != inventory.PlanBuilding || !strings.Contains(s.Why, "does not say") { t.Fatalf("the module is not handed back to be asked of build-agent: %+v %s", s, p.State) } for _, m := range unaskedOf(*p) { askModule(t.Context(), p, m, byName) } - if s := p.Modules["mesh-build-go"]; s.Key || s.Build != "build-9" || len(*asked) != 1 { + if s := p.Modules["sudo"]; s.Key || s.Build != "build-9" || len(*asked) != 1 { t.Fatalf("the next look did not ask build-agent: %+v, asked %v", s, *asked) } } @@ -150,17 +150,34 @@ func TestAnOpenWalkWaitingOnAnUnreferencedKeyIsAskedOfBuildAgent(t *testing.T) { // asked of build-agent, whose build is the one delivered. func TestAFailedKeyOfABuilderThatDoesNotSayFailsNoWalk(t *testing.T) { theBuilderSays(t, true, nil) - p, byName := aWalkOfOne("mesh-build-go") - askModule(t.Context(), p, "mesh-build-go", byName) + p, byName := aWalkOfOne("sudo") + askModule(t.Context(), p, "sudo", byName) _ = byName fp := "src2:" + strings.Repeat("f", 64) - keys := planKeysOf{notSaid: true, values: map[string]sdkdelivery.Module{"mesh-build-go@c0ffee0123456789": {FP2: fp}}, - keys: map[string]builderkeys.Key{"mesh-build-go": {Module: "mesh-build-go", Fingerprint: fp, Found: true, + keys := planKeysOf{notSaid: true, values: map[string]sdkdelivery.Module{"sudo@c0ffee0123456789": {FP2: fp}}, + keys: map[string]builderkeys.Key{"sudo": {Module: "sudo", Fingerprint: fp, Found: true, Build: sdkdelivery.BuilderBuild{State: sdkdelivery.BuildFailed, Error: "not stored: tools: no reference"}}}} if _, err := settleKeys(t.Context(), recordsOf{}, keys, p, p.Tiers[0], func(string, sdkdelivery.Module, string) {}); err != nil { t.Fatal(err) } - if s := p.Modules["mesh-build-go"]; p.State != inventory.PlanBuilding || s.State != "" || s.Key { + if s := p.Modules["sudo"]; p.State != inventory.PlanBuilding || s.State != "" || s.Key { t.Fatalf("a failed key of a builder that does not say failed the walk: %s %q %+v", p.State, p.Note, s) } } + +// 1886/AC6 (ADR 0522 §6): the parts that write references, the builder and the build modules, are build-agent's own until +// R4g: asked of build-agent and never taken from keys they write themselves, whatever the builder says. +func TestTheReferenceWritersAreAskedOfBuildAgentWhateverTheBuilderSays(t *testing.T) { + theBuilderSays(t, true, nil) + asked := askingBuildAgent(t) + for _, m := range []string{"mesh-build-go", "mesh-build-node", "mesh-build-image", "mesh-delivery-builder"} { + p, byName := aWalkOfOne(m) + askModule(t.Context(), p, m, byName) + if s := p.Modules[m]; s.Key || s.Build != "build-9" { + t.Errorf("%s is not asked of build-agent: %+v", m, s) + } + } + if len(*asked) != 4 { + t.Fatalf("build-agent was asked %v", *asked) + } +} diff --git a/cmd/mesh-controller/plan_keys.go b/cmd/mesh-controller/plan_keys.go index 5380e954..ade65305 100644 --- a/cmd/mesh-controller/plan_keys.go +++ b/cmd/mesh-controller/plan_keys.go @@ -32,7 +32,12 @@ func isBuildersOwn(module string) bool { return buildersOwn(module) } // repository's; and mesh-tools, the toolchain image build-agent's builds stand on. The builder writes no key of any of them, // and a walk waiting for one would hold its tier for ever. mesh-tools-go is not among them: it builds from a Dockerfile in // its own directory on a pinned base, and has its key from mesh-build-image. -var buildAgentsOwn = map[string]bool{sdkdelivery.BuilderModule: true, builder.AgentModule: true, "mesh-tools": true} +// +// **And the parts that write references** (ADR 0522 §6, issue 1886): the build modules push and answer each reference, +// and the builder writes it into the key, so none of them is ever taken from a key they write themselves. Until R4g +// build-agent is the way they are delivered whatever the builder says. +var buildAgentsOwn = map[string]bool{sdkdelivery.BuilderModule: true, builder.AgentModule: true, "mesh-tools": true, + "mesh-build-go": true, "mesh-build-node": true, "mesh-build-image": true} // buildersOwn is isBuildersOwn's rule: a variable only so a test of the asks left (their bus waits, their plan key) may // name modules of its own as build-agent's. From 1414ed7f7b81152aacf38ca9835d924c8fa35832 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 01:03:19 +0200 Subject: [PATCH 3/4] Judge an ask by the builder's say the walk read, never a second read (issue 1886, review round 1) A second read that failed, or that turned true in between, refused the ask and failed the whole plan. --- cmd/mesh-controller/builder_says_test.go | 40 ++++++++++++++++++++++++ cmd/mesh-controller/plan_keys.go | 14 +++++++++ cmd/mesh-controller/release_plan.go | 14 +++++++-- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/cmd/mesh-controller/builder_says_test.go b/cmd/mesh-controller/builder_says_test.go index 1fc5c5ab..cafe16cd 100644 --- a/cmd/mesh-controller/builder_says_test.go +++ b/cmd/mesh-controller/builder_says_test.go @@ -181,3 +181,43 @@ func TestTheReferenceWritersAreAskedOfBuildAgentWhateverTheBuilderSays(t *testin t.Fatalf("build-agent was asked %v", *asked) } } + +// 1886 review round 1 (ADR 0522 §2): the guard of the one ask function judges by the say askModule read, never a second +// read: a second read that fails, or that says otherwise, fails no plan. +func TestTheAskGuardUsesTheSayTheWalkRead(t *testing.T) { + was := askedModuleOf + t.Cleanup(func() { askedModuleOf = was }) + askedModuleOf = func(_ context.Context, _ buildSource, path string) (string, error) { + return strings.TrimPrefix(path, "modules/"), nil + } + reads := 0 + wasSays := builderSaysReferences + t.Cleanup(func() { builderSaysReferences = wasSays }) + for _, second := range []struct { + name string + says bool + err error + }{{"a second read that fails", false, errors.New("the bus did not answer")}, {"a say that turned true", true, nil}} { + reads = 0 + builderSaysReferences = func(context.Context) (bool, error) { + reads++ + if reads == 1 { + return false, nil + } + return second.says, second.err + } + wasAsk := askABuild + askABuild = func(ctx context.Context, source buildSource, path, _ string) (string, error) { + if err := onlyTheBuildersOwn(ctx, source, path); err != nil { + return "", err + } + return "build-9", nil + } + p, byName := aWalkOfOne("sudo") + askModule(t.Context(), p, "sudo", byName) + askABuild = wasAsk + if s := p.Modules["sudo"]; p.State == inventory.PlanFailed || s.Build != "build-9" || s.Key { + t.Errorf("%s: the walk %s, the module %+v", second.name, p.State, s) + } + } +} diff --git a/cmd/mesh-controller/plan_keys.go b/cmd/mesh-controller/plan_keys.go index ade65305..abe732bd 100644 --- a/cmd/mesh-controller/plan_keys.go +++ b/cmd/mesh-controller/plan_keys.go @@ -51,6 +51,20 @@ type planKeyReads interface { SaysReferences(ctx context.Context) (bool, error) } +// builderSayKey carries, on an ask's context, the builder's say the walk read before asking (withBuilderSay). +type builderSayKey struct{} + +// withBuilderSay hands the say a walk read to the ask's guard, so the guard never reads it a second time. +func withBuilderSay(ctx context.Context, says bool) context.Context { + return context.WithValue(ctx, builderSayKey{}, says) +} + +// builderSayOf is the say a walk handed the ask, and whether it handed one. +func builderSayOf(ctx context.Context) (says, handed bool) { + says, handed = ctx.Value(builderSayKey{}).(bool) + return says, handed +} + // builderSaysReferences is whether a running builder instance says, in its own bucket of instances, that the keys it // writes name references (issue 1886). Until one does, a walk asks build-agent for every module, as before R4e: the // keys the running builder writes could never be delivered from. A variable so a test needs no bus. diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 83024e40..fda995b1 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -457,8 +457,16 @@ func onlyTheBuildersOwn(ctx context.Context, source buildSource, path string) er if isBuildersOwn(module) { return nil } - // **While no running builder says it writes references, build-agent builds every module** (issue 1886). - if says, err := builderSaysReferences(ctx); err == nil && !says { + // **While no running builder says it writes references, build-agent builds every module** (issue 1886): judged by the + // say the walk read when it has one, read here only for an ask that carries none. + says, read := builderSayOf(ctx) + if !read { + var err error + if says, err = builderSaysReferences(ctx); err != nil { + says = true // not known: refused below, as before the say existed + } + } + if !says { return nil } if module == "" { @@ -547,6 +555,8 @@ func askModule(ctx context.Context, p *inventory.Plan, name string, byName map[s takeFromKey(p, state, e, now) return } + // The ask's guard judges by this say, never by a second read that could fail or differ (review of issue 1886). + ctx = withBuilderSay(ctx, says) } source := buildSource{Repository: e.Source.Repository, Seat: e.Source.Seat} fmt.Printf(" tier %d: ", p.Tier) From bbadbb2af0215284b9b87733858b87170d334de7 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 01:59:18 +0200 Subject: [PATCH 4/4] Judge the ask and the settle by one say of the builder (issue 1886, merge check hang) The settle read the say from the builder's bucket while the ask read it through builderSaysReferences; where they differed (a test's say against a bus with no bucket) a module was handed back and taken from its key again at every turn, and the look's loop never ended. --- cmd/mesh-controller/builder_says_test.go | 24 ++++++++++++++++++++++-- cmd/mesh-controller/plan_keys.go | 6 +++--- cmd/mesh-controller/plan_keys_test.go | 4 ---- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/cmd/mesh-controller/builder_says_test.go b/cmd/mesh-controller/builder_says_test.go index cafe16cd..c85bdc17 100644 --- a/cmd/mesh-controller/builder_says_test.go +++ b/cmd/mesh-controller/builder_says_test.go @@ -127,7 +127,6 @@ func TestAnOpenWalkWaitingOnAnUnreferencedKeyIsAskedOfBuildAgent(t *testing.T) { // Not said: handed back, and the next look asks build-agent. p, byName, keys := walk() - keys.notSaid = true theBuilderSays(t, false, nil) asked := askingBuildAgent(t) changed, err := settleKeys(t.Context(), records, keys, p, p.Tiers[0], take) @@ -154,7 +153,8 @@ func TestAFailedKeyOfABuilderThatDoesNotSayFailsNoWalk(t *testing.T) { askModule(t.Context(), p, "sudo", byName) _ = byName fp := "src2:" + strings.Repeat("f", 64) - keys := planKeysOf{notSaid: true, values: map[string]sdkdelivery.Module{"sudo@c0ffee0123456789": {FP2: fp}}, + theBuilderSays(t, false, nil) + keys := planKeysOf{values: map[string]sdkdelivery.Module{"sudo@c0ffee0123456789": {FP2: fp}}, keys: map[string]builderkeys.Key{"sudo": {Module: "sudo", Fingerprint: fp, Found: true, Build: sdkdelivery.BuilderBuild{State: sdkdelivery.BuildFailed, Error: "not stored: tools: no reference"}}}} if _, err := settleKeys(t.Context(), recordsOf{}, keys, p, p.Tiers[0], func(string, sdkdelivery.Module, string) {}); err != nil { @@ -221,3 +221,23 @@ func TestTheAskGuardUsesTheSayTheWalkRead(t *testing.T) { } } } + +// 1886 merge check hang: the look must judge an ask and a settle by one say. A settle that read the say from another +// source than the ask (the bus's bucket, against the walk's read) handed a module back, the next ask took it from its key +// again, and the look's loop never ended: walk_from_keys_integration_test hung. +func TestTheSettleJudgesByTheSameSayAsTheAsk(t *testing.T) { + theBuilderSays(t, true, nil) + p, _ := aWalkOfOne("sudo") + p.Modules["sudo"] = &inventory.PlanModule{} + askModule(t.Context(), p, "sudo", map[string]inventory.Entry{"sudo": {Source: inventory.Source{Repository: "novox/mesh-catalog", Path: "modules/sudo", Ref: "main"}}}) + fp := "src2:" + strings.Repeat("1", 64) + keys := planKeysOf{values: map[string]sdkdelivery.Module{"sudo@c0ffee0123456789": {FP2: fp}}, + keys: map[string]builderkeys.Key{"sudo": {Module: "sudo", Fingerprint: fp, Found: true, + Build: sdkdelivery.BuilderBuild{State: sdkdelivery.BuildBuilding, By: "shanks"}}}} + if _, err := settleKeys(t.Context(), recordsOf{}, keys, p, p.Tiers[0], func(string, sdkdelivery.Module, string) {}); err != nil { + t.Fatal(err) + } + if s := p.Modules["sudo"]; !s.Key || s.State != "asked" { + t.Fatalf("the settle judged by another say than the ask's: %+v", s) + } +} diff --git a/cmd/mesh-controller/plan_keys.go b/cmd/mesh-controller/plan_keys.go index abe732bd..1bdb1bcc 100644 --- a/cmd/mesh-controller/plan_keys.go +++ b/cmd/mesh-controller/plan_keys.go @@ -47,8 +47,6 @@ var buildersOwn = func(module string) bool { return buildAgentsOwn[module] } type planKeyReads interface { ModuleAt(ctx context.Context, module, commit string) (sdkdelivery.Module, error) Build(ctx context.Context, module, fp string) (builderkeys.Key, error) - // SaysReferences is whether a running builder says it writes references (builderkeys.Reader.SaysReferences). - SaysReferences(ctx context.Context) (bool, error) } // builderSayKey carries, on an ask's context, the builder's say the walk read before asking (withBuilderSay). @@ -135,7 +133,9 @@ func settleKeys(ctx context.Context, inv keyRecords, keys planKeyReads, p *inven changed = true } } - says, err := keys.SaysReferences(ctx) + // One say for the ask and the settle (builderSaysReferences): two sources that differ hand a module back and take it + // from its key again at every turn of the look, which then never ends. + says, err := builderSaysReferences(ctx) if err != nil { return changed, fmt.Errorf("whether the builder writes references cannot be read: %w", err) } diff --git a/cmd/mesh-controller/plan_keys_test.go b/cmd/mesh-controller/plan_keys_test.go index 1a04a48b..29190f32 100644 --- a/cmd/mesh-controller/plan_keys_test.go +++ b/cmd/mesh-controller/plan_keys_test.go @@ -16,12 +16,8 @@ import ( type planKeysOf struct { values map[string]sdkdelivery.Module // by module + "@" + commit keys map[string]builderkeys.Key - // notSaid is a builder that does not say it writes references (issue 1886); the zero value says it. - notSaid bool } -func (k planKeysOf) SaysReferences(context.Context) (bool, error) { return !k.notSaid, nil } - func (k planKeysOf) ModuleAt(_ context.Context, module, commit string) (sdkdelivery.Module, error) { if v, ok := k.values[module+"@"+commit]; ok { return v, nil