From 563a624a6d3d70ca9aa5228142c25fdb6b6636b3 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 8 Nov 2026 17:00:15 +0100 Subject: [PATCH] Grants read the have's build for memberships and holdings, and a failed read of it holds the pass back (issue 1966, review round 1) Claude-Session: https://claude.ai/code/session_01NcbgfFfQ4Z9UE5jK32PmE5 --- modules/mesh-grants/grants_test.go | 49 +++++++++++++++++++ modules/mesh-grants/internal/grants/inputs.go | 6 +++ .../internal/grants/memberships.go | 5 +- .../mesh-grants/internal/grants/records.go | 5 +- 4 files changed, 63 insertions(+), 2 deletions(-) diff --git a/modules/mesh-grants/grants_test.go b/modules/mesh-grants/grants_test.go index 07e47eb3..01c04158 100644 --- a/modules/mesh-grants/grants_test.go +++ b/modules/mesh-grants/grants_test.go @@ -4042,3 +4042,52 @@ func TestMembershipKeepsWhatTheRunningBuildReads(t *testing.T) { }) } } + +// A pass reads each machine's have for the build it runs (issue 1966): its membership keeps what that older build reads, a +// have or build that cannot be read holds the pass back, and the seats held are the ones that build claims (ADR 0660 ยง2). +func TestPassComposesMembershipFromTheHaveBuild(t *testing.T) { + const old = `{"module":"mod-y","version":"1","own-secrets":{"broker":"/var/lib/y/broker"},"reads":["mod-x.kept","mod-x.old"]}` + runs := func(b *fakeBus) { + b.manifestOf("mod-y", yManifest) + digest := meshcatalogclient.ManifestDigest([]byte(old)) + b.put(grants.ReadManifests, digest, meshcatalog.ManifestsByDigestValue{V: keys.Version, Module: "mod-y", Manifest: old}) + fp := "src2:" + strings.Repeat("cd", 32) + b.put(grants.ReadHave, "b.mod-y", nodeengine.HaveByNodeModuleValue{V: keys.Version, Phase: nodeengineclient.PhaseRunning, Health: nodeengineclient.HealthOK, Fp: fp}) + key, _ := builds.BuilderBuildKey("mod-y", fp) + b.put(grants.ReadBuilds, key, map[string]any{"v": keys.Version, "state": "done", "manifest": digest}) + } + t.Run("the published membership keeps a read of the build the have names", func(t *testing.T) { + b := fixture() + runs(b) + partOn(b).Pass() + if got := string(b.own[grants.StateMemberships]["b.mod-y"]); !strings.Contains(got, "mod-x.old") || !strings.Contains(got, "mod-x.kept") { + t.Errorf("b.mod-y's membership lacks the running build's read: %s", got) + } + }) + t.Run("a build that cannot be read is a failed read", func(t *testing.T) { + b := fixture() + runs(b) + b.refused[grants.ReadBuilds] = true + if f := grants.ReadInputs(b, &grants.Manifests{}).ReadFailed(); !strings.Contains(strings.Join(f, "\n"), grants.ReadBuilds) { + t.Errorf("a build read refused is not a failed read: %v", f) + } + }) + t.Run("the seats held are the have build's claims, then the want's, then the trunk's", func(t *testing.T) { + claims := catalogue.Manifest{Module: "m", Version: "1", Claims: []catalogue.Claim{{Name: "mesh-catalog", Scope: catalogue.ScopeMesh}}} + bare := catalogue.Manifest{Module: "m", Version: "1"} + for name, row := range map[string]struct { + had, running map[string]catalogue.Manifest + held bool + }{ + "trunk alone": {nil, nil, false}, + "have claims": {map[string]catalogue.Manifest{"a.m": claims}, map[string]catalogue.Manifest{"a.m": bare}, true}, + "want claims": {nil, map[string]catalogue.Manifest{"a.m": claims}, true}, + "have is bare": {map[string]catalogue.Manifest{"a.m": bare}, map[string]catalogue.Manifest{"a.m": claims}, false}, + } { + in := grants.Inputs{Nodes: []string{"a"}, Assigned: map[string][]string{"a": {"m"}}, Manifests: map[string]catalogue.Manifest{"m": bare}, Had: row.had, Running: row.running} + if got := len(grants.HoldingsOf(in).Claimed) > 0; got != row.held { + t.Errorf("%s: claimed %v, want %v", name, got, row.held) + } + } + }) +} diff --git a/modules/mesh-grants/internal/grants/inputs.go b/modules/mesh-grants/internal/grants/inputs.go index b397006e..ae603936 100644 --- a/modules/mesh-grants/internal/grants/inputs.go +++ b/modules/mesh-grants/internal/grants/inputs.go @@ -754,6 +754,9 @@ func runningByHaves(r Reader, in *Inputs, cache *Manifests, into map[string]cata for node, modules := range in.Assigned { for _, module := range modules { raw, found, err := r.Get(ReadHave, node+"."+module) + if err != nil { + in.Gaps = append(in.Gaps, Gap{Input: ReadHave + " " + node + "." + module, Why: "cannot be read, so the build it runs is unknown: " + err.Error(), Failed: true}) + } if err != nil || !found { fromTrunk = append(fromTrunk, node+"."+module) continue @@ -769,6 +772,9 @@ func runningByHaves(r Reader, in *Inputs, cache *Manifests, into map[string]cata continue } braw, found, err := r.Get(ReadBuilds, bk) + if err != nil { + in.Gaps = append(in.Gaps, Gap{Input: ReadBuilds + " " + bk, Why: "cannot be read, so the manifest of the build " + node + "." + module + " runs is unknown: " + err.Error(), Failed: true}) + } if err != nil || !found { fromTrunk = append(fromTrunk, node+"."+module) continue diff --git a/modules/mesh-grants/internal/grants/memberships.go b/modules/mesh-grants/internal/grants/memberships.go index 55946fc1..165c12b6 100644 --- a/modules/mesh-grants/internal/grants/memberships.go +++ b/modules/mesh-grants/internal/grants/memberships.go @@ -206,7 +206,10 @@ func HoldingsOf(in Inputs) Holdings { } for node, modules := range in.Assigned { for _, module := range modules { - m, ok := in.Running[node+"."+module] + m, ok := in.Had[node+"."+module] + if !ok { + m, ok = in.Running[node+"."+module] + } if !ok { m, ok = in.Manifests[module] } diff --git a/modules/mesh-grants/internal/grants/records.go b/modules/mesh-grants/internal/grants/records.go index b82f4dd1..06e674f9 100644 --- a/modules/mesh-grants/internal/grants/records.go +++ b/modules/mesh-grants/internal/grants/records.go @@ -448,7 +448,10 @@ func holdersAfterPass(in Inputs) ([]catalogue.Held, []Gap) { var assignments []catalogue.Assignment for _, node := range slices.Sorted(maps.Keys(in.Assigned)) { for _, module := range in.Assigned[node] { - m, ok := in.Running[node+"."+module] + m, ok := in.Had[node+"."+module] + if !ok { + m, ok = in.Running[node+"."+module] + } if !ok { m, ok = in.Manifests[module] }