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.
This commit is contained in:
2026-10-23 00:47:15 +02:00
parent 289bbf68e4
commit fae7710bed
9 changed files with 444 additions and 4 deletions
@@ -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 }
}
+166
View File
@@ -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)
}
}
+36 -1
View File
@@ -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
}
}
+37
View File
@@ -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.
+4
View File
@@ -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
+17 -3
View File
@@ -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)
+97
View File
@@ -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, `<owner module>_<name>`.
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
}
@@ -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)
}
}
+24
View File
@@ -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)
}
}
}