From 07a28b03c46cbadd41a33245c4690ef29a6f4ffb Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:16:16 +0200 Subject: [PATCH 01/14] The indexer writes each module's directory into modules. and keeps it (issue 1606, ADR 0478 clause 8) --- .../mesh-delivery-indexer/integration_test.go | 8 ++--- .../internal/index/modules.go | 2 +- .../internal/index/modules_test.go | 31 +++++++++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go index c6aadb7..11d9623 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go @@ -215,8 +215,8 @@ func TestIndexerWritesModulesAtAMerge(t *testing.T) { // 1239/AC9 } } } - if m.FP != want || m.Repository != "novox/mesh-delivery" || m.Commit != merge { - t.Errorf("module = %+v within 5 s, want fp %s", m, want) + if m.FP != want || m.Repository != "novox/mesh-delivery" || m.Commit != merge || m.Path != "modules/a" { // 1606/AC2 + t.Errorf("module = %+v within 5 s, want fp %s and path modules/a", m, want) } } @@ -262,8 +262,8 @@ func TestIndexerKeepsManifestsAtAMerge(t *testing.T) { // 1444/AC4, 1449/AC4 } digest := delivery.ManifestDigest(manifest) if m.Manifest != digest || m.None2 != "toolchain not yet reported" || m.FP2 != "" || m.Inputs2 == nil || m.Inputs2.Tree != "newtree" || - m.Inputs2.Locks["tools"] != goSum { - t.Errorf("module = %+v within 5 s, want its manifest digest %s, its inputs and none2 'toolchain not yet reported'", m, digest) + m.Inputs2.Locks["tools"] != goSum || m.Path != "modules/avahi" { // 1606/AC2 + t.Errorf("module = %+v within 5 s, want its path modules/avahi, its manifest digest %s, its inputs and none2 'toolchain not yet reported'", m, digest) } kv, err := js.KeyValue(ctx, delivery.ManifestsBucket) if err != nil { diff --git a/modules/mesh-delivery-indexer/internal/index/modules.go b/modules/mesh-delivery-indexer/internal/index/modules.go index 6869bd4..23cf31a 100644 --- a/modules/mesh-delivery-indexer/internal/index/modules.go +++ b/modules/mesh-delivery-indexer/internal/index/modules.go @@ -63,7 +63,7 @@ func (ix *Indexer) applyMerged(ctx context.Context, f delivery.Forge) error { continue } fp, none := fingerprintOf(k, f.Trees) - next := delivery.Module{Repository: f.Repository, Commit: f.Merge, FP: fp, None: none, Src: ix.srcOf(f.Repository, f.Merge), Manifest: digests[k.Path]} + next := delivery.Module{Repository: f.Repository, Commit: f.Merge, FP: fp, None: none, Src: ix.srcOf(f.Repository, f.Merge), Manifest: digests[k.Path], Path: k.Path} // The new form (issue 1444, R4c-2): no module has a fingerprint of it yet, so each has the reason it has none, which // names what is missing, from its manifest and lock hashes (inputs.go). raw, has := f.Manifests[k.Path] diff --git a/modules/mesh-delivery-indexer/internal/index/modules_test.go b/modules/mesh-delivery-indexer/internal/index/modules_test.go index 7fd2ac7..db2d58f 100644 --- a/modules/mesh-delivery-indexer/internal/index/modules_test.go +++ b/modules/mesh-delivery-indexer/internal/index/modules_test.go @@ -225,3 +225,34 @@ func TestARecordAsTheControllerWritesItGivesAFingerprint(t *testing.T) { // 1281 t.Errorf("module = %+v (%v), want a fingerprint, not none", m, ok) } } + +func TestAMergeWritesTheModulesDirectoryAndKeepsItThroughSourceAndRecompute(t *testing.T) { // 1606/AC1, AC2 + ix, mods := modulesIndexer(t, + rec(t, "build-1", "a", "novox/mesh-delivery", "modules/a", "main", "", "2026-10-19T09:00:00Z", aInputs), + rec(t, "build-2", "r", "novox/mesh-delivery", "", "main", "", "2026-10-19T09:00:00Z", strings.Replace(aInputs, `"Module":"a"`, `"Module":"r"`, 1))) + ctx := context.Background() + if err := ix.Apply(ctx, mergedMsg("novox/mesh-delivery", map[string]string{"modules/a": "newtree", "": "roottree"})); err != nil { + t.Fatal(err) + } + for module, want := range map[string]string{"a": "modules/a", "r": ""} { + if m, ok := moduleOf(t, mods, module); !ok || m.Path != want { + t.Errorf("after the merge %s = %+v (%v), want path %q", module, m, ok, want) + } + } + if err := ix.ApplySource(ctx, 7, sourceOf("novox/mesh-delivery", merge)); err != nil { + t.Fatal(err) + } + if err := ix.Recompute(ctx); err != nil { + t.Fatal(err) + } + if m, _ := moduleOf(t, mods, "a"); m.Path != "modules/a" || m.Src != 7 { + t.Errorf("after the source and a recompute a = %+v, want path modules/a kept and Src 7", m) + } + raw := string(mods.vals["a"]) + if !strings.Contains(raw, `"path":"modules/a"`) { + t.Errorf("the stored value %s has no path", raw) + } + if strings.Contains(string(mods.vals["r"]), `"path"`) { + t.Errorf("the root module's value %s names a path", mods.vals["r"]) + } +} From c3804b012fe108a5d59a1a0f081ba2fd400e5810 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:18:30 +0200 Subject: [PATCH 02/14] Add the builder part: its manifest, its credential and its bus connection (issue 1607, AC1) --- .../cmd/mesh-delivery-builder/main.go | 118 ++++++++++++ modules/mesh-delivery-builder/go.mod | 22 +++ modules/mesh-delivery-builder/go.sum | 27 +++ .../mesh-delivery-builder/manifest_test.go | 168 ++++++++++++++++++ modules/mesh-delivery-builder/module.json | 75 ++++++++ 5 files changed, 410 insertions(+) create mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go create mode 100644 modules/mesh-delivery-builder/go.mod create mode 100644 modules/mesh-delivery-builder/go.sum create mode 100644 modules/mesh-delivery-builder/manifest_test.go create mode 100644 modules/mesh-delivery-builder/module.json diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go new file mode 100644 index 0000000..c023e2e --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go @@ -0,0 +1,118 @@ +// Command mesh-delivery-builder is delivery's builder (novox/hq ADR 0453 clause 4, ADR 0478, issue 1607): it builds every +// new-form fingerprint the indexer writes, dark, into its own build keys (builds..), through the build +// modules' fetch and build tools. Nothing reads its keys to deliver until release R4e. It serves no request subject and +// holds no timer: every reaction is a watch or a bus event (ADR 0437 §8). +package main + +import ( + "context" + "crypto/sha256" + "crypto/tls" + "crypto/x509" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "log" + "os" + "os/signal" + "strings" + "syscall" + + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +func main() { + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + if err := run(ctx, os.Getenv, os.Stderr); err != nil { + fmt.Fprintln(os.Stderr, "mesh-delivery-builder:", err) + os.Exit(1) + } +} + +// credential is the broker file the controller issues to the module: JSON of the bus address, the certificate +// fingerprint to pin, the principal and its password (as the indexer reads it; issue 1242 moves this into the SDK). +type credential struct { + URL string `json:"url"` + Fingerprint string `json:"fingerprint"` + Node string `json:"node"` + Module string `json:"module"` + User string `json:"user"` + Password string `json:"password"` +} + +func readCredential(path string) (credential, error) { + raw, err := os.ReadFile(path) + if err != nil { + return credential{}, err + } + var c credential + if err := json.Unmarshal(raw, &c); err != nil { + return credential{}, fmt.Errorf("the broker credential is not one: %w", err) + } + if c.URL == "" || c.User == "" { + return credential{}, errors.New("the broker credential names no bus address or user") + } + return c, nil +} + +// pinned accepts exactly the certificate with this SHA-256 and no other. +func pinned(want string) *tls.Config { + return &tls.Config{ + InsecureSkipVerify: true, //nolint:gosec // replaced by the pin below, which is stricter + MinVersion: tls.VersionTLS12, + VerifyPeerCertificate: func(rawCerts [][]byte, _ [][]*x509.Certificate) error { + if len(rawCerts) == 0 { + return errors.New("the bus presented no certificate") + } + sum := sha256.Sum256(rawCerts[0]) + if got := "sha256:" + hex.EncodeToString(sum[:]); got != want { + return fmt.Errorf("the bus presented a certificate this mesh does not know (%.23s…)", got) + } + return nil + }, + } +} + +// run is the program: it returns when ctx ends or the bus cannot be used. +func run(ctx context.Context, getenv func(string) string, logOut io.Writer) error { + logger := log.New(logOut, "mesh-delivery-builder: ", log.LstdFlags) + path := strings.TrimSpace(getenv("MESH_BROKER_FILE")) + if path == "" { + return errors.New("MESH_BROKER_FILE is not set, so there is no credential to hear the bus with") + } + cred, err := readCredential(path) + if err != nil { + return err + } + opts := []nats.Option{ + nats.Name(cred.Node + "." + cred.Module), + nats.UserInfo(cred.User, cred.Password), + nats.CustomInboxPrefix("_INBOX." + cred.User), + nats.MaxReconnects(-1), + nats.RetryOnFailedConnect(true), + } + if strings.TrimSpace(cred.Fingerprint) != "" { + opts = append(opts, nats.Secure(pinned(cred.Fingerprint))) + } + conn, err := nats.Connect(cred.URL, opts...) + if err != nil { + return fmt.Errorf("connecting to the bus: %w", err) + } + defer conn.Close() + js, err := jetstream.New(conn) + if err != nil { + return err + } + if _, err := js.KeyValue(ctx, delivery.BuilderBuildsBucket); err != nil { + return fmt.Errorf("the bucket %s: %w", delivery.BuilderBuildsBucket, err) + } + logger.Printf("started on %s", cred.Node) + <-ctx.Done() + return nil +} diff --git a/modules/mesh-delivery-builder/go.mod b/modules/mesh-delivery-builder/go.mod new file mode 100644 index 0000000..956a222 --- /dev/null +++ b/modules/mesh-delivery-builder/go.mod @@ -0,0 +1,22 @@ +module mesh-delivery-builder + +go 1.26.0 + +require ( + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020182814-4024909a2960 + github.com/nats-io/nats-server/v2 v2.11.17 + github.com/nats-io/nats.go v1.54.0 +) + +require ( + github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op // indirect + github.com/google/go-tpm v0.9.8 // indirect + github.com/klauspost/compress v1.20.0 // indirect + github.com/minio/highwayhash v1.0.4 // indirect + github.com/nats-io/jwt/v2 v2.8.1 // indirect + github.com/nats-io/nkeys v0.4.16 // indirect + github.com/nats-io/nuid v1.0.1 // indirect + golang.org/x/crypto v0.57.0 // indirect + golang.org/x/sys v0.48.0 // indirect + golang.org/x/time v0.15.0 // indirect +) diff --git a/modules/mesh-delivery-builder/go.sum b/modules/mesh-delivery-builder/go.sum new file mode 100644 index 0000000..0e67d23 --- /dev/null +++ b/modules/mesh-delivery-builder/go.sum @@ -0,0 +1,27 @@ +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020182814-4024909a2960 h1:FCId3IuRScG03X7Gx9vfKfGNZH9jWnfx9RChfJ3eWfo= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020182814-4024909a2960/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= +github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= +github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= +github.com/google/go-tpm v0.9.8/go.mod h1:h9jEsEECg7gtLis0upRBQU+GhYVH6jMjrFxI8u6bVUY= +github.com/klauspost/compress v1.20.0 h1:a3C1ke2ohxFymNlb2HWAHjDeKCI90scRskErZkR0ezA= +github.com/klauspost/compress v1.20.0/go.mod h1:LUdAzn7YLVvxLpc7y3V1m40wESHTgc1422pwwBSKYuI= +github.com/minio/highwayhash v1.0.4 h1:asJizugGgchQod2ja9NJlGOWq4s7KsAWr5XUc9Clgl4= +github.com/minio/highwayhash v1.0.4/go.mod h1:GGYsuwP/fPD6Y9hMiXuapVvlIUEhFhMTh0rxU3ik1LQ= +github.com/nats-io/jwt/v2 v2.8.1 h1:V0xpGuD/N8Mi+fQNDynXohVvp7ZztevW5io8CUWlPmU= +github.com/nats-io/jwt/v2 v2.8.1/go.mod h1:nWnOEEiVMiKHQpnAy4eXlizVEtSfzacZ1Q43LIRavZg= +github.com/nats-io/nats-server/v2 v2.11.17 h1:GKEghcFK6A+aFx11Yf1LjgLC3txAwvyhnYzhBIQZA8I= +github.com/nats-io/nats-server/v2 v2.11.17/go.mod h1:B1sFVz4StNosQ903ak4N1G01Fl/9f8e06mXpFIE2K24= +github.com/nats-io/nats.go v1.54.0 h1:vsXoOxjHp/GmPUN+EcI7uOf/uB+iAP+kEsAFNQN0yzA= +github.com/nats-io/nats.go v1.54.0/go.mod h1:y+DZoD1oBOYfZTU681eTUiUjI0vbqYGixNVFHcjHJ0k= +github.com/nats-io/nkeys v0.4.16 h1:rd5oAuLOb8mnAycB0xleuEBNS1pVVnN0fv/FF34Eypg= +github.com/nats-io/nkeys v0.4.16/go.mod h1:llLgWoI0o4z/Q57q2R1kHfmocyhGV6VG/U18Glg1Afs= +github.com/nats-io/nuid v1.0.1 h1:5iA8DT8V7q8WK2EScv2padNa/rTESc1KdnPw4TC2paw= +github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c= +golang.org/x/crypto v0.57.0 h1:3ZVCjf8Ggz7zneR/EHRVx68Ctf+2pmIMP2UFhh9cC6M= +golang.org/x/crypto v0.57.0/go.mod h1:Fdz0i5U6CoizGwLda9DttjSk6qlZo25zYNtR+ycvuZA= +golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= +golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= +golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= +golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= diff --git a/modules/mesh-delivery-builder/manifest_test.go b/modules/mesh-delivery-builder/manifest_test.go new file mode 100644 index 0000000..5d224e7 --- /dev/null +++ b/modules/mesh-delivery-builder/manifest_test.go @@ -0,0 +1,168 @@ +package manifest + +import ( + "bytes" + "encoding/json" + "io/fs" + "os" + "path/filepath" + "reflect" + "slices" + "strings" + "testing" +) + +// The controller's manifest reader is mesh-controller internal/catalogue (Manifest, manifest.go: Consumes :521, +// Invokes :590, Streams :613, Reads :659; StreamDeclaration, streams.go), an internal package of another repository. +// These tests decode the manifest strictly into the fields those types name (DisallowUnknownFields) and hold the +// rules of issue 1607 (ADR 0453 §4, ADR 0478). The merge gate's module check proves the controller takes the manifest. + +type state struct { + Name string `json:"name"` + History int `json:"history,omitempty"` +} + +type artifact struct { + Name string `json:"name"` + Kind string `json:"kind"` + Language string `json:"language"` + System string `json:"system"` + From string `json:"from"` + Binary string `json:"binary"` +} + +type manifest struct { + Module string `json:"module"` + Version string `json:"version"` + Slug string `json:"slug"` + Consumes []string `json:"consumes"` + State []state `json:"state"` + Reads []string `json:"reads"` + Invokes []string `json:"invokes"` + OwnSecrets map[string]string `json:"own-secrets"` + SecretsOwner string `json:"secrets-owner"` + Resources []map[string]any `json:"resources"` + Build struct { + Artifacts []artifact `json:"artifacts"` + } `json:"build"` +} + +func load(t *testing.T) (manifest, map[string]json.RawMessage) { + t.Helper() + raw, err := os.ReadFile("module.json") + if err != nil { + t.Fatal(err) + } + var m manifest + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.DisallowUnknownFields() + if err := dec.Decode(&m); err != nil { + t.Fatalf("the controller's reader refuses a key this manifest has: %v", err) + } + var top map[string]json.RawMessage + if err := json.Unmarshal(raw, &top); err != nil { + t.Fatal(err) + } + return m, top +} + +func sorted(s []string) []string { + s = slices.Clone(s) + slices.Sort(s) + return s +} + +func TestManifestDeclarations(t *testing.T) { // 1607/AC1 + m, top := load(t) + 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 + t.Errorf("state = %+v, want %+v", m.State, want) + } + if want := []string{"mesh-build-go.steps", "mesh-build-go.toolchains", "mesh-build-node.steps", "mesh-build-node.toolchains", + "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules"}; !reflect.DeepEqual(sorted(m.Reads), want) { + t.Errorf("reads = %v, want %v", m.Reads, want) + } + if want := []string{"mesh-build-go.build_go_build", "mesh-build-go.build_go_fetch", "mesh-build-node.build_node_build", "mesh-build-node.build_node_fetch"}; !reflect.DeepEqual(sorted(m.Invokes), want) { + t.Errorf("invokes = %v, want only the build modules' fetch and build", m.Invokes) + } + // The builder is woken by a build module's report of a toolchain, as the indexer is: the bucket may not exist when it starts. + if want := []string{"mesh-build-go.toolchain.reported", "mesh-build-node.toolchain.reported"}; !reflect.DeepEqual(sorted(m.Consumes), want) { + t.Errorf("consumes = %v, want %v", m.Consumes, want) + } + if b, ok := m.OwnSecrets["broker"]; !ok || len(m.OwnSecrets) != 1 || b != "${dir:state}/broker" { + t.Errorf("own-secrets = %v, want exactly broker", m.OwnSecrets) + } + if m.SecretsOwner != "mesh-delivery-builder" { + t.Errorf("secrets-owner = %q", m.SecretsOwner) + } + for _, k := range []string{"streams", "logs", "emits", "claims", "provides", "requires", "uses", "defines-seats", "tools", "seat", "seats", "bus"} { + if _, ok := top[k]; ok { + t.Errorf("the manifest declares %q: the builder has none", k) + } + } +} + +func TestManifestProcess(t *testing.T) { // 1607/AC1 + m, _ := load(t) + var procs []map[string]any + for _, r := range m.Resources { + if r["type"] == "process" { + procs = append(procs, r) + } + if r["type"] == "tool" || r["type"] == "seat" { + t.Errorf("resource %v: the builder serves no tool and holds no seat", r) + } + } + if len(procs) != 1 { + t.Fatalf("%d process resources, want one", len(procs)) + } + p := procs[0] + if p["artifact"] != "program" || !reflect.DeepEqual(p["run"], []any{"./mesh-delivery-builder"}) { + t.Errorf("process = %v", p) + } + env, _ := p["env"].(map[string]any) + if len(env) != 1 || env["MESH_BROKER_FILE"] != "${dir:state}/broker" { + t.Errorf("env = %v", p["env"]) + } + if _, bus := p["bus"]; bus { + t.Errorf("the process declares a bus: it runs on its own broker credential") + } + if len(m.Build.Artifacts) != 1 { + t.Fatalf("artifacts = %+v", m.Build.Artifacts) + } + a := m.Build.Artifacts[0] + if st, err := os.Stat(a.From); err != nil || !st.IsDir() || a.Binary != "mesh-delivery-builder" || !strings.HasSuffix(a.From, a.Binary) || a.Kind != "bundle" || a.Language != "go" { + t.Errorf("artifact = %+v", a) + } +} + +func TestServesNoRequestSubjectAndHoldsNoTicker(t *testing.T) { // 1607/AC6 (ADR 0437 §8: no timer, a wait is a context's bound) + forbidden := []string{"Subscribe(", "QueueSubscribe", "ChanSubscribe", "time.NewTicker", "time.Tick", "time.NewTimer", "time.AfterFunc", "time.After(", "time.Sleep"} + var files int + for _, root := range []string{"cmd", "internal"} { + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return err + } + files++ + src, err := os.ReadFile(path) + if err != nil { + return err + } + for _, f := range forbidden { + if strings.Contains(string(src), f) { + t.Errorf("%s holds %s: the builder serves no request subject and holds no timer", path, f) + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } + } + if files < 1 { + t.Fatalf("read %d source files, want the program's", files) + } +} diff --git a/modules/mesh-delivery-builder/module.json b/modules/mesh-delivery-builder/module.json new file mode 100644 index 0000000..9d27205 --- /dev/null +++ b/modules/mesh-delivery-builder/module.json @@ -0,0 +1,75 @@ +{ + "module": "mesh-delivery-builder", + "version": "1", + "slug": "builder", + "consumes": [ + "mesh-build-go.toolchain.reported", + "mesh-build-node.toolchain.reported" + ], + "state": [ + { "name": "builds", "history": 5 } + ], + "reads": [ + "mesh-build-go.steps", + "mesh-build-go.toolchains", + "mesh-build-node.steps", + "mesh-build-node.toolchains", + "mesh-delivery-indexer.manifests", + "mesh-delivery-indexer.modules" + ], + "invokes": [ + "mesh-build-go.build_go_build", + "mesh-build-go.build_go_fetch", + "mesh-build-node.build_node_build", + "mesh-build-node.build_node_fetch" + ], + "own-secrets": { + "broker": "${dir:state}/broker" + }, + "secrets-owner": "mesh-delivery-builder", + "resources": [ + { + "id": "account", + "type": "user", + "name": "mesh-delivery-builder", + "shell": "/usr/bin/nologin", + "home": "/var/lib/mesh-delivery-builder" + }, + { + "id": "state", + "type": "directory", + "mode": "0700", + "owner": "mesh-delivery-builder", + "place": ".", + "unknown": "program" + }, + { + "id": "builder", + "type": "process", + "name": "mesh-delivery-builder", + "artifact": "program", + "run": [ + "./mesh-delivery-builder" + ], + "env": { + "MESH_BROKER_FILE": "${dir:state}/broker" + }, + "user": "mesh-delivery-builder", + "health": { + "kind": "unit" + } + } + ], + "build": { + "artifacts": [ + { + "name": "program", + "kind": "bundle", + "language": "go", + "system": "arch", + "from": "cmd/mesh-delivery-builder", + "binary": "mesh-delivery-builder" + } + ] + } +} From 87e550681f5f32f174c0f5ae5fb309cb01d96644 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:19:38 +0200 Subject: [PATCH 03/14] Plan a manifest's artifacts for the build modules and refuse image, upstream and package by kind (issue 1607, AC2) --- .../internal/build/plan.go | 83 +++++++++++++++++++ .../internal/build/plan_test.go | 75 +++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 modules/mesh-delivery-builder/internal/build/plan.go create mode 100644 modules/mesh-delivery-builder/internal/build/plan_test.go diff --git a/modules/mesh-delivery-builder/internal/build/plan.go b/modules/mesh-delivery-builder/internal/build/plan.go new file mode 100644 index 0000000..78aff09 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/plan.go @@ -0,0 +1,83 @@ +// Package build is the builder's logic (novox/hq ADR 0453 clause 4, ADR 0478), apart from the bus: what a manifest asks +// to be built and where, the claim of a build key, the choice of build nodes, the build modules' long steps and the run of +// one build. The bus is reached through the small interfaces here, so each part is tested with fakes. +package build + +import ( + "fmt" + "sort" + + "git.novox.be/novox/mesh-sdk/go/catalogue" +) + +// The build modules a task is given to, and the language whose toolchain each reports (the indexer's ToolchainModules). +const ( + GoBuildModule = "mesh-build-go" + NodeBuildModule = "mesh-build-node" +) + +// RefusedPrefix opens the error of a build key that is failed for a kind no builder builds yet (ADR 0478 clause 2): such +// a key is not built again as a new revision. +const RefusedPrefix = "refused kind:" + +// Task is one artifact to build: its name and kind in the manifest, the build module that builds it and the language +// of the toolchain it is built with. +type Task struct { + Artifact string + Kind string + Module string + Language string +} + +// Plan is what a manifest asks the builder to build, by artifact name. +type Plan struct{ Tasks []Task } + +// Modules are the build modules the plan calls, sorted. +func (p Plan) Modules() []string { + seen := map[string]bool{} + var out []string + for _, t := range p.Tasks { + if !seen[t.Module] { + seen[t.Module] = true + out = append(out, t.Module) + } + } + sort.Strings(out) + return out +} + +// Refused is the error of a plan the builder does not build: an artifact of a kind (or of a language) it has no build +// module for. +type Refused struct{ Artifact, What string } + +func (r *Refused) Error() string { + return fmt.Sprintf("%s %s (artifact %s): the builder builds Go and TypeScript bundles and archives; build-agent builds it", RefusedPrefix, r.What, r.Artifact) +} + +// PlanOf is the plan of a manifest (ADR 0478 clauses 1 and 2): a Go bundle and an archive are built by the Go build +// module, a TypeScript bundle by the Node one, each in the order of the artifacts' names. A manifest with an image, +// upstream or package artifact, or a bundle of another language, is refused as a whole, naming the first such artifact +// by name. A manifest with no artifact has an empty plan. +func PlanOf(m catalogue.Manifest) (Plan, error) { + if m.Build == nil { + return Plan{}, nil + } + artifacts := append([]catalogue.Artifact(nil), m.Build.Artifacts...) + sort.Slice(artifacts, func(i, j int) bool { return artifacts[i].Name < artifacts[j].Name }) + var p Plan + for _, a := range artifacts { + switch { + case a.Kind == catalogue.ArtifactArchive: + p.Tasks = append(p.Tasks, Task{Artifact: a.Name, Kind: a.Kind, Module: GoBuildModule, Language: "go"}) + case a.Kind == catalogue.ArtifactBundle && a.Language == "go": + p.Tasks = append(p.Tasks, Task{Artifact: a.Name, Kind: a.Kind, Module: GoBuildModule, Language: "go"}) + case a.Kind == catalogue.ArtifactBundle && a.Language == "typescript": + p.Tasks = append(p.Tasks, Task{Artifact: a.Name, Kind: a.Kind, Module: NodeBuildModule, Language: "typescript"}) + case a.Kind == catalogue.ArtifactBundle: + return Plan{}, &Refused{Artifact: a.Name, What: fmt.Sprintf("bundle of language %q", a.Language)} + default: + return Plan{}, &Refused{Artifact: a.Name, What: a.Kind} + } + } + return p, nil +} diff --git a/modules/mesh-delivery-builder/internal/build/plan_test.go b/modules/mesh-delivery-builder/internal/build/plan_test.go new file mode 100644 index 0000000..bbeb0c3 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/plan_test.go @@ -0,0 +1,75 @@ +package build + +import ( + "errors" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/catalogue" +) + +// manifestOf is a manifest holding these artifacts: the plan reads only their name, kind and language. +func manifestOf(artifacts ...catalogue.Artifact) catalogue.Manifest { + return catalogue.Manifest{Module: "m", Build: &catalogue.Build{Artifacts: artifacts}} +} + +func TestPlanRoutesEachKindToItsBuildModule(t *testing.T) { // 1607/AC2 + m := manifestOf( + catalogue.Artifact{Name: "tool", Kind: "bundle", Language: "go"}, + catalogue.Artifact{Name: "web", Kind: "bundle", Language: "typescript"}, + catalogue.Artifact{Name: "files", Kind: "archive"}) + p, err := PlanOf(m) + if err != nil { + t.Fatal(err) + } + want := []Task{ + {Artifact: "files", Kind: "archive", Module: GoBuildModule, Language: "go"}, + {Artifact: "tool", Kind: "bundle", Module: GoBuildModule, Language: "go"}, + {Artifact: "web", Kind: "bundle", Module: NodeBuildModule, Language: "typescript"}, + } + if len(p.Tasks) != len(want) { + t.Fatalf("tasks = %+v, want %+v", p.Tasks, want) + } + for i := range want { + if p.Tasks[i] != want[i] { + t.Errorf("task %d = %+v, want %+v", i, p.Tasks[i], want[i]) + } + } + if modules := p.Modules(); len(modules) != 2 || modules[0] != GoBuildModule || modules[1] != NodeBuildModule { + t.Errorf("modules = %v", modules) + } +} + +func TestPlanRefusesImageUpstreamAndPackageByName(t *testing.T) { // 1607/AC2, ADR 0478 clause 2 + for _, c := range []struct { + artifact catalogue.Artifact + kind string + }{ + {catalogue.Artifact{Name: "img", Kind: "image"}, "image"}, + {catalogue.Artifact{Name: "up", Kind: "upstream"}, "upstream"}, + {catalogue.Artifact{Name: "pkg", Kind: "package", Language: "typescript"}, "package"}, + } { + m := manifestOf(catalogue.Artifact{Name: "ok", Kind: "archive"}, c.artifact) + p, err := PlanOf(m) + if err == nil { + t.Errorf("%s: plan %+v, want a refusal", c.kind, p) + continue + } + var r *Refused + if !asRefused(err, &r) || !strings.HasPrefix(err.Error(), RefusedPrefix) || !strings.Contains(err.Error(), c.kind) || !strings.Contains(err.Error(), "build-agent") { + t.Errorf("%s: error = %v, want it to open %q and name the kind and build-agent", c.kind, err, RefusedPrefix) + } + } +} + +func TestPlanRefusesABundleOfAnotherLanguageAndAModuleWithNoArtifact(t *testing.T) { + if _, err := PlanOf(manifestOf(catalogue.Artifact{Name: "x", Kind: "bundle", Language: "python"})); err == nil || !strings.HasPrefix(err.Error(), RefusedPrefix) { + t.Errorf("a python bundle: error = %v, want a refusal", err) + } + p, err := PlanOf(manifestOf()) + if err != nil || len(p.Tasks) != 0 { + t.Errorf("no artifact: plan %+v error %v, want an empty plan", p, err) + } +} + +func asRefused(err error, target **Refused) bool { return errors.As(err, target) } From 1e6d00b4c7558b236e95912d99c08a796fa76236 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:20:09 +0200 Subject: [PATCH 04/14] Claim a build key by create and compare-and-set, never twice, retry a failed key except a refused kind (issue 1607, AC3) --- .../internal/build/claim.go | 107 ++++++++++ .../internal/build/claim_test.go | 188 ++++++++++++++++++ 2 files changed, 295 insertions(+) create mode 100644 modules/mesh-delivery-builder/internal/build/claim.go create mode 100644 modules/mesh-delivery-builder/internal/build/claim_test.go diff --git a/modules/mesh-delivery-builder/internal/build/claim.go b/modules/mesh-delivery-builder/internal/build/claim.go new file mode 100644 index 0000000..0411c4d --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/claim.go @@ -0,0 +1,107 @@ +package build + +import ( + "context" + "errors" + "fmt" + "strings" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// ErrConflict is what a Store answers when a Create finds the key or an Update finds another revision than the one given. +var ErrConflict = errors.New("the key exists or changed") + +// Store is the builder's bucket (builds): create and compare-and-set, each answering the key's new revision. Only the +// builder's credential writes it (ADR 0429 clause 5). +type Store interface { + Get(ctx context.Context, key string) (value []byte, revision uint64, found bool, err error) + Create(ctx context.Context, key string, value []byte) (uint64, error) + Update(ctx context.Context, key string, value []byte, revision uint64) (uint64, error) +} + +// Claimer claims fingerprints for the builder instance By (ADR 0429 clause 5: a builder claims a fingerprint by creating +// its key and moves it by compare-and-set). +type Claimer struct { + Store Store + By string +} + +// Claim is a build the builder holds: its key, the revision the next write must be made at and the value last written. +type Claim struct { + Key string + Rev uint64 + Build delivery.BuilderBuild +} + +// Claim takes the build of module at fingerprint, or answers nil when it is not this builder's to build now: +// - no key: it is created building, by this builder; a create that finds the key made meanwhile gets nothing; +// - a done key is never taken again (the same source is never built twice); +// - a failed key is taken again as a new revision of the same key (ADR 0429 clause 18), except one failed for a kind no +// builder builds yet (RefusedPrefix, ADR 0478 clause 2), which waits for another fingerprint; +// - a key left queued or building by this builder is taken as it stands (ADR 0478 clause 5: a stopped builder takes its +// own again when it starts); one held by another builder is left, since nothing reclaims without presence. +func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string) (*Claim, error) { + key, err := delivery.BuilderBuildKey(module, fingerprint) + if err != nil { + return nil, err + } + building := delivery.BuilderBuild{State: delivery.BuildBuilding, By: c.By, Step: "claimed", Manifest: manifest} + value, rev, found, err := c.Store.Get(ctx, key) + if err != nil { + return nil, fmt.Errorf("%s: %w", key, err) + } + if !found { + return c.write(ctx, key, building, func(data []byte) (uint64, error) { return c.Store.Create(ctx, key, data) }) + } + cur, err := delivery.DecodeBuilderBuild(value) + if err != nil { + return nil, fmt.Errorf("%s holds what is not a build: %w", key, err) + } + switch { + case cur.State == delivery.BuildDone: + return nil, nil + case cur.State == delivery.BuildFailed && strings.HasPrefix(cur.Error, RefusedPrefix): + return nil, nil + case cur.State == delivery.BuildFailed: + return c.write(ctx, key, building, func(data []byte) (uint64, error) { return c.Store.Update(ctx, key, data, rev) }) + case cur.By == c.By: + return &Claim{Key: key, Rev: rev, Build: cur}, nil + } + return nil, nil +} + +func (c Claimer) write(_ context.Context, key string, b delivery.BuilderBuild, put func([]byte) (uint64, error)) (*Claim, error) { + data, err := delivery.EncodeBuilderBuild(b) + if err != nil { + return nil, err + } + rev, err := put(data) + if errors.Is(err, ErrConflict) { + return nil, nil // another builder made it first + } + if err != nil { + return nil, fmt.Errorf("%s: %w", key, err) + } + b.V = delivery.Version + return &Claim{Key: key, Rev: rev, Build: b}, nil +} + +// Finish moves the claimed key to b by compare-and-set at the claim's revision, and writes nothing when b is the value +// the key holds. A key that changed meanwhile is ErrConflict: the claim is no longer this builder's. +func (c Claimer) Finish(ctx context.Context, cl *Claim, b delivery.BuilderBuild) error { + b.V = delivery.Version + if delivery.SameBuilderBuild(cl.Build, b) { + return nil + } + data, err := delivery.EncodeBuilderBuild(b) + if err != nil { + return err + } + rev, err := c.Store.Update(ctx, cl.Key, data, cl.Rev) + if err != nil { + return fmt.Errorf("%s: %w", cl.Key, err) + } + cl.Rev, cl.Build = rev, b + return nil +} diff --git a/modules/mesh-delivery-builder/internal/build/claim_test.go b/modules/mesh-delivery-builder/internal/build/claim_test.go new file mode 100644 index 0000000..c6a8e36 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/claim_test.go @@ -0,0 +1,188 @@ +package build + +import ( + "context" + "errors" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// memStore is a bucket's create and compare-and-set in memory, with a revision per write. +type memStore struct { + vals map[string][]byte + revs map[string]uint64 + next uint64 + writes int + // before runs once ahead of the next write, so a test changes the key under the writer. + before func() +} + +func newMem() *memStore { return &memStore{vals: map[string][]byte{}, revs: map[string]uint64{}} } + +func (m *memStore) Get(_ context.Context, key string) ([]byte, uint64, bool, error) { + v, ok := m.vals[key] + return v, m.revs[key], ok, nil +} + +func (m *memStore) hook() { + if h := m.before; h != nil { + m.before = nil + h() + } +} + +func (m *memStore) put(key string, value []byte) uint64 { + m.next++ + m.writes++ + m.vals[key], m.revs[key] = value, m.next + return m.next +} + +func (m *memStore) Create(_ context.Context, key string, value []byte) (uint64, error) { + m.hook() + if _, ok := m.vals[key]; ok { + return 0, ErrConflict + } + return m.put(key, value), nil +} + +func (m *memStore) Update(_ context.Context, key string, value []byte, rev uint64) (uint64, error) { + m.hook() + if _, ok := m.vals[key]; !ok || m.revs[key] != rev { + return 0, ErrConflict + } + return m.put(key, value), nil +} + +const ( + fp1 = "src2:1111111111111111111111111111111111111111111111111111111111111111" + digest = "sha256:2222222222222222222222222222222222222222222222222222222222222222" +) + +func seed(t *testing.T, st *memStore, b delivery.BuilderBuild) string { + t.Helper() + key, err := delivery.BuilderBuildKey("m", fp1) + if err != nil { + t.Fatal(err) + } + data, err := delivery.EncodeBuilderBuild(b) + if err != nil { + t.Fatal(err) + } + st.put(key, data) + return key +} + +func valueOf(t *testing.T, st *memStore, key string) delivery.BuilderBuild { + t.Helper() + b, err := delivery.DecodeBuilderBuild(st.vals[key]) + if err != nil { + t.Fatal(err) + } + return b +} + +func TestClaimCreatesTheKeyBuildingAndASecondBuilderGetsNothing(t *testing.T) { // 1607/AC3 + st := newMem() + a, b := Claimer{Store: st, By: "novox.a"}, Claimer{Store: st, By: "novox.b"} + got, err := a.Claim(context.Background(), "m", fp1, hex64('a')) + if err != nil || got == nil { + t.Fatalf("first claim = %v, %v, want a claim", got, err) + } + if v := valueOf(t, st, got.Key); v.State != delivery.BuildBuilding || v.By != "novox.a" || v.Manifest != hex64('a') { + t.Errorf("the key holds %+v, want building by novox.a with the manifest digest", v) + } + if second, err := b.Claim(context.Background(), "m", fp1, hex64('a')); err != nil || second != nil { + t.Errorf("second builder's claim = %v, %v, want none: the same source is never built twice", second, err) + } +} + +func TestClaimLosingTheCreateRaceGetsNothing(t *testing.T) { // 1607/AC3 + st := newMem() + st.before = func() { seed(t, st, delivery.BuilderBuild{State: delivery.BuildBuilding, By: "novox.b"}) } + got, err := Claimer{Store: st, By: "novox.a"}.Claim(context.Background(), "m", fp1, "") + if err != nil || got != nil { + t.Errorf("claim = %v, %v, want none: the create failed because the key exists", got, err) + } +} + +func TestADoneKeyIsNeverTakenAgain(t *testing.T) { // 1607/AC3 + st := newMem() + seed(t, st, delivery.BuilderBuild{State: delivery.BuildDone, By: "novox.a", Manifest: hex64('a'), Artifacts: []delivery.BuiltArtifact{{Name: "x", Digest: digest}}}) + writes := st.writes + got, err := Claimer{Store: st, By: "novox.a"}.Claim(context.Background(), "m", fp1, hex64('a')) + if err != nil || got != nil || st.writes != writes { + t.Errorf("claim = %v, %v, writes %d (was %d), want none and no write", got, err, st.writes, writes) + } +} + +func TestAFailedKeyIsTakenAgainAsANewRevisionExceptARefusedKind(t *testing.T) { // 1607/AC3, ADR 0478 clause 2 + st := newMem() + key := seed(t, st, delivery.BuilderBuild{State: delivery.BuildFailed, By: "novox.a", Error: "the step failed"}) + rev := st.revs[key] + got, err := Claimer{Store: st, By: "novox.a"}.Claim(context.Background(), "m", fp1, hex64('a')) + if err != nil || got == nil || st.revs[key] <= rev { + t.Fatalf("claim = %v, %v, revision %d (was %d), want a claim as a new revision of the same key", got, err, st.revs[key], rev) + } + if v := valueOf(t, st, key); v.State != delivery.BuildBuilding || v.Error != "" { + t.Errorf("the key holds %+v, want building with no error", v) + } + + st = newMem() + key = seed(t, st, delivery.BuilderBuild{State: delivery.BuildFailed, By: "novox.a", Error: RefusedPrefix + " image (artifact x): build-agent builds it"}) + writes := st.writes + if got, err := (Claimer{Store: st, By: "novox.a"}).Claim(context.Background(), "m", fp1, ""); err != nil || got != nil || st.writes != writes { + t.Errorf("a refused kind: claim = %v, %v, writes %d (was %d), want none and no write", got, err, st.writes, writes) + } +} + +func TestABuildLeftBuildingBySameBuilderIsResumedAndAnotherBuildersIsLeft(t *testing.T) { // 1607/AC3, ADR 0478 clause 5 + st := newMem() + seed(t, st, delivery.BuilderBuild{State: delivery.BuildBuilding, By: "novox.a"}) + if got, err := (Claimer{Store: st, By: "novox.a"}).Claim(context.Background(), "m", fp1, ""); err != nil || got == nil { + t.Errorf("own building key: claim = %v, %v, want it taken again", got, err) + } + if got, err := (Claimer{Store: st, By: "novox.b"}).Claim(context.Background(), "m", fp1, ""); err != nil || got != nil { + t.Errorf("another builder's building key: claim = %v, %v, want none: nothing reclaims without presence", got, err) + } +} + +func TestFinishMovesTheKeyByCompareAndSetAndWritesOnlyADifferentValue(t *testing.T) { // 1607/AC3 + st := newMem() + c := Claimer{Store: st, By: "novox.a"} + got, _ := c.Claim(context.Background(), "m", fp1, hex64('a')) + done := delivery.BuilderBuild{State: delivery.BuildDone, By: "novox.a", Manifest: hex64('a'), Artifacts: []delivery.BuiltArtifact{{Name: "x", Kind: "archive", Digest: digest}}} + if err := c.Finish(context.Background(), got, done); err != nil { + t.Fatal(err) + } + if v := valueOf(t, st, got.Key); v.State != delivery.BuildDone || v.Artifacts[0].Digest != digest { + t.Errorf("the key holds %+v", v) + } + writes := st.writes + if err := c.Finish(context.Background(), got, done); err != nil && !errors.Is(err, ErrConflict) { + t.Fatal(err) + } + if st.writes != writes { + t.Errorf("the same value was written again (%d writes, was %d)", st.writes, writes) + } +} + +func TestFinishAfterTheKeyChangedIsAConflictNotAnOverwrite(t *testing.T) { // 1607/AC3 + st := newMem() + c := Claimer{Store: st, By: "novox.a"} + got, _ := c.Claim(context.Background(), "m", fp1, "") + st.put(got.Key, []byte(`{"v":1,"state":"failed","error":"someone else"}`)) + err := c.Finish(context.Background(), got, delivery.BuilderBuild{State: delivery.BuildFailed, Error: "mine"}) + if !errors.Is(err, ErrConflict) { + t.Errorf("Finish = %v, want ErrConflict", err) + } +} + +func hex64(c byte) string { + b := make([]byte, 64) + for i := range b { + b[i] = c + } + return string(b) +} From 74d11f95a248e81bd793f248fd81c4dc2b3378a2 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:20:31 +0200 Subject: [PATCH 05/14] Choose build nodes by the agreed toolchain and fewest running steps, and agree the two nodes' digests (issue 1607, AC4) --- .../internal/build/choose.go | 63 +++++++++++++++++++ .../internal/build/choose_test.go | 63 +++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 modules/mesh-delivery-builder/internal/build/choose.go create mode 100644 modules/mesh-delivery-builder/internal/build/choose_test.go diff --git a/modules/mesh-delivery-builder/internal/build/choose.go b/modules/mesh-delivery-builder/internal/build/choose.go new file mode 100644 index 0000000..c4cf30d --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/choose.go @@ -0,0 +1,63 @@ +package build + +import ( + "fmt" + "sort" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// ChooseNodes picks up to want build nodes for a language (ADR 0478 clause 3): the nodes whose toolchains key reports the +// version the nodes agree on, the ones with the fewest steps the builder has running there first, a tie broken by the +// node's name. It answers the toolchain they agree on. When nothing is reported, or the nodes report different versions +// (so the indexer agreed none), it fails naming the language and why. The build module's health is not read here: a node +// whose module refuses on its toolchain refuses the step, and the build fails naming it. +func ChooseNodes(language string, entries []delivery.NodeToolchain, running map[string]int, want int) ([]string, delivery.Toolchain2, error) { + tc, none := delivery.AgreedToolchain(entries) + if none != "" { + return nil, delivery.Toolchain2{}, fmt.Errorf("no %s build node qualifies: %s", language, none) + } + nodes := make([]string, 0, len(entries)) + seen := map[string]bool{} + for _, e := range entries { + if !seen[e.Node] { + seen[e.Node] = true + nodes = append(nodes, e.Node) + } + } + sort.Slice(nodes, func(i, j int) bool { + if running[nodes[i]] != running[nodes[j]] { + return running[nodes[i]] < running[nodes[j]] + } + return nodes[i] < nodes[j] + }) + if len(nodes) > want { + nodes = nodes[:want] + } + return nodes, tc, nil +} + +// Agree is the one digest every node built an artifact to (ADR 0478 clause 4), or an error naming each node and its +// digest when they differ. One node's digest agrees with itself. +func Agree(artifact string, byNode map[string]string) (string, error) { + nodes := make([]string, 0, len(byNode)) + for n := range byNode { + nodes = append(nodes, n) + } + sort.Strings(nodes) + var first string + for i, n := range nodes { + if i == 0 { + first = byNode[n] + continue + } + if byNode[n] != first { + msg := fmt.Sprintf("the nodes built %s to different digests:", artifact) + for _, n := range nodes { + msg += fmt.Sprintf(" %s %s;", n, byNode[n]) + } + return "", fmt.Errorf("%s", msg[:len(msg)-1]) + } + } + return first, nil +} diff --git a/modules/mesh-delivery-builder/internal/build/choose_test.go b/modules/mesh-delivery-builder/internal/build/choose_test.go new file mode 100644 index 0000000..c085dcd --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/choose_test.go @@ -0,0 +1,63 @@ +package build + +import ( + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +func tcs(language, version string, nodes ...string) []delivery.NodeToolchain { + var out []delivery.NodeToolchain + for _, n := range nodes { + out = append(out, delivery.NodeToolchain{Node: n, Toolchain: delivery.Toolchain{Language: language, Version: version, Recipe: "1"}}) + } + return out +} + +func TestChooseTakesTheAgreedNodesWithFewestRunningStepsTiesByName(t *testing.T) { // 1607/AC4, ADR 0478 clause 3 + entries := tcs("go", "go1.27.1", "novox", "ace", "g14", "shanks") + nodes, tc, err := ChooseNodes("go", entries, map[string]int{"novox": 3, "ace": 1, "g14": 1, "shanks": 0}, 2) + if err != nil { + t.Fatal(err) + } + if len(nodes) != 2 || nodes[0] != "shanks" || nodes[1] != "ace" { + t.Errorf("nodes = %v, want [shanks ace]: fewest steps first, a tie by name", nodes) + } + if tc.Version != "go1.27.1" || tc.Recipe != "1" || tc.Language != "go" { + t.Errorf("toolchain = %+v", tc) + } +} + +func TestChooseWithOneNodeAnswersOneNode(t *testing.T) { // 1607/AC4 + nodes, _, err := ChooseNodes("go", tcs("go", "go1.27.1", "novox"), nil, 2) + if err != nil || len(nodes) != 1 || nodes[0] != "novox" { + t.Errorf("nodes = %v, err %v, want [novox]", nodes, err) + } +} + +func TestChooseFailsNamingTheLanguageAndTheVersionsWhenNoNodeQualifies(t *testing.T) { // 1607/AC4 + _, _, err := ChooseNodes("typescript", nil, nil, 2) + if err == nil || !strings.Contains(err.Error(), "typescript") || !strings.Contains(err.Error(), delivery.ToolchainNoReport) { + t.Errorf("no report: error = %v, want it to name typescript and %q", err, delivery.ToolchainNoReport) + } + entries := append(tcs("go", "go1.27.1", "novox"), tcs("go", "go1.26.0", "ace")...) + _, _, err = ChooseNodes("go", entries, nil, 2) + if err == nil || !strings.Contains(err.Error(), "go1.27.1") || !strings.Contains(err.Error(), "go1.26.0") || !strings.Contains(err.Error(), "go") { + t.Errorf("disagreeing nodes: error = %v, want both versions named", err) + } +} + +func TestAgreeNeedsEqualDigestsAndNamesBothNodesWhenTheyDiffer(t *testing.T) { // 1607/AC4, ADR 0478 clause 4 + d, err := Agree("tool", map[string]string{"ace": digest, "novox": digest}) + if err != nil || d != digest { + t.Errorf("Agree = %q, %v, want %q", d, err, digest) + } + _, err = Agree("tool", map[string]string{"ace": digest, "novox": "sha256:" + hex64('9')}) + if err == nil || !strings.Contains(err.Error(), "ace") || !strings.Contains(err.Error(), "novox") || !strings.Contains(err.Error(), digest) || !strings.Contains(err.Error(), hex64('9')) || !strings.Contains(err.Error(), "tool") { + t.Errorf("differing digests: error = %v, want the artifact, both nodes and both digests", err) + } + if d, err := Agree("tool", map[string]string{"ace": digest}); err != nil || d != digest { + t.Errorf("one node: %q, %v", d, err) + } +} From 3298ad58d375826c1e58ccedd98ae817e63813a5 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:21:14 +0200 Subject: [PATCH 06/14] Run a build module's long step: call it on its node, read the result at its steps key, bound the wait (issue 1607, AC5) --- .../internal/build/steps.go | 67 ++++++++++++ .../internal/build/steps_test.go | 103 ++++++++++++++++++ 2 files changed, 170 insertions(+) create mode 100644 modules/mesh-delivery-builder/internal/build/steps.go create mode 100644 modules/mesh-delivery-builder/internal/build/steps_test.go diff --git a/modules/mesh-delivery-builder/internal/build/steps.go b/modules/mesh-delivery-builder/internal/build/steps.go new file mode 100644 index 0000000..3a84936 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/steps.go @@ -0,0 +1,67 @@ +package build + +import ( + "context" + "encoding/json" + "fmt" + "time" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// Caller calls a tool of a module on one named machine (mesh-sdk process.Conn.CallOn; ADR 0469 clause 3). +type Caller interface { + CallOn(node, key string, body any) (json.RawMessage, error) +} + +// Steps waits for the end of a build module's long step: the value of its key `.` in the module's steps state +// (ADR 0469 clause 2) once its state is not running. It is a watch of that key, not a poll; it returns when the step +// ends or ctx does. +type Steps interface { + Wait(ctx context.Context, module, key string) (delivery.Step, error) +} + +// Runner runs a build module's long steps: the call answers at once with the step's key, and the result is read from that +// key when the step ends (ADR 0469 clause 2). +type Runner struct { + Caller Caller + Steps Steps + // Bound is how long a step may run before the builder stops waiting for it. + Bound time.Duration +} + +// Run calls tool of the build module on node with args, waits for the step's end and answers its result. A step that +// failed or was refused is an error naming the node, the state and the reason. +func (r Runner) Run(ctx context.Context, module, node, tool string, args map[string]any) (json.RawMessage, error) { + raw, err := r.Caller.CallOn(node, module+"."+tool, args) + if err != nil { + return nil, fmt.Errorf("%s on %s: %w", tool, node, err) + } + var answer struct { + Step string `json:"step"` + Key string `json:"key"` + State string `json:"state"` + } + if err := json.Unmarshal(raw, &answer); err != nil { + return nil, fmt.Errorf("%s on %s answered what is not a step: %w", tool, node, err) + } + if answer.Key == "" { + return nil, fmt.Errorf("%s on %s answered no step key: %s", tool, node, raw) + } + if n, _, err := delivery.ParseStepKey(answer.Key); err != nil || n != node { + return nil, fmt.Errorf("%s on %s answered the step key %q, which is not a key of that node", tool, node, answer.Key) + } + wait, cancel := context.WithTimeoutCause(ctx, r.Bound, fmt.Errorf("the step %s did not end within %s", answer.Key, r.Bound)) + defer cancel() + step, err := r.Steps.Wait(wait, module, answer.Key) + if err != nil { + if cause := context.Cause(wait); cause != nil && ctx.Err() == nil { + err = cause + } + return nil, fmt.Errorf("%s on %s: %w", tool, node, err) + } + if step.State != delivery.StepPassed { + return nil, fmt.Errorf("%s on %s: the step %s %s: %s", tool, node, answer.Key, step.State, step.Reason) + } + return step.Result, nil +} diff --git a/modules/mesh-delivery-builder/internal/build/steps_test.go b/modules/mesh-delivery-builder/internal/build/steps_test.go new file mode 100644 index 0000000..f6963ac --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/steps_test.go @@ -0,0 +1,103 @@ +package build + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +type call struct { + node, key string + body any +} + +// fakeCaller answers a long tool as the build modules do: at once, with the step and its key. +type fakeCaller struct { + calls []call + answer string + err error +} + +func (f *fakeCaller) CallOn(node, key string, body any) (json.RawMessage, error) { + f.calls = append(f.calls, call{node, key, body}) + if f.err != nil { + return nil, f.err + } + return json.RawMessage(f.answer), nil +} + +// fakeSteps holds the steps values by `/`; Wait on a key it does not hold blocks until the context ends. +type fakeSteps struct{ steps map[string]delivery.Step } + +func (f *fakeSteps) Wait(ctx context.Context, module, key string) (delivery.Step, error) { + if s, ok := f.steps[module+"/"+key]; ok { + return s, nil + } + <-ctx.Done() + return delivery.Step{}, context.Cause(ctx) +} + +func stepAt(node, id, state, reason string, result string) delivery.Step { + s := delivery.Step{ID: id, Tool: "build_go_fetch", Node: node, State: state, Reason: reason} + if result != "" { + s.Result = json.RawMessage(result) + } + return s +} + +func TestRunCallsTheToolOnTheNodeAndReadsTheStepsKey(t *testing.T) { // 1607/AC5 + caller := &fakeCaller{answer: `{"step":"abc1","key":"ace.abc1","state":"running"}`} + steps := &fakeSteps{steps: map[string]delivery.Step{"mesh-build-go/ace.abc1": stepAt("ace", "abc1", delivery.StepPassed, "", `{"workspace":"w1","commit":"c"}`)}} + res, err := Runner{Caller: caller, Steps: steps, Bound: time.Second}.Run(context.Background(), "mesh-build-go", "ace", "build_go_fetch", map[string]any{"repository": "novox/r", "commit": "c"}) + if err != nil { + t.Fatal(err) + } + if len(caller.calls) != 1 || caller.calls[0].node != "ace" || caller.calls[0].key != "mesh-build-go.build_go_fetch" { + t.Errorf("calls = %+v, want one on ace for mesh-build-go.build_go_fetch", caller.calls) + } + var fr struct{ Workspace string } + if err := json.Unmarshal(res, &fr); err != nil || fr.Workspace != "w1" { + t.Errorf("result = %s (%v), want the step's result", res, err) + } +} + +func TestRunFailsOnAFailedOrRefusedStepNamingItsReason(t *testing.T) { // 1607/AC5 + for _, state := range []string{delivery.StepFailed, delivery.StepRefused} { + caller := &fakeCaller{answer: `{"step":"abc1","key":"ace.abc1","state":"running"}`} + steps := &fakeSteps{steps: map[string]delivery.Step{"mesh-build-go/ace.abc1": stepAt("ace", "abc1", state, "go is go1.26.0, the recipe builds with go1.27.1", "")}} + _, err := Runner{Caller: caller, Steps: steps, Bound: time.Second}.Run(context.Background(), "mesh-build-go", "ace", "build_go_build", nil) + if err == nil || !strings.Contains(err.Error(), "go1.27.1") || !strings.Contains(err.Error(), state) || !strings.Contains(err.Error(), "ace") { + t.Errorf("%s: error = %v, want the node, the state and the reason", state, err) + } + } +} + +func TestRunStopsWaitingAtTheBound(t *testing.T) { // 1607/AC5 + caller := &fakeCaller{answer: `{"step":"abc1","key":"ace.abc1","state":"running"}`} + _, err := Runner{Caller: caller, Steps: &fakeSteps{}, Bound: 20 * time.Millisecond}.Run(context.Background(), "mesh-build-go", "ace", "build_go_build", nil) + if err == nil || !strings.Contains(err.Error(), "ace.abc1") || !strings.Contains(err.Error(), "did not end") { + t.Errorf("error = %v, want the step key and that it did not end within its bound", err) + } +} + +func TestRunRefusesAnAnswerWithNoKeyAndAKeyOfAnotherNode(t *testing.T) { // 1607/AC5 + for _, answer := range []string{`{"state":"running"}`, `{"step":"abc1","key":"novox.abc1","state":"running"}`, `not json`} { + _, err := Runner{Caller: &fakeCaller{answer: answer}, Steps: &fakeSteps{}, Bound: time.Second}.Run(context.Background(), "mesh-build-go", "ace", "build_go_fetch", nil) + if err == nil { + t.Errorf("answer %s: no error", answer) + } + } +} + +func TestRunReturnsTheCallsOwnError(t *testing.T) { // 1607/AC5 + boom := errors.New("no responders") + _, err := Runner{Caller: &fakeCaller{err: boom}, Steps: &fakeSteps{}, Bound: time.Second}.Run(context.Background(), "mesh-build-go", "ace", "build_go_fetch", nil) + if !errors.Is(err, boom) || !strings.Contains(err.Error(), "ace") { + t.Errorf("error = %v, want the call's error naming the node", err) + } +} From 71bb5a5fb39a3ebd691c4b98c5e3467de0b9d5fa Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:21:46 +0200 Subject: [PATCH 07/14] Watch the build modules' toolchains and steps and keep the toolchains by node, with no timer (issue 1607, AC6) --- .../cmd/mesh-delivery-builder/bus.go | 151 ++++++++++++++++++ .../cmd/mesh-delivery-builder/toolchains.go | 104 ++++++++++++ .../internal/build/manifests.go | 12 ++ .../internal/build/toolchains.go | 64 ++++++++ .../internal/build/toolchains_test.go | 51 ++++++ 5 files changed, 382 insertions(+) create mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go create mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/toolchains.go create mode 100644 modules/mesh-delivery-builder/internal/build/manifests.go create mode 100644 modules/mesh-delivery-builder/internal/build/toolchains.go create mode 100644 modules/mesh-delivery-builder/internal/build/toolchains_test.go diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go new file mode 100644 index 0000000..5798b3a --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/bus.go @@ -0,0 +1,151 @@ +package main + +import ( + "context" + "errors" + "fmt" + + "github.com/nats-io/nats.go/jetstream" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/delivery" + + "mesh-delivery-builder/internal/build" +) + +// bucketOpener is the part of the bus the watches use. +type bucketOpener interface { + KeyValue(ctx context.Context, bucket string) (jetstream.KeyValue, error) +} + +// kvStore is the builds bucket as the builder's store: create and compare-and-set. +type kvStore struct{ kv jetstream.KeyValue } + +func conflict(err error) error { + if errors.Is(err, jetstream.ErrKeyExists) || errors.Is(err, jetstream.ErrKeyRevisionMismatch) { + return build.ErrConflict + } + return err +} + +func (s kvStore) Get(ctx context.Context, key string) ([]byte, uint64, bool, error) { + e, err := s.kv.Get(ctx, key) + if errors.Is(err, jetstream.ErrKeyNotFound) { + return nil, 0, false, nil + } + if err != nil { + return nil, 0, false, err + } + return e.Value(), e.Revision(), true, nil +} + +func (s kvStore) Create(ctx context.Context, key string, value []byte) (uint64, error) { + rev, err := s.kv.Create(ctx, key, value) + return rev, conflict(err) +} + +func (s kvStore) Update(ctx context.Context, key string, value []byte, revision uint64) (uint64, error) { + rev, err := s.kv.Update(ctx, key, value, revision) + return rev, conflict(err) +} + +func opOf(e jetstream.KeyValueEntry) bool { return e.Operation() == jetstream.KeyValuePut } + +// settle reads the entries a watcher holds now, up to its mark of the last values, and returns the watcher for the +// changes that follow. +func settle(ctx context.Context, kv jetstream.KeyValue, each func(jetstream.KeyValueEntry)) (jetstream.KeyWatcher, error) { + w, err := kv.WatchAll(ctx) + if err != nil { + return nil, err + } + for { + select { + case <-ctx.Done(): + w.Stop() + return nil, ctx.Err() + case e, ok := <-w.Updates(): + if !ok { + return nil, errors.New("the watch ended") + } + if e == nil { + return w, nil + } + each(e) + } + } +} + +// follow hands each change of a watcher to each until ctx ends; a watch that ends is an error, and the process manager +// starts the program again. +func follow(ctx context.Context, w jetstream.KeyWatcher, each func(jetstream.KeyValueEntry)) error { + defer w.Stop() + for { + select { + case <-ctx.Done(): + return nil + case e, ok := <-w.Updates(): + if !ok { + return errors.New("a watch ended") + } + if e != nil { + each(e) + } + } + } +} + +// stepsWaiter waits for a build module's step by watching its key in the module's steps bucket (ADR 0469 clause 2): the +// watch hands over the value the key holds now, so a step that ended before the watch began is read at once, and every +// change after. No timer: the caller bounds the wait with its context. +type stepsWaiter struct{ js bucketOpener } + +func (w stepsWaiter) Wait(ctx context.Context, module, key string) (delivery.Step, error) { + kv, err := w.js.KeyValue(ctx, delivery.StepsBucket(module)) + if err != nil { + return delivery.Step{}, fmt.Errorf("the bucket %s: %w", delivery.StepsBucket(module), err) + } + watch, err := kv.Watch(ctx, key) + if err != nil { + return delivery.Step{}, err + } + defer watch.Stop() + for { + select { + case <-ctx.Done(): + return delivery.Step{}, ctx.Err() + case e, ok := <-watch.Updates(): + if !ok { + return delivery.Step{}, errors.New("the watch of the step ended") + } + if e == nil || !opOf(e) { + continue + } + step, err := delivery.DecodeStep(e.Value()) + if err != nil { + return delivery.Step{}, fmt.Errorf("the step %s is not a step: %w", key, err) + } + if step.State != delivery.StepRunning { + return step, nil + } + } + } +} + +// manifestReader reads manifests. of the indexer and parses the manifest it holds. +type manifestReader struct{ kv jetstream.KeyValue } + +func (m manifestReader) Manifest(ctx context.Context, digest string) (catalogue.Manifest, error) { + key, err := delivery.ManifestsKey(digest) + if err != nil { + return catalogue.Manifest{}, err + } + e, err := m.kv.Get(ctx, key) + if err != nil { + return catalogue.Manifest{}, fmt.Errorf("manifests.%s: %w", key, err) + } + kept, err := delivery.DecodeManifest(e.Value()) + if err != nil { + return catalogue.Manifest{}, err + } + return catalogue.ParseManifest([]byte(kept.JSON)) +} diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/toolchains.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/toolchains.go new file mode 100644 index 0000000..a7afc4d --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/toolchains.go @@ -0,0 +1,104 @@ +package main + +import ( + "context" + "errors" + "fmt" + "log" + "strings" + "sync" + + "github.com/nats-io/nats.go/jetstream" + + "git.novox.be/novox/mesh-sdk/go/delivery" + + "mesh-delivery-builder/internal/build" +) + +// toolchainStreams are the stream each build module carries its toolchain.reported event on, as the bus names it +// (declared by the build modules, heard by this module through consumes). +var toolchainStreams = map[string]string{build.GoBuildModule: "TOOLCHAINS_GO", build.NodeBuildModule: "TOOLCHAINS_NODE"} + +// toolchainWatch keeps the build modules' toolchains in a build.ToolchainSet and tells the builder when one changed. A +// build module's bucket is made when it is assigned, after the builder started, and the builder holds no timer (ADR 0437 +// §8): it watches the buckets that exist at start, and a build module says each time it wrote its toolchains key +// (toolchain.reported, handled by Reported); a bucket not watched yet is then watched. +type toolchainWatch struct { + ctx context.Context + js bucketOpener + set *build.ToolchainSet + changed func(ctx context.Context) // a language's entries changed: the failed builds are tried again + logger *log.Logger + errs chan error + mu sync.Mutex + watched map[string]bool +} + +func newToolchainWatch(ctx context.Context, js bucketOpener, set *build.ToolchainSet, changed func(context.Context), logger *log.Logger) *toolchainWatch { + return &toolchainWatch{ctx: ctx, js: js, set: set, changed: changed, logger: logger, errs: make(chan error, len(build.LanguageOf)), watched: map[string]bool{}} +} + +// start watches the buckets that exist now; a missing one is said and waits for its build module's event. +func (w *toolchainWatch) start() error { + for module := range build.LanguageOf { + if err := w.ensure(module); err != nil { + return err + } + } + return nil +} + +func (w *toolchainWatch) ensure(module string) error { + w.mu.Lock() + defer w.mu.Unlock() + if w.watched[module] { + return nil + } + name := delivery.ToolchainsBucket(module) + kv, err := w.js.KeyValue(w.ctx, name) + switch { + case errors.Is(err, jetstream.ErrBucketNotFound): + w.logger.Printf("the bucket %s does not exist yet: it is watched when %s reports a toolchain", name, module) + return nil + case err != nil: + return fmt.Errorf("the bucket %s: %w", name, err) + } + watcher, err := settle(w.ctx, kv, func(e jetstream.KeyValueEntry) { w.set.Put(module, e.Key(), e.Value(), !opOf(e)) }) + if err != nil { + return err + } + w.watched[module] = true + go func() { + w.errs <- follow(w.ctx, watcher, func(e jetstream.KeyValueEntry) { + if w.set.Put(module, e.Key(), e.Value(), !opOf(e)) { + w.changed(w.ctx) + } + }) + }() + w.changed(w.ctx) // what the settle read is new to the builder + return nil +} + +// delivered is the part of a JetStream message the watch uses. +type delivered interface { + Subject() string + Ack() error + Term() error +} + +// Reported takes one toolchain.reported event of a build module in: the module is read off the subject, +// mesh.stream..event.toolchain.reported, and its bucket is watched if it is not. A subject of no build module is +// terminated; a failure stays unacknowledged, and the server delivers it again. +func (w *toolchainWatch) Reported(m delivered) { + module, _, _ := strings.Cut(strings.TrimPrefix(m.Subject(), "mesh.stream."), ".event.") + if _, known := build.LanguageOf[module]; !known || !strings.HasPrefix(m.Subject(), "mesh.stream.") { + w.logger.Printf("%s: not a build module's event, dropped", m.Subject()) + _ = m.Term() + return + } + if err := w.ensure(module); err != nil { + w.logger.Printf("%s: left for redelivery: %v", m.Subject(), err) + return + } + _ = m.Ack() +} diff --git a/modules/mesh-delivery-builder/internal/build/manifests.go b/modules/mesh-delivery-builder/internal/build/manifests.go new file mode 100644 index 0000000..dbdbc04 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/manifests.go @@ -0,0 +1,12 @@ +package build + +import ( + "context" + + "git.novox.be/novox/mesh-sdk/go/catalogue" +) + +// Manifests reads a module's manifest by its digest, the key of the indexer's manifests bucket. +type Manifests interface { + Manifest(ctx context.Context, digest string) (catalogue.Manifest, error) +} diff --git a/modules/mesh-delivery-builder/internal/build/toolchains.go b/modules/mesh-delivery-builder/internal/build/toolchains.go new file mode 100644 index 0000000..44af6e6 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/toolchains.go @@ -0,0 +1,64 @@ +package build + +import ( + "sync" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// LanguageOf is the language each build module's toolchains state holds (the indexer's ToolchainModules, reversed). +var LanguageOf = map[string]string{GoBuildModule: "go", NodeBuildModule: "typescript"} + +// ToolchainSet holds what the build modules' toolchains buckets hold: each node's entry of a language, as the watches +// put them in. Safe for use from the watches and from a build at once. +type ToolchainSet struct { + mu sync.Mutex + by map[string]map[string]delivery.Toolchain // language -> node -> entry +} + +// Put takes a change of a build module's toolchains bucket in: a put of a node's entry (key .) or its +// delete. It answers whether the language's entries changed (a node's version or recipe, or a node appearing or going). +// A key or a value that is not an entry of the module's language is left out. +func (s *ToolchainSet) Put(module, key string, value []byte, deleted bool) bool { + language, ok := LanguageOf[module] + if !ok { + return false + } + node, keyLanguage, err := delivery.ParseToolchainKey(key) + if err != nil || keyLanguage != language { + return false + } + s.mu.Lock() + defer s.mu.Unlock() + if deleted { + if _, had := s.by[language][node]; !had { + return false + } + delete(s.by[language], node) + return true + } + tc, err := delivery.DecodeToolchain(value) + if err != nil || tc.Language != language { + return false + } + if s.by == nil { + s.by = map[string]map[string]delivery.Toolchain{} + } + if s.by[language] == nil { + s.by[language] = map[string]delivery.Toolchain{} + } + cur, had := s.by[language][node] + s.by[language][node] = tc + return !had || cur.Version != tc.Version || cur.Recipe != tc.Recipe +} + +// Entries are the nodes' entries of a language, in no particular order. +func (s *ToolchainSet) Entries(language string) []delivery.NodeToolchain { + s.mu.Lock() + defer s.mu.Unlock() + var out []delivery.NodeToolchain + for node, tc := range s.by[language] { + out = append(out, delivery.NodeToolchain{Node: node, Toolchain: tc}) + } + return out +} diff --git a/modules/mesh-delivery-builder/internal/build/toolchains_test.go b/modules/mesh-delivery-builder/internal/build/toolchains_test.go new file mode 100644 index 0000000..83b95ca --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/toolchains_test.go @@ -0,0 +1,51 @@ +package build + +import ( + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +func tcValue(t *testing.T, language, version string) []byte { + t.Helper() + b, err := delivery.EncodeToolchain(delivery.Toolchain{Language: language, Version: version, Recipe: "1", At: time.Now()}) + if err != nil { + t.Fatal(err) + } + return b +} + +func TestToolchainSetKeepsEachNodesEntryByLanguageAndSaysWhenOneChanged(t *testing.T) { // 1607/AC6 + var s ToolchainSet + if !s.Put(GoBuildModule, "novox.go", tcValue(t, "go", "go1.27.1"), false) { + t.Error("a first entry did not count as a change") + } + if s.Put(GoBuildModule, "novox.go", tcValue(t, "go", "go1.27.1"), false) { + t.Error("the same entry again counted as a change") + } + s.Put(GoBuildModule, "ace.go", tcValue(t, "go", "go1.27.1"), false) + s.Put(NodeBuildModule, "ace.typescript", tcValue(t, "typescript", "tsc5"), false) + if got := s.Entries("go"); len(got) != 2 { + t.Errorf("go entries = %+v, want two", got) + } + if got := s.Entries("typescript"); len(got) != 1 || got[0].Node != "ace" || got[0].Version != "tsc5" { + t.Errorf("typescript entries = %+v", got) + } + if !s.Put(GoBuildModule, "ace.go", nil, true) || len(s.Entries("go")) != 1 { + t.Error("a delete did not remove the node's entry") + } +} + +func TestToolchainSetLeavesOutWhatIsNotAToolchainOfTheModulesLanguage(t *testing.T) { // 1607/AC6 + var s ToolchainSet + if s.Put(GoBuildModule, "novox.typescript", tcValue(t, "typescript", "x"), false) { + t.Error("a typescript toolchain in the go build module's bucket counted") + } + if s.Put(GoBuildModule, "novox.go", []byte("not json"), false) || s.Put("other-module", "novox.go", tcValue(t, "go", "x"), false) { + t.Error("garbage or an unknown module counted") + } + if len(s.Entries("go")) != 0 { + t.Error("an entry was kept") + } +} From 46c9f33d649b0bc2e2f91f634003a4767f1aecef Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:23:39 +0200 Subject: [PATCH 08/14] Build each new-form fingerprint through the build modules on two nodes and write its key done or failed (issue 1607, AC7, AC8) --- .../cmd/mesh-delivery-builder/main.go | 105 +++++- .../internal/build/claim_test.go | 8 + .../internal/build/run.go | 324 ++++++++++++++++++ .../internal/build/run_test.go | 295 ++++++++++++++++ .../internal/build/steps_test.go | 11 +- 5 files changed, 737 insertions(+), 6 deletions(-) create mode 100644 modules/mesh-delivery-builder/internal/build/run.go create mode 100644 modules/mesh-delivery-builder/internal/build/run_test.go 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 c023e2e..c85cfe4 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go @@ -19,11 +19,23 @@ import ( "os/signal" "strings" "syscall" + "time" "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" "git.novox.be/novox/mesh-sdk/go/delivery" + "git.novox.be/novox/mesh-sdk/go/process" + + "mesh-delivery-builder/internal/build" +) + +const ( + // defaultStepBound is how long the builder waits for one step of a build module (a fetch downloads the Go modules, a + // build compiles). + defaultStepBound = 30 * time.Minute + // nodesPerFingerprint is how many build nodes build each fingerprint until release R4e (ADR 0478 clause 4). + nodesPerFingerprint = 2 ) func main() { @@ -109,10 +121,95 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro if err != nil { return err } - if _, err := js.KeyValue(ctx, delivery.BuilderBuildsBucket); err != nil { - return fmt.Errorf("the bucket %s: %w", delivery.BuilderBuildsBucket, err) + bucket := func(name string) (jetstream.KeyValue, error) { + kv, err := js.KeyValue(ctx, name) + if err != nil { + return nil, fmt.Errorf("the bucket %s: %w", name, err) + } + return kv, nil + } + builds, err := bucket(delivery.BuilderBuildsBucket) + if err != nil { + return err + } + modules, err := bucket(delivery.ModulesBucket) + if err != nil { + return err + } + manifests, err := bucket(delivery.ManifestsBucket) + if err != nil { + return err + } + // Calls to the build modules go out as this module's own account, naming the machine (mesh-sdk process.CallOn); the + // buckets are read and written on the connection above. + account, err := process.ReadAccount(path) + if err != nil { + return err + } + caller, err := process.Connect(account, nats.Name(cred.Node+"."+cred.Module+".calls"), nats.RetryOnFailedConnect(true)) + if err != nil { + return err + } + defer caller.Close() + + bound := defaultStepBound + if v := strings.TrimSpace(getenv("MESH_BUILDER_STEP_BOUND")); v != "" { + if bound, err = time.ParseDuration(v); err != nil { + return fmt.Errorf("MESH_BUILDER_STEP_BOUND: %w", err) + } + } + tools := &build.ToolchainSet{} + b := &build.Builder{ + Claimer: build.Claimer{Store: kvStore{builds}, By: cred.Node + "." + cred.Module}, + Runner: build.Runner{Caller: caller, Steps: stepsWaiter{js}, Bound: bound}, + Toolchains: tools, + Manifests: manifestReader{manifests}, + Nodes: nodesPerFingerprint, + Logf: logger.Printf, + } + // The toolchains first, so a build that starts from the first modules value finds the nodes. + watch := newToolchainWatch(ctx, js, tools, b.Reconsider, logger) + if err := watch.start(); err != nil { + return err + } + // A build module says each time it wrote its toolchains key: its stream, and the builder's durable consumer on it, are + // made by the controller. Where the stream is not there yet the builder says so and is started again with the build + // module. + for _, module := range []string{build.GoBuildModule, build.NodeBuildModule} { + stream := toolchainStreams[module] + cons, err := js.Consumer(ctx, stream, cred.Node+"_"+cred.Module) + if err != nil { + logger.Printf("the consumer %s_%s on %s does not exist yet, so %s's reports are not heard until the builder starts again: %v", cred.Node, cred.Module, stream, module, err) + continue + } + cc, err := cons.Consume(func(m jetstream.Msg) { watch.Reported(m) }, + jetstream.ConsumeErrHandler(func(_ jetstream.ConsumeContext, err error) { logger.Printf("hearing the bus: %v", err) })) + if err != nil { + return err + } + defer cc.Stop() + } + each := func(e jetstream.KeyValueEntry) { + if opOf(e) { + b.Module(ctx, e.Key(), e.Value()) + } else { + b.Forget(e.Key()) + } + } + w, err := settle(ctx, modules, each) + if err != nil { + return err } logger.Printf("started on %s", cred.Node) - <-ctx.Done() - return nil + errs := make(chan error, 1) + go func() { errs <- follow(ctx, w, each) }() + select { + case <-ctx.Done(): + b.Wait() + return nil + case err := <-errs: + return err + case err := <-watch.errs: + return err + } } diff --git a/modules/mesh-delivery-builder/internal/build/claim_test.go b/modules/mesh-delivery-builder/internal/build/claim_test.go index c6a8e36..7589524 100644 --- a/modules/mesh-delivery-builder/internal/build/claim_test.go +++ b/modules/mesh-delivery-builder/internal/build/claim_test.go @@ -3,6 +3,7 @@ package build import ( "context" "errors" + "sync" "testing" "git.novox.be/novox/mesh-sdk/go/delivery" @@ -10,6 +11,7 @@ import ( // memStore is a bucket's create and compare-and-set in memory, with a revision per write. type memStore struct { + mu sync.Mutex vals map[string][]byte revs map[string]uint64 next uint64 @@ -21,6 +23,8 @@ type memStore struct { func newMem() *memStore { return &memStore{vals: map[string][]byte{}, revs: map[string]uint64{}} } func (m *memStore) Get(_ context.Context, key string) ([]byte, uint64, bool, error) { + m.mu.Lock() + defer m.mu.Unlock() v, ok := m.vals[key] return v, m.revs[key], ok, nil } @@ -40,6 +44,8 @@ func (m *memStore) put(key string, value []byte) uint64 { } func (m *memStore) Create(_ context.Context, key string, value []byte) (uint64, error) { + m.mu.Lock() + defer m.mu.Unlock() m.hook() if _, ok := m.vals[key]; ok { return 0, ErrConflict @@ -48,6 +54,8 @@ func (m *memStore) Create(_ context.Context, key string, value []byte) (uint64, } func (m *memStore) Update(_ context.Context, key string, value []byte, rev uint64) (uint64, error) { + m.mu.Lock() + defer m.mu.Unlock() m.hook() if _, ok := m.vals[key]; !ok || m.revs[key] != rev { return 0, ErrConflict diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go new file mode 100644 index 0000000..8e6c35a --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -0,0 +1,324 @@ +package build + +import ( + "context" + "encoding/json" + "fmt" + "sort" + "strings" + "sync" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// ToolchainEntries are the nodes' toolchain entries of a language (ToolchainSet). +type ToolchainEntries interface { + Entries(language string) []delivery.NodeToolchain +} + +// toolPrefix is the start of each build module's tool names: fetch and build. +var toolPrefix = map[string]string{GoBuildModule: "build_go_", NodeBuildModule: "build_node_"} + +// Builder builds, dark, every new-form fingerprint the indexer writes (novox/hq ADR 0453 clause 4, ADR 0478): it is +// handed each modules value as the watch of the indexer's bucket sees it, and for each module with a fingerprint and an +// artifact to build claims the fingerprint's build key, builds it through the build modules on the nodes +// the toolchains choose, and writes the key done or failed. It holds no timer; a build starts from a modules value or a +// change of the toolchains. +type Builder struct { + Claimer Claimer + Runner Runner + Toolchains ToolchainEntries + Manifests Manifests + // Nodes is how many build nodes build each fingerprint: two until release R4e (ADR 0478 clause 4). + Nodes int + // Limit is how many builds run at once; four when 0. + Limit int + Logf func(format string, args ...any) + + once sync.Once + sem chan struct{} + wg sync.WaitGroup + mu sync.Mutex + known map[string]delivery.Module // by modules key + inflight map[string]bool // by build key + running map[string]int // steps in flight, by `/` + plans map[string]planned // by manifest digest +} + +type planned struct { + plan Plan + err error +} + +func (b *Builder) init() { + b.once.Do(func() { + limit := b.Limit + if limit <= 0 { + limit = 4 + } + b.sem = make(chan struct{}, limit) + b.known, b.inflight, b.running, b.plans = map[string]delivery.Module{}, map[string]bool{}, map[string]int{}, map[string]planned{} + }) +} + +func (b *Builder) logf(format string, args ...any) { + if b.Logf != nil { + b.Logf(format, args...) + } +} + +// Wait returns when every build started so far has ended. +func (b *Builder) Wait() { b.wg.Wait() } + +// Module takes one value of the indexer's modules bucket in (the key is the module's name) and builds it if it is +// to be built. A value that is not a module is said and left. +func (b *Builder) Module(ctx context.Context, key string, value []byte) { + b.init() + m, err := delivery.DecodeModule(value) + if err != nil { + b.logf("modules.%s holds what is not a module, left: %v", key, err) + return + } + b.mu.Lock() + b.known[key] = m + b.mu.Unlock() + b.consider(ctx, key, m) +} + +// Forget takes a module out (its key was deleted). +func (b *Builder) Forget(key string) { + b.init() + b.mu.Lock() + delete(b.known, key) + b.mu.Unlock() +} + +// Reconsider looks at every module again: the toolchains changed, so a build that failed for them may build now. +func (b *Builder) Reconsider(ctx context.Context) { + b.init() + b.mu.Lock() + known := make(map[string]delivery.Module, len(b.known)) + for k, m := range b.known { + known[k] = m + } + b.mu.Unlock() + for _, k := range sortedKeys(known) { + b.consider(ctx, k, known[k]) + } +} + +func sortedKeys(m map[string]delivery.Module) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +// planOf is the plan of the manifest with this digest, read once: a manifest by digest never changes. A manifest that +// cannot be read is not kept, so it is read again when the module is. +func (b *Builder) planOf(ctx context.Context, digest string) (Plan, error, bool) { + b.mu.Lock() + p, ok := b.plans[digest] + b.mu.Unlock() + if ok { + return p.plan, p.err, true + } + m, err := b.Manifests.Manifest(ctx, digest) + if err != nil { + return Plan{}, fmt.Errorf("its manifest %s cannot be read: %w", digest, err), false + } + plan, err := PlanOf(m) + b.mu.Lock() + b.plans[digest] = planned{plan, err} + b.mu.Unlock() + return plan, err, true +} + +func (b *Builder) consider(ctx context.Context, module string, m delivery.Module) { + if m.FP2 == "" || m.Manifest == "" { + return + } + key, err := delivery.BuilderBuildKey(module, m.FP2) + if err != nil { + b.logf("modules.%s: %v", module, err) + return + } + b.mu.Lock() + if b.inflight[key] { + b.mu.Unlock() + return + } + b.inflight[key] = true + b.mu.Unlock() + b.wg.Add(1) + go func() { + defer b.wg.Done() + defer func() { b.mu.Lock(); delete(b.inflight, key); b.mu.Unlock() }() + b.sem <- struct{}{} + defer func() { <-b.sem }() + b.try(ctx, module, m) + }() +} + +// try claims the fingerprint and builds it, writing the key done or failed. A build that ends because ctx did is left +// building by this builder: it takes it again when it starts (ADR 0478 clause 5). +func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { + plan, planErr, read := b.planOf(ctx, m.Manifest) + if read && planErr == nil && len(plan.Tasks) == 0 { + return // nothing in the manifest for a builder to build + } + cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest) + if err != nil { + b.logf("%s: %v", module, err) + return + } + if cl == nil { + return + } + result := b.build(ctx, module, m, plan, planErr) + if ctx.Err() != nil { + return + } + result.By, result.Manifest = b.Claimer.By, m.Manifest + if err := b.Claimer.Finish(ctx, cl, result); err != nil { + b.logf("%s: the build could not be written: %v", cl.Key, err) + return + } + if result.State == delivery.BuildFailed { + b.logf("%s: failed: %s", cl.Key, result.Error) + } +} + +func failed(format string, args ...any) delivery.BuilderBuild { + return delivery.BuilderBuild{State: delivery.BuildFailed, Error: fmt.Sprintf(format, args...)} +} + +// lane is the build of a plan's tasks of one build module on one node. +type lane struct { + module, node string + tasks []Task + digests map[string]string // artifact -> digest + err error +} + +func (b *Builder) build(ctx context.Context, module string, m delivery.Module, plan Plan, planErr error) delivery.BuilderBuild { + switch { + case planErr != nil: + return failed("%s", planErr) + case m.Path == "": + return failed("the module's key names no directory (path), so the build tools cannot be told where %s lives", module) + } + repository := delivery.RepositoryName(m.Repository) + var lanes []*lane + used := map[string]delivery.ToolchainUsed{} + var steps []string + for _, bm := range plan.Modules() { + language := LanguageOf[bm] + nodes, tc, err := ChooseNodes(language, b.Toolchains.Entries(language), b.runningOn(bm), b.Nodes) + if err != nil { + return failed("%s", err) + } + used[language] = delivery.ToolchainUsed{Version: tc.Version, Recipe: tc.Recipe} + if len(nodes) == 1 { + steps = append(steps, fmt.Sprintf("%s on one node, %s", bm, nodes[0])) + } else { + sorted := append([]string(nil), nodes...) + sort.Strings(sorted) + steps = append(steps, fmt.Sprintf("%s on %s", bm, strings.Join(sorted, " and "))) + } + var tasks []Task + for _, t := range plan.Tasks { + if t.Module == bm { + tasks = append(tasks, t) + } + } + for _, n := range nodes { + lanes = append(lanes, &lane{module: bm, node: n, tasks: tasks, digests: map[string]string{}}) + } + } + var wg sync.WaitGroup + for _, l := range lanes { + wg.Add(1) + go func() { defer wg.Done(); b.runLane(ctx, l, repository, m, module) }() + } + wg.Wait() + var errs []string + for _, l := range lanes { + if l.err != nil { + errs = append(errs, l.err.Error()) + } + } + if len(errs) > 0 { + return failed("%s", strings.Join(errs, "; ")) + } + var artifacts []delivery.BuiltArtifact + for _, t := range plan.Tasks { + byNode := map[string]string{} + for _, l := range lanes { + if d, ok := l.digests[t.Artifact]; ok && l.module == t.Module { + byNode[l.node] = d + } + } + d, err := Agree(t.Artifact, byNode) + if err != nil { + return failed("%s", err) + } + artifacts = append(artifacts, delivery.BuiltArtifact{Name: t.Artifact, Kind: t.Kind, Digest: d}) + } + return delivery.BuilderBuild{State: delivery.BuildDone, Step: "built " + strings.Join(steps, "; "), Artifacts: artifacts, Toolchains: used} +} + +func (b *Builder) runningOn(buildModule string) map[string]int { + b.mu.Lock() + defer b.mu.Unlock() + out := map[string]int{} + for k, n := range b.running { + if mod, node, _ := strings.Cut(k, "/"); mod == buildModule { + out[node] = n + } + } + return out +} + +// step runs one step of a build module, counting it as running on the node while it does. +func (b *Builder) step(ctx context.Context, bm, node, tool string, args map[string]any) (json.RawMessage, error) { + k := bm + "/" + node + b.mu.Lock() + b.running[k]++ + b.mu.Unlock() + defer func() { b.mu.Lock(); b.running[k]--; b.mu.Unlock() }() + return b.Runner.Run(ctx, bm, node, toolPrefix[bm]+tool, args) +} + +// runLane fetches the commit on the node and builds each of the lane's artifacts there. +func (b *Builder) runLane(ctx context.Context, l *lane, repository string, m delivery.Module, module string) { + raw, err := b.step(ctx, l.module, l.node, "fetch", map[string]any{"repository": repository, "commit": m.Commit}) + if err != nil { + l.err = err + return + } + var fetched struct { + Workspace string `json:"workspace"` + } + if err := json.Unmarshal(raw, &fetched); err != nil || fetched.Workspace == "" { + l.err = fmt.Errorf("the fetch on %s answered no workspace: %s", l.node, raw) + return + } + for _, t := range l.tasks { + raw, err := b.step(ctx, l.module, l.node, "build", map[string]any{"workspace": fetched.Workspace, "module": m.Path, "artifact": t.Artifact}) + if err != nil { + l.err = err + return + } + var built struct { + Digest string `json:"digest"` + } + if err := json.Unmarshal(raw, &built); err != nil || built.Digest == "" { + l.err = fmt.Errorf("the build of %s of %s on %s answered no digest: %s", t.Artifact, module, l.node, raw) + return + } + l.digests[t.Artifact] = built.Digest + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run_test.go b/modules/mesh-delivery-builder/internal/build/run_test.go new file mode 100644 index 0000000..fb71751 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -0,0 +1,295 @@ +package build + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "sync" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// buildModules simulates the two build modules on the nodes: a long tool answers a step and key at once and writes the +// step's end into the steps (as mesh-build-go and mesh-build-node do), with a digest by node and artifact, or a failure. +type buildModules struct { + mu sync.Mutex + n int + calls []call + steps *fakeSteps + digests map[string]string // `/` -> digest; the default is `digest` + fails map[string]string // `/` -> the reason the step fails with +} + +func newBuildModules() *buildModules { + return &buildModules{steps: &fakeSteps{steps: map[string]delivery.Step{}}, digests: map[string]string{}, fails: map[string]string{}} +} + +func (b *buildModules) CallOn(node, key string, body any) (json.RawMessage, error) { + b.mu.Lock() + defer b.mu.Unlock() + b.calls = append(b.calls, call{node, key, body}) + b.n++ + id := fmt.Sprintf("s%d", b.n) + module, tool, _ := strings.Cut(key, ".") + args, _ := body.(map[string]any) + step := delivery.Step{ID: id, Tool: tool, Node: node, State: delivery.StepPassed} + if reason, ok := b.fails[node+"/"+tool]; ok { + step.State, step.Reason = delivery.StepFailed, reason + } else if strings.HasSuffix(tool, "_fetch") { + step.Result = json.RawMessage(`{"workspace":"w-` + node + `"}`) + } else { + d := digest + if v, ok := b.digests[node+"/"+fmt.Sprint(args["artifact"])]; ok { + d = v + } + step.Result = json.RawMessage(`{"digest":"` + d + `"}`) + } + sk, _ := delivery.StepKey(node, id) + b.steps.mu.Lock() + b.steps.steps[module+"/"+sk] = step + b.steps.mu.Unlock() + return json.RawMessage(`{"step":"` + id + `","key":"` + sk + `","state":"running"}`), nil +} + +func (b *buildModules) callsOf(tool string) (out []call) { + b.mu.Lock() + defer b.mu.Unlock() + for _, c := range b.calls { + if strings.HasSuffix(c.key, "."+tool) { + out = append(out, c) + } + } + return out +} + +type fakeManifests struct { + byDigest map[string]catalogue.Manifest +} + +func (f *fakeManifests) Manifest(_ context.Context, d string) (catalogue.Manifest, error) { + m, ok := f.byDigest[d] + if !ok { + return catalogue.Manifest{}, fmt.Errorf("manifests.%s is not there", d) + } + return m, nil +} + +func mdigest(c byte) string { return hex64(c) } + +type rig struct { + b *Builder + st *memStore + bm *buildModules + tools *ToolchainSet + mf *fakeManifests + key string +} + +func newRig(t *testing.T, artifacts ...catalogue.Artifact) *rig { + t.Helper() + r := &rig{st: newMem(), bm: newBuildModules(), tools: &ToolchainSet{}, mf: &fakeManifests{byDigest: map[string]catalogue.Manifest{mdigest('a'): {Module: "m", Build: &catalogue.Build{Artifacts: artifacts}}}}} + for _, n := range []string{"ace", "novox"} { + r.tools.Put(GoBuildModule, n+".go", tcValue(t, "go", "go1.27.1"), false) + r.tools.Put(NodeBuildModule, n+".typescript", tcValue(t, "typescript", "tsc5"), false) + } + r.b = &Builder{ + Claimer: Claimer{Store: r.st, By: "novox.builder"}, Runner: Runner{Caller: r.bm, Steps: r.bm.steps, Bound: time.Second}, + Toolchains: r.tools, Manifests: r.mf, Nodes: 2, Logf: t.Logf, + } + var err error + if r.key, err = delivery.BuilderBuildKey("m", fp1); err != nil { + t.Fatal(err) + } + return r +} + +func moduleValue(t *testing.T, path string) []byte { + t.Helper() + data, err := delivery.EncodeModule(delivery.Module{Repository: "novox/mesh-catalog", Commit: "7a74cee83f88e6585a5511c3cc53bdf0f5303d20", FP: "src1:" + hex64('f'), FP2: fp1, Manifest: mdigest('a'), Path: path}) + if err != nil { + t.Fatal(err) + } + return data +} + +func (r *rig) consider(t *testing.T, path string) delivery.BuilderBuild { + t.Helper() + r.b.Module(context.Background(), "m", moduleValue(t, path)) + r.b.Wait() + if _, ok := r.st.vals[r.key]; !ok { + t.Fatal("no build key was written") + } + return valueOf(t, r.st, r.key) +} + +var ( + goBundle = catalogue.Artifact{Name: "tool", Kind: "bundle", Language: "go"} + tsBundle = catalogue.Artifact{Name: "web", Kind: "bundle", Language: "typescript"} + archiveA = catalogue.Artifact{Name: "files", Kind: "archive"} + imageArt = catalogue.Artifact{Name: "img", Kind: "image"} + manifestD = mdigest('a') +) + +func TestABundleAndAnArchiveAreBuiltOnTwoNodesAndDoneWhenTheDigestsAgree(t *testing.T) { // 1607/AC7, ADR 0478 clauses 1, 4 + r := newRig(t, goBundle, archiveA, tsBundle) + v := r.consider(t, "modules/m") + if v.State != delivery.BuildDone || v.By != "novox.builder" || v.Manifest != manifestD || len(v.Artifacts) != 3 { + t.Fatalf("build = %+v, want done by novox.builder with three artifacts", v) + } + for i, want := range []string{"files", "tool", "web"} { + if a := v.Artifacts[i]; a.Name != want || a.Digest != digest || a.Reference != "" { + t.Errorf("artifact %d = %+v, want %s with the digest and no reference (ADR 0478 clause 6)", i, a, want) + } + } + if v.Artifacts[0].Kind != "archive" || v.Artifacts[1].Kind != "bundle" { + t.Errorf("kinds = %+v", v.Artifacts) + } + if got := v.Toolchains["go"]; got.Version != "go1.27.1" || got.Recipe != "1" { + t.Errorf("toolchains = %+v, want go and typescript named", v.Toolchains) + } + if got := v.Toolchains["typescript"]; got.Version != "tsc5" { + t.Errorf("toolchains = %+v", v.Toolchains) + } + // Two fetches per build module, one on each node; every build names the module's directory. + if n := len(r.bm.callsOf("build_go_fetch")); n != 2 { + t.Errorf("%d go fetches, want one per node", n) + } + if n := len(r.bm.callsOf("build_go_build")); n != 4 { // tool and files, each on two nodes + t.Errorf("%d go builds, want 4", n) + } + if n := len(r.bm.callsOf("build_node_build")); n != 2 { + t.Errorf("%d node builds, want 2", n) + } + c := r.bm.callsOf("build_go_build")[0] + if args := c.body.(map[string]any); args["module"] != "modules/m" || !strings.HasPrefix(fmt.Sprint(args["workspace"]), "w-") { + t.Errorf("build args = %+v, want the module's path and the fetch's workspace", args) + } + f := r.bm.callsOf("build_go_fetch")[0].body.(map[string]any) + if f["repository"] != "novox/mesh-catalog" || f["commit"] != "7a74cee83f88e6585a5511c3cc53bdf0f5303d20" { + t.Errorf("fetch args = %+v", f) + } +} + +func TestDifferingDigestsFailTheBuildNamingBothNodes(t *testing.T) { // 1607/AC4, ADR 0478 clause 4 + r := newRig(t, goBundle) + r.bm.digests["ace/tool"] = "sha256:" + hex64('9') + v := r.consider(t, "modules/m") + if v.State != delivery.BuildFailed || !strings.Contains(v.Error, "ace") || !strings.Contains(v.Error, "novox") || !strings.Contains(v.Error, digest) || !strings.Contains(v.Error, hex64('9')) { + t.Errorf("build = %+v, want failed naming both nodes and both digests", v) + } +} + +func TestOneQualifyingNodeBuildsOnceAndTheKeySaysSo(t *testing.T) { // 1607/AC4, ADR 0478 clause 4 + r := newRig(t, goBundle) + r.tools.Put(GoBuildModule, "ace.go", nil, true) + v := r.consider(t, "modules/m") + if v.State != delivery.BuildDone || !strings.Contains(v.Step, "one node") || !strings.Contains(v.Step, "novox") { + t.Errorf("build = %+v, want done with its step saying one node, novox", v) + } + if n := len(r.bm.callsOf("build_go_build")); n != 1 { + t.Errorf("%d builds, want one", n) + } +} + +func TestAFailingStepFailsTheBuildWithItsReasonAndNodeAndIsTriedAgainAsANewRevision(t *testing.T) { // 1607/AC7, ADR 0429 clause 18 + r := newRig(t, goBundle) + r.bm.fails["ace/build_go_build"] = "go build of tool: undefined: x" + v := r.consider(t, "modules/m") + if v.State != delivery.BuildFailed || !strings.Contains(v.Error, "ace") || !strings.Contains(v.Error, "undefined: x") { + t.Fatalf("build = %+v, want failed with the node and the reason", v) + } + rev := r.st.revs[r.key] + delete(r.bm.fails, "ace/build_go_build") + v = r.consider(t, "modules/m") + if v.State != delivery.BuildDone || r.st.revs[r.key] <= rev { + t.Errorf("build = %+v at revision %d (was %d), want done as a new revision of the same key", v, r.st.revs[r.key], rev) + } +} + +func TestAModuleWithAnImageGetsARefusedKeyNeverBuiltAgain(t *testing.T) { // 1607/AC2, ADR 0478 clause 2 + r := newRig(t, archiveA, imageArt) + v := r.consider(t, "modules/m") + if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, RefusedPrefix) || !strings.Contains(v.Error, "image") || !strings.Contains(v.Error, "build-agent") { + t.Fatalf("build = %+v, want failed 'refused kind: image' naming build-agent", v) + } + if len(r.bm.calls) != 0 { + t.Errorf("%d calls, want none for a refused kind", len(r.bm.calls)) + } + writes := r.st.writes + r.consider(t, "modules/m") + if r.st.writes != writes { + t.Errorf("a refused key was written again (%d writes, was %d)", r.st.writes, writes) + } +} + +func TestAModuleKeyWithNoPathFailsNamingThatReason(t *testing.T) { // 1607/AC8, ADR 0478 clause 8 + r := newRig(t, goBundle) + v := r.consider(t, "") + if v.State != delivery.BuildFailed || !strings.Contains(v.Error, "path") || len(r.bm.calls) != 0 { + t.Errorf("build = %+v with %d calls, want failed naming the missing path and nothing called", v, len(r.bm.calls)) + } +} + +func TestNoQualifyingNodeFailsTheBuildNamingTheLanguage(t *testing.T) { // 1607/AC4 + r := newRig(t, tsBundle) + r.tools.Put(NodeBuildModule, "ace.typescript", nil, true) + r.tools.Put(NodeBuildModule, "novox.typescript", nil, true) + v := r.consider(t, "modules/m") + if v.State != delivery.BuildFailed || !strings.Contains(v.Error, "typescript") || !strings.Contains(v.Error, delivery.ToolchainNoReport) { + t.Errorf("build = %+v, want failed naming typescript and that no toolchain is reported", v) + } +} + +func TestAnUnreadableManifestFailsTheBuild(t *testing.T) { + r := newRig(t, goBundle) + delete(r.mf.byDigest, mdigest('a')) + v := r.consider(t, "modules/m") + if v.State != delivery.BuildFailed || !strings.Contains(v.Error, "manifest") { + t.Errorf("build = %+v, want failed naming the manifest", v) + } +} + +func TestAModuleWithNoFingerprintOrNoArtifactIsNotBuilt(t *testing.T) { // 1607/AC2 + r := newRig(t) + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) + data, _ := delivery.EncodeModule(delivery.Module{Repository: "novox/r", Commit: "c", FP: "src1:" + hex64('f'), None2: "toolchain not yet reported"}) + r.b.Module(context.Background(), "n", data) + r.b.Wait() + if len(r.st.vals) != 0 { + t.Errorf("keys %v were written: a module with no new-form fingerprint or no artifact has nothing to build", r.st.vals) + } +} + +func TestAKeyTheBuilderLeftBuildingIsBuiltAtStartAndDoneKeysAreNotBuiltTwice(t *testing.T) { // 1607/AC3, AC7 + r := newRig(t, goBundle) + seed(t, r.st, delivery.BuilderBuild{State: delivery.BuildBuilding, By: "novox.builder", Manifest: manifestD}) + if v := r.consider(t, "modules/m"); v.State != delivery.BuildDone { + t.Fatalf("build = %+v, want the builder's own building key built", v) + } + calls := len(r.bm.calls) + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) + r.b.Reconsider(context.Background()) + r.b.Wait() + if len(r.bm.calls) != calls { + t.Errorf("a done key was built again (%d calls, was %d)", len(r.bm.calls), calls) + } +} + +func TestTwoInstancesBuildOneFingerprintOnce(t *testing.T) { // 1607/AC7 + r := newRig(t, goBundle) + other := &Builder{Claimer: Claimer{Store: r.st, By: "ace.builder"}, Runner: r.b.Runner, Toolchains: r.tools, Manifests: r.mf, Nodes: 2, Logf: t.Logf} + value := moduleValue(t, "modules/m") + var wg sync.WaitGroup + for _, b := range []*Builder{r.b, other} { + wg.Add(1) + go func() { defer wg.Done(); b.Module(context.Background(), "m", value); b.Wait() }() + } + wg.Wait() + if n := len(r.bm.callsOf("build_go_build")); n != 2 { // one instance, two nodes + t.Errorf("%d builds, want 2: one instance built it", n) + } +} diff --git a/modules/mesh-delivery-builder/internal/build/steps_test.go b/modules/mesh-delivery-builder/internal/build/steps_test.go index f6963ac..4a39fb9 100644 --- a/modules/mesh-delivery-builder/internal/build/steps_test.go +++ b/modules/mesh-delivery-builder/internal/build/steps_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "strings" + "sync" "testing" "time" @@ -32,10 +33,16 @@ func (f *fakeCaller) CallOn(node, key string, body any) (json.RawMessage, error) } // fakeSteps holds the steps values by `/`; Wait on a key it does not hold blocks until the context ends. -type fakeSteps struct{ steps map[string]delivery.Step } +type fakeSteps struct { + mu sync.Mutex + steps map[string]delivery.Step +} func (f *fakeSteps) Wait(ctx context.Context, module, key string) (delivery.Step, error) { - if s, ok := f.steps[module+"/"+key]; ok { + f.mu.Lock() + s, ok := f.steps[module+"/"+key] + f.mu.Unlock() + if ok { return s, nil } <-ctx.Done() From 0b0cfb6e37f908aa943d7e7397c635068fef08de Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:25:13 +0200 Subject: [PATCH 09/14] Test the builder whole over the real bus with fake build modules, and look again at a value that arrives while its key is built (issue 1607, AC6, AC7) --- .../mesh-delivery-builder/integration_test.go | 366 ++++++++++++++++++ .../catalogue-a67f0196-ca-trust.module.json | 66 ++++ .../catalogue-a67f0196-netcheck.module.json | 57 +++ .../internal/build/run.go | 19 +- .../internal/build/run_test.go | 27 ++ 5 files changed, 532 insertions(+), 3 deletions(-) create mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go create mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-ca-trust.module.json create mode 100644 modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-netcheck.module.json 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 new file mode 100644 index 0000000..eeaa473 --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go @@ -0,0 +1,366 @@ +//go:build integration + +package main + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/nats-io/nats-server/v2/server" + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// busConf is the shape of the composed accounts file (mesh-controller internal/broker): one account with JetStream, the +// controller's user and the builder's own principals . (two instances, for the test of one build). +const busConf = ` +listen: 127.0.0.1:-1 +jetstream { store_dir: %q } +accounts { + MESH { + jetstream: enabled + users = [ + { user: "ctl", password: "pw" } + { user: "n1.mesh-delivery-builder", password: "pw" } + { user: "n2.mesh-delivery-builder", password: "pw" } + ] + } +} +` + +const ( + commit = "7a74cee83f88e6585a5511c3cc53bdf0f5303d20" + fp = "src2:3333333333333333333333333333333333333333333333333333333333333333" +) + +// fakeBuildModules serve the tools of mesh-build-go and mesh-build-node as the real ones do (ADR 0469 clause 2): a call on +// mesh.mod..call... answers at once with the step and its key, and the step's end is written to +// the key . of the module's steps bucket, some milliseconds later. +type fakeBuildModules struct { + js jetstream.JetStream + ctx context.Context + n atomic.Int64 + failBuild atomic.Bool + mu sync.Mutex + calls map[string]int // by `.` +} + +func (f *fakeBuildModules) count(tool string) int { + f.mu.Lock() + defer f.mu.Unlock() + return f.calls[tool] +} + +// digestOf is what every node builds an artifact to. +func digestOf(artifact string) string { + sum := sha256.Sum256([]byte(artifact)) + return "sha256:" + hex.EncodeToString(sum[:]) +} + +func (f *fakeBuildModules) serve(t *testing.T, nc *nats.Conn, module string) { + steps, err := f.js.CreateKeyValue(f.ctx, jetstream.KeyValueConfig{Bucket: delivery.StepsBucket(module), History: 1}) + if err != nil { + t.Fatal(err) + } + sub, err := nc.Subscribe("mesh.mod."+module+".call.>", func(m *nats.Msg) { + tokens := strings.Split(m.Subject, ".") // mesh mod call + tool, node := tokens[4], tokens[5] + f.mu.Lock() + f.calls[module+"."+tool]++ + f.mu.Unlock() + var args map[string]any + _ = json.Unmarshal(m.Data, &args) + id := fmt.Sprintf("s%d", f.n.Add(1)) + key, _ := delivery.StepKey(node, id) + put := func(s delivery.Step) { + data, err := delivery.EncodeStep(s) + if err == nil { + _, err = steps.Put(f.ctx, key, data) + } + if err != nil { + t.Errorf("writing step %s: %v", key, err) + } + } + step := delivery.Step{ID: id, Tool: tool, Node: node, State: delivery.StepRunning, Started: time.Now().UTC()} + put(step) + answer, _ := json.Marshal(map[string]any{"result": map[string]any{"step": id, "key": key, "state": "running"}, "error": "", "node": node}) + _ = m.Respond(answer) + go func() { + time.Sleep(30 * time.Millisecond) + step.Ended = time.Now().UTC() + switch { + case strings.HasSuffix(tool, "_fetch"): + step.State, step.Result = delivery.StepPassed, json.RawMessage(`{"workspace":"w`+id+`","commit":"`+commit+`"}`) + case f.failBuild.Load() && node == "n1": + step.State, step.Reason = delivery.StepFailed, fmt.Sprintf("go build of %v: boom", args["artifact"]) + default: + step.State, step.Result = delivery.StepPassed, json.RawMessage(`{"digest":"`+digestOf(fmt.Sprint(args["artifact"]))+`"}`) + } + put(step) + }() + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = sub.Unsubscribe() }) +} + +type bed struct { + ctx context.Context + js jetstream.JetStream + modules jetstream.KeyValue + builds jetstream.KeyValue + tools map[string]jetstream.KeyValue + fake *fakeBuildModules + srvURL string + dir string +} + +// startBed starts the bus, the buckets the controller makes from the builder's and the build modules' manifests, the +// build modules' streams and the builder's durable consumers on them, two nodes' toolchains, the indexer's manifests, and +// the fake build modules. It runs no builder. +func startBed(t *testing.T) *bed { + dir, err := os.MkdirTemp(os.Getenv("TMPDIR"), "b") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(dir) }) + conf := filepath.Join(dir, "bus.conf") + if err := os.WriteFile(conf, []byte(fmt.Sprintf(busConf, filepath.Join(dir, "js"))), 0o600); err != nil { + t.Fatal(err) + } + opts, err := server.ProcessConfigFile(conf) + if err != nil { + t.Fatal(err) + } + opts.NoLog, opts.NoSigs = true, true + srv, err := server.NewServer(opts) + if err != nil { + t.Fatal(err) + } + go srv.Start() + if !srv.ReadyForConnections(10 * time.Second) { + t.Fatal("the bus did not come up") + } + t.Cleanup(srv.Shutdown) + ctl, err := nats.Connect(srv.ClientURL(), nats.UserInfo("ctl", "pw")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(ctl.Close) + js, err := jetstream.New(ctl) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + t.Cleanup(cancel) + b := &bed{ctx: ctx, js: js, tools: map[string]jetstream.KeyValue{}, srvURL: srv.ClientURL(), dir: dir} + for module, stream := range toolchainStreams { + if _, err := js.CreateStream(ctx, jetstream.StreamConfig{Name: stream, Subjects: []string{"mesh.stream." + module + ".event.toolchain.reported"}}); err != nil { + t.Fatal(err) + } + for _, node := range []string{"n1", "n2"} { + if _, err := js.CreateOrUpdateConsumer(ctx, stream, jetstream.ConsumerConfig{Durable: node + "_mesh-delivery-builder", AckPolicy: jetstream.AckExplicitPolicy}); err != nil { + t.Fatal(err) + } + } + kv, err := js.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: delivery.ToolchainsBucket(module), History: 1}) + if err != nil { + t.Fatal(err) + } + b.tools[module] = kv + } + mk := func(name string, history int) jetstream.KeyValue { + kv, err := js.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: name, History: uint8(history)}) + if err != nil { + t.Fatal(err) + } + return kv + } + b.modules = mk(delivery.ModulesBucket, delivery.ModulesHistory) + mk(delivery.ManifestsBucket, delivery.ManifestsHistory) + b.builds = mk(delivery.BuilderBuildsBucket, delivery.BuilderBuildsHistory) + b.fake = &fakeBuildModules{js: js, ctx: ctx, calls: map[string]int{}} + b.fake.serve(t, ctl, "mesh-build-go") + b.fake.serve(t, ctl, "mesh-build-node") + b.report(t, "mesh-build-go", "n1", "go", "go1.27.1", "1") + b.report(t, "mesh-build-go", "n2", "go", "go1.27.1", "1") + b.report(t, "mesh-build-node", "n1", "typescript", "tsc5", "1") + b.report(t, "mesh-build-node", "n2", "typescript", "tsc5", "1") + return b +} + +// report writes a node's toolchains key of a build module and says so on the module's stream, as the module does. +func (b *bed) report(t *testing.T, module, node, language, version, recipe string) { + t.Helper() + data, err := delivery.EncodeToolchain(delivery.Toolchain{Language: language, Version: version, Recipe: recipe, At: time.Now().UTC()}) + if err != nil { + t.Fatal(err) + } + key, _ := delivery.ToolchainKey(node, language) + if _, err := b.tools[module].Put(b.ctx, key, data); err != nil { + t.Fatal(err) + } + if _, err := b.js.Publish(b.ctx, "mesh.stream."+module+".event.toolchain.reported", []byte(`{}`)); err != nil { + t.Fatal(err) + } +} + +// module writes modules. as the indexer does for a module with a new-form fingerprint, and its manifest by digest. +func (b *bed) module(t *testing.T, name, fixture, path string) string { + t.Helper() + raw, err := os.ReadFile(filepath.Join("testdata", fixture)) + if err != nil { + t.Fatal(err) + } + digest := delivery.ManifestDigest(raw) + manifests, err := b.js.KeyValue(b.ctx, delivery.ManifestsBucket) + if err != nil { + t.Fatal(err) + } + mv, err := delivery.EncodeManifest(delivery.Manifest{Module: name, JSON: string(raw)}) + if err != nil { + t.Fatal(err) + } + if _, err := manifests.Put(b.ctx, digest, mv); err != nil { + t.Fatal(err) + } + data, err := delivery.EncodeModule(delivery.Module{Repository: "novox/mesh-catalog", Commit: commit, FP: "src1:" + strings.Repeat("f", 64), FP2: fp, + Manifest: digest, Path: path}) + if err != nil { + t.Fatal(err) + } + if _, err := b.modules.Put(b.ctx, name, data); err != nil { + t.Fatal(err) + } + return digest +} + +// start runs the program as the builder of a node. +func (b *bed) start(t *testing.T, node string, env map[string]string) { + t.Helper() + broker := filepath.Join(t.TempDir(), "broker") + cred, _ := json.Marshal(credential{URL: b.srvURL, Node: node, Module: "mesh-delivery-builder", User: node + ".mesh-delivery-builder", Password: "pw"}) + if err := os.WriteFile(broker, cred, 0o600); err != nil { + t.Fatal(err) + } + e := map[string]string{"MESH_BROKER_FILE": broker} + for k, v := range env { + e[k] = v + } + 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 }) +} + +// await reads the build key until pred holds of it, within 20 s. +func (b *bed) await(t *testing.T, module string, pred func(delivery.BuilderBuild) bool) delivery.BuilderBuild { + t.Helper() + key, err := delivery.BuilderBuildKey(module, fp) + if err != nil { + t.Fatal(err) + } + var last delivery.BuilderBuild + for deadline := time.Now().Add(20 * time.Second); time.Now().Before(deadline); time.Sleep(50 * time.Millisecond) { + e, err := b.builds.Get(b.ctx, key) + if err != nil { + continue + } + if last, err = delivery.DecodeBuilderBuild(e.Value()); err == nil && pred(last) { + return last + } + } + t.Fatalf("builds.%s = %+v within 20 s", key, last) + return last +} + +func isState(state string) func(delivery.BuilderBuild) bool { + return func(v delivery.BuilderBuild) bool { return v.State == state } +} + +// TestBuilderMakesADoneKeyWithTheDigests: modules.netcheck (a Go and a TypeScript bundle) and its manifest are in the +// indexer's buckets, and two nodes report each build module's toolchain; the program writes builds.netcheck. done, +// the digests of both bundles, both build modules' toolchains, after a fetch and a build on each of the two nodes. +func TestBuilderMakesADoneKeyWithTheDigests(t *testing.T) { // 1607/AC7 + b := startBed(t) + digest := b.module(t, "netcheck", "catalogue-a67f0196-netcheck.module.json", "modules/netcheck") + b.start(t, "n1", nil) + v := b.await(t, "netcheck", isState(delivery.BuildDone)) + if v.By != "n1.mesh-delivery-builder" || v.Manifest != digest || len(v.Artifacts) != 2 { + t.Fatalf("build = %+v", v) + } + for _, a := range v.Artifacts { + if a.Digest != digestOf(a.Name) || a.Kind != "bundle" || a.Reference != "" { + t.Errorf("artifact = %+v, want the digest every node built and no reference", a) + } + } + if v.Toolchains["go"].Version != "go1.27.1" || v.Toolchains["typescript"].Version != "tsc5" { + t.Errorf("toolchains = %+v", v.Toolchains) + } + for tool, want := range map[string]int{"mesh-build-go.build_go_fetch": 2, "mesh-build-go.build_go_build": 2, "mesh-build-node.build_node_fetch": 2, "mesh-build-node.build_node_build": 2} { + if got := b.fake.count(tool); got != want { + t.Errorf("%s called %d times, want %d (once on each of two nodes)", tool, got, want) + } + } +} + +// TestASecondBuilderInstanceDoesNotBuildTheSameSourceAgain: two builders on two nodes start together on one module key; the +// fingerprint is built once. +func TestASecondBuilderInstanceDoesNotBuildTheSameSourceAgain(t *testing.T) { // 1607/AC7 + b := startBed(t) + b.module(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust") + b.start(t, "n1", nil) + b.start(t, "n2", nil) + v := b.await(t, "ca-trust", isState(delivery.BuildDone)) + if len(v.Artifacts) != 1 || v.Artifacts[0].Kind != "archive" || v.Artifacts[0].Digest != digestOf("files") { + t.Fatalf("build = %+v, want the archive's digest", v) + } + if n := b.fake.count("mesh-build-go.build_go_build"); n != 2 { // one instance built it, on two nodes + t.Errorf("build_go_build was called %d times, want 2", n) + } +} + +// TestAFailedKeyIsBuiltAgainAsANewRevisionWhenAToolchainChanges: a build step fails on one node, so the key is failed +// naming it; a toolchain report of a node then wakes the builder, which builds the key again as a new revision of the same +// key, now done. +func TestAFailedKeyIsBuiltAgainAsANewRevisionWhenAToolchainChanges(t *testing.T) { // 1607/AC6, AC7 + b := startBed(t) + b.fake.failBuild.Store(true) + b.module(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust") + b.start(t, "n1", nil) + v := b.await(t, "ca-trust", isState(delivery.BuildFailed)) + if !strings.Contains(v.Error, "n1") || !strings.Contains(v.Error, "boom") { + t.Errorf("build = %+v, want failed naming the node and the step's reason", v) + } + key, _ := delivery.BuilderBuildKey("ca-trust", fp) + failed, err := b.builds.Get(b.ctx, key) + if err != nil { + t.Fatal(err) + } + b.fake.failBuild.Store(false) + // The build module's recipe moved on both nodes: the toolchain's entries changed, twice (between the two the nodes disagree). + b.report(t, "mesh-build-go", "n1", "go", "go1.27.1", "2") + b.report(t, "mesh-build-go", "n2", "go", "go1.27.1", "2") + b.await(t, "ca-trust", isState(delivery.BuildDone)) + done, err := b.builds.Get(b.ctx, key) + if err != nil { + t.Fatal(err) + } + if done.Revision() <= failed.Revision() { + t.Errorf("revision %d, was %d: the key was not written as a new revision", done.Revision(), failed.Revision()) + } +} diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-ca-trust.module.json b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-ca-trust.module.json new file mode 100644 index 0000000..b3c9dc9 --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-ca-trust.module.json @@ -0,0 +1,66 @@ +{ + "module": "ca-trust", + "version": "1", + "slug": "catrust", + "capabilities": [ + "service-manager" + ], + "requires": [ + "internal-acme-ca" + ], + "seats": [ + { + "name": "the-mesh-trust-anchor", + "scope": "node" + } + ], + "claims": [ + { + "name": "the-mesh-trust-anchor", + "scope": "node" + } + ], + "resources": [ + { + "id": "state", + "type": "directory", + "mode": "0700", + "place": ".", + "unknown": "program" + }, + { + "id": "anchor", + "type": "file", + "path": "${dir:state}/anchor", + "mode": "0755", + "template": "files/anchor" + }, + { + "id": "unit", + "type": "file", + "path": "/etc/systemd/system/mesh-ca-trust.service", + "mode": "0644", + "template": "files/mesh-ca-trust.service" + }, + { + "id": "trust", + "type": "service", + "unit": "mesh-ca-trust.service", + "state": "running", + "boot": "enabled", + "restart-on": [ + "anchor", + "unit" + ] + } + ], + "build": { + "artifacts": [ + { + "name": "files", + "kind": "archive", + "from": "files" + } + ] + } +} diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-netcheck.module.json b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-netcheck.module.json new file mode 100644 index 0000000..ec01003 --- /dev/null +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/testdata/catalogue-a67f0196-netcheck.module.json @@ -0,0 +1,57 @@ +{ + "module": "netcheck", + "version": "1", + "emits": [ + "tunnel.endpoint-changed", + "tunnel.handshake-stale", + "tunnel.handshake-fresh" + ], + "tools": [ + "netcheck_tunnel", + "netcheck_tcp", + "netcheck_dns", + "netcheck_http", + "netcheck_listening" + ], + "replaces": { + "netcheck_listening": [ + "ss -lunt", + "ss -ltn", + "netstat -lnt" + ], + "netcheck_dns": [ + "dig", + "nslookup" + ], + "netcheck_tunnel": [ + "wg show", + "wg show mesh0 dump" + ] + }, + "build": { + "artifacts": [ + { + "name": "tools-go", + "kind": "bundle", + "language": "go", + "system": "arch", + "from": "cmd/netcheck", + "binary": "netcheck", + "loads": [ + "netcheck" + ] + }, + { + "name": "tools-typescript", + "kind": "bundle", + "language": "typescript", + "entrypoints": [ + "tools/index.js" + ], + "loads": [ + "tools/index.js" + ] + } + ] + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index 8e6c35a..b927a9a 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -41,6 +41,7 @@ type Builder struct { mu sync.Mutex known map[string]delivery.Module // by modules key inflight map[string]bool // by build key + again map[string]delivery.Module // by build key: the newest value that arrived while the key was being built running map[string]int // steps in flight, by `/` plans map[string]planned // by manifest digest } @@ -57,7 +58,7 @@ func (b *Builder) init() { limit = 4 } b.sem = make(chan struct{}, limit) - b.known, b.inflight, b.running, b.plans = map[string]delivery.Module{}, map[string]bool{}, map[string]int{}, map[string]planned{} + b.known, b.inflight, b.again, b.running, b.plans = map[string]delivery.Module{}, map[string]bool{}, map[string]delivery.Module{}, map[string]int{}, map[string]planned{} }) } @@ -147,6 +148,7 @@ func (b *Builder) consider(ctx context.Context, module string, m delivery.Module } b.mu.Lock() if b.inflight[key] { + b.again[key] = m // looked at once more when the build ends, so a change is never lost b.mu.Unlock() return } @@ -155,10 +157,21 @@ func (b *Builder) consider(ctx context.Context, module string, m delivery.Module b.wg.Add(1) go func() { defer b.wg.Done() - defer func() { b.mu.Lock(); delete(b.inflight, key); b.mu.Unlock() }() b.sem <- struct{}{} defer func() { <-b.sem }() - b.try(ctx, module, m) + for { + b.try(ctx, module, m) + b.mu.Lock() + next, more := b.again[key] + delete(b.again, key) + if !more || ctx.Err() != nil { + delete(b.inflight, key) + b.mu.Unlock() + return + } + b.mu.Unlock() + m = next + } }() } diff --git a/modules/mesh-delivery-builder/internal/build/run_test.go b/modules/mesh-delivery-builder/internal/build/run_test.go index fb71751..23427b0 100644 --- a/modules/mesh-delivery-builder/internal/build/run_test.go +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -22,6 +22,9 @@ type buildModules struct { steps *fakeSteps digests map[string]string // `/` -> digest; the default is `digest` fails map[string]string // `/` -> the reason the step fails with + // hold, when set before the builds start, blocks every call until it is closed; entered is sent to as a call arrives. + hold chan struct{} + entered chan struct{} } func newBuildModules() *buildModules { @@ -29,6 +32,13 @@ func newBuildModules() *buildModules { } func (b *buildModules) CallOn(node, key string, body any) (json.RawMessage, error) { + if b.hold != nil { + select { + case b.entered <- struct{}{}: + default: + } + <-b.hold + } b.mu.Lock() defer b.mu.Unlock() b.calls = append(b.calls, call{node, key, body}) @@ -293,3 +303,20 @@ func TestTwoInstancesBuildOneFingerprintOnce(t *testing.T) { // 1607/AC7 t.Errorf("%d builds, want 2: one instance built it", n) } } + +func TestAModulesValueThatArrivesWhileItsKeyIsBeingBuiltIsLookedAtAgainWhenItEnds(t *testing.T) { // 1607/AC6 (no event is dropped, no timer) + r := newRig(t, goBundle) + r.bm.fails["ace/build_go_build"] = "go build of tool: boom" + r.bm.hold, r.bm.entered = make(chan struct{}), make(chan struct{}, 8) + value := moduleValue(t, "modules/m") + r.b.Module(context.Background(), "m", value) + <-r.bm.entered // the first build is under way + r.b.Module(context.Background(), "m", value) + r.b.Reconsider(context.Background()) + delete(r.bm.fails, "ace/build_go_build") // what the event is about: the build can succeed now + close(r.bm.hold) + r.b.Wait() + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildDone { + t.Errorf("build = %+v, want the failed key built again as a new revision after the first build ended", v) + } +} From 3bd2a065f90c9e62b4b8c6c7fd43f55d0f435e63 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:33:49 +0200 Subject: [PATCH 10/14] The indexer writes '.' for a root module's path and the toolchains each fingerprint was computed with (issue 1615) --- .../internal/index/modules.go | 10 ++++- .../internal/index/modules_test.go | 13 +++++-- .../internal/index/source_test.go | 3 +- .../internal/index/toolchains.go | 38 ++++++++++++++----- .../internal/index/toolchains_test.go | 28 ++++++++++++++ 5 files changed, 77 insertions(+), 15 deletions(-) diff --git a/modules/mesh-delivery-indexer/internal/index/modules.go b/modules/mesh-delivery-indexer/internal/index/modules.go index 23cf31a..7e55e43 100644 --- a/modules/mesh-delivery-indexer/internal/index/modules.go +++ b/modules/mesh-delivery-indexer/internal/index/modules.go @@ -63,7 +63,7 @@ func (ix *Indexer) applyMerged(ctx context.Context, f delivery.Forge) error { continue } fp, none := fingerprintOf(k, f.Trees) - next := delivery.Module{Repository: f.Repository, Commit: f.Merge, FP: fp, None: none, Src: ix.srcOf(f.Repository, f.Merge), Manifest: digests[k.Path], Path: k.Path} + next := delivery.Module{Repository: f.Repository, Commit: f.Merge, FP: fp, None: none, Src: ix.srcOf(f.Repository, f.Merge), Manifest: digests[k.Path], Path: pathOf(k.Path)} // The new form (issue 1444, R4c-2): no module has a fingerprint of it yet, so each has the reason it has none, which // names what is missing, from its manifest and lock hashes (inputs.go). raw, has := f.Manifests[k.Path] @@ -109,4 +109,12 @@ func (ix *Indexer) baseFP(ctx context.Context) baseFP { } } +// pathOf is the module's directory as Module.Path holds it: "." for the repository's root, since empty means not known. +func pathOf(dir string) string { + if dir == "" { + return delivery.RootPath + } + return dir +} + func withV(m delivery.Module) delivery.Module { m.V = delivery.Version; return m } diff --git a/modules/mesh-delivery-indexer/internal/index/modules_test.go b/modules/mesh-delivery-indexer/internal/index/modules_test.go index db2d58f..36eee4f 100644 --- a/modules/mesh-delivery-indexer/internal/index/modules_test.go +++ b/modules/mesh-delivery-indexer/internal/index/modules_test.go @@ -234,7 +234,7 @@ func TestAMergeWritesTheModulesDirectoryAndKeepsItThroughSourceAndRecompute(t *t if err := ix.Apply(ctx, mergedMsg("novox/mesh-delivery", map[string]string{"modules/a": "newtree", "": "roottree"})); err != nil { t.Fatal(err) } - for module, want := range map[string]string{"a": "modules/a", "r": ""} { + for module, want := range map[string]string{"a": "modules/a", "r": delivery.RootPath} { if m, ok := moduleOf(t, mods, module); !ok || m.Path != want { t.Errorf("after the merge %s = %+v (%v), want path %q", module, m, ok, want) } @@ -252,7 +252,14 @@ func TestAMergeWritesTheModulesDirectoryAndKeepsItThroughSourceAndRecompute(t *t if !strings.Contains(raw, `"path":"modules/a"`) { t.Errorf("the stored value %s has no path", raw) } - if strings.Contains(string(mods.vals["r"]), `"path"`) { - t.Errorf("the root module's value %s names a path", mods.vals["r"]) +} + +func TestARootModuleHasTheDirectoryDot(t *testing.T) { // 1615/AC3: empty means not known, a root module is "." + ix, mods := modulesIndexer(t, rec(t, "build-1", "r", "novox/mesh-sdk", "", "main", "", "2026-10-19T09:00:00Z", strings.Replace(aInputs, `"Module":"a"`, `"Module":"r"`, 1))) + if err := ix.Apply(context.Background(), mergedMsg("novox/mesh-sdk", map[string]string{"": "roottree"})); err != nil { + t.Fatal(err) + } + if m, ok := moduleOf(t, mods, "r"); !ok || m.Path != delivery.RootPath { + t.Errorf("module = %+v (%v), want path %q", m, ok, delivery.RootPath) } } diff --git a/modules/mesh-delivery-indexer/internal/index/source_test.go b/modules/mesh-delivery-indexer/internal/index/source_test.go index 8f8fd7a..634e216 100644 --- a/modules/mesh-delivery-indexer/internal/index/source_test.go +++ b/modules/mesh-delivery-indexer/internal/index/source_test.go @@ -2,6 +2,7 @@ package index import ( "context" + "reflect" "testing" "git.novox.be/novox/mesh-sdk/go/delivery" @@ -32,7 +33,7 @@ func TestASourceRevisionStampsTheModulesOfTheRepositoriesHeadItNames(t *testing. m, ok := moduleOf(t, mods, "a") want := before want.Src = 7 - if !ok || m != want { + if !ok || !reflect.DeepEqual(m, want) { t.Errorf("module a = %+v (%v), want %+v: the fingerprint and the commit as the merge wrote them, with Src 7", m, ok, want) } if c, ok := moduleOf(t, mods, "c"); ok { diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains.go b/modules/mesh-delivery-indexer/internal/index/toolchains.go index fd5ce2c..8fbb0de 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains.go @@ -179,7 +179,10 @@ func (ix *Indexer) recomputeWith(ctx context.Context, overlay map[string]deliver all[key] = &known{key: key, cur: cur, languages: languagesOf(m), bases: basesOf(m), contexts: hasContext(m)} } tools := ix.agreed() - type result struct{ fp, none string } + type result struct { + fp, none string + tcs map[string]delivery.ToolchainUsed // by language, when there is a fingerprint + } done := map[string]result{} visiting := map[string]bool{} var of func(module string, chain []string) result @@ -192,16 +195,16 @@ func (ix *Indexer) recomputeWith(ctx context.Context, overlay map[string]deliver cur, has := held[module] switch { case !has: - return result{"", "the indexer holds no record of it"} + return result{fp: "", none: "the indexer holds no record of it"} case cur.FP2 != "": - return result{cur.FP2, ""} + return result{fp: cur.FP2, none: ""} case cur.None2 != "": - return result{"", cur.None2} + return result{fp: "", none: cur.None2} } - return result{"", "its record carries no fingerprint of the new form"} + return result{fp: "", none: "its record carries no fingerprint of the new form"} } if visiting[module] { - return result{"", "a cycle of bases: " + strings.Join(append(chain, module), " -> ")} + return result{fp: "", none: "a cycle of bases: " + strings.Join(append(chain, module), " -> ")} } visiting[module] = true defer delete(visiting, module) @@ -258,6 +261,9 @@ func (ix *Indexer) recomputeWith(ctx context.Context, overlay map[string]deliver r.none = noBuilder default: r.fp, r.none = delivery.SourceFingerprint2(kept.With(module, toolchains)) + if r.fp != "" { + r.tcs = toolchainsUsed(toolchains) + } } done[module] = r k.cur.Inputs2 = &kept @@ -269,7 +275,7 @@ func (ix *Indexer) recomputeWith(ctx context.Context, overlay map[string]deliver r := of(key, nil) if _, merged := overlay[key]; merged { final := overlay[key] - final.FP2, final.None2, final.Inputs2 = r.fp, r.none, k.cur.Inputs2 + final.FP2, final.None2, final.Inputs2, final.Toolchains = r.fp, r.none, k.cur.Inputs2, r.tcs // What the bases and the builder gave is kept as it was while the value it gave does not change, so a merge // that moves nothing for the module writes nothing (as Recompute does). if value, _, found, err := ix.Modules.Get(ctx, key); err == nil && found { @@ -282,7 +288,7 @@ func (ix *Indexer) recomputeWith(ctx context.Context, overlay map[string]deliver finals[key] = final continue } - if k.cur.FP2 == r.fp && k.cur.None2 == r.none { + if k.cur.FP2 == r.fp && k.cur.None2 == r.none && reflect.DeepEqual(k.cur.Toolchains, r.tcs) { continue } digest, inputs := k.cur.Manifest, k.cur.Inputs2 @@ -294,10 +300,10 @@ func (ix *Indexer) recomputeWith(ctx context.Context, overlay map[string]deliver if err != nil || cur.Manifest != digest || cur.Inputs2 == nil { return nil, false, nil // a merge changed it meanwhile; its own recompute follows } - if cur.FP2 == r.fp && cur.None2 == r.none { + if cur.FP2 == r.fp && cur.None2 == r.none && reflect.DeepEqual(cur.Toolchains, r.tcs) { return nil, false, nil } - cur.FP2, cur.None2 = r.fp, r.none + cur.FP2, cur.None2, cur.Toolchains = r.fp, r.none, r.tcs cur.Inputs2 = inputs data, err := delivery.EncodeModule(cur) return data, false, err @@ -365,3 +371,15 @@ func hasContext(m catalogue.Manifest) bool { } return false } + +// toolchainsUsed are the toolchains a fingerprint was computed with, by language (an artifact's toolchain is its language's). +func toolchainsUsed(by map[string]delivery.Toolchain2) map[string]delivery.ToolchainUsed { + if len(by) == 0 { + return nil + } + out := map[string]delivery.ToolchainUsed{} + for _, t := range by { + out[t.Language] = delivery.ToolchainUsed{Version: t.Version, Recipe: t.Recipe} + } + return out +} diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains_test.go b/modules/mesh-delivery-indexer/internal/index/toolchains_test.go index d9fd7f4..3cd1184 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains_test.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains_test.go @@ -2,6 +2,7 @@ package index import ( "context" + "reflect" "strings" "testing" "time" @@ -238,3 +239,30 @@ func TestAMergeKeepsAFingerprintTheModuleStillHas(t *testing.T) { // 1444 (revie t.Errorf("fp2 = %q (was %q), writes %d more", after.FP2, before.FP2, mods.writes-w) } } + +func TestAModuleKeepsTheToolchainsItsFingerprintWasComputedWith(t *testing.T) { // 1615/AC3 + ix, mods := newFormIndexer(t, builderModules(t)...) + if m, _ := moduleOf(t, mods, "avahi"); m.Toolchains != nil { + t.Fatalf("before a toolchain is reported: toolchains = %+v, want none", m.Toolchains) + } + reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.1") + m, _ := moduleOf(t, mods, "avahi") + want := map[string]delivery.ToolchainUsed{"go": {Version: "go1.27.1", Recipe: "1"}} + if m.FP2 == "" || !reflect.DeepEqual(m.Toolchains, want) { + t.Fatalf("avahi: fp2 %q toolchains %+v, want %+v", m.FP2, m.Toolchains, want) + } + if o, _ := moduleOf(t, mods, "other"); o.FP2 != "" || o.Toolchains != nil { + t.Errorf("a module with no fingerprint carries toolchains %+v", o.Toolchains) + } + // An upgrade moves the fingerprint and the toolchains together, with no merge. + reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.2") + n, _ := moduleOf(t, mods, "avahi") + if n.FP2 == m.FP2 || n.Toolchains["go"].Version != "go1.27.2" { + t.Errorf("after the upgrade: fp2 %q (was %q) toolchains %+v, want the new version with the new fingerprint", n.FP2, m.FP2, n.Toolchains) + } + // Nodes that disagree leave no fingerprint and no toolchains. + reportToolchain(t, ix, "mesh-build-go", "ace", "go", "go1.26.0") + if d, _ := moduleOf(t, mods, "avahi"); d.FP2 != "" || d.Toolchains != nil { + t.Errorf("disagreeing nodes: fp2 %q toolchains %+v, want neither", d.FP2, d.Toolchains) + } +} From f555a05cfb39dc467f51510ff9036611e2d40806 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 11:36:32 +0200 Subject: [PATCH 11/14] Build a key only with the toolchain its fingerprint names, accept '.' as a root module's path, and choose only online nodes (issues 1615, 1607 review) --- .../mesh-delivery-builder/integration_test.go | 75 +++++++++++-- .../cmd/mesh-delivery-builder/main.go | 25 +++++ .../internal/build/choose.go | 24 +++- .../internal/build/choose_test.go | 24 +++- .../internal/build/presence.go | 53 +++++++++ .../internal/build/presence_test.go | 43 ++++++++ .../internal/build/run.go | 41 ++++++- .../internal/build/run_test.go | 104 +++++++++++++++--- .../mesh-delivery-builder/manifest_test.go | 2 +- modules/mesh-delivery-builder/module.json | 3 +- 10 files changed, 356 insertions(+), 38 deletions(-) create mode 100644 modules/mesh-delivery-builder/internal/build/presence.go create mode 100644 modules/mesh-delivery-builder/internal/build/presence_test.go 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 eeaa473..2131046 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 @@ -193,6 +193,13 @@ func startBed(t *testing.T) *bed { b.modules = mk(delivery.ModulesBucket, delivery.ModulesHistory) mk(delivery.ManifestsBucket, delivery.ManifestsHistory) b.builds = mk(delivery.BuilderBuildsBucket, delivery.BuilderBuildsHistory) + presence := mk(delivery.PresenceBucket, delivery.PresenceHistory) + for _, node := range []string{"n1", "n2"} { + data, _ := delivery.EncodePresence(delivery.Presence{Online: true}) + if _, err := presence.Put(ctx, node, data); err != nil { + t.Fatal(err) + } + } b.fake = &fakeBuildModules{js: js, ctx: ctx, calls: map[string]int{}} b.fake.serve(t, ctl, "mesh-build-go") b.fake.serve(t, ctl, "mesh-build-node") @@ -221,6 +228,15 @@ func (b *bed) report(t *testing.T, module, node, language, version, recipe strin // module writes modules. as the indexer does for a module with a new-form fingerprint, and its manifest by digest. func (b *bed) module(t *testing.T, name, fixture, path string) string { + return b.moduleWith(t, name, fixture, path, map[string]delivery.ToolchainUsed{"go": {Version: "go1.27.1", Recipe: "1"}, "typescript": {Version: "tsc5", Recipe: "1"}}) +} + +// moduleWith is module with the toolchains its fingerprint was computed with. +func (b *bed) moduleWith(t *testing.T, name, fixture, path string, toolchains map[string]delivery.ToolchainUsed) string { + return b.moduleCommit(t, name, fixture, path, toolchains, commit) +} + +func (b *bed) moduleCommit(t *testing.T, name, fixture, path string, toolchains map[string]delivery.ToolchainUsed, at string) string { t.Helper() raw, err := os.ReadFile(filepath.Join("testdata", fixture)) if err != nil { @@ -238,8 +254,8 @@ func (b *bed) module(t *testing.T, name, fixture, path string) string { if _, err := manifests.Put(b.ctx, digest, mv); err != nil { t.Fatal(err) } - data, err := delivery.EncodeModule(delivery.Module{Repository: "novox/mesh-catalog", Commit: commit, FP: "src1:" + strings.Repeat("f", 64), FP2: fp, - Manifest: digest, Path: path}) + data, err := delivery.EncodeModule(delivery.Module{Repository: "novox/mesh-catalog", Commit: at, FP: "src1:" + strings.Repeat("f", 64), FP2: fp, + Manifest: digest, Path: path, Toolchains: toolchains}) if err != nil { t.Fatal(err) } @@ -334,10 +350,10 @@ func TestASecondBuilderInstanceDoesNotBuildTheSameSourceAgain(t *testing.T) { // } } -// TestAFailedKeyIsBuiltAgainAsANewRevisionWhenAToolchainChanges: a build step fails on one node, so the key is failed -// naming it; a toolchain report of a node then wakes the builder, which builds the key again as a new revision of the same -// key, now done. -func TestAFailedKeyIsBuiltAgainAsANewRevisionWhenAToolchainChanges(t *testing.T) { // 1607/AC6, AC7 +// TestAFailedKeyIsBuiltAgainAsANewRevisionFromANewModulesValueWithTheSameFingerprint: a build step fails on one node, so the +// key is failed naming it; the indexer writing modules. again with the same fingerprint wakes the builder, which builds +// the key again as a new revision of the same key, now done. +func TestAFailedKeyIsBuiltAgainAsANewRevisionFromANewModulesValueWithTheSameFingerprint(t *testing.T) { // 1607/AC6, AC7 b := startBed(t) b.fake.failBuild.Store(true) b.module(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust") @@ -352,9 +368,8 @@ func TestAFailedKeyIsBuiltAgainAsANewRevisionWhenAToolchainChanges(t *testing.T) t.Fatal(err) } b.fake.failBuild.Store(false) - // The build module's recipe moved on both nodes: the toolchain's entries changed, twice (between the two the nodes disagree). - b.report(t, "mesh-build-go", "n1", "go", "go1.27.1", "2") - b.report(t, "mesh-build-go", "n2", "go", "go1.27.1", "2") + // The indexer writes the module again (here at a later commit), with the same fingerprint. + b.moduleAt(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust", "go1.27.1", "89abcdef0123456789abcdef0123456789abcdef") b.await(t, "ca-trust", isState(delivery.BuildDone)) done, err := b.builds.Get(b.ctx, key) if err != nil { @@ -364,3 +379,45 @@ func TestAFailedKeyIsBuiltAgainAsANewRevisionWhenAToolchainChanges(t *testing.T) t.Errorf("revision %d, was %d: the key was not written as a new revision", done.Revision(), failed.Revision()) } } + +// TestAToolchainChangeDoesNotBuildTheOldFingerprint: the fingerprint names go1.27.1, the nodes then agree on go1.27.2 (a +// report of both nodes), and the builder writes no key for the old fingerprint although it was woken by the report; a +// module value naming go1.27.2 (the new fingerprint, computed by the indexer) is built. +func TestAToolchainChangeDoesNotBuildTheOldFingerprint(t *testing.T) { // 1615/AC4 + b := startBed(t) + b.report(t, "mesh-build-go", "n1", "go", "go1.27.2", "1") + b.report(t, "mesh-build-go", "n2", "go", "go1.27.2", "1") + b.module(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust") // names go1.27.1 + b.start(t, "n1", nil) + // Another module, whose fingerprint names the new toolchain, is built; by then the old one has been looked at. + b.moduleAt(t, "ca-trust2", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust", "go1.27.2", commit) + b.await(t, "ca-trust2", isState(delivery.BuildDone)) + if _, err := b.builds.Get(b.ctx, mustKey(t, "ca-trust")); err == nil { + t.Error("the old fingerprint, naming go1.27.1, was built with go1.27.2") + } + if n := b.fake.count("mesh-build-go.build_go_build"); n != 2 { + t.Errorf("build_go_build was called %d times, want 2 (the new fingerprint on two nodes only)", n) + } +} + +// TestARootModuleIsBuilt: a module whose path is "." is given to the build tools as it is. +func TestARootModuleIsBuilt(t *testing.T) { // 1615/AC4 + b := startBed(t) + b.module(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", delivery.RootPath) + b.start(t, "n1", nil) + b.await(t, "ca-trust", isState(delivery.BuildDone)) +} + +func mustKey(t *testing.T, module string) string { + k, err := delivery.BuilderBuildKey(module, fp) + if err != nil { + t.Fatal(err) + } + return k +} + +// moduleAt is module with the go toolchain version the fingerprint names and the commit it was computed at. +func (b *bed) moduleAt(t *testing.T, name, fixture, path, goVersion, at string) { + t.Helper() + b.moduleCommit(t, name, fixture, path, map[string]delivery.ToolchainUsed{"go": {Version: goVersion, Recipe: "1"}}, at) +} 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 c85cfe4..c3d70df 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go @@ -159,10 +159,12 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro } } tools := &build.ToolchainSet{} + presence := &build.PresenceSet{} b := &build.Builder{ Claimer: build.Claimer{Store: kvStore{builds}, By: cred.Node + "." + cred.Module}, Runner: build.Runner{Caller: caller, Steps: stepsWaiter{js}, Bound: bound}, Toolchains: tools, + Presence: presence, Manifests: manifestReader{manifests}, Nodes: nodesPerFingerprint, Logf: logger.Printf, @@ -189,6 +191,27 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro } defer cc.Stop() } + // The watchman's presence says which nodes are asleep. Where its bucket is not there yet no node is online, so nothing + // builds; the builder says so and is started again with the watchman. + presenceErrs := make(chan error, 1) + switch kv, err := js.KeyValue(ctx, delivery.PresenceBucket); { + case errors.Is(err, jetstream.ErrBucketNotFound): + logger.Printf("the bucket %s does not exist yet: no node counts as online until the builder starts again", delivery.PresenceBucket) + case err != nil: + return fmt.Errorf("the bucket %s: %w", delivery.PresenceBucket, err) + default: + pw, err := settle(ctx, kv, func(e jetstream.KeyValueEntry) { presence.Put(e.Key(), e.Value(), !opOf(e)) }) + if err != nil { + return err + } + go func() { + presenceErrs <- follow(ctx, pw, func(e jetstream.KeyValueEntry) { + if presence.Put(e.Key(), e.Value(), !opOf(e)) { + b.Reconsider(ctx) // a node came online or went: a build that waited for one may go on + } + }) + }() + } each := func(e jetstream.KeyValueEntry) { if opOf(e) { b.Module(ctx, e.Key(), e.Value()) @@ -211,5 +234,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro return err case err := <-watch.errs: return err + case err := <-presenceErrs: + return err } } diff --git a/modules/mesh-delivery-builder/internal/build/choose.go b/modules/mesh-delivery-builder/internal/build/choose.go index c4cf30d..88e09d3 100644 --- a/modules/mesh-delivery-builder/internal/build/choose.go +++ b/modules/mesh-delivery-builder/internal/build/choose.go @@ -3,28 +3,40 @@ package build import ( "fmt" "sort" + "strings" "git.novox.be/novox/mesh-sdk/go/delivery" ) // ChooseNodes picks up to want build nodes for a language (ADR 0478 clause 3): the nodes whose toolchains key reports the // version the nodes agree on, the ones with the fewest steps the builder has running there first, a tie broken by the -// node's name. It answers the toolchain they agree on. When nothing is reported, or the nodes report different versions -// (so the indexer agreed none), it fails naming the language and why. The build module's health is not read here: a node -// whose module refuses on its toolchain refuses the step, and the build fails naming it. -func ChooseNodes(language string, entries []delivery.NodeToolchain, running map[string]int, want int) ([]string, delivery.Toolchain2, error) { +// node's name. A node qualifies only when online is true of it (the watchman's presence of the node; nil means every +// node is). It answers the toolchain all the reporting nodes agree on, online or not, as the indexer computes it. When +// nothing is reported, or the nodes report different versions (so the indexer agreed none), it fails naming the language +// and why; when no reporting node is online it fails naming the nodes. A node whose build module refuses on its toolchain +// refuses the step, and the build fails naming it. +func ChooseNodes(language string, entries []delivery.NodeToolchain, running map[string]int, online func(node string) bool, want int) ([]string, delivery.Toolchain2, error) { tc, none := delivery.AgreedToolchain(entries) if none != "" { return nil, delivery.Toolchain2{}, fmt.Errorf("no %s build node qualifies: %s", language, none) } nodes := make([]string, 0, len(entries)) + var reporting []string seen := map[string]bool{} for _, e := range entries { - if !seen[e.Node] { - seen[e.Node] = true + if seen[e.Node] { + continue + } + seen[e.Node] = true + reporting = append(reporting, e.Node) + if online == nil || online(e.Node) { nodes = append(nodes, e.Node) } } + if len(nodes) == 0 { + sort.Strings(reporting) + return nil, tc, fmt.Errorf("no %s build node qualifies: %s report the toolchain, and none is online", language, strings.Join(reporting, ", ")) + } sort.Slice(nodes, func(i, j int) bool { if running[nodes[i]] != running[nodes[j]] { return running[nodes[i]] < running[nodes[j]] diff --git a/modules/mesh-delivery-builder/internal/build/choose_test.go b/modules/mesh-delivery-builder/internal/build/choose_test.go index c085dcd..7057f3f 100644 --- a/modules/mesh-delivery-builder/internal/build/choose_test.go +++ b/modules/mesh-delivery-builder/internal/build/choose_test.go @@ -17,7 +17,7 @@ func tcs(language, version string, nodes ...string) []delivery.NodeToolchain { func TestChooseTakesTheAgreedNodesWithFewestRunningStepsTiesByName(t *testing.T) { // 1607/AC4, ADR 0478 clause 3 entries := tcs("go", "go1.27.1", "novox", "ace", "g14", "shanks") - nodes, tc, err := ChooseNodes("go", entries, map[string]int{"novox": 3, "ace": 1, "g14": 1, "shanks": 0}, 2) + nodes, tc, err := ChooseNodes("go", entries, map[string]int{"novox": 3, "ace": 1, "g14": 1, "shanks": 0}, nil, 2) if err != nil { t.Fatal(err) } @@ -30,19 +30,19 @@ func TestChooseTakesTheAgreedNodesWithFewestRunningStepsTiesByName(t *testing.T) } func TestChooseWithOneNodeAnswersOneNode(t *testing.T) { // 1607/AC4 - nodes, _, err := ChooseNodes("go", tcs("go", "go1.27.1", "novox"), nil, 2) + nodes, _, err := ChooseNodes("go", tcs("go", "go1.27.1", "novox"), nil, nil, 2) if err != nil || len(nodes) != 1 || nodes[0] != "novox" { t.Errorf("nodes = %v, err %v, want [novox]", nodes, err) } } func TestChooseFailsNamingTheLanguageAndTheVersionsWhenNoNodeQualifies(t *testing.T) { // 1607/AC4 - _, _, err := ChooseNodes("typescript", nil, nil, 2) + _, _, err := ChooseNodes("typescript", nil, nil, nil, 2) if err == nil || !strings.Contains(err.Error(), "typescript") || !strings.Contains(err.Error(), delivery.ToolchainNoReport) { t.Errorf("no report: error = %v, want it to name typescript and %q", err, delivery.ToolchainNoReport) } entries := append(tcs("go", "go1.27.1", "novox"), tcs("go", "go1.26.0", "ace")...) - _, _, err = ChooseNodes("go", entries, nil, 2) + _, _, err = ChooseNodes("go", entries, nil, nil, 2) if err == nil || !strings.Contains(err.Error(), "go1.27.1") || !strings.Contains(err.Error(), "go1.26.0") || !strings.Contains(err.Error(), "go") { t.Errorf("disagreeing nodes: error = %v, want both versions named", err) } @@ -61,3 +61,19 @@ func TestAgreeNeedsEqualDigestsAndNamesBothNodesWhenTheyDiffer(t *testing.T) { / t.Errorf("one node: %q, %v", d, err) } } + +func TestChooseSkipsNodesThatAreNotOnlineAndFailsNamingThemWhenNoneIs(t *testing.T) { // 1607 review 3, ADR 0478 clause 3 + entries := tcs("go", "go1.27.1", "ace", "g14", "novox") + online := func(n string) bool { return n != "g14" } // a sleeping laptop + nodes, tc, err := ChooseNodes("go", entries, map[string]int{"novox": 5}, online, 2) + if err != nil || len(nodes) != 2 || nodes[0] != "ace" || nodes[1] != "novox" || tc.Version != "go1.27.1" { + t.Errorf("nodes = %v, toolchain %+v, err %v, want [ace novox] with g14 left out although it runs nothing", nodes, tc, err) + } + _, tc, err = ChooseNodes("go", entries, nil, func(string) bool { return false }, 2) + if err == nil || !strings.Contains(err.Error(), "online") || !strings.Contains(err.Error(), "g14") || !strings.Contains(err.Error(), "go") { + t.Errorf("none online: error = %v, want it to name the nodes that report and that none is online", err) + } + if tc.Version != "go1.27.1" { + t.Errorf("none online: the agreed toolchain %+v is still answered", tc) + } +} diff --git a/modules/mesh-delivery-builder/internal/build/presence.go b/modules/mesh-delivery-builder/internal/build/presence.go new file mode 100644 index 0000000..b1131aa --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/presence.go @@ -0,0 +1,53 @@ +package build + +import ( + "sync" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// PresenceSet holds which nodes are online, as the watchman writes it (mesh-delivery-watchman_presence, key ``, the +// node-engine's connection): a node asleep or off the bus is offline and its build module cannot answer. A node the +// watchman has no key of is not online. Safe for use from the watch and from a build at once. +type PresenceSet struct { + mu sync.Mutex + online map[string]bool +} + +// Put takes a change of the presence bucket in and answers whether a node's online state changed. A part's key and a +// value that is not a presence are left out. +func (s *PresenceSet) Put(key string, value []byte, deleted bool) bool { + node, part, _, err := delivery.ParsePresenceKey(key) + if err != nil || part != "" { + return false + } + on := false + if !deleted { + p, err := delivery.DecodePresence(value) + if err != nil { + return false + } + on = p.Online + } + s.mu.Lock() + defer s.mu.Unlock() + if s.online[node] == on { + return false + } + if s.online == nil { + s.online = map[string]bool{} + } + if on { + s.online[node] = true + } else { + delete(s.online, node) + } + return true +} + +// Online is whether the watchman last saw the node online. +func (s *PresenceSet) Online(node string) bool { + s.mu.Lock() + defer s.mu.Unlock() + return s.online[node] +} diff --git a/modules/mesh-delivery-builder/internal/build/presence_test.go b/modules/mesh-delivery-builder/internal/build/presence_test.go new file mode 100644 index 0000000..c1f3de7 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/presence_test.go @@ -0,0 +1,43 @@ +package build + +import ( + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +func presenceValue(t *testing.T, online bool) []byte { + t.Helper() + b, err := delivery.EncodePresence(delivery.Presence{Online: online}) + if err != nil { + t.Fatal(err) + } + return b +} + +func TestPresenceSetKnowsOnlyNodesTheWatchmanSawOnline(t *testing.T) { // 1607 review 3 + var s PresenceSet + if s.Online("ace") { + t.Error("a node the watchman never wrote is online") + } + if !s.Put("ace", presenceValue(t, true), false) || !s.Online("ace") { + t.Error("ace online was not taken in") + } + if s.Put("ace", presenceValue(t, true), false) { + t.Error("the same presence again counted as a change") + } + if !s.Put("ace", presenceValue(t, false), false) || s.Online("ace") { + t.Error("ace going offline was not taken in") + } + s.Put("g14", presenceValue(t, true), false) + if !s.Put("g14", nil, true) || s.Online("g14") { + t.Error("a deleted key is still online") + } + // A part's instance (part..) is not a node. + if s.Put("part.mesh-delivery-builder.novox", presenceValue(t, true), false) || s.Online("part") { + t.Error("a part's key was taken for a node") + } + if s.Put("novox", []byte("junk"), false) { + t.Error("junk counted") + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index b927a9a..4fad468 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -28,7 +28,9 @@ type Builder struct { Claimer Claimer Runner Runner Toolchains ToolchainEntries - Manifests Manifests + // Presence says which nodes are online (the watchman's); a node that is not is not chosen. + Presence interface{ Online(node string) bool } + Manifests Manifests // Nodes is how many build nodes build each fingerprint: two until release R4e (ADR 0478 clause 4). Nodes int // Limit is how many builds run at once; four when 0. @@ -182,6 +184,14 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { if read && planErr == nil && len(plan.Tasks) == 0 { return // nothing in the manifest for a builder to build } + if planErr == nil { + if why := b.stale(m, plan); why != "" { + // The fingerprint names a toolchain the build nodes no longer agree on: a new fingerprint is coming, and a build + // now would be of this source with another toolchain (ADR 0477 clause 2). Nothing is written. + b.logf("%s at %s is not built: %s", module, m.FP2, why) + return + } + } cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest) if err != nil { b.logf("%s: %v", module, err) @@ -194,6 +204,10 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { if ctx.Err() != nil { return } + if result.State == "" { + b.logf("%s: left building, the toolchains moved under it", cl.Key) + return + } result.By, result.Manifest = b.Claimer.By, m.Manifest if err := b.Claimer.Finish(ctx, cl, result); err != nil { b.logf("%s: the build could not be written: %v", cl.Key, err) @@ -204,6 +218,23 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { } } +// stale is why the module's fingerprint is not built now, or empty when it is: the toolchain each language of the plan +// has agreed (novox/hq ADR 0465) must be the one the fingerprint was computed with (Module.Toolchains). +func (b *Builder) stale(m delivery.Module, plan Plan) string { + for _, bm := range plan.Modules() { + language := LanguageOf[bm] + tc, none := delivery.AgreedToolchain(b.Toolchains.Entries(language)) + if none != "" { + return fmt.Sprintf("the %s toolchain is not agreed: %s", language, none) + } + want, ok := m.Toolchains[language] + if !ok || want.Version != tc.Version || want.Recipe != tc.Recipe { + return fmt.Sprintf("the fingerprint names %s %s (recipe %s) and the nodes agree on %s (recipe %s)", language, want.Version, want.Recipe, tc.Version, tc.Recipe) + } + } + return "" +} + func failed(format string, args ...any) delivery.BuilderBuild { return delivery.BuilderBuild{State: delivery.BuildFailed, Error: fmt.Sprintf(format, args...)} } @@ -221,6 +252,7 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p case planErr != nil: return failed("%s", planErr) case m.Path == "": + // Empty is not known (a root module's path is "."): the indexer writes it again (issue 1615). return failed("the module's key names no directory (path), so the build tools cannot be told where %s lives", module) } repository := delivery.RepositoryName(m.Repository) @@ -229,10 +261,13 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p var steps []string for _, bm := range plan.Modules() { language := LanguageOf[bm] - nodes, tc, err := ChooseNodes(language, b.Toolchains.Entries(language), b.runningOn(bm), b.Nodes) + nodes, tc, err := ChooseNodes(language, b.Toolchains.Entries(language), b.runningOn(bm), b.online, b.Nodes) if err != nil { return failed("%s", err) } + if want := m.Toolchains[language]; want.Version != tc.Version || want.Recipe != tc.Recipe { + return delivery.BuilderBuild{} // the agreed toolchain moved since try looked: nothing is written + } used[language] = delivery.ToolchainUsed{Version: tc.Version, Recipe: tc.Recipe} if len(nodes) == 1 { steps = append(steps, fmt.Sprintf("%s on one node, %s", bm, nodes[0])) @@ -283,6 +318,8 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p return delivery.BuilderBuild{State: delivery.BuildDone, Step: "built " + strings.Join(steps, "; "), Artifacts: artifacts, Toolchains: used} } +func (b *Builder) online(node string) bool { return b.Presence == nil || b.Presence.Online(node) } + func (b *Builder) runningOn(buildModule string) map[string]int { b.mu.Lock() defer b.mu.Unlock() diff --git a/modules/mesh-delivery-builder/internal/build/run_test.go b/modules/mesh-delivery-builder/internal/build/run_test.go index 23427b0..3a00ba8 100644 --- a/modules/mesh-delivery-builder/internal/build/run_test.go +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -95,20 +95,22 @@ type rig struct { st *memStore bm *buildModules tools *ToolchainSet + pres *PresenceSet mf *fakeManifests key string } func newRig(t *testing.T, artifacts ...catalogue.Artifact) *rig { t.Helper() - r := &rig{st: newMem(), bm: newBuildModules(), tools: &ToolchainSet{}, mf: &fakeManifests{byDigest: map[string]catalogue.Manifest{mdigest('a'): {Module: "m", Build: &catalogue.Build{Artifacts: artifacts}}}}} + r := &rig{st: newMem(), bm: newBuildModules(), tools: &ToolchainSet{}, pres: &PresenceSet{}, mf: &fakeManifests{byDigest: map[string]catalogue.Manifest{mdigest('a'): {Module: "m", Build: &catalogue.Build{Artifacts: artifacts}}}}} for _, n := range []string{"ace", "novox"} { r.tools.Put(GoBuildModule, n+".go", tcValue(t, "go", "go1.27.1"), false) r.tools.Put(NodeBuildModule, n+".typescript", tcValue(t, "typescript", "tsc5"), false) + r.pres.Put(n, presenceValue(t, true), false) } r.b = &Builder{ Claimer: Claimer{Store: r.st, By: "novox.builder"}, Runner: Runner{Caller: r.bm, Steps: r.bm.steps, Bound: time.Second}, - Toolchains: r.tools, Manifests: r.mf, Nodes: 2, Logf: t.Logf, + Toolchains: r.tools, Presence: r.pres, Manifests: r.mf, Nodes: 2, Logf: t.Logf, } var err error if r.key, err = delivery.BuilderBuildKey("m", fp1); err != nil { @@ -117,9 +119,14 @@ func newRig(t *testing.T, artifacts ...catalogue.Artifact) *rig { return r } -func moduleValue(t *testing.T, path string) []byte { +// agreedToolchains are what the rig's nodes report, and so what a fingerprint computed now names. +var agreedToolchains = map[string]delivery.ToolchainUsed{"go": {Version: "go1.27.1", Recipe: "1"}, "typescript": {Version: "tsc5", Recipe: "1"}} + +func moduleValue(t *testing.T, path string) []byte { return moduleValueWith(t, path, agreedToolchains) } + +func moduleValueWith(t *testing.T, path string, toolchains map[string]delivery.ToolchainUsed) []byte { t.Helper() - data, err := delivery.EncodeModule(delivery.Module{Repository: "novox/mesh-catalog", Commit: "7a74cee83f88e6585a5511c3cc53bdf0f5303d20", FP: "src1:" + hex64('f'), FP2: fp1, Manifest: mdigest('a'), Path: path}) + data, err := delivery.EncodeModule(delivery.Module{Repository: "novox/mesh-catalog", Commit: "7a74cee83f88e6585a5511c3cc53bdf0f5303d20", FP: "src1:" + hex64('f'), FP2: fp1, Manifest: mdigest('a'), Path: path, Toolchains: toolchains}) if err != nil { t.Fatal(err) } @@ -244,16 +251,6 @@ func TestAModuleKeyWithNoPathFailsNamingThatReason(t *testing.T) { // 1607/AC8, } } -func TestNoQualifyingNodeFailsTheBuildNamingTheLanguage(t *testing.T) { // 1607/AC4 - r := newRig(t, tsBundle) - r.tools.Put(NodeBuildModule, "ace.typescript", nil, true) - r.tools.Put(NodeBuildModule, "novox.typescript", nil, true) - v := r.consider(t, "modules/m") - if v.State != delivery.BuildFailed || !strings.Contains(v.Error, "typescript") || !strings.Contains(v.Error, delivery.ToolchainNoReport) { - t.Errorf("build = %+v, want failed naming typescript and that no toolchain is reported", v) - } -} - func TestAnUnreadableManifestFailsTheBuild(t *testing.T) { r := newRig(t, goBundle) delete(r.mf.byDigest, mdigest('a')) @@ -291,7 +288,7 @@ func TestAKeyTheBuilderLeftBuildingIsBuiltAtStartAndDoneKeysAreNotBuiltTwice(t * func TestTwoInstancesBuildOneFingerprintOnce(t *testing.T) { // 1607/AC7 r := newRig(t, goBundle) - other := &Builder{Claimer: Claimer{Store: r.st, By: "ace.builder"}, Runner: r.b.Runner, Toolchains: r.tools, Manifests: r.mf, Nodes: 2, Logf: t.Logf} + other := &Builder{Claimer: Claimer{Store: r.st, By: "ace.builder"}, Runner: r.b.Runner, Toolchains: r.tools, Presence: r.pres, Manifests: r.mf, Nodes: 2, Logf: t.Logf} value := moduleValue(t, "modules/m") var wg sync.WaitGroup for _, b := range []*Builder{r.b, other} { @@ -320,3 +317,80 @@ func TestAModulesValueThatArrivesWhileItsKeyIsBeingBuiltIsLookedAtAgainWhenItEnd t.Errorf("build = %+v, want the failed key built again as a new revision after the first build ended", v) } } + +func TestAKeyIsBuiltOnlyWithTheToolchainItsFingerprintNamesAndNothingIsWrittenOtherwise(t *testing.T) { // 1615/AC4 + r := newRig(t, goBundle) + old := map[string]delivery.ToolchainUsed{"go": {Version: "go1.26.0", Recipe: "1"}} + r.b.Module(context.Background(), "m", moduleValueWith(t, "modules/m", old)) + r.b.Wait() + if len(r.st.vals) != 0 || len(r.bm.calls) != 0 { + t.Errorf("keys %v and %d calls for a fingerprint of go1.26.0 while the nodes agree on go1.27.1, want neither", r.st.vals, len(r.bm.calls)) + } + // A recipe that differs is a different toolchain too, and a value that names none (written before) is not built. + r.b.Module(context.Background(), "m", moduleValueWith(t, "modules/m", map[string]delivery.ToolchainUsed{"go": {Version: "go1.27.1", Recipe: "0"}})) + r.b.Module(context.Background(), "m", moduleValueWith(t, "modules/m", nil)) + r.b.Wait() + if len(r.st.vals) != 0 || len(r.bm.calls) != 0 { + t.Errorf("keys %v and %d calls for a recipe that differs or a value that names no toolchain", r.st.vals, len(r.bm.calls)) + } + // The toolchain moves under a key that is not built yet: the old fingerprint stays unbuilt, the new one builds. + r.tools.Put(GoBuildModule, "ace.go", tcValue(t, "go", "go1.27.2"), false) + r.tools.Put(GoBuildModule, "novox.go", tcValue(t, "go", "go1.27.2"), false) + r.b.Reconsider(context.Background()) + r.b.Wait() + if len(r.st.vals) != 0 { + t.Errorf("keys %v: the old fingerprint was built with the new toolchain", r.st.vals) + } + r.b.Module(context.Background(), "m", moduleValueWith(t, "modules/m", map[string]delivery.ToolchainUsed{"go": {Version: "go1.27.2", Recipe: "1"}})) + r.b.Wait() + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildDone || v.Toolchains["go"].Version != "go1.27.2" { + t.Errorf("build = %+v, want done with go1.27.2", v) + } +} + +func TestNodesTheyDisagreeOnLeaveTheKeyUnwritten(t *testing.T) { // 1615/AC4 + r := newRig(t, goBundle) + r.tools.Put(GoBuildModule, "ace.go", tcValue(t, "go", "go1.26.0"), false) + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) + r.b.Wait() + if len(r.st.vals) != 0 || len(r.bm.calls) != 0 { + t.Errorf("keys %v and %d calls while the nodes disagree", r.st.vals, len(r.bm.calls)) + } +} + +func TestARootModulesPathIsDotAndIsGivenToTheBuildTools(t *testing.T) { // 1615/AC4 + r := newRig(t, goBundle) + if v := r.consider(t, delivery.RootPath); v.State != delivery.BuildDone { + t.Fatalf("build = %+v, want done for a root module", v) + } + if args := r.bm.callsOf("build_go_build")[0].body.(map[string]any); args["module"] != "." { + t.Errorf("build args = %+v, want module \".\"", args) + } +} + +func TestANodeThatIsNotOnlineIsNotChosenAndComingBackWakesTheBuild(t *testing.T) { // 1607 review 3 + r := newRig(t, goBundle) + r.tools.Put(GoBuildModule, "g14.go", tcValue(t, "go", "go1.27.1"), false) // a laptop that reports its toolchain and sleeps + v := r.consider(t, "modules/m") + if v.State != delivery.BuildDone { + t.Fatalf("build = %+v", v) + } + for _, c := range r.bm.calls { + if c.node == "g14" { + t.Errorf("a call went to g14, which is not online: %+v", c) + } + } + // With no node online the key fails naming why; a node coming online builds it as a new revision. + r2 := newRig(t, goBundle) + r2.pres.Put("ace", nil, true) + r2.pres.Put("novox", nil, true) + if v := r2.consider(t, "modules/m"); v.State != delivery.BuildFailed || !strings.Contains(v.Error, "online") { + t.Fatalf("build = %+v, want failed because no node is online", v) + } + r2.pres.Put("ace", presenceValue(t, true), false) + r2.b.Reconsider(context.Background()) + r2.b.Wait() + if v := valueOf(t, r2.st, r2.key); v.State != delivery.BuildDone { + t.Errorf("build = %+v, want done once a node is online", v) + } +} diff --git a/modules/mesh-delivery-builder/manifest_test.go b/modules/mesh-delivery-builder/manifest_test.go index 5d224e7..d957467 100644 --- a/modules/mesh-delivery-builder/manifest_test.go +++ b/modules/mesh-delivery-builder/manifest_test.go @@ -81,7 +81,7 @@ func TestManifestDeclarations(t *testing.T) { // 1607/AC1 t.Errorf("state = %+v, want %+v", m.State, want) } if want := []string{"mesh-build-go.steps", "mesh-build-go.toolchains", "mesh-build-node.steps", "mesh-build-node.toolchains", - "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules"}; !reflect.DeepEqual(sorted(m.Reads), want) { + "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules", "mesh-delivery-watchman.presence"}; !reflect.DeepEqual(sorted(m.Reads), want) { t.Errorf("reads = %v, want %v", m.Reads, want) } if want := []string{"mesh-build-go.build_go_build", "mesh-build-go.build_go_fetch", "mesh-build-node.build_node_build", "mesh-build-node.build_node_fetch"}; !reflect.DeepEqual(sorted(m.Invokes), want) { diff --git a/modules/mesh-delivery-builder/module.json b/modules/mesh-delivery-builder/module.json index 9d27205..1a902a2 100644 --- a/modules/mesh-delivery-builder/module.json +++ b/modules/mesh-delivery-builder/module.json @@ -15,7 +15,8 @@ "mesh-build-node.steps", "mesh-build-node.toolchains", "mesh-delivery-indexer.manifests", - "mesh-delivery-indexer.modules" + "mesh-delivery-indexer.modules", + "mesh-delivery-watchman.presence" ], "invokes": [ "mesh-build-go.build_go_build", From 27e4c3891f908958976a9835d29d89e02b7ca722 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 12:41:42 +0200 Subject: [PATCH 12/14] Run at most a node's build capacity of build steps on it at once, and hold a step back for a place with no timer (issue 1607, round 3) --- .../cmd/mesh-delivery-builder/main.go | 5 + .../internal/build/choose.go | 17 +-- .../internal/build/choose_test.go | 20 ++- .../internal/build/places.go | 118 +++++++++++++++ .../internal/build/places_test.go | 140 ++++++++++++++++++ .../internal/build/run.go | 34 ++--- .../internal/build/steps_test.go | 6 + .../mesh-delivery-builder/manifest_test.go | 4 +- modules/mesh-delivery-builder/module.json | 3 +- 9 files changed, 313 insertions(+), 34 deletions(-) create mode 100644 modules/mesh-delivery-builder/internal/build/places.go create mode 100644 modules/mesh-delivery-builder/internal/build/places_test.go 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 c3d70df..70521de 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go @@ -158,6 +158,10 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro return fmt.Errorf("MESH_BUILDER_STEP_BOUND: %w", err) } } + capacity, err := build.ParseCapacity(getenv("MESH_BUILDER_CAPACITY")) + if err != nil { + return fmt.Errorf("MESH_BUILDER_CAPACITY: %w", err) + } tools := &build.ToolchainSet{} presence := &build.PresenceSet{} b := &build.Builder{ @@ -165,6 +169,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro Runner: build.Runner{Caller: caller, Steps: stepsWaiter{js}, Bound: bound}, Toolchains: tools, Presence: presence, + Places: &build.Places{Capacity: capacity}, Manifests: manifestReader{manifests}, Nodes: nodesPerFingerprint, Logf: logger.Printf, diff --git a/modules/mesh-delivery-builder/internal/build/choose.go b/modules/mesh-delivery-builder/internal/build/choose.go index 88e09d3..656684a 100644 --- a/modules/mesh-delivery-builder/internal/build/choose.go +++ b/modules/mesh-delivery-builder/internal/build/choose.go @@ -9,13 +9,14 @@ import ( ) // ChooseNodes picks up to want build nodes for a language (ADR 0478 clause 3): the nodes whose toolchains key reports the -// version the nodes agree on, the ones with the fewest steps the builder has running there first, a tie broken by the -// node's name. A node qualifies only when online is true of it (the watchman's presence of the node; nil means every +// version the nodes agree on, the ones with room under their build capacity first (places), then the fewest steps the +// builder has running there, a tie broken by the node's name. A node at its capacity is still chosen when the others are +// too (fewer than want have room): its steps wait for a place (Places.Acquire). A node qualifies only when online is true of it (the watchman's presence of the node; nil means every // node is). It answers the toolchain all the reporting nodes agree on, online or not, as the indexer computes it. When // nothing is reported, or the nodes report different versions (so the indexer agreed none), it fails naming the language // and why; when no reporting node is online it fails naming the nodes. A node whose build module refuses on its toolchain // refuses the step, and the build fails naming it. -func ChooseNodes(language string, entries []delivery.NodeToolchain, running map[string]int, online func(node string) bool, want int) ([]string, delivery.Toolchain2, error) { +func ChooseNodes(language string, entries []delivery.NodeToolchain, places *Places, online func(node string) bool, want int) ([]string, delivery.Toolchain2, error) { tc, none := delivery.AgreedToolchain(entries) if none != "" { return nil, delivery.Toolchain2{}, fmt.Errorf("no %s build node qualifies: %s", language, none) @@ -37,12 +38,10 @@ func ChooseNodes(language string, entries []delivery.NodeToolchain, running map[ sort.Strings(reporting) return nil, tc, fmt.Errorf("no %s build node qualifies: %s report the toolchain, and none is online", language, strings.Join(reporting, ", ")) } - sort.Slice(nodes, func(i, j int) bool { - if running[nodes[i]] != running[nodes[j]] { - return running[nodes[i]] < running[nodes[j]] - } - return nodes[i] < nodes[j] - }) + if places == nil { + places = &Places{} + } + places.order(nodes) // nodes with room first, then the fewest steps running, then by name if len(nodes) > want { nodes = nodes[:want] } diff --git a/modules/mesh-delivery-builder/internal/build/choose_test.go b/modules/mesh-delivery-builder/internal/build/choose_test.go index 7057f3f..88a6b53 100644 --- a/modules/mesh-delivery-builder/internal/build/choose_test.go +++ b/modules/mesh-delivery-builder/internal/build/choose_test.go @@ -17,7 +17,7 @@ func tcs(language, version string, nodes ...string) []delivery.NodeToolchain { func TestChooseTakesTheAgreedNodesWithFewestRunningStepsTiesByName(t *testing.T) { // 1607/AC4, ADR 0478 clause 3 entries := tcs("go", "go1.27.1", "novox", "ace", "g14", "shanks") - nodes, tc, err := ChooseNodes("go", entries, map[string]int{"novox": 3, "ace": 1, "g14": 1, "shanks": 0}, nil, 2) + nodes, tc, err := ChooseNodes("go", entries, placesRunning(map[string]int{"novox": 3, "ace": 1, "g14": 1, "shanks": 0}), nil, 2) if err != nil { t.Fatal(err) } @@ -65,7 +65,7 @@ func TestAgreeNeedsEqualDigestsAndNamesBothNodesWhenTheyDiffer(t *testing.T) { / func TestChooseSkipsNodesThatAreNotOnlineAndFailsNamingThemWhenNoneIs(t *testing.T) { // 1607 review 3, ADR 0478 clause 3 entries := tcs("go", "go1.27.1", "ace", "g14", "novox") online := func(n string) bool { return n != "g14" } // a sleeping laptop - nodes, tc, err := ChooseNodes("go", entries, map[string]int{"novox": 5}, online, 2) + nodes, tc, err := ChooseNodes("go", entries, placesRunning(map[string]int{"novox": 5}), online, 2) if err != nil || len(nodes) != 2 || nodes[0] != "ace" || nodes[1] != "novox" || tc.Version != "go1.27.1" { t.Errorf("nodes = %v, toolchain %+v, err %v, want [ace novox] with g14 left out although it runs nothing", nodes, tc, err) } @@ -77,3 +77,19 @@ func TestChooseSkipsNodesThatAreNotOnlineAndFailsNamingThemWhenNoneIs(t *testing t.Errorf("none online: the agreed toolchain %+v is still answered", tc) } } + +// placesRunning is Places with these steps counted as running, for a test of the choice. +func placesRunning(running map[string]int) *Places { + return &Places{running: running, changed: make(chan struct{})} +} + +func TestChoosePrefersNodesBelowTheirCapacity(t *testing.T) { // round 3 + entries := tcs("go", "go1.27.1", "ace", "novox", "shanks") + // shanks has capacity 3 with 2 running: room; ace has the default 1 with 1 running: none; novox has none running. + p := placesRunning(map[string]int{"shanks": 2, "ace": 1}) + p.Capacity = map[string]int{"shanks": 3} + nodes, _, err := ChooseNodes("go", entries, p, nil, 2) + if err != nil || len(nodes) != 2 || nodes[0] != "novox" || nodes[1] != "shanks" { + t.Errorf("nodes = %v, err %v, want [novox shanks]: the nodes with room, ace at capacity left out", nodes, err) + } +} diff --git a/modules/mesh-delivery-builder/internal/build/places.go b/modules/mesh-delivery-builder/internal/build/places.go new file mode 100644 index 0000000..3ff6701 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/places.go @@ -0,0 +1,118 @@ +package build + +import ( + "context" + "fmt" + "sort" + "strconv" + "strings" + "sync" +) + +// DefaultCapacity is the build capacity of a node that has no setting: one build step at a time. +const DefaultCapacity = 1 + +// ParseCapacity reads the builder's capacity setting: `node=steps` pairs separated by commas, such as +// `shanks=3,ace=2`. A node without a pair has DefaultCapacity. A pair with no node, a count that is not a whole number of +// at least 1, and a node named twice are refused. +func ParseCapacity(s string) (map[string]int, error) { + out := map[string]int{} + for _, pair := range strings.Split(s, ",") { + pair = strings.TrimSpace(pair) + if pair == "" { + continue + } + node, count, ok := strings.Cut(pair, "=") + node, count = strings.TrimSpace(node), strings.TrimSpace(count) + n, err := strconv.Atoi(count) + if !ok || node == "" || err != nil || n < 1 { + return nil, fmt.Errorf("the capacity %q is not =", pair) + } + if _, dup := out[node]; dup { + return nil, fmt.Errorf("the capacity names %s twice", node) + } + out[node] = n + } + return out, nil +} + +// Places counts the build steps the builder runs on each node and holds a step back while its node is at its +// capacity (the operator's decision of 2026-10-21; ADR 0478 clause 3). A step waits for a place by watching the ends of +// the other steps, not by a timer: ending a step wakes the waiting ones. +type Places struct { + // Capacity is the number of steps at once by node; a node not in it has DefaultCapacity. + Capacity map[string]int + + mu sync.Mutex + running map[string]int + changed chan struct{} // closed, and replaced, each time a step ends +} + +// CapacityOf is how many steps the builder runs on the node at once. +func (p *Places) CapacityOf(node string) int { + if n, ok := p.Capacity[node]; ok && n >= 1 { + return n + } + return DefaultCapacity +} + +// Running is the steps in flight now, by node. +func (p *Places) Running() map[string]int { + p.mu.Lock() + defer p.mu.Unlock() + out := make(map[string]int, len(p.running)) + for n, c := range p.running { + if c > 0 { + out[n] = c + } + } + return out +} + +// Acquire takes a place on the node, waiting while the node is at its capacity, and answers the function that gives it +// back. It returns an error when ctx ends first. +func (p *Places) Acquire(ctx context.Context, node string) (release func(), err error) { + for { + p.mu.Lock() + if p.running == nil { + p.running = map[string]int{} + p.changed = make(chan struct{}) + } + if p.running[node] < p.CapacityOf(node) { + p.running[node]++ + p.mu.Unlock() + var once sync.Once + return func() { + once.Do(func() { + p.mu.Lock() + p.running[node]-- + close(p.changed) + p.changed = make(chan struct{}) + p.mu.Unlock() + }) + }, nil + } + wait := p.changed + p.mu.Unlock() + select { + case <-wait: + case <-ctx.Done(): + return nil, fmt.Errorf("waiting for a place on %s: %w", node, ctx.Err()) + } + } +} + +// order orders the nodes for a choice: those under their capacity first, then fewest running, then by name. +func (p *Places) order(nodes []string) { + running := p.Running() + sort.SliceStable(nodes, func(i, j int) bool { + ri, rj := running[nodes[i]] < p.CapacityOf(nodes[i]), running[nodes[j]] < p.CapacityOf(nodes[j]) + if ri != rj { + return ri + } + if running[nodes[i]] != running[nodes[j]] { + return running[nodes[i]] < running[nodes[j]] + } + return nodes[i] < nodes[j] + }) +} diff --git a/modules/mesh-delivery-builder/internal/build/places_test.go b/modules/mesh-delivery-builder/internal/build/places_test.go new file mode 100644 index 0000000..5cb3d24 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/places_test.go @@ -0,0 +1,140 @@ +package build + +import ( + "context" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/catalogue" +) + +func TestParseCapacityReadsNodeEqualsCountPairs(t *testing.T) { + c, err := ParseCapacity(" shanks=3, ace=2 ") + if err != nil || c["shanks"] != 3 || c["ace"] != 2 || len(c) != 2 { + t.Errorf("capacity = %v, %v", c, err) + } + if c, err := ParseCapacity(""); err != nil || len(c) != 0 { + t.Errorf("empty: %v, %v", c, err) + } + for _, bad := range []string{"shanks", "shanks=0", "shanks=x", "=2", "a=1,a=2", "shanks=-1"} { + if _, err := ParseCapacity(bad); err == nil { + t.Errorf("%q accepted", bad) + } + } +} + +func acquireAsync(p *Places, node string) chan func() { + got := make(chan func(), 1) + go func() { + release, err := p.Acquire(context.Background(), node) + if err != nil { + close(got) + return + } + got <- release + }() + return got +} + +func TestThreeStepsOnANodeOfCapacityTwoRunTwoAtOnceAndTheThirdStartsWhenOneEnds(t *testing.T) { // round 3 + p := &Places{Capacity: map[string]int{"ace": 2}} + a, b := acquireAsync(p, "ace"), acquireAsync(p, "ace") + ra, rb := <-a, <-b + third := acquireAsync(p, "ace") + select { + case <-third: + t.Fatal("a third step started on a node of capacity 2 with two running") + case <-time.After(50 * time.Millisecond): + } + if p.Running()["ace"] != 2 { + t.Errorf("running = %v, want 2 on ace", p.Running()) + } + ra() + select { + case r := <-third: + r() + case <-time.After(2 * time.Second): + t.Fatal("the third step did not start when one ended") + } + rb() +} + +func TestANodeWithNoSettingRunsOneStepAtATime(t *testing.T) { // round 3 + p := &Places{Capacity: map[string]int{"shanks": 3}} + first := <-acquireAsync(p, "g14") + second := acquireAsync(p, "g14") + select { + case <-second: + t.Fatal("two steps on a node without a setting") + case <-time.After(50 * time.Millisecond): + } + first() + r := <-second + r() + if p.CapacityOf("g14") != 1 || p.CapacityOf("shanks") != 3 { + t.Errorf("capacities = %d, %d", p.CapacityOf("g14"), p.CapacityOf("shanks")) + } +} + +func TestAWaitingStepStopsWhenItsContextEnds(t *testing.T) { // round 3 + p := &Places{} + r := <-acquireAsync(p, "ace") + defer r() + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { _, err := p.Acquire(ctx, "ace"); done <- err }() + cancel() + select { + case err := <-done: + if err == nil { + t.Error("Acquire returned no error for a cancelled wait") + } + case <-time.After(2 * time.Second): + t.Fatal("a cancelled wait did not return") + } +} + +func TestStepsOfSeveralBuildsNeverExceedANodesCapacity(t *testing.T) { // round 3, through the builder + r := newRig(t, catalogue.Artifact{Name: "tool", Kind: "bundle", Language: "go"}) + r.b.Places = &Places{Capacity: map[string]int{"ace": 2}} // novox has none: one at a time + var now, most atomic.Int32 + var mu sync.Mutex + perNode := map[string]int{} + r.bm.steps.during = func(key string) func() { + node, _, _ := strings.Cut(key, ".") + mu.Lock() + perNode[node]++ + if int32(perNode[node]) > most.Load() && node == "ace" { + most.Store(int32(perNode[node])) + } + if node == "novox" && perNode[node] > 1 { + t.Errorf("%d steps at once on novox, which has capacity 1", perNode[node]) + } + mu.Unlock() + now.Add(1) + time.Sleep(15 * time.Millisecond) + return func() { mu.Lock(); perNode[node]--; mu.Unlock(); now.Add(-1) } + } + for _, m := range []string{"m1", "m2", "m3", "m4"} { + r.b.Module(context.Background(), m, moduleValue(t, "modules/"+m)) + } + r.b.Wait() + if most.Load() > 2 { + t.Errorf("%d steps at once on ace, which has capacity 2", most.Load()) + } + if most.Load() < 2 { + t.Errorf("at most %d steps at once on ace: the capacity was not used", most.Load()) + } +} + +func TestTwoNodeAgreementStillTakesTwoDifferentNodesWhenCapacitiesAreSet(t *testing.T) { // round 3, ADR 0478 clause 4 + r := newRig(t, catalogue.Artifact{Name: "tool", Kind: "bundle", Language: "go"}) + r.b.Places = &Places{Capacity: map[string]int{"ace": 2, "novox": 3}} + v := r.consider(t, "modules/m") + if v.State != "done" || !strings.Contains(v.Step, "ace and novox") { + t.Errorf("build = %+v, want done on ace and novox", v) + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index 4fad468..cd25c70 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -31,6 +31,8 @@ type Builder struct { // Presence says which nodes are online (the watchman's); a node that is not is not chosen. Presence interface{ Online(node string) bool } Manifests Manifests + // Places holds a step back while its node is at its build capacity; a Builder with none runs one step at a time on each node. + Places *Places // Nodes is how many build nodes build each fingerprint: two until release R4e (ADR 0478 clause 4). Nodes int // Limit is how many builds run at once; four when 0. @@ -44,7 +46,6 @@ type Builder struct { known map[string]delivery.Module // by modules key inflight map[string]bool // by build key again map[string]delivery.Module // by build key: the newest value that arrived while the key was being built - running map[string]int // steps in flight, by `/` plans map[string]planned // by manifest digest } @@ -60,7 +61,10 @@ func (b *Builder) init() { limit = 4 } b.sem = make(chan struct{}, limit) - b.known, b.inflight, b.again, b.running, b.plans = map[string]delivery.Module{}, map[string]bool{}, map[string]delivery.Module{}, map[string]int{}, map[string]planned{} + b.known, b.inflight, b.again, b.plans = map[string]delivery.Module{}, map[string]bool{}, map[string]delivery.Module{}, map[string]planned{} + if b.Places == nil { + b.Places = &Places{} + } }) } @@ -261,7 +265,7 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p var steps []string for _, bm := range plan.Modules() { language := LanguageOf[bm] - nodes, tc, err := ChooseNodes(language, b.Toolchains.Entries(language), b.runningOn(bm), b.online, b.Nodes) + nodes, tc, err := ChooseNodes(language, b.Toolchains.Entries(language), b.Places, b.online, b.Nodes) if err != nil { return failed("%s", err) } @@ -320,25 +324,13 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p func (b *Builder) online(node string) bool { return b.Presence == nil || b.Presence.Online(node) } -func (b *Builder) runningOn(buildModule string) map[string]int { - b.mu.Lock() - defer b.mu.Unlock() - out := map[string]int{} - for k, n := range b.running { - if mod, node, _ := strings.Cut(k, "/"); mod == buildModule { - out[node] = n - } - } - return out -} - -// step runs one step of a build module, counting it as running on the node while it does. +// step runs one step of a build module, once its node has a place for it. func (b *Builder) step(ctx context.Context, bm, node, tool string, args map[string]any) (json.RawMessage, error) { - k := bm + "/" + node - b.mu.Lock() - b.running[k]++ - b.mu.Unlock() - defer func() { b.mu.Lock(); b.running[k]--; b.mu.Unlock() }() + release, err := b.Places.Acquire(ctx, node) + if err != nil { + return nil, err + } + defer release() return b.Runner.Run(ctx, bm, node, toolPrefix[bm]+tool, args) } diff --git a/modules/mesh-delivery-builder/internal/build/steps_test.go b/modules/mesh-delivery-builder/internal/build/steps_test.go index 4a39fb9..4a0503b 100644 --- a/modules/mesh-delivery-builder/internal/build/steps_test.go +++ b/modules/mesh-delivery-builder/internal/build/steps_test.go @@ -36,13 +36,19 @@ func (f *fakeCaller) CallOn(node, key string, body any) (json.RawMessage, error) type fakeSteps struct { mu sync.Mutex steps map[string]delivery.Step + // during, when set, is called as a wait starts and the function it answers as the wait ends. + during func(key string) func() } func (f *fakeSteps) Wait(ctx context.Context, module, key string) (delivery.Step, error) { f.mu.Lock() s, ok := f.steps[module+"/"+key] + during := f.during f.mu.Unlock() if ok { + if during != nil { + defer during(key)() + } return s, nil } <-ctx.Done() diff --git a/modules/mesh-delivery-builder/manifest_test.go b/modules/mesh-delivery-builder/manifest_test.go index d957467..87781bc 100644 --- a/modules/mesh-delivery-builder/manifest_test.go +++ b/modules/mesh-delivery-builder/manifest_test.go @@ -123,7 +123,9 @@ func TestManifestProcess(t *testing.T) { // 1607/AC1 t.Errorf("process = %v", p) } env, _ := p["env"].(map[string]any) - if len(env) != 1 || env["MESH_BROKER_FILE"] != "${dir:state}/broker" { + // The build capacity of each node: how many build steps the builder runs on it at once (a node not named runs one). No + // part of delivery reads a per-node setting yet, so the value is the manifest's own; changing it is a change of this line. + if len(env) != 2 || env["MESH_BROKER_FILE"] != "${dir:state}/broker" || env["MESH_BUILDER_CAPACITY"] != "shanks=3,ace=2" { t.Errorf("env = %v", p["env"]) } if _, bus := p["bus"]; bus { diff --git a/modules/mesh-delivery-builder/module.json b/modules/mesh-delivery-builder/module.json index 1a902a2..a886fbe 100644 --- a/modules/mesh-delivery-builder/module.json +++ b/modules/mesh-delivery-builder/module.json @@ -53,7 +53,8 @@ "./mesh-delivery-builder" ], "env": { - "MESH_BROKER_FILE": "${dir:state}/broker" + "MESH_BROKER_FILE": "${dir:state}/broker", + "MESH_BUILDER_CAPACITY": "shanks=3,ace=2" }, "user": "mesh-delivery-builder", "health": { From ebac167cd60ec9e17b1147bb51a56ec1908c85d3 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 12:45:10 +0200 Subject: [PATCH 13/14] Move the indexer and builder to mesh-sdk main with the toolchains and root path (issue 1615) --- modules/mesh-delivery-builder/go.mod | 2 +- modules/mesh-delivery-builder/go.sum | 4 ++-- modules/mesh-delivery-indexer/go.mod | 2 +- modules/mesh-delivery-indexer/go.sum | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/mesh-delivery-builder/go.mod b/modules/mesh-delivery-builder/go.mod index 956a222..4431597 100644 --- a/modules/mesh-delivery-builder/go.mod +++ b/modules/mesh-delivery-builder/go.mod @@ -3,7 +3,7 @@ module mesh-delivery-builder go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020182814-4024909a2960 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021100209-cb54756c3865 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-delivery-builder/go.sum b/modules/mesh-delivery-builder/go.sum index 0e67d23..639aa71 100644 --- a/modules/mesh-delivery-builder/go.sum +++ b/modules/mesh-delivery-builder/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020182814-4024909a2960 h1:FCId3IuRScG03X7Gx9vfKfGNZH9jWnfx9RChfJ3eWfo= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020182814-4024909a2960/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021100209-cb54756c3865 h1:VLir9NZbyi1LdH1YwWDIVRuL+lDC+AYlm6FZPlvmlm4= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021100209-cb54756c3865/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-delivery-indexer/go.mod b/modules/mesh-delivery-indexer/go.mod index b1780db..c7b22bb 100644 --- a/modules/mesh-delivery-indexer/go.mod +++ b/modules/mesh-delivery-indexer/go.mod @@ -3,7 +3,7 @@ module mesh-delivery-indexer go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020182814-4024909a2960 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021100209-cb54756c3865 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-delivery-indexer/go.sum b/modules/mesh-delivery-indexer/go.sum index 0e67d23..639aa71 100644 --- a/modules/mesh-delivery-indexer/go.sum +++ b/modules/mesh-delivery-indexer/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020182814-4024909a2960 h1:FCId3IuRScG03X7Gx9vfKfGNZH9jWnfx9RChfJ3eWfo= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020182814-4024909a2960/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021100209-cb54756c3865 h1:VLir9NZbyi1LdH1YwWDIVRuL+lDC+AYlm6FZPlvmlm4= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021100209-cb54756c3865/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= From bfc64ac65ea1af14f3feddf4e3b7dad85219d909 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 13:15:28 +0200 Subject: [PATCH 14/14] Build an archive with no toolchain, mark digests that differ and a one-node key so each is handled, reserve a place when a node is chosen, choose only nodes whose build module is healthy, and take over any key found building (issue 1607, review round 4) --- .../mesh-delivery-builder/integration_test.go | 131 ++++++-- .../cmd/mesh-delivery-builder/main.go | 25 ++ .../internal/build/choose.go | 46 ++- .../internal/build/claim.go | 45 ++- .../internal/build/claim_test.go | 27 +- .../internal/build/health.go | 56 ++++ .../internal/build/places.go | 86 ++++- .../internal/build/plan.go | 11 + .../internal/build/round4_test.go | 293 ++++++++++++++++++ .../internal/build/run.go | 117 +++++-- .../internal/build/run_test.go | 12 +- .../mesh-delivery-builder/manifest_test.go | 2 +- modules/mesh-delivery-builder/module.json | 3 +- 13 files changed, 749 insertions(+), 105 deletions(-) create mode 100644 modules/mesh-delivery-builder/internal/build/health.go create mode 100644 modules/mesh-delivery-builder/internal/build/round4_test.go 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 2131046..780387e 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 @@ -119,14 +119,16 @@ func (f *fakeBuildModules) serve(t *testing.T, nc *nats.Conn, module string) { } type bed struct { - ctx context.Context - js jetstream.JetStream - modules jetstream.KeyValue - builds jetstream.KeyValue - tools map[string]jetstream.KeyValue - fake *fakeBuildModules - srvURL string - dir string + ctx context.Context + js jetstream.JetStream + modules jetstream.KeyValue + builds jetstream.KeyValue + tools map[string]jetstream.KeyValue + presence jetstream.KeyValue + have jetstream.KeyValue + fake *fakeBuildModules + srvURL string + dir string } // startBed starts the bus, the buckets the controller makes from the builder's and the build modules' manifests, the @@ -194,10 +196,11 @@ func startBed(t *testing.T) *bed { mk(delivery.ManifestsBucket, delivery.ManifestsHistory) b.builds = mk(delivery.BuilderBuildsBucket, delivery.BuilderBuildsHistory) presence := mk(delivery.PresenceBucket, delivery.PresenceHistory) + b.presence, b.have = presence, mk(delivery.HaveBucket, delivery.HaveHistory) for _, node := range []string{"n1", "n2"} { - data, _ := delivery.EncodePresence(delivery.Presence{Online: true}) - if _, err := presence.Put(ctx, node, data); err != nil { - t.Fatal(err) + b.setOnline(t, node, true) + for _, module := range []string{"mesh-build-go", "mesh-build-node"} { + b.setHealth(t, node, module, delivery.HealthOK) } } b.fake = &fakeBuildModules{js: js, ctx: ctx, calls: map[string]int{}} @@ -210,6 +213,28 @@ func startBed(t *testing.T) *bed { return b } +// setOnline writes the watchman's presence of a node. +func (b *bed) setOnline(t *testing.T, node string, online bool) { + t.Helper() + data, _ := delivery.EncodePresence(delivery.Presence{Online: online}) + if _, err := b.presence.Put(b.ctx, node, data); err != nil { + t.Fatal(err) + } +} + +// setHealth writes the node-engine's have of a build module on a node, running with this health. +func (b *bed) setHealth(t *testing.T, node, module string, health delivery.Health) { + t.Helper() + data, err := delivery.EncodeHave(delivery.Have{Phase: delivery.PhaseRunning, Health: health}) + if err != nil { + t.Fatal(err) + } + key, _ := delivery.HaveKey(node, module) + if _, err := b.have.Put(b.ctx, key, data); err != nil { + t.Fatal(err) + } +} + // report writes a node's toolchains key of a build module and says so on the module's stream, as the module does. func (b *bed) report(t *testing.T, module, node, language, version, recipe string) { t.Helper() @@ -265,6 +290,13 @@ func (b *bed) moduleCommit(t *testing.T, name, fixture, path string, toolchains return digest } +// archive writes modules. as the real indexer does for a module of an archive only: its fingerprint names no +// toolchain (mesh-delivery-indexer internal/index inputs.go languagesOf gives an archive no language), so Toolchains is empty. +func (b *bed) archive(t *testing.T, name, path, at string) string { + t.Helper() + return b.moduleCommit(t, name, "catalogue-a67f0196-ca-trust.module.json", path, nil, at) +} + // start runs the program as the builder of a node. func (b *bed) start(t *testing.T, node string, env map[string]string) { t.Helper() @@ -334,19 +366,69 @@ func TestBuilderMakesADoneKeyWithTheDigests(t *testing.T) { // 1607/AC7 } } -// TestASecondBuilderInstanceDoesNotBuildTheSameSourceAgain: two builders on two nodes start together on one module key; the -// fingerprint is built once. -func TestASecondBuilderInstanceDoesNotBuildTheSameSourceAgain(t *testing.T) { // 1607/AC7 +// TestTwoBuilderInstancesStartingTogetherEndWithOneDoneKey: two builders on two nodes start together on one module key of an +// archive only (no toolchain named, as the indexer writes it); the second takes over the key the first holds building +// (ADR 0478 clause 6), and the key ends done, written once. +func TestTwoBuilderInstancesStartingTogetherEndWithOneDoneKey(t *testing.T) { // 1607/AC7 b := startBed(t) - b.module(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust") + b.archive(t, "ca-trust", "modules/ca-trust", commit) b.start(t, "n1", nil) b.start(t, "n2", nil) v := b.await(t, "ca-trust", isState(delivery.BuildDone)) if len(v.Artifacts) != 1 || v.Artifacts[0].Kind != "archive" || v.Artifacts[0].Digest != digestOf("files") { t.Fatalf("build = %+v, want the archive's digest", v) } - if n := b.fake.count("mesh-build-go.build_go_build"); n != 2 { // one instance built it, on two nodes - t.Errorf("build_go_build was called %d times, want 2", n) + if n := b.fake.count("mesh-build-go.build_go_build"); n < 2 || n%2 != 0 { // each instance that built, on two nodes + t.Errorf("build_go_build was called %d times, want two for each instance that built", n) + } +} + +// TestAnArchiveOnlyModuleNamingNoToolchainIsBuilt: the indexer gives an archive no language, so modules.ca-trust has no +// toolchains; the builder builds it on the build module's nodes all the same (review round 4). +func TestAnArchiveOnlyModuleNamingNoToolchainIsBuilt(t *testing.T) { + b := startBed(t) + b.archive(t, "ca-trust", "modules/ca-trust", commit) + b.start(t, "n1", nil) + v := b.await(t, "ca-trust", isState(delivery.BuildDone)) + if len(v.Toolchains) != 0 || len(v.Artifacts) != 1 || strings.HasPrefix(v.Step, "one node (") { + t.Errorf("build = %+v, want done on two nodes naming no toolchain", v) + } +} + +// TestAKeyBuiltOnOneNodeIsBuiltAgainWhenASecondNodeComesOnline: n2 is offline, so the key is done on n1 alone and marked; +// n2 coming online makes the builder build it again on both nodes, as a new revision (ADR 0478 clause 4). +func TestAKeyBuiltOnOneNodeIsBuiltAgainWhenASecondNodeComesOnline(t *testing.T) { + b := startBed(t) + b.setOnline(t, "n2", false) + b.archive(t, "ca-trust", "modules/ca-trust", commit) + b.start(t, "n1", nil) + one := b.await(t, "ca-trust", isState(delivery.BuildDone)) + if !strings.HasPrefix(one.Step, "one node (") { + t.Fatalf("build = %+v, want it marked as built on one node", one) + } + key := mustKey(t, "ca-trust") + e, err := b.builds.Get(b.ctx, key) + if err != nil { + t.Fatal(err) + } + b.setOnline(t, "n2", true) + two := b.await(t, "ca-trust", func(v delivery.BuilderBuild) bool { + return v.State == delivery.BuildDone && !strings.HasPrefix(v.Step, "one node (") + }) + if e2, _ := b.builds.Get(b.ctx, key); e2.Revision() <= e.Revision() || !strings.Contains(two.Step, "n1 and n2") { + t.Errorf("build = %+v at revision %d (was %d), want a new revision built on n1 and n2", two, e2.Revision(), e.Revision()) + } +} + +// TestANodeWhoseBuildModuleIsUnhealthyIsNotUsed: the node-engine's have of mesh-build-go on n2 says failed, so only n1 builds. +func TestANodeWhoseBuildModuleIsUnhealthyIsNotUsed(t *testing.T) { + b := startBed(t) + b.setHealth(t, "n2", "mesh-build-go", delivery.HealthFailed) + b.archive(t, "ca-trust", "modules/ca-trust", commit) + b.start(t, "n1", nil) + v := b.await(t, "ca-trust", isState(delivery.BuildDone)) + if !strings.Contains(v.Step, "on one node, n1") { + t.Errorf("build = %+v, want it built on n1 alone", v) } } @@ -356,7 +438,7 @@ func TestASecondBuilderInstanceDoesNotBuildTheSameSourceAgain(t *testing.T) { // func TestAFailedKeyIsBuiltAgainAsANewRevisionFromANewModulesValueWithTheSameFingerprint(t *testing.T) { // 1607/AC6, AC7 b := startBed(t) b.fake.failBuild.Store(true) - b.module(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust") + b.archive(t, "ca-trust", "modules/ca-trust", commit) b.start(t, "n1", nil) v := b.await(t, "ca-trust", isState(delivery.BuildFailed)) if !strings.Contains(v.Error, "n1") || !strings.Contains(v.Error, "boom") { @@ -369,7 +451,7 @@ func TestAFailedKeyIsBuiltAgainAsANewRevisionFromANewModulesValueWithTheSameFing } b.fake.failBuild.Store(false) // The indexer writes the module again (here at a later commit), with the same fingerprint. - b.moduleAt(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust", "go1.27.1", "89abcdef0123456789abcdef0123456789abcdef") + b.archive(t, "ca-trust", "modules/ca-trust", "89abcdef0123456789abcdef0123456789abcdef") b.await(t, "ca-trust", isState(delivery.BuildDone)) done, err := b.builds.Get(b.ctx, key) if err != nil { @@ -387,12 +469,13 @@ func TestAToolchainChangeDoesNotBuildTheOldFingerprint(t *testing.T) { // 1615/A b := startBed(t) b.report(t, "mesh-build-go", "n1", "go", "go1.27.2", "1") b.report(t, "mesh-build-go", "n2", "go", "go1.27.2", "1") - b.module(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust") // names go1.27.1 + b.module(t, "netcheck", "catalogue-a67f0196-netcheck.module.json", "modules/netcheck") // names go1.27.1 b.start(t, "n1", nil) // Another module, whose fingerprint names the new toolchain, is built; by then the old one has been looked at. - b.moduleAt(t, "ca-trust2", "catalogue-a67f0196-ca-trust.module.json", "modules/ca-trust", "go1.27.2", commit) - b.await(t, "ca-trust2", isState(delivery.BuildDone)) - if _, err := b.builds.Get(b.ctx, mustKey(t, "ca-trust")); err == nil { + b.moduleCommit(t, "netcheck2", "catalogue-a67f0196-netcheck.module.json", "modules/netcheck", + map[string]delivery.ToolchainUsed{"go": {Version: "go1.27.2", Recipe: "1"}, "typescript": {Version: "tsc5", Recipe: "1"}}, commit) + b.await(t, "netcheck2", isState(delivery.BuildDone)) + if _, err := b.builds.Get(b.ctx, mustKey(t, "netcheck")); err == nil { t.Error("the old fingerprint, naming go1.27.1, was built with go1.27.2") } if n := b.fake.count("mesh-build-go.build_go_build"); n != 2 { @@ -403,7 +486,7 @@ func TestAToolchainChangeDoesNotBuildTheOldFingerprint(t *testing.T) { // 1615/A // TestARootModuleIsBuilt: a module whose path is "." is given to the build tools as it is. func TestARootModuleIsBuilt(t *testing.T) { // 1615/AC4 b := startBed(t) - b.module(t, "ca-trust", "catalogue-a67f0196-ca-trust.module.json", delivery.RootPath) + b.archive(t, "ca-trust", delivery.RootPath, commit) b.start(t, "n1", nil) b.await(t, "ca-trust", isState(delivery.BuildDone)) } 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 70521de..f93a061 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go @@ -164,11 +164,13 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro } tools := &build.ToolchainSet{} presence := &build.PresenceSet{} + have := &build.HaveSet{} b := &build.Builder{ Claimer: build.Claimer{Store: kvStore{builds}, By: cred.Node + "." + cred.Module}, Runner: build.Runner{Caller: caller, Steps: stepsWaiter{js}, Bound: bound}, Toolchains: tools, Presence: presence, + Health: have, Places: &build.Places{Capacity: capacity}, Manifests: manifestReader{manifests}, Nodes: nodesPerFingerprint, @@ -217,6 +219,27 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro }) }() } + // The node-engine's have says whether a node's build module runs and is healthy: a node whose build module is not is not + // chosen (ADR 0478 clause 3). Where its bucket is not there yet no build module counts as healthy, so nothing builds. + haveErrs := make(chan error, 1) + switch kv, err := js.KeyValue(ctx, delivery.HaveBucket); { + case errors.Is(err, jetstream.ErrBucketNotFound): + logger.Printf("the bucket %s does not exist yet: no build module counts as healthy until the builder starts again", delivery.HaveBucket) + case err != nil: + return fmt.Errorf("the bucket %s: %w", delivery.HaveBucket, err) + default: + hw, err := settle(ctx, kv, func(e jetstream.KeyValueEntry) { have.Put(e.Key(), e.Value(), !opOf(e)) }) + if err != nil { + return err + } + go func() { + haveErrs <- follow(ctx, hw, func(e jetstream.KeyValueEntry) { + if have.Put(e.Key(), e.Value(), !opOf(e)) { + b.Reconsider(ctx) // a build module became healthy or stopped being: a build that waited may go on + } + }) + }() + } each := func(e jetstream.KeyValueEntry) { if opOf(e) { b.Module(ctx, e.Key(), e.Value()) @@ -241,5 +264,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro return err case err := <-presenceErrs: return err + case err := <-haveErrs: + return err } } diff --git a/modules/mesh-delivery-builder/internal/build/choose.go b/modules/mesh-delivery-builder/internal/build/choose.go index 656684a..a71c61c 100644 --- a/modules/mesh-delivery-builder/internal/build/choose.go +++ b/modules/mesh-delivery-builder/internal/build/choose.go @@ -9,18 +9,31 @@ import ( ) // ChooseNodes picks up to want build nodes for a language (ADR 0478 clause 3): the nodes whose toolchains key reports the -// version the nodes agree on, the ones with room under their build capacity first (places), then the fewest steps the -// builder has running there, a tie broken by the node's name. A node at its capacity is still chosen when the others are -// too (fewer than want have room): its steps wait for a place (Places.Acquire). A node qualifies only when online is true of it (the watchman's presence of the node; nil means every -// node is). It answers the toolchain all the reporting nodes agree on, online or not, as the indexer computes it. When -// nothing is reported, or the nodes report different versions (so the indexer agreed none), it fails naming the language -// and why; when no reporting node is online it fails naming the nodes. A node whose build module refuses on its toolchain -// refuses the step, and the build fails naming it. -func ChooseNodes(language string, entries []delivery.NodeToolchain, places *Places, online func(node string) bool, want int) ([]string, delivery.Toolchain2, error) { +// version the nodes agree on, the ones with room under their build capacity first (places), then the fewest lanes the +// builder has running or chosen there, a tie broken by the node's name. A place is reserved on each node chosen, in the +// same step as the choice, so builds choosing at once do not choose the same full node while another has room; the +// caller takes the place (Places.Acquire) or gives it up (Places.Cancel). A node at its capacity is still chosen when +// the others are too (fewer than want have room): its lane waits for a place. A node qualifies only when qualifies is true +// of it (online and its build module healthy; nil means every node does). It answers the toolchain all the reporting nodes +// agree on, qualifying or not, as the indexer computes it. When nothing is reported, or the nodes report different +// versions (so the indexer agreed none), it fails naming the language and why; when no reporting node qualifies it fails +// naming the nodes. A node whose build module refuses on its toolchain refuses the step, and the build fails naming it. +func ChooseNodes(language string, entries []delivery.NodeToolchain, places *Places, qualifies func(node string) bool, want int) ([]string, delivery.Toolchain2, error) { tc, none := delivery.AgreedToolchain(entries) if none != "" { return nil, delivery.Toolchain2{}, fmt.Errorf("no %s build node qualifies: %s", language, none) } + nodes, err := choose(language, entries, places, qualifies, want) + return nodes, tc, err +} + +// ChooseAnyNodes is ChooseNodes for artifacts that need no toolchain, such as an archive (ADR 0478 clause 1): the +// build module's nodes agree on nothing, so any node of the build module that qualifies is chosen, by capacity and load only. +func ChooseAnyNodes(language string, entries []delivery.NodeToolchain, places *Places, qualifies func(node string) bool, want int) ([]string, error) { + return choose(language, entries, places, qualifies, want) +} + +func choose(language string, entries []delivery.NodeToolchain, places *Places, qualifies func(node string) bool, want int) ([]string, error) { nodes := make([]string, 0, len(entries)) var reporting []string seen := map[string]bool{} @@ -30,24 +43,25 @@ func ChooseNodes(language string, entries []delivery.NodeToolchain, places *Plac } seen[e.Node] = true reporting = append(reporting, e.Node) - if online == nil || online(e.Node) { + if qualifies == nil || qualifies(e.Node) { nodes = append(nodes, e.Node) } } if len(nodes) == 0 { sort.Strings(reporting) - return nil, tc, fmt.Errorf("no %s build node qualifies: %s report the toolchain, and none is online", language, strings.Join(reporting, ", ")) + return nil, fmt.Errorf("no %s build node qualifies: %s report the toolchain, and none is online with a healthy build module", language, strings.Join(reporting, ", ")) } if places == nil { places = &Places{} } - places.order(nodes) // nodes with room first, then the fewest steps running, then by name - if len(nodes) > want { - nodes = nodes[:want] - } - return nodes, tc, nil + return places.choose(nodes, want), nil } +// DigestsDifferPrefix opens the error of a build whose nodes built an artifact to different digests (ADR 0478 clause 4): +// such a key is not built again as a new revision, since the same source on the same toolchains gave two answers and +// building again settles nothing; it waits for another fingerprint. +const DigestsDifferPrefix = "digests differ:" + // Agree is the one digest every node built an artifact to (ADR 0478 clause 4), or an error naming each node and its // digest when they differ. One node's digest agrees with itself. func Agree(artifact string, byNode map[string]string) (string, error) { @@ -63,7 +77,7 @@ func Agree(artifact string, byNode map[string]string) (string, error) { continue } if byNode[n] != first { - msg := fmt.Sprintf("the nodes built %s to different digests:", artifact) + msg := fmt.Sprintf("%s the nodes built %s to different digests:", DigestsDifferPrefix, artifact) for _, n := range nodes { msg += fmt.Sprintf(" %s %s;", n, byNode[n]) } diff --git a/modules/mesh-delivery-builder/internal/build/claim.go b/modules/mesh-delivery-builder/internal/build/claim.go index 0411c4d..2875d57 100644 --- a/modules/mesh-delivery-builder/internal/build/claim.go +++ b/modules/mesh-delivery-builder/internal/build/claim.go @@ -34,14 +34,44 @@ type Claim struct { Build delivery.BuilderBuild } +// OneNodePrefix opens the Step of a done build that a build module built on one node only, because no second node +// qualified (ADR 0478 clause 4); the build modules that did so follow in parentheses, then ": ". Claim reads it to build +// the key again, as a new revision, once a second node qualifies. +const OneNodePrefix = "one node (" + +// OneNodeModules are the build modules a done build built on one node only, or none when it is not such a build. +func OneNodeModules(b delivery.BuilderBuild) []string { + if b.State != delivery.BuildDone || !strings.HasPrefix(b.Step, OneNodePrefix) { + return nil + } + list, _, ok := strings.Cut(strings.TrimPrefix(b.Step, OneNodePrefix), "): ") + if !ok || list == "" { + return nil + } + return strings.Split(list, ", ") +} + +// ClaimOptions say what the builder knows about the moment of a claim. +type ClaimOptions struct { + // TakeOver takes a key found building whoever holds it: the builder has just started and looks at the key for the first + // time, and a build left building by a builder that is gone, on this node or another, is nobody's (ADR 0478 clause 6). + TakeOver bool + // MoreNodes says whether one of these build modules, which built a done key on one node only, has a second node that + // qualifies now. Nil means none does. + MoreNodes func(modules []string) bool +} + // Claim takes the build of module at fingerprint, or answers nil when it is not this builder's to build now: // - no key: it is created building, by this builder; a create that finds the key made meanwhile gets nothing; -// - a done key is never taken again (the same source is never built twice); +// - a done key is not taken again (the same source is never built twice), except one built on one node only (OneNodePrefix) +// when a second node qualifies now (ClaimOptions.MoreNodes): that is taken as it stands, with the done value, and its +// builder writes the new revision when the second build ends (ADR 0478 clause 4); // - a failed key is taken again as a new revision of the same key (ADR 0429 clause 18), except one failed for a kind no -// builder builds yet (RefusedPrefix, ADR 0478 clause 2), which waits for another fingerprint; +// builder builds yet (RefusedPrefix, ADR 0478 clause 2) or for digests that differ (DigestsDifferPrefix, clause 4), +// which wait for another fingerprint; // - a key left queued or building by this builder is taken as it stands (ADR 0478 clause 5: a stopped builder takes its -// own again when it starts); one held by another builder is left, since nothing reclaims without presence. -func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string) (*Claim, error) { +// own again when it starts), and so is one left by another builder when ClaimOptions.TakeOver (clause 6). +func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string, opt ClaimOptions) (*Claim, error) { key, err := delivery.BuilderBuildKey(module, fingerprint) if err != nil { return nil, err @@ -60,12 +90,15 @@ func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string } switch { case cur.State == delivery.BuildDone: + if modules := OneNodeModules(cur); len(modules) > 0 && opt.MoreNodes != nil && opt.MoreNodes(modules) { + return &Claim{Key: key, Rev: rev, Build: cur}, nil + } return nil, nil - case cur.State == delivery.BuildFailed && strings.HasPrefix(cur.Error, RefusedPrefix): + case cur.State == delivery.BuildFailed && (strings.HasPrefix(cur.Error, RefusedPrefix) || strings.HasPrefix(cur.Error, DigestsDifferPrefix)): return nil, nil case cur.State == delivery.BuildFailed: return c.write(ctx, key, building, func(data []byte) (uint64, error) { return c.Store.Update(ctx, key, data, rev) }) - case cur.By == c.By: + case cur.By == c.By || opt.TakeOver: return &Claim{Key: key, Rev: rev, Build: cur}, nil } return nil, nil diff --git a/modules/mesh-delivery-builder/internal/build/claim_test.go b/modules/mesh-delivery-builder/internal/build/claim_test.go index 7589524..644b586 100644 --- a/modules/mesh-delivery-builder/internal/build/claim_test.go +++ b/modules/mesh-delivery-builder/internal/build/claim_test.go @@ -94,14 +94,14 @@ func valueOf(t *testing.T, st *memStore, key string) delivery.BuilderBuild { func TestClaimCreatesTheKeyBuildingAndASecondBuilderGetsNothing(t *testing.T) { // 1607/AC3 st := newMem() a, b := Claimer{Store: st, By: "novox.a"}, Claimer{Store: st, By: "novox.b"} - got, err := a.Claim(context.Background(), "m", fp1, hex64('a')) + got, err := a.Claim(context.Background(), "m", fp1, hex64('a'), ClaimOptions{}) if err != nil || got == nil { t.Fatalf("first claim = %v, %v, want a claim", got, err) } if v := valueOf(t, st, got.Key); v.State != delivery.BuildBuilding || v.By != "novox.a" || v.Manifest != hex64('a') { t.Errorf("the key holds %+v, want building by novox.a with the manifest digest", v) } - if second, err := b.Claim(context.Background(), "m", fp1, hex64('a')); err != nil || second != nil { + if second, err := b.Claim(context.Background(), "m", fp1, hex64('a'), ClaimOptions{}); err != nil || second != nil { t.Errorf("second builder's claim = %v, %v, want none: the same source is never built twice", second, err) } } @@ -109,7 +109,7 @@ func TestClaimCreatesTheKeyBuildingAndASecondBuilderGetsNothing(t *testing.T) { func TestClaimLosingTheCreateRaceGetsNothing(t *testing.T) { // 1607/AC3 st := newMem() st.before = func() { seed(t, st, delivery.BuilderBuild{State: delivery.BuildBuilding, By: "novox.b"}) } - got, err := Claimer{Store: st, By: "novox.a"}.Claim(context.Background(), "m", fp1, "") + got, err := Claimer{Store: st, By: "novox.a"}.Claim(context.Background(), "m", fp1, "", ClaimOptions{}) if err != nil || got != nil { t.Errorf("claim = %v, %v, want none: the create failed because the key exists", got, err) } @@ -119,7 +119,7 @@ func TestADoneKeyIsNeverTakenAgain(t *testing.T) { // 1607/AC3 st := newMem() seed(t, st, delivery.BuilderBuild{State: delivery.BuildDone, By: "novox.a", Manifest: hex64('a'), Artifacts: []delivery.BuiltArtifact{{Name: "x", Digest: digest}}}) writes := st.writes - got, err := Claimer{Store: st, By: "novox.a"}.Claim(context.Background(), "m", fp1, hex64('a')) + got, err := Claimer{Store: st, By: "novox.a"}.Claim(context.Background(), "m", fp1, hex64('a'), ClaimOptions{}) if err != nil || got != nil || st.writes != writes { t.Errorf("claim = %v, %v, writes %d (was %d), want none and no write", got, err, st.writes, writes) } @@ -129,7 +129,7 @@ func TestAFailedKeyIsTakenAgainAsANewRevisionExceptARefusedKind(t *testing.T) { st := newMem() key := seed(t, st, delivery.BuilderBuild{State: delivery.BuildFailed, By: "novox.a", Error: "the step failed"}) rev := st.revs[key] - got, err := Claimer{Store: st, By: "novox.a"}.Claim(context.Background(), "m", fp1, hex64('a')) + got, err := Claimer{Store: st, By: "novox.a"}.Claim(context.Background(), "m", fp1, hex64('a'), ClaimOptions{}) if err != nil || got == nil || st.revs[key] <= rev { t.Fatalf("claim = %v, %v, revision %d (was %d), want a claim as a new revision of the same key", got, err, st.revs[key], rev) } @@ -140,26 +140,15 @@ func TestAFailedKeyIsTakenAgainAsANewRevisionExceptARefusedKind(t *testing.T) { st = newMem() key = seed(t, st, delivery.BuilderBuild{State: delivery.BuildFailed, By: "novox.a", Error: RefusedPrefix + " image (artifact x): build-agent builds it"}) writes := st.writes - if got, err := (Claimer{Store: st, By: "novox.a"}).Claim(context.Background(), "m", fp1, ""); err != nil || got != nil || st.writes != writes { + if got, err := (Claimer{Store: st, By: "novox.a"}).Claim(context.Background(), "m", fp1, "", ClaimOptions{}); err != nil || got != nil || st.writes != writes { t.Errorf("a refused kind: claim = %v, %v, writes %d (was %d), want none and no write", got, err, st.writes, writes) } } -func TestABuildLeftBuildingBySameBuilderIsResumedAndAnotherBuildersIsLeft(t *testing.T) { // 1607/AC3, ADR 0478 clause 5 - st := newMem() - seed(t, st, delivery.BuilderBuild{State: delivery.BuildBuilding, By: "novox.a"}) - if got, err := (Claimer{Store: st, By: "novox.a"}).Claim(context.Background(), "m", fp1, ""); err != nil || got == nil { - t.Errorf("own building key: claim = %v, %v, want it taken again", got, err) - } - if got, err := (Claimer{Store: st, By: "novox.b"}).Claim(context.Background(), "m", fp1, ""); err != nil || got != nil { - t.Errorf("another builder's building key: claim = %v, %v, want none: nothing reclaims without presence", got, err) - } -} - func TestFinishMovesTheKeyByCompareAndSetAndWritesOnlyADifferentValue(t *testing.T) { // 1607/AC3 st := newMem() c := Claimer{Store: st, By: "novox.a"} - got, _ := c.Claim(context.Background(), "m", fp1, hex64('a')) + got, _ := c.Claim(context.Background(), "m", fp1, hex64('a'), ClaimOptions{}) done := delivery.BuilderBuild{State: delivery.BuildDone, By: "novox.a", Manifest: hex64('a'), Artifacts: []delivery.BuiltArtifact{{Name: "x", Kind: "archive", Digest: digest}}} if err := c.Finish(context.Background(), got, done); err != nil { t.Fatal(err) @@ -179,7 +168,7 @@ func TestFinishMovesTheKeyByCompareAndSetAndWritesOnlyADifferentValue(t *testing func TestFinishAfterTheKeyChangedIsAConflictNotAnOverwrite(t *testing.T) { // 1607/AC3 st := newMem() c := Claimer{Store: st, By: "novox.a"} - got, _ := c.Claim(context.Background(), "m", fp1, "") + got, _ := c.Claim(context.Background(), "m", fp1, "", ClaimOptions{}) st.put(got.Key, []byte(`{"v":1,"state":"failed","error":"someone else"}`)) err := c.Finish(context.Background(), got, delivery.BuilderBuild{State: delivery.BuildFailed, Error: "mine"}) if !errors.Is(err, ErrConflict) { diff --git a/modules/mesh-delivery-builder/internal/build/health.go b/modules/mesh-delivery-builder/internal/build/health.go new file mode 100644 index 0000000..d75e40b --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/health.go @@ -0,0 +1,56 @@ +package build + +import ( + "sync" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// HaveSet holds what each node's node-engine says of each module on it (node-engine_have, key `.`): the +// builder reads it to know whether a node's build module is running and healthy, since a build module whose installed +// toolchain is not the one its recipe builds with is unhealthy (mesh-build-go build_go_health) and its steps would +// refuse (ADR 0478 clause 3). A module the node-engine has written no key of is not healthy. Safe for use from the watch +// and from a build at once. +type HaveSet struct { + mu sync.Mutex + healthy map[string]bool // by `.` +} + +// Put takes a change of the have bucket in and answers whether a module's healthy state changed. A node's own key +// (`.node`) and a value that is not a have are left out. +func (s *HaveSet) Put(key string, value []byte, deleted bool) bool { + node, module, isNode, err := delivery.ParseHaveKey(key) + if err != nil || isNode { + return false + } + ok := false + if !deleted { + h, err := delivery.DecodeHave(value) + if err != nil { + return false + } + ok = h.Phase == delivery.PhaseRunning && h.Health == delivery.HealthOK + } + k := node + "." + module + s.mu.Lock() + defer s.mu.Unlock() + if s.healthy[k] == ok { + return false + } + if s.healthy == nil { + s.healthy = map[string]bool{} + } + if ok { + s.healthy[k] = true + } else { + delete(s.healthy, k) + } + return true +} + +// Healthy is whether the module on the node was last seen running and healthy. +func (s *HaveSet) Healthy(node, module string) bool { + s.mu.Lock() + defer s.mu.Unlock() + return s.healthy[node+"."+module] +} diff --git a/modules/mesh-delivery-builder/internal/build/places.go b/modules/mesh-delivery-builder/internal/build/places.go index 3ff6701..18e2c30 100644 --- a/modules/mesh-delivery-builder/internal/build/places.go +++ b/modules/mesh-delivery-builder/internal/build/places.go @@ -36,16 +36,20 @@ func ParseCapacity(s string) (map[string]int, error) { return out, nil } -// Places counts the build steps the builder runs on each node and holds a step back while its node is at its -// capacity (the operator's decision of 2026-10-21; ADR 0478 clause 3). A step waits for a place by watching the ends of -// the other steps, not by a timer: ending a step wakes the waiting ones. +// Places counts the build steps the builder runs on each node and holds a lane back while its node is at its +// capacity (the operator's decision of 2026-10-21; ADR 0478 clause 3). A lane is the fetch and the builds of one build +// module on one node, run one step at a time, so a lane holds one place while it runs. A node is chosen for a lane by +// reserving a place on it at the moment of the choice (choose), so that builds choosing at once see each other's +// choices and none waits on a full node while another has room. A lane waits for its place by watching the ends of the +// other lanes, not by a timer: ending a lane wakes the waiting ones. type Places struct { // Capacity is the number of steps at once by node; a node not in it has DefaultCapacity. Capacity map[string]int - mu sync.Mutex - running map[string]int - changed chan struct{} // closed, and replaced, each time a step ends + mu sync.Mutex + running map[string]int // lanes holding a place + reserved map[string]int // lanes chosen for the node that do not hold a place yet + changed chan struct{} // closed, and replaced, each time a lane ends } // CapacityOf is how many steps the builder runs on the node at once. @@ -69,17 +73,43 @@ func (p *Places) Running() map[string]int { return out } +// Reserved is the lanes chosen for a node that hold no place yet, by node. +func (p *Places) Reserved() map[string]int { + p.mu.Lock() + defer p.mu.Unlock() + out := make(map[string]int, len(p.reserved)) + for n, c := range p.reserved { + if c > 0 { + out[n] = c + } + } + return out +} + +func (p *Places) init() { + if p.running == nil { + p.running = map[string]int{} + } + if p.reserved == nil { + p.reserved = map[string]int{} + } + if p.changed == nil { + p.changed = make(chan struct{}) + } +} + // Acquire takes a place on the node, waiting while the node is at its capacity, and answers the function that gives it -// back. It returns an error when ctx ends first. +// back. A place reserved for the node by a choice is the one taken. It returns an error when ctx ends first, and then +// gives up the reservation too. func (p *Places) Acquire(ctx context.Context, node string) (release func(), err error) { for { p.mu.Lock() - if p.running == nil { - p.running = map[string]int{} - p.changed = make(chan struct{}) - } + p.init() if p.running[node] < p.CapacityOf(node) { p.running[node]++ + if p.reserved[node] > 0 { + p.reserved[node]-- + } p.mu.Unlock() var once sync.Once return func() { @@ -97,22 +127,44 @@ func (p *Places) Acquire(ctx context.Context, node string) (release func(), err select { case <-wait: case <-ctx.Done(): + p.Cancel(node) return nil, fmt.Errorf("waiting for a place on %s: %w", node, ctx.Err()) } } } -// order orders the nodes for a choice: those under their capacity first, then fewest running, then by name. -func (p *Places) order(nodes []string) { - running := p.Running() +// Cancel gives up a place reserved for the node that will not be taken. +func (p *Places) Cancel(node string) { + p.mu.Lock() + defer p.mu.Unlock() + p.init() + if p.reserved[node] > 0 { + p.reserved[node]-- + } +} + +// choose orders the nodes (those with room first, counting the lanes running and the lanes reserved, then the fewest of +// them, then by name), keeps the first want and reserves a place on each, all at once. +func (p *Places) choose(nodes []string, want int) []string { + p.mu.Lock() + defer p.mu.Unlock() + p.init() + load := func(n string) int { return p.running[n] + p.reserved[n] } sort.SliceStable(nodes, func(i, j int) bool { - ri, rj := running[nodes[i]] < p.CapacityOf(nodes[i]), running[nodes[j]] < p.CapacityOf(nodes[j]) + ri, rj := load(nodes[i]) < p.CapacityOf(nodes[i]), load(nodes[j]) < p.CapacityOf(nodes[j]) if ri != rj { return ri } - if running[nodes[i]] != running[nodes[j]] { - return running[nodes[i]] < running[nodes[j]] + if load(nodes[i]) != load(nodes[j]) { + return load(nodes[i]) < load(nodes[j]) } return nodes[i] < nodes[j] }) + if len(nodes) > want { + nodes = nodes[:want] + } + for _, n := range nodes { + p.reserved[n]++ + } + return nodes } diff --git a/modules/mesh-delivery-builder/internal/build/plan.go b/modules/mesh-delivery-builder/internal/build/plan.go index 78aff09..d765825 100644 --- a/modules/mesh-delivery-builder/internal/build/plan.go +++ b/modules/mesh-delivery-builder/internal/build/plan.go @@ -46,6 +46,17 @@ func (p Plan) Modules() []string { return out } +// NeedsToolchain is whether any task the build module builds is built with a toolchain: a bundle is, an archive is not +// (the indexer gives an archive no language, so a fingerprint names no toolchain for it; ADR 0478 clause 1). +func (p Plan) NeedsToolchain(module string) bool { + for _, t := range p.Tasks { + if t.Module == module && t.Kind != catalogue.ArtifactArchive { + return true + } + } + return false +} + // Refused is the error of a plan the builder does not build: an artifact of a kind (or of a language) it has no build // module for. type Refused struct{ Artifact, What string } diff --git a/modules/mesh-delivery-builder/internal/build/round4_test.go b/modules/mesh-delivery-builder/internal/build/round4_test.go new file mode 100644 index 0000000..fc7f638 --- /dev/null +++ b/modules/mesh-delivery-builder/internal/build/round4_test.go @@ -0,0 +1,293 @@ +package build + +import ( + "context" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +func haveValue(t *testing.T, phase delivery.Phase, health delivery.Health) []byte { + t.Helper() + b, err := delivery.EncodeHave(delivery.Have{Phase: phase, Health: health}) + if err != nil { + t.Fatal(err) + } + return b +} + +// 1. A module of an archive only: the real indexer gives an archive no language (inputs.go languagesOf), so its +// fingerprint names no toolchain, and the builder builds it on the build module's nodes with no toolchain agreed. +func TestAnArchiveOnlyModuleIsBuiltAlthoughItsFingerprintNamesNoToolchain(t *testing.T) { // review round 4, 1 + r := newRig(t, archiveA) + v := func() delivery.BuilderBuild { + r.b.Module(context.Background(), "m", moduleValueWith(t, "modules/m", nil)) + r.b.Wait() + if _, ok := r.st.vals[r.key]; !ok { + t.Fatal("no build key was written for the archive-only module") + } + return valueOf(t, r.st, r.key) + }() + if v.State != delivery.BuildDone || len(v.Artifacts) != 1 || v.Artifacts[0].Kind != "archive" || len(v.Toolchains) != 0 { + t.Fatalf("build = %+v, want done with the archive and no toolchain named", v) + } + if n := len(r.bm.callsOf("build_go_build")); n != 2 { + t.Errorf("%d go builds, want one on each of two nodes", n) + } +} + +func TestAnArchiveOnlyModuleNeedsNoAgreedToolchainButNeedsNodesThatQualify(t *testing.T) { // review round 4, 1 + r := newRig(t, archiveA) + r.tools.Put(GoBuildModule, "ace.go", tcValue(t, "go", "go1.26.0"), false) // the nodes disagree: a bundle would wait + r.b.Health = &HaveSet{} // no build module is healthy anywhere + r.b.Module(context.Background(), "m", moduleValueWith(t, "modules/m", nil)) + r.b.Wait() + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildFailed || !strings.Contains(v.Error, "healthy") { + t.Errorf("build = %+v, want failed: no node's build module is healthy", v) + } + have := &HaveSet{} + have.Put("novox.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) + r.b.Health = have + r.b.Reconsider(context.Background()) + r.b.Wait() + v := valueOf(t, r.st, r.key) + if v.State != delivery.BuildDone || !strings.HasPrefix(v.Step, OneNodePrefix) { + t.Errorf("build = %+v, want done on novox, the one node that qualifies, marked as one node", v) + } +} + +func TestABundleAndAnArchiveOfOneBuildModuleStillNeedTheAgreedToolchain(t *testing.T) { // review round 4, 1 + p, err := PlanOf(catalogue.Manifest{Module: "m", Build: &catalogue.Build{Artifacts: []catalogue.Artifact{goBundle, archiveA}}}) + if err != nil || !p.NeedsToolchain(GoBuildModule) { + t.Errorf("a plan with a Go bundle: %v, %v", p.NeedsToolchain(GoBuildModule), err) + } + p, _ = PlanOf(catalogue.Manifest{Module: "m", Build: &catalogue.Build{Artifacts: []catalogue.Artifact{archiveA}}}) + if p.NeedsToolchain(GoBuildModule) { + t.Error("a plan of an archive needs a toolchain") + } +} + +// 2. The digests-differ failure opens with its prefix, and nothing builds it again. +func TestDifferingDigestsFailTheKeyWithThePrefixAndNothingBuildsItAgain(t *testing.T) { // review round 4, 2 + r := newRig(t, goBundle) + r.bm.digests["ace/tool"] = "sha256:" + hex64('9') + v := r.consider(t, "modules/m") + if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, "digests differ:") { + t.Fatalf("build = %+v, want a failure opening with exactly %q", v, "digests differ:") + } + calls, writes := len(r.bm.calls), r.st.writes + r.bm.digests = map[string]string{} // the nodes would now agree + r.b.Reconsider(context.Background()) + r.b.Wait() + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) + r.b.Wait() + if len(r.bm.calls) != calls || r.st.writes != writes { + t.Errorf("a failed-for-digests key was built again: %d calls (was %d), %d writes (was %d)", len(r.bm.calls), calls, r.st.writes, writes) + } +} + +func TestClaimLeavesAKeyFailedForDifferingDigestsAlone(t *testing.T) { // review round 4, 2 + st := newMem() + seed(t, st, delivery.BuilderBuild{State: delivery.BuildFailed, Error: DigestsDifferPrefix + " the nodes built tool to different digests: ace x; novox y"}) + writes := st.writes + got, err := (Claimer{Store: st, By: "novox.a"}).Claim(context.Background(), "m", fp1, "", ClaimOptions{TakeOver: true}) + if err != nil || got != nil || st.writes != writes { + t.Errorf("claim = %v, %v, %d writes (was %d), want none and no write", got, err, st.writes, writes) + } +} + +// 3. A done key built on one node is built again on a second node once one qualifies. +func TestADoneKeyOfOneNodeIsBuiltAgainOnASecondNodeAndTheDigestsCompared(t *testing.T) { // review round 4, 3 + r := newRig(t, goBundle) + r.pres.Put("ace", presenceValue(t, false), false) // only novox qualifies + v := r.consider(t, "modules/m") + if v.State != delivery.BuildDone || !strings.HasPrefix(v.Step, OneNodePrefix+GoBuildModule+"): ") || !strings.Contains(v.Step, "on one node, novox") { + t.Fatalf("build = %+v, want done on one node, marked", v) + } + if got := OneNodeModules(v); len(got) != 1 || got[0] != GoBuildModule { + t.Errorf("OneNodeModules = %v", got) + } + rev, calls := r.st.revs[r.key], len(r.bm.callsOf("build_go_build")) + // Nothing changes while one node qualifies: looking again builds nothing. + r.b.Reconsider(context.Background()) + r.b.Wait() + if r.st.revs[r.key] != rev || len(r.bm.callsOf("build_go_build")) != calls { + t.Fatal("a one-node key was built again with no second node") + } + // ace comes online: the key is built on ace and novox, as a new revision. + r.pres.Put("ace", presenceValue(t, true), false) + r.b.Reconsider(context.Background()) + r.b.Wait() + v = valueOf(t, r.st, r.key) + if v.State != delivery.BuildDone || strings.HasPrefix(v.Step, OneNodePrefix) || !strings.Contains(v.Step, "ace and novox") { + t.Errorf("build = %+v, want done on ace and novox", v) + } + if r.st.revs[r.key] <= rev { + t.Errorf("revision %d, was %d: not a new revision", r.st.revs[r.key], rev) + } + builds := r.bm.callsOf("build_go_build") + if len(builds) != calls+2 || builds[len(builds)-2].node == builds[len(builds)-1].node { + t.Errorf("%d builds (was %d), want one on each of two nodes", len(builds), calls) + } +} + +func TestASecondNodeThatDisagreesFailsTheDoneKeyWithTheDigestsPrefix(t *testing.T) { // review round 4, 3 + r := newRig(t, goBundle) + r.pres.Put("ace", presenceValue(t, false), false) + r.consider(t, "modules/m") + r.bm.digests["ace/tool"] = "sha256:" + hex64('9') + r.pres.Put("ace", presenceValue(t, true), false) + r.b.Reconsider(context.Background()) + r.b.Wait() + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, DigestsDifferPrefix) { + t.Errorf("build = %+v, want failed with digests differ", v) + } +} + +func TestASecondBuildThatFailsForAnotherReasonLeavesTheDoneKeyStanding(t *testing.T) { // review round 4, 3 + r := newRig(t, goBundle) + r.pres.Put("ace", presenceValue(t, false), false) + first := r.consider(t, "modules/m") + r.bm.fails["ace/build_go_fetch"] = "ace is out of disk" + r.pres.Put("ace", presenceValue(t, true), false) + r.b.Reconsider(context.Background()) + r.b.Wait() + if v := valueOf(t, r.st, r.key); !delivery.SameBuilderBuild(v, first) { + t.Errorf("build = %+v, want the first done build to stand: %+v", v, first) + } +} + +// 4. A step never waits on a full node while another qualifying node has room. +func TestChoosesMadeAtOnceSeeEachOthersReservationsSoNoLaneWaitsWhileANodeHasRoom(t *testing.T) { // review round 4, 4 + p := &Places{Capacity: map[string]int{"ace": 2, "g14": 1, "novox": 1, "shanks": 3}} + entries := tcs("go", "go1.27.1", "ace", "g14", "novox", "shanks") + total := 2 + 1 + 1 + 3 + var waiting int + for i := 1; i <= 4; i++ { // four modules choose before any lane has started + room := 0 + for _, n := range []string{"ace", "g14", "novox", "shanks"} { + room += p.CapacityOf(n) - p.Running()[n] - p.Reserved()[n] + } + nodes, _, err := ChooseNodes("go", entries, p, nil, 2) + if err != nil || len(nodes) != 2 { + t.Fatalf("choice %d: %v, %v", i, nodes, err) + } + if room >= 2 { + for _, n := range nodes { + if p.Reserved()[n] > p.CapacityOf(n) { + t.Errorf("choice %d: %v put a lane on full %s while %d places were free", i, nodes, n, room) + } + } + } else { + waiting += 2 - room + } + } + if want := 4*2 - total; waiting != want { + t.Errorf("%d lanes wait, want %d: the 8 lanes less the %d places", waiting, want, total) + } + loads := p.Reserved() + for n, c := range loads { + if c > p.CapacityOf(n) && n != "shanks" && n != "ace" && n != "g14" && n != "novox" { + t.Errorf("unknown node %s", n) + } + } + if loads["shanks"] != 3 || loads["ace"] != 2 || loads["g14"]+loads["novox"] != 3 { + t.Errorf("reservations = %v, want shanks 3, ace 2 and the single-place nodes taking the rest", loads) + } +} + +func TestACancelledReservationFreesTheChoiceAndAnAcquireTakesItsReservation(t *testing.T) { // review round 4, 4 + p := &Places{} + nodes, _, _ := ChooseNodes("go", tcs("go", "go1.27.1", "ace"), p, nil, 1) + if len(nodes) != 1 || p.Reserved()["ace"] != 1 { + t.Fatalf("nodes %v, reserved %v", nodes, p.Reserved()) + } + release, err := p.Acquire(context.Background(), "ace") + if err != nil || p.Reserved()["ace"] != 0 || p.Running()["ace"] != 1 { + t.Fatalf("acquire: %v, reserved %v, running %v", err, p.Reserved(), p.Running()) + } + release() + ChooseNodes("go", tcs("go", "go1.27.1", "ace"), p, nil, 1) + p.Cancel("ace") + if len(p.Reserved()) != 0 { + t.Errorf("reserved after Cancel = %v", p.Reserved()) + } +} + +// 5. A node whose build module is not healthy is not chosen. +func TestHaveSetKnowsOnlyBuildModulesRunningAndHealthy(t *testing.T) { // review round 4, 5 + var s HaveSet + if s.Healthy("ace", GoBuildModule) { + t.Error("a module with no have is healthy") + } + if !s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) || !s.Healthy("ace", GoBuildModule) { + t.Error("a running, healthy module is not") + } + if s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) { + t.Error("the same state counted as a change") + } + if !s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthFailed), false) || s.Healthy("ace", GoBuildModule) { + t.Error("a module whose health failed is healthy") + } + s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) + if !s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseFailed, delivery.HealthOK), false) || s.Healthy("ace", GoBuildModule) { + t.Error("a module whose phase failed is healthy") + } + s.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) + if !s.Put("ace.mesh-build-go", nil, true) || s.Healthy("ace", GoBuildModule) { + t.Error("a deleted have is healthy") + } + if s.Put("ace.node", []byte(`{}`), false) || s.Put("ace.mesh-build-go", []byte(`not json`), false) { + t.Error("a node's key or a non-have counted") + } +} + +func TestANodeWhoseBuildModuleIsNotHealthyIsNotChosen(t *testing.T) { // review round 4, 5 + r := newRig(t, goBundle) + have := &HaveSet{} + have.Put("ace.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthFailed), false) // go is not the recipe's + have.Put("novox.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false) + r.b.Health = have + v := r.consider(t, "modules/m") + if v.State != delivery.BuildDone || !strings.Contains(v.Step, "on one node, novox") { + t.Fatalf("build = %+v, want done on novox alone", v) + } + for _, c := range r.bm.calls { + if c.node == "ace" { + t.Errorf("a step was called on ace, whose build module is unhealthy: %+v", c) + } + } + have.Put("novox.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthFailed), false) + r.st.vals = map[string][]byte{} // a fresh key + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) + r.b.Wait() + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildFailed || !strings.Contains(v.Error, "healthy") { + t.Errorf("build = %+v, want failed: no node has a healthy build module", v) + } +} + +// 6. A key found building is taken over, whoever holds it. +func TestABuildLeftBuildingByAnyBuilderIsTakenOverAtStart(t *testing.T) { // review round 4, 6; 1607/AC3, ADR 0478 clauses 5, 6 + st := newMem() + seed(t, st, delivery.BuilderBuild{State: delivery.BuildBuilding, By: "novox.a"}) + if got, err := (Claimer{Store: st, By: "novox.a"}).Claim(context.Background(), "m", fp1, "", ClaimOptions{}); err != nil || got == nil { + t.Errorf("own building key: claim = %v, %v, want it taken again", got, err) + } + if got, err := (Claimer{Store: st, By: "ace.b"}).Claim(context.Background(), "m", fp1, "", ClaimOptions{TakeOver: true}); err != nil || got == nil || got.Build.By != "novox.a" { + t.Errorf("another node's building key at start: claim = %v, %v, want it taken over", got, err) + } + if got, _ := (Claimer{Store: st, By: "ace.b"}).Claim(context.Background(), "m", fp1, "", ClaimOptions{}); got != nil { + t.Errorf("another builder's building key after the start was taken: a running builder's key is its own") + } +} + +func TestABuilderFindingAKeyBuildingByAnotherNodeBuildsItOnStart(t *testing.T) { // review round 4, 6 + r := newRig(t, goBundle) + seed(t, r.st, delivery.BuilderBuild{State: delivery.BuildBuilding, By: "ace.builder", Manifest: manifestD}) + v := r.consider(t, "modules/m") + if v.State != delivery.BuildDone || v.By != "novox.builder" { + t.Errorf("build = %+v, want it taken over and done by novox.builder", v) + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index cd25c70..f7ade56 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -31,6 +31,11 @@ type Builder struct { // Presence says which nodes are online (the watchman's); a node that is not is not chosen. Presence interface{ Online(node string) bool } Manifests Manifests + // Health says whether a node's build module is running and healthy (the node-engine's have); a node whose build module + // is not is not chosen. Nil means every node's is. + Health interface { + Healthy(node, module string) bool + } // Places holds a step back while its node is at its build capacity; a Builder with none runs one step at a time on each node. Places *Places // Nodes is how many build nodes build each fingerprint: two until release R4e (ADR 0478 clause 4). @@ -47,6 +52,7 @@ type Builder struct { inflight map[string]bool // by build key again map[string]delivery.Module // by build key: the newest value that arrived while the key was being built plans map[string]planned // by manifest digest + tried map[string]bool // by build key: looked at since this process started } type planned struct { @@ -61,7 +67,7 @@ func (b *Builder) init() { limit = 4 } b.sem = make(chan struct{}, limit) - b.known, b.inflight, b.again, b.plans = map[string]delivery.Module{}, map[string]bool{}, map[string]delivery.Module{}, map[string]planned{} + b.known, b.inflight, b.again, b.plans, b.tried = map[string]delivery.Module{}, map[string]bool{}, map[string]delivery.Module{}, map[string]planned{}, map[string]bool{} if b.Places == nil { b.Places = &Places{} } @@ -196,7 +202,12 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { return } } - cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest) + key, _ := delivery.BuilderBuildKey(module, m.FP2) + b.mu.Lock() + first := !b.tried[key] + b.tried[key] = true + b.mu.Unlock() + cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest, ClaimOptions{TakeOver: first, MoreNodes: func(modules []string) bool { return b.moreNodes(modules) }}) if err != nil { b.logf("%s: %v", module, err) return @@ -213,6 +224,12 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { return } result.By, result.Manifest = b.Claimer.By, m.Manifest + if cl.Build.State == delivery.BuildDone && result.State == delivery.BuildFailed && !strings.HasPrefix(result.Error, DigestsDifferPrefix) { + // A done key built again on a second node keeps its done value when the new build fails for another reason than + // the digests: the first build stands, and a later change of the nodes looks again. + b.logf("%s: built again on a second node and failed, the done build stands: %s", cl.Key, result.Error) + return + } if err := b.Claimer.Finish(ctx, cl, result); err != nil { b.logf("%s: the build could not be written: %v", cl.Key, err) return @@ -226,6 +243,9 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { // has agreed (novox/hq ADR 0465) must be the one the fingerprint was computed with (Module.Toolchains). func (b *Builder) stale(m delivery.Module, plan Plan) string { for _, bm := range plan.Modules() { + if !plan.NeedsToolchain(bm) { + continue // an archive is packed with no toolchain + } language := LanguageOf[bm] tc, none := delivery.AgreedToolchain(b.Toolchains.Entries(language)) if none != "" { @@ -262,18 +282,47 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p repository := delivery.RepositoryName(m.Repository) var lanes []*lane used := map[string]delivery.ToolchainUsed{} - var steps []string + var steps, oneNode []string + // Every node chosen holds a reservation until its lane takes the place; a build that ends before its lanes start gives them up. + giveUp := func(nodes ...string) { + for _, n := range nodes { + b.Places.Cancel(n) + } + } + giveUpLanes := func() { + for _, l := range lanes { + b.Places.Cancel(l.node) + } + } for _, bm := range plan.Modules() { language := LanguageOf[bm] - nodes, tc, err := ChooseNodes(language, b.Toolchains.Entries(language), b.Places, b.online, b.Nodes) - if err != nil { - return failed("%s", err) + entries := b.Toolchains.Entries(language) + qualifies := func(node string) bool { return b.qualifies(node, bm) } + var nodes []string + if plan.NeedsToolchain(bm) { + ns, tc, err := ChooseNodes(language, entries, b.Places, qualifies, b.Nodes) + if err != nil { + giveUpLanes() + return failed("%s", err) + } + if want := m.Toolchains[language]; want.Version != tc.Version || want.Recipe != tc.Recipe { + giveUp(ns...) + giveUpLanes() + return delivery.BuilderBuild{} // the agreed toolchain moved since try looked: nothing is written + } + used[language] = delivery.ToolchainUsed{Version: tc.Version, Recipe: tc.Recipe} + nodes = ns + } else { + // An archive is packed with no toolchain (the fingerprint names none): any node of the build module that qualifies. + ns, err := ChooseAnyNodes(language, entries, b.Places, qualifies, b.Nodes) + if err != nil { + giveUpLanes() + return failed("%s", err) + } + nodes = ns } - if want := m.Toolchains[language]; want.Version != tc.Version || want.Recipe != tc.Recipe { - return delivery.BuilderBuild{} // the agreed toolchain moved since try looked: nothing is written - } - used[language] = delivery.ToolchainUsed{Version: tc.Version, Recipe: tc.Recipe} if len(nodes) == 1 { + oneNode = append(oneNode, bm) steps = append(steps, fmt.Sprintf("%s on one node, %s", bm, nodes[0])) } else { sorted := append([]string(nil), nodes...) @@ -319,23 +368,55 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p } artifacts = append(artifacts, delivery.BuiltArtifact{Name: t.Artifact, Kind: t.Kind, Digest: d}) } - return delivery.BuilderBuild{State: delivery.BuildDone, Step: "built " + strings.Join(steps, "; "), Artifacts: artifacts, Toolchains: used} + step := "built " + strings.Join(steps, "; ") + if len(oneNode) > 0 { + // Marked so that Claim builds the key again once a second node qualifies (ADR 0478 clause 4). + step = OneNodePrefix + strings.Join(oneNode, ", ") + "): " + step + } + if len(used) == 0 { + used = nil + } + return delivery.BuilderBuild{State: delivery.BuildDone, Step: step, Artifacts: artifacts, Toolchains: used} } func (b *Builder) online(node string) bool { return b.Presence == nil || b.Presence.Online(node) } -// step runs one step of a build module, once its node has a place for it. -func (b *Builder) step(ctx context.Context, bm, node, tool string, args map[string]any) (json.RawMessage, error) { - release, err := b.Places.Acquire(ctx, node) - if err != nil { - return nil, err +// qualifies is whether the node can build with the build module now: it is online and the build module on it is healthy. +func (b *Builder) qualifies(node, bm string) bool { + return b.online(node) && (b.Health == nil || b.Health.Healthy(node, bm)) +} + +// moreNodes is whether any of these build modules has two nodes that qualify now. +func (b *Builder) moreNodes(modules []string) bool { + for _, bm := range modules { + seen := map[string]bool{} + n := 0 + for _, e := range b.Toolchains.Entries(LanguageOf[bm]) { + if !seen[e.Node] && b.qualifies(e.Node, bm) { + seen[e.Node] = true + n++ + } + } + if n >= 2 { + return true + } } - defer release() + return false +} + +// step runs one step of a build module; the lane that runs it holds its node's place. +func (b *Builder) step(ctx context.Context, bm, node, tool string, args map[string]any) (json.RawMessage, error) { return b.Runner.Run(ctx, bm, node, toolPrefix[bm]+tool, args) } -// runLane fetches the commit on the node and builds each of the lane's artifacts there. +// runLane takes a place on the node, fetches the commit there and builds each of the lane's artifacts, one step at a time. func (b *Builder) runLane(ctx context.Context, l *lane, repository string, m delivery.Module, module string) { + release, err := b.Places.Acquire(ctx, l.node) + if err != nil { + l.err = err + return + } + defer release() raw, err := b.step(ctx, l.module, l.node, "fetch", map[string]any{"repository": repository, "commit": m.Commit}) if err != nil { l.err = err diff --git a/modules/mesh-delivery-builder/internal/build/run_test.go b/modules/mesh-delivery-builder/internal/build/run_test.go index 3a00ba8..34c89a6 100644 --- a/modules/mesh-delivery-builder/internal/build/run_test.go +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -286,7 +286,10 @@ func TestAKeyTheBuilderLeftBuildingIsBuiltAtStartAndDoneKeysAreNotBuiltTwice(t * } } -func TestTwoInstancesBuildOneFingerprintOnce(t *testing.T) { // 1607/AC7 +// Two instances that start together on one key: the second finds the key building and takes it over (ADR 0478 clause 6: +// a key found building at start is taken over, whoever holds it), so both may build; the key is written once, by the one +// whose compare-and-set comes first, and the other's write is refused, never overwriting it. +func TestTwoInstancesStartingTogetherEndWithOneDoneKey(t *testing.T) { // 1607/AC7, review round 4 r := newRig(t, goBundle) other := &Builder{Claimer: Claimer{Store: r.st, By: "ace.builder"}, Runner: r.b.Runner, Toolchains: r.tools, Presence: r.pres, Manifests: r.mf, Nodes: 2, Logf: t.Logf} value := moduleValue(t, "modules/m") @@ -296,8 +299,11 @@ func TestTwoInstancesBuildOneFingerprintOnce(t *testing.T) { // 1607/AC7 go func() { defer wg.Done(); b.Module(context.Background(), "m", value); b.Wait() }() } wg.Wait() - if n := len(r.bm.callsOf("build_go_build")); n != 2 { // one instance, two nodes - t.Errorf("%d builds, want 2: one instance built it", n) + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildDone || len(v.Artifacts) != 1 { + t.Errorf("build = %+v, want one done key", v) + } + if n := len(r.bm.callsOf("build_go_build")); n < 2 || n%2 != 0 { + t.Errorf("%d builds, want two for each instance that built", n) } } diff --git a/modules/mesh-delivery-builder/manifest_test.go b/modules/mesh-delivery-builder/manifest_test.go index 87781bc..374764b 100644 --- a/modules/mesh-delivery-builder/manifest_test.go +++ b/modules/mesh-delivery-builder/manifest_test.go @@ -81,7 +81,7 @@ func TestManifestDeclarations(t *testing.T) { // 1607/AC1 t.Errorf("state = %+v, want %+v", m.State, want) } if want := []string{"mesh-build-go.steps", "mesh-build-go.toolchains", "mesh-build-node.steps", "mesh-build-node.toolchains", - "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules", "mesh-delivery-watchman.presence"}; !reflect.DeepEqual(sorted(m.Reads), want) { + "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules", "mesh-delivery-watchman.presence", "node-engine.have"}; !reflect.DeepEqual(sorted(m.Reads), want) { t.Errorf("reads = %v, want %v", m.Reads, want) } if want := []string{"mesh-build-go.build_go_build", "mesh-build-go.build_go_fetch", "mesh-build-node.build_node_build", "mesh-build-node.build_node_fetch"}; !reflect.DeepEqual(sorted(m.Invokes), want) { diff --git a/modules/mesh-delivery-builder/module.json b/modules/mesh-delivery-builder/module.json index a886fbe..115de62 100644 --- a/modules/mesh-delivery-builder/module.json +++ b/modules/mesh-delivery-builder/module.json @@ -16,7 +16,8 @@ "mesh-build-node.toolchains", "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules", - "mesh-delivery-watchman.presence" + "mesh-delivery-watchman.presence", + "node-engine.have" ], "invokes": [ "mesh-build-go.build_go_build",