diff --git a/cmd/mesh-controller/delivery_order_test.go b/cmd/mesh-controller/delivery_order_test.go index 7c29c00..878a201 100644 --- a/cmd/mesh-controller/delivery_order_test.go +++ b/cmd/mesh-controller/delivery_order_test.go @@ -6,6 +6,8 @@ import ( "reflect" "strings" "testing" + + "github.com/novox/mesh-controller/internal/link" ) // recordingDelivery is a delivery that writes down what was done, in order, and fails where told. @@ -39,10 +41,11 @@ func ready(names ...string) []readyNode { } // novox/hq issue 249: the grants that come with a module's new declarations are issued before any -// machine is sent the code that uses them. +// machine is sent the code that uses them — except the machine holding the bus, whose declaration +// carries the controller's own right to issue them, and goes first. func TestGrantsAreIssuedBeforeTheDeclarations(t *testing.T) { d := &recordingDelivery{} - digests, err := deliver(t.Context(), d, ready("anchor", "laptop")) + digests, err := deliver(t.Context(), d, "", ready("anchor", "laptop")) if err != nil { t.Fatal(err) } @@ -53,27 +56,71 @@ func TestGrantsAreIssuedBeforeTheDeclarations(t *testing.T) { if digests["anchor"] != "digest-anchor" || digests["laptop"] != "digest-laptop" { t.Fatalf("the digests sent were not answered: %v", digests) } + + held := &recordingDelivery{} + if _, err := deliver(t.Context(), held, "broker", ready("broker", "anchor")); err != nil { + t.Fatal(err) + } + want = []string{"declare broker", "grant broker", "grant anchor", "declare anchor"} + if !reflect.DeepEqual(held.did, want) { + t.Fatalf("with the bus's machine in the send: %v, wanted %v", held.did, want) + } } -// A grant that cannot be issued sends nothing and is an error — never "until the next push". -func TestAGrantThatFailsSendsNothingAndSaysSo(t *testing.T) { - d := &recordingDelivery{grantErr: errors.New("the bus refused the membership")} - _, err := deliver(t.Context(), d, ready("anchor", "laptop")) - if err == nil || !strings.Contains(err.Error(), "the bus refused the membership") { +// A grant that cannot be issued holds back the machines it concerns and is an error the caller +// retries on — never "until the next push" — and the bus's own machine is sent regardless, so the +// grant that would let the controller issue memberships is never held behind them. +func TestAGrantThatFailsHoldsBackWhatItConcerns(t *testing.T) { + // A failure naming no machine (the buckets): everything but the bus's machine. + d := &recordingDelivery{grantErr: errors.New("the bus refused the bucket")} + _, err := deliver(t.Context(), d, "broker", ready("broker", "anchor", "laptop")) + if err == nil || !errors.Is(err, errGrants) || !strings.Contains(err.Error(), "the bus refused the bucket") || + !strings.Contains(err.Error(), "anchor, laptop not sent") { t.Fatalf("a failed grant was not said as the send's failure: %v", err) } - for _, did := range d.did { - if strings.HasPrefix(did, "declare") { - t.Fatalf("code was sent after its grant failed: %v", d.did) - } + if want := []string{"declare broker", "grant broker", "grant anchor", "grant laptop"}; !reflect.DeepEqual(d.did, want) { + t.Fatalf("delivered %v, wanted the bus's machine alone", d.did) } + + // A membership that failed for one machine: that machine alone. + one := &recordingDelivery{grantErr: &grantsRefused{nodes: map[string]error{"laptop": errors.New("no")}}} + _, err = deliver(t.Context(), one, "", ready("anchor", "laptop")) + if !errors.Is(err, errGrants) || !strings.Contains(err.Error(), "laptop not sent") { + t.Fatalf("one machine's refused membership was not said: %v", err) + } + if want := []string{"grant anchor", "grant laptop", "declare anchor"}; !reflect.DeepEqual(one.did, want) { + t.Fatalf("delivered %v, wanted anchor sent and laptop held back", one.did) + } + + // The announced upgrade that hit it is asked again. + if !errors.Is(askAgainOnGrants(err), link.ErrTryAgain) { + t.Fatal("an announcement whose send stopped at its grants is not asked again") + } + if other := errors.New("laptop could not be resolved"); errors.Is(askAgainOnGrants(other), link.ErrTryAgain) { + t.Fatal("any failure is asked again, not only a grant's") + } + // Nothing to send is nothing granted either. none := &recordingDelivery{grantErr: errors.New("never asked")} - if _, err := deliver(t.Context(), none, nil); err != nil || len(none.did) != 0 { + if _, err := deliver(t.Context(), none, "", nil); err != nil || len(none.did) != 0 { t.Fatalf("an empty send granted or failed: %v %v", none.did, err) } } +// Whether the bus's machine goes first is read from the user list alone, by its digest. +func TestTheBusMachineIsBehindByItsUserListAlone(t *testing.T) { + list := "users: [a, b]" + if userListBehind(list, digestOf([]byte(list))) { + t.Fatal("the list it was sent reads as behind") + } + if !userListBehind(list, digestOf([]byte("users: [a]"))) || !userListBehind(list, "") { + t.Fatal("a changed or never-sent list reads as current") + } + if userListBehind("", "") { + t.Fatal("a machine sent no list reads as behind") + } +} + // The machine holding the bus goes first: its declaration carries the user list the new grants are // checked against. Among the machines it is moved to the front; not among them it is added only // when it is behind. diff --git a/cmd/mesh-controller/hold_test.go b/cmd/mesh-controller/hold_test.go index 62660c7..c66d769 100644 --- a/cmd/mesh-controller/hold_test.go +++ b/cmd/mesh-controller/hold_test.go @@ -23,11 +23,11 @@ func TestASendRoundGivesItsHoldBackOnEveryWayOut(t *testing.T) { for name, round := range map[string]func() error{ "a body that cannot be marshalled": func() error { - _, err := sendRound(ctx, open, []string{"anchor"}, unmarshallable, fine) + _, err := sendRound(ctx, open, []string{"anchor"}, unmarshallable, fine, "") return err }, "a send that fails": func() error { - _, err := sendRound(ctx, open, []string{"anchor"}, plain, failing) + _, err := sendRound(ctx, open, []string{"anchor"}, plain, failing, "") return err }, } { diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index a5b773d..2c66137 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -398,7 +398,7 @@ func declarationWith(ctx context.Context, open *stores, node string, return sendable{}, err } return sendable{Resources: composed.Resources, Adoption: adoption, - Received: composed.Received, Mesh: with.Mesh, + Received: composed.Received, Mesh: with.Mesh, BusUsers: with.BusUsers, LeftOut: sortedKeysOf(composed.LeftOut), leftOutWhy: composed.LeftOut}, nil } @@ -1341,6 +1341,20 @@ func composeBusUsers(ctx context.Context, inv *inventory.Inventory, // // Asked of what this push resolves to rather than of the seat's holder mesh-wide: the file is a // resource of that module, so the question is whether it is here. + list, missing, err := busUserList(ctx, inv, onThisNode) + if len(missing) > 0 { + fmt.Printf("the bus's user list leaves out %d user(s) the mesh has minted no credential "+ + "for: %s. Each is a user that cannot connect until one is issued\n", + len(missing), strings.Join(missing, ", ")) + } + return list, err +} + +// busUserList is composeBusUsers without saying anything: the list, and the users left out of it +// for want of a credential. Asked on every send to decide whether the machine holding the bus must +// go first (novox/hq issue 249), where saying the same missing users each time would bury them. +func busUserList(ctx context.Context, inv *inventory.Inventory, + onThisNode []catalogue.Manifest) (string, []string, error) { holdsTheBus := false for _, m := range onThisNode { if m.BusUsers != "" && m.ClaimsSeat("mesh-broker") { @@ -1348,37 +1362,33 @@ func composeBusUsers(ctx context.Context, inv *inventory.Inventory, } } if !holdsTheBus { - return "", nil + return "", nil, nil } records, err := inv.BusRecords(ctx) if err != nil { - return "", err + return "", nil, err } users, err := broker.Users(records) if err != nil { - return "", err + return "", nil, err } kept, err := inv.BusUsers(ctx) if err != nil { - return "", err + return "", nil, err } hashes := make(map[string]string, len(kept)) for name, u := range kept { hashes[name] = u.PasswordHash } filled, missing := broker.WithPasswords(users, hashes) - if len(missing) > 0 { - fmt.Printf("the bus's user list leaves out %d user(s) the mesh has minted no credential "+ - "for: %s. Each is a user that cannot connect until one is issued\n", - len(missing), strings.Join(missing, ", ")) - } if len(filled) == 0 { - return "", fmt.Errorf( + return "", missing, fmt.Errorf( "this machine runs the bus and not one user has a credential, so the composed list " + "would refuse every connection in the mesh") } - return broker.ComposeAccounts(filled) + list, err := broker.ComposeAccounts(filled) + return list, missing, err } // providerModuleOf is which module answers a need on the providing node: the one in this node's diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 65ec086..9a2d3c5 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -402,7 +402,7 @@ func pushCommand(ctx context.Context, args []string) error { // Each machine's memberships first, then the declarations (novox/hq issue 249, ADR 0160): a push // is the one most operators run, and on 2026-10-01 it was the one path that issued none. bus := overTheBus{open: open, server: server, signer: ident} - sentDigest, err := deliver(ctx, bus, sending) + sentDigest, err := deliver(ctx, bus, holder, sending) if err != nil { return err } @@ -477,7 +477,7 @@ func pushCommand(ctx context.Context, args []string) error { } return declared, err }, - bus) + bus, holder) refusals = append(refusals, refused...) if err != nil { return err @@ -607,7 +607,7 @@ func composeEach(names []string, allot func(node string) (int64, error), // still sent. Their memberships go before their declarations, as every send's do (issue 249). func sendRound(ctx context.Context, open *stores, names []string, compose func(held context.Context, node string) (sendable, error), - d delivery) ([]string, error) { + d delivery, holder string) ([]string, error) { held, release, err := holdNodes(ctx, open, names) if err != nil { return nil, err @@ -616,7 +616,7 @@ func sendRound(ctx context.Context, open *stores, names []string, sending, refused := composeEach(names, allotting(held, open.inventory), func(node string) (sendable, error) { return compose(held, node) }) - if _, err := deliver(held, d, sending); err != nil { + if _, err := deliver(held, d, holder, sending); err != nil { return refused, err } return refused, nil @@ -632,7 +632,28 @@ type delivery interface { declare(ctx context.Context, s readyNode, body []byte) (string, error) } -// deliver sends the machines their declarations, **the grants first** (novox/hq issue 249). +// errGrants marks a send that stopped because what the machines' modules may do could not be issued +// (novox/hq issue 249). Nothing about the machines is wrong; asked again, it is likely to work, so an +// announcement that hits it is held and asked again. +var errGrants = errors.New("what the machines' modules may do on the bus could not be issued, and code " + + "sent before its grants is refused there") + +// grantsRefused is a grant that failed for some machines and not others: their memberships could not +// be issued, by machine, and only those machines are held back. +type grantsRefused struct{ nodes map[string]error } + +func (g *grantsRefused) Error() string { + names := make([]string, 0, len(g.nodes)) + for n := range g.nodes { + names = append(names, n) + } + sort.Strings(names) + return fmt.Sprintf("the memberships of %s could not be issued; the first: %v", + strings.Join(names, ", "), g.nodes[names[0]]) +} + +// deliver sends the machines their declarations: **the machine holding the bus, then the grants, +// then the rest** (novox/hq issue 249). // // A merge gave a module a new state; its bundle reached every machine within a minute, and the // machines' permissions on the bus did not include the state until somebody pushed by hand: the code @@ -642,31 +663,82 @@ type delivery interface { // the bus (internal/link/bus.go), so issued first it waits for the runtime that arrives after it. A // runtime still on the old code merely holds a grant it does not use yet. // -// **A grant that cannot be issued sends nothing**, and says so as an error. It used to be said and -// passed over — "the machines keep what they derive until the next push" — which reported a rollout -// as done that had delivered code its machines could not run, and left the remedy to a push nobody -// knew to make. Returned, the caller's rollout is not marked sent and is tried again. -func deliver(ctx context.Context, d delivery, sending []readyNode) (map[string]string, error) { +// **The holder's declaration before the grants, though.** The bus's user list travels in it, and the +// controller's own right to publish memberships and raise buckets is in that list (the precedent of +// issue 183): grants first, and a grant the controller is not yet allowed to make would hold the very +// declaration that allows it — a lock only a hand on the broker could open. A runtime already running +// on that machine follows a membership issued after its declaration, as it always has. +// +// **A grant that cannot be issued holds back what it concerns, and says so as an error.** It used to +// be said and passed over — "the machines keep what they derive until the next push" — which reported +// a rollout done that had delivered code its machines could not run. A membership that failed holds +// back its own machine; a failure that names no machine (the buckets) holds back every machine but the +// holder, already sent. The error carries errGrants, so the caller's rollout is not marked sent and is +// tried again. +// +// The grants are issued, not waited on: a membership is a retained message the runtime reads when it +// comes, and the bus answers its publication; nothing here waits for a runtime to have read one. +func deliver(ctx context.Context, d delivery, holder string, sending []readyNode) (map[string]string, error) { digests := map[string]string{} if len(sending) == 0 { return digests, nil } - if err := d.grant(ctx, sending); err != nil { - return digests, fmt.Errorf("nothing was sent: what the machines' modules may do on the bus "+ - "could not be issued, and code sent before its grants is refused there (novox/hq issue 249): %w", err) - } - for _, s := range sending { + send := func(s readyNode) error { // The number is inside the signed bytes, so a replayed older declaration cannot borrow a // newer one's (novox/hq 04-ISSUES/107); it was taken when the composition began (issue 204). body, err := s.declared.Body() if err != nil { - return digests, err + return err } digest, err := d.declare(ctx, s, body) if err != nil { - return digests, err + return err } digests[s.node] = digest + return nil + } + var rest []readyNode + for _, s := range sending { + if holder != "" && s.node == holder { + if err := send(s); err != nil { + return digests, err + } + continue + } + rest = append(rest, s) + } + held := map[string]error{} + if err := d.grant(ctx, sending); err != nil { + var some *grantsRefused + if !errors.As(err, &some) { + var names []string + for _, s := range rest { + names = append(names, s.node) + } + if len(names) == 0 { + return digests, fmt.Errorf("%w: %w", errGrants, err) + } + return digests, fmt.Errorf("%w; %s not sent: %w", errGrants, strings.Join(names, ", "), err) + } + held = some.nodes + } + var notSent []string + for _, s := range rest { + if _, refused := held[s.node]; refused { + notSent = append(notSent, s.node) + continue + } + if err := send(s); err != nil { + return digests, err + } + } + if len(notSent) > 0 { + return digests, fmt.Errorf("%w; %s not sent: %w", errGrants, strings.Join(notSent, ", "), + &grantsRefused{nodes: held}) + } + if len(held) > 0 { + // Only the holder's own memberships failed, and it was sent before them. + return digests, fmt.Errorf("%w: %w", errGrants, &grantsRefused{nodes: held}) } return digests, nil } @@ -694,6 +766,16 @@ func (b overTheBus) declare(ctx context.Context, s readyNode, body []byte) (stri if err != nil { return "", err } + if s.declared.BusUsers != "" { + // And the user list it carried, so the next send reads whether it must go first from the + // list alone (novox/hq issue 249). On the same outliving context as the send's record. + kept, cancel := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Second) + err := b.open.inventory.RecordSentBusUsers(kept, s.node, digestOf([]byte(s.declared.BusUsers))) + cancel() + if err != nil { + return "", err + } + } fmt.Printf("%ssent %s %d resource(s)\n", b.indent, s.node, len(s.declared.Resources)) return digest, nil } @@ -722,14 +804,13 @@ func brokerFirst(names []string, holder string, behind bool) []string { } // brokerBehind is the machine holding the bus — the one whose declaration carries the user list — -// and, when it is not among the machines named, whether what it should be differs from what it was -// last sent. +// and, when it is not among the machines named, whether the user list it would be sent now differs +// from the one it was last sent (novox/hq issue 249). // -// **Its whole declaration, because that is what is recorded** (novox/hq issue 249). The mesh keeps -// a digest of what each machine was last sent and not of the user list inside it (ADR 0043: the -// list is composed on each push, never kept), so "the user list changed" is read as "the bus's -// machine is behind". That is the safe direction: a machine sent what it should be is never wrong, -// and whatever else it was behind on is what any push would have sent it. +// **The user list alone, not the whole declaration.** Read from the whole declaration, any change +// pending on that machine — an upgrade its policy records rather than rolls out — went with every +// send anywhere, and a module running there always put it in its first wave. A digest of the list +// last sent is kept for this (ADR 0043: the list is composed on each push, never kept itself). func brokerBehind(ctx context.Context, open *stores, names []string) (string, bool, error) { inv := open.inventory holders, err := seatHolders(ctx, inv) @@ -753,23 +834,26 @@ func brokerBehind(ctx context.Context, open *stores, names []string) (string, bo return h.Node, false, nil } } - node, err := inv.NodeByName(ctx, h.Node) + plan, _, err := planFor(ctx, open, h.Node) if err != nil { - return "", false, err - } - would, err := wouldSend(ctx, open, []inventory.Node{node}) - if err != nil { - return "", false, err - } - if would[h.Node] == "" { // It cannot be worked out: sending it would refuse the whole send, and `plan` says why. return h.Node, false, nil } - sent, err := inv.Outstanding(ctx, h.Node) + list, _, err := busUserList(ctx, inv, plan.Modules) + if err != nil { + return h.Node, false, nil + } + sent, err := inv.SentBusUsers(ctx, h.Node) if err != nil { return "", false, err } - return h.Node, would[h.Node] != sent, nil + return h.Node, userListBehind(list, sent), nil +} + +// userListBehind is whether the user list composed now is not the one last sent, by its digest. An +// empty list composed is never behind: there is nothing for it to carry. +func userListBehind(now, sentDigest string) bool { + return now != "" && digestOf([]byte(now)) != sentDigest } // couldNotBeResolved is what a push ends with when some machines could not be worked out. @@ -867,7 +951,7 @@ func sendToEach(ctx context.Context, open *stores, names []string) ([]string, er // composition. **Issued before the declarations** (novox/hq issue 249): the runtime the // membership is for arrives with the declaration, and a membership waits for it on the bus; the // code arriving first was refused its own state until somebody pushed. - if _, err := deliver(ctx, overTheBus{open: open, server: server, signer: ident, indent: " "}, sending); err != nil { + if _, err := deliver(ctx, overTheBus{open: open, server: server, signer: ident, indent: " "}, holder, sending); err != nil { return nil, err } sent := make([]string, 0, len(sending)) @@ -898,9 +982,10 @@ func issueMemberships(ctx context.Context, open *stores, server *link.Server, se // state: its bundle asked for a bucket that did not exist. Idempotent and cheap. // // **A failure here is the send's failure** (novox/hq issue 249). It was said and the push stood, - // because the declarations were already away; they are sent after this now, and a module whose - // state does not exist is a module that fails its start, so nothing is sent and the caller tries - // again rather than reporting the rollout done. + // because the declarations were already away; they are sent after this now — all but the bus's + // own machine, sent before it (deliver) — and a module whose state does not exist is a module + // that fails its start, so they are not sent and the caller tries again rather than reporting the + // rollout done. buckets, err := open.inventory.DeclaredBuckets(ctx) if err != nil { return fmt.Errorf("the modules' state could not be read, so no bucket was asserted: %w", err) @@ -909,8 +994,8 @@ func issueMemberships(ctx context.Context, open *stores, server *link.Server, se return fmt.Errorf("the modules' state could not be asserted on the bus: %w", err) } // Every membership is tried, and the first failure named once. - issued, failed := 0, 0 - var first error + issued := 0 + refused := map[string]error{} for _, s := range sent { node := s.node for _, d := range records.Assigned[node] { @@ -931,10 +1016,9 @@ func issueMemberships(ctx context.Context, open *stores, server *link.Server, se return err } if err := bus.PublishMembership(ctx, node, d.Module, body); err != nil { - if first == nil { - first = err + if refused[node] == nil { + refused[node] = fmt.Errorf("%s: %w", d.Module, err) } - failed++ continue } issued++ @@ -943,10 +1027,10 @@ func issueMemberships(ctx context.Context, open *stores, server *link.Server, se if issued > 0 { fmt.Printf(" issued %d membership(s)\n", issued) } - if failed > 0 { - // Returned, never passed over (novox/hq issue 249): the declarations that need these are not - // sent, and the rollout that asked is tried again rather than waiting for a push. - return fmt.Errorf("%d membership(s) could not be issued; the first: %w", failed, first) + if len(refused) > 0 { + // Returned, never passed over (novox/hq issue 249): the declarations of the machines they are + // for are not sent, and the rollout that asked is tried again rather than waiting for a push. + return &grantsRefused{nodes: refused} } return nil } diff --git a/cmd/mesh-controller/sendable.go b/cmd/mesh-controller/sendable.go index 6722732..4219e97 100644 --- a/cmd/mesh-controller/sendable.go +++ b/cmd/mesh-controller/sendable.go @@ -31,6 +31,10 @@ type sendable struct { // the same composition as its received files, and every machine's private-network address. Received map[string]map[string][]catalogue.Contribution Mesh []string + // BusUsers is the bus's user list this declaration carries, empty for every machine but the one + // holding the bus; not sent apart from the file it is in. Its digest is recorded once sent, so + // whether that machine must go first is read from the list alone (novox/hq issue 249). + BusUsers string // LeftOut is every module of the machine's set left out of this declaration because a stored // setting cannot compose with its definition (novox/hq ADR 0163, rule 6), sorted. The host // keeps that module's held things and touches none of its containers; a machine is told diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index 3c71189..3fa7aab 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -65,13 +65,17 @@ func (f following) Upgraded(ctx context.Context, u link.Upgraded) error { if plans, err := inv.OpenPlans(ctx); err != nil { return notNow(err) } else if id := rolledOutByAPlan(plans, u.Module); id != "" { - fmt.Printf("%s moved to %s; %s rolls it out to %s\n", u.Module, shortCommit(u.Commit), id, readableList(on)) + // Said with its remedy: a plan that ends without sending it — failed, or closed by hand — leaves + // these machines behind, which `status` lists and `push --behind` sends (novox/hq issue 249). + fmt.Printf("%s moved to %s; %s rolls it out to %s — if that plan ends without sending it, "+ + "`status` lists them as behind and `push --behind` sends it\n", + u.Module, shortCommit(u.Commit), id, readableList(on)) return nil } if decision.Together { fmt.Printf("%s moved to %s; sending %s together\n", u.Module, shortCommit(u.Commit), readableList(on)) - return sendTo(ctx, f.open, on) + return askAgainOnGrants(sendTo(ctx, f.open, on)) } // One at a time, and stopping at the first that fails. // @@ -82,18 +86,28 @@ func (f following) Upgraded(ctx context.Context, u link.Upgraded) error { u.Module, shortCommit(u.Commit), readableList(on)) for _, node := range on { if err := sendTo(ctx, f.open, []string{node}); err != nil { - return fmt.Errorf("%s did not take %s, so the machines after it were left alone: %w", - node, u.Module, err) + return askAgainOnGrants(fmt.Errorf("%s did not take %s, so the machines after it were left alone: %w", + node, u.Module, err)) } } return nil } +// askAgainOnGrants marks a send that stopped at its grants as one to ask again (novox/hq issue 249): +// an announcement handled by a send whose memberships could not be issued is held and redelivered, +// rather than taken as handled with the machines left on the old version. +func askAgainOnGrants(err error) error { + if err != nil && errors.Is(err, errGrants) && !errors.Is(err, link.ErrTryAgain) { + return fmt.Errorf("%w: %w", link.ErrTryAgain, err) + } + return err +} + // rolledOutByAPlan is the open plan that will send a module's machines its new build — one holding // the module that has not finished sending it — or empty when none will (novox/hq issue 249). func rolledOutByAPlan(plans []inventory.Plan, module string) string { for _, p := range plans { - if s, holds := p.Modules[module]; p.Open() && holds && (s == nil || s.SentAt == nil) { + if s, holds := p.Modules[module]; p.Open() && holds && (s == nil || (s.SentAt == nil && s.State != "failed")) { return p.ID } } diff --git a/internal/inventory/migrations/0057-a-newer-plan-supersedes-an-older.sql b/internal/inventory/migrations/0058-a-newer-plan-supersedes-an-older.sql similarity index 60% rename from internal/inventory/migrations/0057-a-newer-plan-supersedes-an-older.sql rename to internal/inventory/migrations/0058-a-newer-plan-supersedes-an-older.sql index 8a83eb6..bf3768f 100644 --- a/internal/inventory/migrations/0057-a-newer-plan-supersedes-an-older.sql +++ b/internal/inventory/migrations/0058-a-newer-plan-supersedes-an-older.sql @@ -12,3 +12,11 @@ -- is superseded by the next plan of its repository, whichever branch — nothing is lost by it, since -- what it had not built is folded into the plan that supersedes it. alter table release_plan add column branch text not null default ''; + +-- And the bus's user list the machine holding the bus was last sent, as a digest (novox/hq issue +-- 249). A module's new grants are refused by the bus until its user list says them, so that machine +-- is sent first whenever the list it would be sent differs from the one it was. Read from its whole +-- declaration, every pending change on it — a recorded upgrade the operator chose not to roll out — +-- went with every send anywhere. A digest and never the list (ADR 0043: the list is composed on each +-- push, never kept). Empty for a machine never sent one, which reads as behind once. +alter table node add column sent_bus_users text not null default ''; diff --git a/internal/inventory/nodes.go b/internal/inventory/nodes.go index fef70aa..e16e323 100644 --- a/internal/inventory/nodes.go +++ b/internal/inventory/nodes.go @@ -921,6 +921,26 @@ func (i *Inventory) RecordSent(ctx context.Context, node, digest string) error { return err } +// RecordSentBusUsers keeps a digest of the bus's user list a machine was just sent, by its name +// (novox/hq issue 249): whether the machine holding the bus must go first is whether this differs +// from the list composed now. +func (i *Inventory) RecordSentBusUsers(ctx context.Context, name, digest string) error { + _, err := i.store.Pool().Exec(ctx, + `update node set sent_bus_users = $2 where name = $1`, name, digest) + return err +} + +// SentBusUsers is the digest of the bus's user list a machine was last sent, empty for none. +func (i *Inventory) SentBusUsers(ctx context.Context, name string) (string, error) { + var sent string + err := i.store.Pool().QueryRow(ctx, + `select sent_bus_users from node where name = $1`, name).Scan(&sent) + if errors.Is(err, pgx.ErrNoRows) { + return "", nil + } + return sent, err +} + // Outstanding is the digest of the declaration a machine was last sent, by its name, and empty // for one that has never been sent anything. // diff --git a/internal/inventory/sent_bus_users_test.go b/internal/inventory/sent_bus_users_test.go new file mode 100644 index 0000000..5717571 --- /dev/null +++ b/internal/inventory/sent_bus_users_test.go @@ -0,0 +1,25 @@ +package inventory + +import "testing" + +// novox/hq issue 249: the digest of the user list a machine was last sent is kept, by its name, and +// is empty for a machine never sent one. +func TestTheUserListAMachineWasSentIsKept(t *testing.T) { + inv := ForTest(t) + ctx := t.Context() + if _, err := inv.AddNode(ctx, "anchor"); err != nil { + t.Fatal(err) + } + if sent, err := inv.SentBusUsers(ctx, "anchor"); err != nil || sent != "" { + t.Fatalf("a machine never sent a list has %q: %v", sent, err) + } + if err := inv.RecordSentBusUsers(ctx, "anchor", "abc"); err != nil { + t.Fatal(err) + } + if sent, err := inv.SentBusUsers(ctx, "anchor"); err != nil || sent != "abc" { + t.Fatalf("the list sent was not kept: %q %v", sent, err) + } + if sent, err := inv.SentBusUsers(ctx, "nobody"); err != nil || sent != "" { + t.Fatalf("a machine the mesh does not know: %q %v", sent, err) + } +}