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..da5896d 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,23 @@ func TestAManifestFallingBackToAPredefinedKeyIsBuilt(t *testing.T) { t.Errorf("build = %+v, want done: the manifest is read", v) } } + +// 1886/AC5: a builder started on n1 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 TestTheStartedBuilderSaysItWritesReferencesAndStoppedSaysNothing(t *testing.T) { + b := startBed(t) + 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 started 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..b51db07 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go @@ -270,6 +270,27 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro return err } logger.Printf("started on %s", cred.Node) + // **Said once started, taken away when stopped** (issue 1886): 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: + unsay, err := say(ctx, kv, cred.Node) + if err != nil { + return err + } + defer func() { + stopping, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second) + defer cancel() + if err := unsay(stopping); err != nil { + logger.Printf("the say in %s could not be taken away: %v", instancesBucket, err) + } + }() + } errs := make(chan error, 2) go func() { errs <- follow(ctx, w, each) }() go func() { errs <- follow(ctx, cw, eachCandidate) }() 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..8ec0e9c --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/says.go @@ -0,0 +1,33 @@ +package main + +import ( + "context" + "fmt" + + "github.com/nats-io/nats.go/jetstream" +) + +// What the builder says it writes (novox/hq issue 1886): 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 controller delivers from the builder's keys only while +// an instance says so, and asks build-agent as before R4e otherwise; a builder from before R4e keeps no such key. 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 +} + +// say puts this instance's key once it has started, and returns what takes it away again when it stops: a builder +// replaced by one that does not write references leaves no say behind it. +func say(ctx context.Context, s sayStore, node string) (unsay func(context.Context) error, err error) { + if _, err := s.Put(ctx, node, []byte(writesReferences)); err != nil { + return nil, fmt.Errorf("saying in %s that this builder writes references: %w", instancesBucket, err) + } + return func(ctx context.Context) error { return s.Delete(ctx, node) }, 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..314d005 --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/says_test.go @@ -0,0 +1,42 @@ +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 started builder says, under its node, that it writes references; stopped, it says nothing any more. +func TestTheBuilderSaysItWritesReferencesWhileItRuns(t *testing.T) { + s := saysOf{} + unsay, err := say(t.Context(), s, "shanks") + if err != nil { + t.Fatal(err) + } + 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 started builder's say: %q %v", s["shanks"], err) + } + if err := 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/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",