diff --git a/modules/mesh-grants/grants_test.go b/modules/mesh-grants/grants_test.go index 1cfe5cd9..117f1ee0 100644 --- a/modules/mesh-grants/grants_test.go +++ b/modules/mesh-grants/grants_test.go @@ -4000,3 +4000,87 @@ func (g *seatGate) MayAct(seats ...string) bool { func short(d string) string { return strings.TrimPrefix(d, "sha256:")[:min(12, len(strings.TrimPrefix(d, "sha256:")))] } + +// A membership is composed from the build each machine runs now, the want's and the trunk's, so none loses a read (issue 1966: +// the vault build running on the node-engines read mesh-delivery-assigner.placement, which the trunk's manifest no longer does). +func TestMembershipKeepsWhatTheRunningBuildReads(t *testing.T) { + reader := func(reads ...string) catalogue.Manifest { + return catalogue.Manifest{Module: "mesh-vault", Version: "1", RunsAs: "mesh-vault", Reads: reads, + OwnSecrets: catalogue.OwnSecrets{"broker": {Path: "/var/lib/mesh-vault/broker"}}} + } + owner := catalogue.Manifest{Module: "mesh-delivery-assigner", Version: "1", RunsAs: "mesh-delivery-assigner", + OwnSecrets: catalogue.OwnSecrets{"broker": {Path: "/var/lib/a/broker"}}} + rows := []struct { + name string + had, running map[string]catalogue.Manifest + reads []string + want []string + }{ + {"the trunk alone", nil, nil, []string{"mesh-config.values"}, []string{"mesh-config.values"}}, + {"the build run reads what the trunk dropped", map[string]catalogue.Manifest{"a.mesh-vault": reader("mesh-delivery-assigner.placement")}, nil, []string{"mesh-config.values"}, []string{"mesh-config.values", "mesh-delivery-assigner.placement"}}, + {"the want's build reads what the trunk dropped", nil, map[string]catalogue.Manifest{"a.mesh-vault": reader("mesh-delivery-assigner.placement")}, []string{"mesh-config.values"}, []string{"mesh-config.values", "mesh-delivery-assigner.placement"}}, + } + for _, row := range rows { + t.Run(row.name, func(t *testing.T) { + in := grants.Inputs{Nodes: []string{"a"}, Assigned: map[string][]string{"a": {"mesh-vault", "mesh-delivery-assigner"}}, + Manifests: map[string]catalogue.Manifest{"mesh-vault": reader(row.reads...), "mesh-delivery-assigner": owner}, + Running: map[string]catalogue.Manifest{}, Had: map[string]catalogue.Manifest{}} + maps.Copy(in.Running, row.running) + maps.Copy(in.Had, row.had) + recs, _ := grants.ListRecords(in) + var got []string + for _, s := range grants.Memberships(recs)["a.mesh-vault"].Membership.State { + if !s.Writes { + got = append(got, s.Name) + } + } + slices.Sort(got) + slices.Sort(row.want) + if !slices.Equal(got, row.want) { + t.Errorf("reads = %v, want %v", got, row.want) + } + }) + } +} + +// 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 pass that cannot read the build a have names keeps the membership last published", func(t *testing.T) { + b := fixture() + runs(b) + part := partOn(b) + part.Pass() + b.refused[grants.ReadBuilds] = true + part.Pass() + if got := string(b.own[grants.StateMemberships]["b.mod-y"]); !strings.Contains(got, "mod-x.old") { + t.Errorf("b.mod-y's membership lost the running build's read when its build could not be 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) + } + }) +} diff --git a/modules/mesh-grants/internal/grants/inputs.go b/modules/mesh-grants/internal/grants/inputs.go index 84dc881d..4b63aa60 100644 --- a/modules/mesh-grants/internal/grants/inputs.go +++ b/modules/mesh-grants/internal/grants/inputs.go @@ -151,6 +151,12 @@ type Inputs struct { // Running is the manifest a machine is wanted to run, by `.`: the one the builder built at the // fingerprint the assigner's want names. An assignment with none is composed from the trunk's manifest. Running map[string]catalogue.Manifest + // Had is the manifest of the build each machine runs now (its have's fingerprint, the builder's build of it), by + // `.`; the want's Running may already be a newer build. A membership is composed from both and the trunk's. + Had map[string]catalogue.Manifest + // HadFailed is why the build a machine runs could not be read this pass (a have or a build read that failed), by + // `.`: its membership is then composed without that build's reads, and the part keeps the last published. + HadFailed map[string]string // Values is each resolved config value, by `//`. Values map[string]any // BusNode is the machine running the bus's module, and Loaded what its node agent says it loaded. @@ -322,7 +328,7 @@ func ReadInputs(r Reader, cache *Manifests) Inputs { gate := cache.gate cache.mu.Unlock() in := Inputs{Assigned: map[string][]string{}, Manifests: map[string]catalogue.Manifest{}, Digests: map[string]string{}, Fingerprints: map[string]string{}, - NoFingerprint: map[string]string{}, Running: map[string]catalogue.Manifest{}, + NoFingerprint: map[string]string{}, Running: map[string]catalogue.Manifest{}, Had: map[string]catalogue.Manifest{}, HadFailed: map[string]string{}, Values: map[string]any{}, Gaps: append([]Gap(nil), standingGaps...)} in.SeatListUnread = gate != nil && !gate.MayAct() nodes := map[string]bool{} @@ -515,11 +521,14 @@ func ReadInputs(r Reader, cache *Manifests) Inputs { wants := map[string][]byte{} if in.ByHaves == "" { wants = each(r, ReadWants, &in.Gaps) + // What each machine runs now is its have's build, which the want may already have left (issue 1966): the + // memberships keep what either build reads, calls and consumes. + runningByHaves(r, &in, cache, in.Had) } else { // Placed by the haves, what runs is what each have's fingerprint names, not a want: the assigner's wants are its // view too, and not read in such a pass. in.Gaps = append(in.Gaps, Gap{Input: ReadWants, Why: "not read: the modules are placed by the node-engines' haves this pass, so what runs is composed from each have's fingerprint"}) - fromTrunk = runningByHaves(r, &in, cache) + fromTrunk = runningByHaves(r, &in, cache, in.Running) } for key, raw := range wants { node, module, err := meshwantsclient.ParseWantKey(key) @@ -743,11 +752,16 @@ func placeByHaves(r Reader, in *Inputs, haveKeys []string) { // runningByHaves is, in a pass placed by the haves, the manifest each placed module runs: its have's fingerprint, the // builder's build of it, the manifest it built; the placed modules with none, `.`, to compose from the trunk. -func runningByHaves(r Reader, in *Inputs, cache *Manifests) []string { +func runningByHaves(r Reader, in *Inputs, cache *Manifests, into map[string]catalogue.Manifest) []string { var fromTrunk []string 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}) + in.HadFailed[node+"."+module] = ReadHave + " cannot be read" + + } if err != nil || !found { fromTrunk = append(fromTrunk, node+"."+module) continue @@ -763,6 +777,10 @@ func runningByHaves(r Reader, in *Inputs, cache *Manifests) []string { continue } braw, found, err := r.Get(ReadBuilds, bk) + if err != nil { + in.HadFailed[node+"."+module] = ReadBuilds + " cannot be read" + 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 @@ -773,7 +791,7 @@ func runningByHaves(r Reader, in *Inputs, cache *Manifests) []string { continue } if m, err := cache.get(r, b.Manifest); err == nil { - in.Running[node+"."+module] = m + into[node+"."+module] = m } else { fromTrunk = append(fromTrunk, node+"."+module) } diff --git a/modules/mesh-grants/internal/grants/part.go b/modules/mesh-grants/internal/grants/part.go index 923c30fa..6590b9ec 100644 --- a/modules/mesh-grants/internal/grants/part.go +++ b/modules/mesh-grants/internal/grants/part.go @@ -407,7 +407,7 @@ func (p *Part) Compose() Result { } p.writeSealed(&res, list, n, leftOut, dropped, in, bus, lost, users, loaded) } - ms := Memberships(recs) + ms := Memberships(listRecs) res.Memberships = len(ms) members := make([]membership.Membership, 0, len(ms)) for _, m := range ms { @@ -436,7 +436,11 @@ func (p *Part) Compose() Result { // A pass that composes part of the mesh only grows a published membership: it publishes the union of the one last // published and the one composed (keeploaded.go, issue 1966), so a tool runner or a seat holder goes on serving what it // serves and still receives what the pass that ends the hold needs. - if why := in.keepsLoaded(); found && why != "" { + why := in.keepsLoaded() + if why == "" && in.HadFailed[key] != "" { + why = "the build it runs is unread (" + in.HadFailed[key] + ")" + } + if found && why != "" { if bare, err := membership.BareGrantsMembership(was); err == nil { if body, err := json.Marshal(ms[key].Membership); err == nil { if grown, kept, err := UnionMembership(bare, body); err == nil && len(kept) > 0 { diff --git a/modules/mesh-grants/internal/grants/records.go b/modules/mesh-grants/internal/grants/records.go index 86da3a79..b82f4dd1 100644 --- a/modules/mesh-grants/internal/grants/records.go +++ b/modules/mesh-grants/internal/grants/records.go @@ -21,7 +21,9 @@ func Records(in Inputs) (buscompose.Records, []Gap) { return records(in, false) // ListRecords is Records for the user list and the grants (issue 774): a module whose wanted build's manifest is not the // trunk's is declared with what both grant (unionDeclared), so the list carries the trunk manifest's subjects before the // assigner moves the want to it, and a grant at the trunk's fingerprint waits for nothing the want's move would bring. -// Memberships and consumers stay with Records: they say what runs. Pure. +// Memberships are composed from it too, and from the build each machine runs now (Inputs.Had): while a machine's build +// differs from the want's or the trunk's, neither loses a read, call or consume (ADR 0660 §2 applied to memberships, +// issue 1966). Consumers stay with Records. Pure. func ListRecords(in Inputs) (buscompose.Records, []Gap) { return records(in, true) } func records(in Inputs, union bool) (buscompose.Records, []Gap) { @@ -106,6 +108,9 @@ func records(in Inputs, union bool) (buscompose.Records, []Gap) { if union && onTrunk && !reflect.DeepEqual(m, trunk) { d = unionDeclared(d, declare(trunk)) } + if had, ok := in.Had[node+"."+module]; union && ok && !reflect.DeepEqual(m, had) { + d = unionDeclared(d, declare(had)) + } out.Assigned[node] = append(out.Assigned[node], d) if m.Instances == catalogue.InstancesInterchangeable || union && onTrunk && trunk.Instances == catalogue.InstancesInterchangeable { out.Interchangeable[m.Module] = true diff --git a/modules/mesh-grants/internal/grants/tools.go b/modules/mesh-grants/internal/grants/tools.go index f68c247e..445a9761 100644 --- a/modules/mesh-grants/internal/grants/tools.go +++ b/modules/mesh-grants/internal/grants/tools.go @@ -111,7 +111,7 @@ func (p *Part) MembershipOf(node, module string) (any, error) { return nil, errors.New("membership names a node and a module, or neither for every membership held back") } in := p.inputs() - recs, left := Records(in) + recs, left := ListRecords(in) notFree := WithRootFree(&recs, p.Call) m, ok := Memberships(recs)[node+"."+module] out := map[string]any{"node": node, "module": module, "gaps": MembershipGaps, "root_free": recs.RootFree, "root_not_free": notFree}