Say in the builder's own bucket of instances that it writes references (issue 1886)

The controller delivers from the builder's keys only while a running builder says its done keys name references; a builder from before R4e says nothing, so R4e's own delivery is asked of build-agent instead of waiting for ever. The say is put once started and taken away when stopped.
2026-10-23 00:48:28 +02:00
parent b714eab995
commit 4b8b1c047c
6 changed files with 126 additions and 3 deletions
@@ -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)
+	}
+}
@@ -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) }()
@@ -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
+}
@@ -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)
+	}
+}
@@ -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",
@@ -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",