diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go index 5ae38b7..182885b 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go @@ -7,6 +7,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "io" "os" @@ -155,6 +156,8 @@ type bed struct { tools map[string]jetstream.KeyValue presence jetstream.KeyValue have jetstream.KeyValue + instances jetstream.KeyValue + stop func() // stops the builder started last fake *fakeBuildModules srvURL string dir string @@ -227,6 +230,7 @@ func startBed(t *testing.T) *bed { b.builds = mk(delivery.BuilderBuildsBucket, delivery.BuilderBuildsHistory) presence := mk(delivery.PresenceBucket, delivery.PresenceHistory) b.presence, b.have = presence, mk(delivery.HaveBucket, delivery.HaveHistory) + b.instances = mk(instancesBucket, 1) for _, node := range []string{"n1", "n2"} { b.setOnline(t, node, true) for _, module := range []string{"mesh-build-go", "mesh-build-node"} { @@ -344,7 +348,9 @@ func (b *bed) start(t *testing.T, node string, env map[string]string) { runCtx, stop := context.WithCancel(b.ctx) done := make(chan error, 1) go func() { done <- run(runCtx, func(k string) string { return e[k] }, io.Discard) }() - t.Cleanup(func() { stop(); <-done }) + var once sync.Once + b.stop = func() { once.Do(func() { stop(); <-done }) } + t.Cleanup(b.stop) } // await reads the build key until pred holds of it, within 20 s. @@ -713,3 +719,26 @@ func TestAManifestFallingBackToAPredefinedKeyIsBuilt(t *testing.T) { t.Errorf("build = %+v, want done: the manifest is read", v) } } + +// 1886/AC5: a builder on n1 whose Go, Node and image build modules each answered a reference says under n1, in its bucket +// of instances, that it writes references, and takes the say away when it stops: the controller delivers from its keys +// only while it holds. +func TestABuilderWhoseBuildModulesAnsweredReferencesSaysItAndStoppedSaysNothing(t *testing.T) { + b := startBed(t) + b.module(t, "netcheck", "catalogue-a67f0196-netcheck.module.json", "modules/netcheck") // a Go and a TypeScript bundle + b.moduleCommit(t, "imaged", "catalogue-a67f0196-imaged.module.json", "modules/imaged", nil, commit) // an image, for the trunk + b.start(t, "n1", nil) + var said string + for deadline := time.Now().Add(20 * time.Second); time.Now().Before(deadline) && said == ""; time.Sleep(50 * time.Millisecond) { + if e, err := b.instances.Get(b.ctx, "n1"); err == nil { + said = string(e.Value()) + } + } + if !strings.Contains(said, `"writes":["references"]`) { + t.Fatalf("the builder said %q", said) + } + b.stop() + if _, err := b.instances.Get(b.ctx, "n1"); !errors.Is(err, jetstream.ErrKeyNotFound) { + t.Fatalf("the stopped builder still says: %v", err) + } +} diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go index 40b6e29..822b325 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go @@ -178,6 +178,25 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro Manifests: manifestReader{manifests}, Logf: logger.Printf, } + // **Said once every build module answered a reference, taken away when stopped** (issue 1886, ADR 0522): the + // controller delivers from this builder's keys only while an instance says it writes references. A bucket not there + // yet (the manifest's state not made) is said, and the controller keeps asking build-agent. + switch kv, err := js.KeyValue(ctx, instancesBucket); { + case errors.Is(err, jetstream.ErrBucketNotFound): + logger.Printf("the bucket %s does not exist yet: the controller is not told that this builder writes references until it starts again", instancesBucket) + case err != nil: + return fmt.Errorf("the bucket %s: %w", instancesBucket, err) + default: + says := newSayer(ctx, kv, cred.Node, []string{buildrun.GoBuildModule, buildrun.NodeBuildModule, build.ImageBuildModule}, logger.Printf) + b.Answered = says.Answered + defer func() { + stopping, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second) + defer cancel() + if err := says.unsay(stopping); err != nil { + logger.Printf("%v", err) + } + }() + } // The toolchains first, so a build that starts from the first modules value finds the nodes. watch := newToolchainWatch(ctx, js, tools, b.NodesChanged, logger) if err := watch.start(); err != nil { diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/says.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/says.go new file mode 100644 index 0000000..8ff0302 --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/says.go @@ -0,0 +1,79 @@ +package main + +import ( + "context" + "fmt" + "sync" + + "github.com/nats-io/nats.go/jetstream" +) + +// What the builder says it writes (novox/hq issue 1886, ADR 0522): each running instance keeps a key, named by its node, +// in its own bucket of instances (`mesh-delivery-builder_instances`, the manifest's state `instances`), saying that every +// done key it writes names where its artifacts are (ADR 0498 ยง1). The references come from the build modules' own pushes, +// so an instance says it only once a step of every build module it calls answered one since it started: a builder beside +// a build module from before R4e says nothing. The controller delivers from the builder's keys only while an instance +// says so, and asks build-agent as before R4e otherwise. The bucket is read by the controller alone, so its name and shape +// are not in mesh-sdk. +const ( + instancesBucket = "mesh-delivery-builder_instances" + writesReferences = `{"v":1,"writes":["references"]}` +) + +// sayStore is the bucket of instances as the builder writes it. +type sayStore interface { + Put(ctx context.Context, key string, value []byte) (uint64, error) + Delete(ctx context.Context, key string, opts ...jetstream.KVDeleteOpt) error +} + +// sayer puts this instance's say once every build module in need answered a reference, and takes it away at the stop. +type sayer struct { + ctx context.Context + store sayStore + node string + need []string + logf func(string, ...any) + + mu sync.Mutex + answered map[string]bool + said bool +} + +func newSayer(ctx context.Context, store sayStore, node string, need []string, logf func(string, ...any)) *sayer { + return &sayer{ctx: ctx, store: store, node: node, need: need, logf: logf, answered: map[string]bool{}} +} + +// Answered is told the build module whose step answered a reference (build.Builder.Answered). +func (s *sayer) Answered(module string) { + s.mu.Lock() + defer s.mu.Unlock() + s.answered[module] = true + if s.said { + return + } + for _, m := range s.need { + if !s.answered[m] { + return + } + } + if _, err := s.store.Put(s.ctx, s.node, []byte(writesReferences)); err != nil { + s.logf("saying in %s that this builder writes references: %v; said again at the next reference", instancesBucket, err) + return + } + s.said = true + s.logf("every build module answered a reference: said in %s that this builder writes references", instancesBucket) +} + +// unsay takes the say away, if it was made: a builder replaced by one that does not write references leaves none. +func (s *sayer) unsay(ctx context.Context) error { + s.mu.Lock() + defer s.mu.Unlock() + if !s.said { + return nil + } + if err := s.store.Delete(ctx, s.node); err != nil { + return fmt.Errorf("taking away the say in %s: %w", instancesBucket, err) + } + s.said = false + return nil +} diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/says_test.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/says_test.go new file mode 100644 index 0000000..f219959 --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/says_test.go @@ -0,0 +1,46 @@ +package main + +import ( + "context" + "encoding/json" + "testing" + + "github.com/nats-io/nats.go/jetstream" +) + +type saysOf map[string]string + +func (s saysOf) Put(_ context.Context, key string, value []byte) (uint64, error) { + s[key] = string(value) + return 1, nil +} + +func (s saysOf) Delete(_ context.Context, key string, _ ...jetstream.KVDeleteOpt) error { + delete(s, key) + return nil +} + +// 1886/AC5: a builder says, under its node, that it writes references only once every build module it calls answered +// one; stopped, it says nothing any more. +func TestTheBuilderSaysItWritesReferencesOnceEveryBuildModuleAnsweredOne(t *testing.T) { + s := saysOf{} + say := newSayer(t.Context(), s, "shanks", []string{"mesh-build-go", "mesh-build-node", "mesh-build-image"}, t.Logf) + say.Answered("mesh-build-go") + say.Answered("mesh-build-node") + if v, ok := s["shanks"]; ok { + t.Fatalf("said before the image build module answered a reference: %q", v) + } + say.Answered("mesh-build-image") + var v struct { + Writes []string `json:"writes"` + } + if err := json.Unmarshal([]byte(s["shanks"]), &v); err != nil || len(v.Writes) != 1 || v.Writes[0] != "references" { + t.Fatalf("the builder's say: %q %v", s["shanks"], err) + } + if err := say.unsay(t.Context()); err != nil { + t.Fatal(err) + } + if _, ok := s["shanks"]; ok { + t.Fatalf("the stopped builder still says: %v", s) + } +} diff --git a/modules/mesh-delivery-builder/internal/build/r4e_test.go b/modules/mesh-delivery-builder/internal/build/r4e_test.go index 1c53597..d3d851c 100644 --- a/modules/mesh-delivery-builder/internal/build/r4e_test.go +++ b/modules/mesh-delivery-builder/internal/build/r4e_test.go @@ -383,3 +383,21 @@ func TestACandidatesFetchNamesTheNamedPathsTheIndexerGaveIt(t *testing.T) { // A t.Errorf("fetch request = %+v, %v; want the candidate's base, the module's directory and its named paths", req, err) } } + +// 1886/AC5: the builder tells who answered a reference: the build module whose step named one, and none for a step that +// answered a digest alone (a build module from before R4e). +func TestTheBuilderTellsWhichBuildModuleAnsweredAReference(t *testing.T) { + r := newRig(t, goBundle) + var answered []string + r.b.Answered = func(module string) { answered = append(answered, module) } + r.bm.noRef["ace/tool"] = true + r.consider(t, "modules/m") + if len(answered) != 0 { + t.Fatalf("a step that answered no reference was told as one: %v", answered) + } + delete(r.bm.noRef, "ace/tool") + r.consider(t, "modules/m") + if len(answered) != 1 || answered[0] != "mesh-build-go" { + t.Fatalf("the build module that answered a reference: %v, want mesh-build-go", answered) + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index 9c6f23e..3021dd0 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -45,6 +45,9 @@ type Builder struct { // Limit is how many builds run at once; four when 0. Limit int Logf func(format string, args ...any) + // Answered is told the build module whose step answered a reference (issue 1886): what the builder says it writes + // rests on it. Nil tells nobody. + Answered func(buildModule string) once sync.Once sem chan struct{} @@ -749,6 +752,9 @@ func (b *Builder) runLane(ctx context.Context, l *lane, repository string, m del if built.Reference != "" && (stored(t.Kind) || !candidate) { l.references[t.Artifact] = built.Reference } + if built.Reference != "" && b.Answered != nil { + b.Answered(l.module) + } } } diff --git a/modules/mesh-delivery-builder/manifest_test.go b/modules/mesh-delivery-builder/manifest_test.go index 3fd061d..6f13144 100644 --- a/modules/mesh-delivery-builder/manifest_test.go +++ b/modules/mesh-delivery-builder/manifest_test.go @@ -83,7 +83,7 @@ func TestManifestDeclarations(t *testing.T) { // 1607/AC1 if m.Module != "mesh-delivery-builder" { t.Errorf("module = %q", m.Module) } - if want := []state{{Name: "builds", History: 5}}; !reflect.DeepEqual(m.State, want) { // delivery.BuilderBuildsHistory + if want := []state{{Name: "builds", History: 5}, {Name: "instances", History: 1}}; !reflect.DeepEqual(m.State, want) { // delivery.BuilderBuildsHistory t.Errorf("state = %+v, want %+v", m.State, want) } if want := []string{"mesh-build-go.steps", "mesh-build-go.toolchains", "mesh-build-image.steps", "mesh-build-node.steps", "mesh-build-node.toolchains", diff --git a/modules/mesh-delivery-builder/module.json b/modules/mesh-delivery-builder/module.json index 460ae22..f18e708 100644 --- a/modules/mesh-delivery-builder/module.json +++ b/modules/mesh-delivery-builder/module.json @@ -7,7 +7,8 @@ "mesh-build-node.toolchain.reported" ], "state": [ - { "name": "builds", "history": 5 } + { "name": "builds", "history": 5 }, + { "name": "instances", "history": 1 } ], "reads": [ "mesh-build-go.steps",