diff --git a/cmd/mesh-controller/asker.go b/cmd/mesh-controller/asker.go index 8a7233dd..4bf3b117 100644 --- a/cmd/mesh-controller/asker.go +++ b/cmd/mesh-controller/asker.go @@ -213,6 +213,8 @@ type asker struct { saidNoRouter bool saidNoGrant bool + // saidGrantCaveat is whether a grant held with a caveat was said, so it is said once. + saidGrantCaveat bool mu sync.Mutex nudged chan struct{} @@ -317,6 +319,13 @@ func (a *asker) reconcile(ctx context.Context) error { } return a.sayUnasked(ctx, unasked, "the bus does not hold the controller's grant to ask yet: "+why) } + if why != "" && !a.saidGrantCaveat { + // Held, with a caveat worth one line (an older bus module cannot confirm the reload). + a.logf("the controller asks: %s", why) + a.saidGrantCaveat = true + } else if why == "" { + a.saidGrantCaveat = false + } a.saidNoGrant = false } channels := "" diff --git a/cmd/mesh-controller/asker_test.go b/cmd/mesh-controller/asker_test.go index 96654618..554214d9 100644 --- a/cmd/mesh-controller/asker_test.go +++ b/cmd/mesh-controller/asker_test.go @@ -12,6 +12,7 @@ import ( "git.novox.be/novox/mesh-sdk/go/asks" + "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/conditions" "github.com/novox/mesh-controller/internal/link" ) @@ -808,3 +809,207 @@ func TestARefusalIsAskedAgainAndTheUndeliveredConditionClearsOnceTaken(t *testin t.Errorf("the explanation says its verdict twice: %q", e) } } + +// hq issue 353, the review: the bus holds the controller's grant to ask only when both facts hold — the mesh +// composes the controller's own grant, and the bus's module says it is in force on the running server — and +// each fact not known is not held, said with why. +func TestTheGrantIsHeldOnlyWhenComposedAndInForceOnTheBus(t *testing.T) { + yes := func(context.Context) (bool, string, error) { return true, "", nil } + no := func(why string) grantJudge { + return func(context.Context) (bool, string, error) { return false, why, nil } + } + broken := func(context.Context) (bool, string, error) { return false, "", errors.New("the store is away") } + for _, c := range []struct { + name string + composed, inForce grantJudge + held bool + says string + }{ + {"composed and in force", yes, yes, true, ""}, + {"not composed", no("no router is assigned"), yes, false, "no router is assigned"}, + {"composed, written, not reloaded", yes, no("the server has not reloaded"), false, "has not reloaded"}, + {"the composition unknown", broken, yes, false, "could not be worked out: the store is away"}, + {"the bus unknown", yes, broken, false, "could not be worked out: the store is away"}, + {"nothing judges the bus", yes, nil, false, "not judged"}, + } { + held, why, err := grantHeldBy(c.composed, c.inForce, time.Now)(context.Background()) + if err != nil || held != c.held || !strings.Contains(why, c.says) { + t.Errorf("%s: held %v (%q, %v)", c.name, held, why, err) + } + } + // Remembered for grantLookEvery, then judged again. + now := time.Date(2026, 10, 9, 18, 0, 0, 0, time.UTC) + inForce := false + judge := grantHeldBy(yes, func(context.Context) (bool, string, error) { return inForce, "not yet", nil }, + func() time.Time { return now }) + if held, _, _ := judge(context.Background()); held { + t.Fatal("held before the bus enforces it") + } + inForce = true + if held, _, _ := judge(context.Background()); held { + t.Error("judged again inside grantLookEvery") + } + now = now.Add(grantLookEvery) + if held, _, _ := judge(context.Background()); !held { + t.Error("not judged again after grantLookEvery") + } +} + +// The controller's own grant, as composed: present while a router that takes asks is assigned, absent +// otherwise — whatever else in the user list changes. +func TestTheControllersOwnGrantToAskIsReadFromTheComposition(t *testing.T) { + router := broker.Declared{Module: "messenger", Holds: []broker.Seat{{Name: broker.AsksSeat, Scope: "mesh", + Accepts: []string{"ask", "cancel"}, Emits: []string{"decided"}, ByCaller: []string{"ask", "cancel", "decided"}}}} + other := broker.Declared{Module: "dunst"} + with := broker.Records{Nodes: []string{"anchor"}, Assigned: map[string][]broker.Declared{"anchor": {router, other}}} + without := broker.Records{Nodes: []string{"anchor"}, Assigned: map[string][]broker.Declared{"anchor": {other}}} + if may, err := controllerMayAsk(with); err != nil || !may { + t.Errorf("with a router assigned: %v %v", may, err) + } + if may, err := controllerMayAsk(without); err != nil || may { + t.Errorf("with no router assigned: %v %v", may, err) + } +} + +// The bus module's answer, with the record's word on the list sent as the check only where the answer does +// not settle it (reviews 22 and 31 of PR 188); each way it is not held names its own remedy. +func TestTheBusModulesAnswerNamesTheRemedyForEachCase(t *testing.T) { + const ( + reloaded = `{"allowed":true,"in_force":true,"file_written":"2026-10-09T15:55:56Z","server_loaded":"2026-10-09T15:56:05Z"}` + notLoaded = `{"allowed":true,"in_force":false,"why":"the user list on the machine allows it, and the server has not reloaded since it was written","file_written":"2026-10-09T15:55:56Z","server_loaded":"2026-10-09T15:50:00Z"}` + noVarz = `{"allowed":true,"in_force":false,"why":"the server's last configuration load could not be read: refused"}` + noStat = `{"allowed":true,"in_force":false,"why":"when the bus's user list was written could not be read inside mesh-broker-nats: exit 1"}` + notAllowed = `{"allowed":false,"in_force":false,"why":"the user list on the machine does not allow it"}` + ) + current := busListState{Machine: "anchor"} + behind := busListState{Machine: "anchor", Behind: true} + stepped := busListState{Machine: "anchor", Behind: true, BusStep: "sending anchor would replace the bus; `bus upgrade --why …`"} + uncomposed := busListState{Machine: "anchor", Behind: true, Uncomposed: errors.New("module foo requires a provision nobody provides")} + for _, c := range []struct { + name, answer string + askErr error + list busListState + listErr error + held bool + says, not string + }{ + {"in force", reloaded, nil, behind, nil, true, "", "push"}, + {"written, not reloaded", notLoaded, nil, current, nil, false, "within about 5 s", "push"}, + {"in force not known: varz", noVarz, nil, current, nil, false, "is not known: the server's last configuration load", "wait"}, + {"in force not known: stat", noStat, nil, current, nil, false, "is not known: when the bus's user list was written", "wait"}, + {"not allowed, list behind", notAllowed, nil, behind, nil, false, "`push anchor` carries it", "5 s"}, + {"not allowed, bus step pending", notAllowed, nil, stepped, nil, false, "`bus upgrade", "`push anchor`"}, + {"not allowed, list uncomposed", notAllowed, nil, uncomposed, nil, false, "`plan anchor`", "`push anchor`"}, + {"older module, allowed, list current", `{"allowed":true}`, nil, current, nil, true, "not confirmed", "push"}, + {"older module, allowed, list behind", `{"allowed":true}`, nil, behind, nil, true, "not confirmed", "push"}, + {"older module, allowed, list unknown", `{"allowed":true}`, nil, current, errors.New("the store is away"), true, "`bus upgrade`", "push"}, + {"older module, not allowed, list behind", `{"allowed":false}`, nil, behind, nil, false, "`push anchor` carries it", "5 s"}, + {"module cannot answer, list current", "", errors.New("no responders"), current, nil, true, "could not answer", "push"}, + {"module cannot answer, list behind", "", errors.New("no responders"), behind, nil, false, "`push anchor` carries it", ""}, + {"module cannot answer, list unknown", "", errors.New("no responders"), current, errors.New("the store is away"), false, "could not be worked out", ""}, + {"answer unreadable, list behind", `not json`, nil, behind, nil, false, "could not be read", ""}, + } { + judge := busGrantBy( + func(context.Context) (string, json.RawMessage, error) { + return "anchor", json.RawMessage(c.answer), c.askErr + }, + func(context.Context) (busListState, error) { return c.list, c.listErr }) + held, why, _ := judgeGrantOnce(judge) + if held != c.held || !strings.Contains(why, c.says) || (c.not != "" && strings.Contains(why, c.not)) { + t.Errorf("%s: held %v (%q)", c.name, held, why) + } + } + none := busGrantBy(func(context.Context) (string, json.RawMessage, error) { return "", nil, nil }, nil) + if held, why, _ := judgeGrantOnce(none); held || !strings.Contains(why, "no module holds the bus") { + t.Errorf("no bus: held %v (%q)", held, why) + } +} + +// judgeGrantOnce is the whole judgement with the composed grant held, as the asker makes it. +func judgeGrantOnce(bus grantJudge) (bool, string, error) { + yes := func(context.Context) (bool, string, error) { return true, "", nil } + return grantHeldBy(yes, bus, time.Now)(context.Background()) +} + +// The orders reviews 22 and 31 of PR 188 found: this controller live before the bus module that answers +// in_force, with the bus's machine behind on its list — and, after mesh-catalog #166 merged, unpushable until a +// person's bus upgrade. An older module that allows the ask asks (once, with the caveat said); a new module that +// has not reloaded names the wait, not a push; one that cannot say whether it reloaded names what is not known. +func TestAnOlderBusModuleNeitherSilencesTheAsksNorNamesTheWrongRemedy(t *testing.T) { + for _, c := range []struct { + name, answer string + list busListState + asks bool + says, not string + }{ + {"older module, list current", `{"allowed":true}`, busListState{Machine: "anchor"}, true, "not confirmed", ""}, + {"older module, list behind, bus step pending", `{"allowed":true}`, + busListState{Machine: "anchor", Behind: true, BusStep: "a planned step"}, true, "not confirmed", ""}, + {"new module, not reloaded", + `{"allowed":true,"in_force":false,"why":"not reloaded","file_written":"2026-10-09T15:55:56Z","server_loaded":"2026-10-09T15:50:00Z"}`, + busListState{Machine: "anchor"}, false, "within about 5 s", "push"}, + {"new module, reload not known", `{"allowed":true,"in_force":false,"why":"the server's last configuration load could not be read: refused"}`, + busListState{Machine: "anchor"}, false, "is not known", "wait"}, + } { + t.Run(c.name, func(t *testing.T) { + r := newAskerRig(t) + var said []string + r.a.logf = func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) } + var raised [][]conditions.Observation + r.a.raise = func(_ context.Context, obs []conditions.Observation) error { + raised = append(raised, obs) + return nil + } + yes := func(context.Context) (bool, string, error) { return true, "", nil } + r.a.grantHeld = grantHeldBy(yes, busGrantBy( + func(context.Context) (string, json.RawMessage, error) { + return "anchor", json.RawMessage(c.answer), nil + }, + func(context.Context) (busListState, error) { return c.list, nil }), + func() time.Time { return r.now }) + r.open = []conditions.Condition{heldCondition()} + _ = r.a.reconcile(context.Background()) + _ = r.a.reconcile(context.Background()) + sent := r.asksSent(t) + if c.asks { + if len(sent) != 1 || sent[0].About != heldCondition().Key { + t.Fatalf("not asked: %+v", sent) + } + n := 0 + for _, s := range said { + if strings.Contains(s, c.says) { + n++ + } + } + if n != 1 { + t.Errorf("the caveat said %d times: %q", n, said) + } + return + } + if len(sent) != 0 { + t.Fatalf("asked while the bus does not hold the grant: %+v", sent) + } + if len(raised) == 0 || len(raised[len(raised)-1]) != 1 || + raised[len(raised)-1][0].Kind != "asks-undelivered" { + t.Fatalf("not said as undelivered: %+v", raised) + } + summary := raised[len(raised)-1][0].Summary + if !strings.Contains(summary, c.says) || (c.not != "" && strings.Contains(summary, c.not)) { + t.Errorf("the condition names the wrong remedy: %q", summary) + } + }) + } +} + +// The serving asker is wired to the real judgement: with records and a bus it cannot read, the grant is not +// held, and so nothing is asked (a stub saying yes, or no judgement at all, would ask). +func TestTheServingAskerJudgesTheGrantForReal(t *testing.T) { + a := servingAsker(&stores{}, nil, nil, nil) + if a.grantHeld == nil { + t.Fatal("the serving asker does not judge the bus's grant") + } + held, why, err := a.grantHeld(context.Background()) + if err != nil || held || !strings.Contains(why, "could not be worked out") { + t.Errorf("held %v (%q, %v) with nothing to read", held, why, err) + } +} diff --git a/cmd/mesh-controller/asker_wire.go b/cmd/mesh-controller/asker_wire.go index 345cf5e5..aadff952 100644 --- a/cmd/mesh-controller/asker_wire.go +++ b/cmd/mesh-controller/asker_wire.go @@ -235,12 +235,27 @@ func routerHereIn(inv *inventory.Inventory) func(ctx context.Context) (bool, err } } -// grantHeldIn says whether the bus holds the controller's grant to ask (novox/hq issue 353): the user list the -// machine holding the bus was last sent is the one the mesh composes now (brokerBehind, the same judgement a -// push makes to send that machine first). While it is behind, the controller's ask is refused by the bus, -// whatever the record says of the router, so nothing is asked and the operator is told to push that machine. -// Judged at most every grantLookEvery: composing the list resolves the bus's machine whole. -func grantHeldIn(open *stores) func(ctx context.Context) (bool, string, error) { +// grantHeldIn says whether the bus holds the controller's grant to ask (novox/hq issue 353), judged on the two +// facts that make it so, and failing closed on each: +// +// 1. **composed**: the user list the mesh composes now grants the controller itself to publish its ask +// (controllerMayAsk) — the controller's own grant, not whether any user of the list changed; +// 2. **in force**: the bus's own module, on the machine holding the bus, says the user list there allows it +// and the server reloaded after that list was written (the nats module's `nats_user_can`, `in_force`) — a +// list sent is not a list the bus enforces until it reloads. +// +// Anything that cannot be worked out is not held, with why, so the conditions that need the operator are said +// as undelivered rather than asked into a refusal. +func grantHeldIn(open *stores, conn *nats.Conn) func(ctx context.Context) (bool, string, error) { + return grantHeldBy(composedGrantIn(open), busGrantIn(open, conn), time.Now) +} + +// grantJudge is one of the two facts: whether it holds, and why not. +type grantJudge func(ctx context.Context) (bool, string, error) + +// grantHeldBy is the judgement over the two facts, remembered for grantLookEvery: composing the list resolves +// the bus's machine whole, and asking the bus's module is a call. +func grantHeldBy(composed, inForce grantJudge, now func() time.Time) func(ctx context.Context) (bool, string, error) { var mu sync.Mutex var at time.Time var held bool @@ -248,22 +263,251 @@ func grantHeldIn(open *stores) func(ctx context.Context) (bool, string, error) { return func(ctx context.Context) (bool, string, error) { mu.Lock() defer mu.Unlock() - if !at.IsZero() && time.Since(at) < grantLookEvery { + if !at.IsZero() && now().Sub(at) < grantLookEvery { return held, why, nil } - machine, behind, err := brokerBehind(ctx, open, nil) + held, why = judgeGrant(ctx, composed, inForce) + at = now() + return held, why, nil + } +} + +// judgeGrant is held only when both facts hold; an error is a fact not known, never a yes. A fact that holds +// with a caveat (a reload not confirmed) gives that caveat as the why of a yes. +func judgeGrant(ctx context.Context, composed, inForce grantJudge) (bool, string) { + var caveat []string + for _, fact := range []struct { + judge grantJudge + what string + }{{composed, "whether the mesh composes the controller a grant to ask"}, {inForce, "whether the bus enforces it"}} { + if fact.judge == nil { + return false, fact.what + " is not judged here" + } + ok, why, err := fact.judge(ctx) + if err != nil { + return false, fact.what + " could not be worked out: " + err.Error() + } + if !ok { + return false, why + } + if why != "" { + caveat = append(caveat, why) + } + } + return true, strings.Join(caveat, "; ") +} + +// controllerMayAsk says whether the controller's own grant, as composed from these records, lets it publish its +// ask (ADR 0259 §3: composed while a router that takes asks under its asker's name is assigned). +func controllerMayAsk(records broker.Records) (bool, error) { + users, err := broker.Users(records) + if err != nil { + return false, err + } + for _, u := range users { + if u.Kind != broker.KindController { + continue + } + perms, err := broker.PermissionsFor(u) + if err != nil { + return false, err + } + return broker.MayPublish(perms, asks.AskSubject(askerName)), nil + } + return false, errors.New("the composition holds no controller") +} + +// composedGrantIn is the first fact, from the mesh's records. +func composedGrantIn(open *stores) grantJudge { + return func(ctx context.Context) (bool, string, error) { + if open == nil || open.inventory == nil { + return false, "", errors.New("the mesh's records are not open") + } + records, err := open.inventory.BusRecords(ctx) if err != nil { return false, "", err } - at, held, why = time.Now(), !behind, "" - if behind { - why = fmt.Sprintf("the bus's user list on %s is behind what the mesh composes, so the bus has not been "+ - "given the controller's grant to ask; `push %s` carries it", machine, machine) + may, err := controllerMayAsk(records) + if err != nil || may { + return may, "", err } - return held, why, nil + return false, "the mesh composes the controller no grant to ask: no router that takes asks under its " + + "asker's name is assigned", nil } } +// busGrantIn is the second fact, asked of the bus's own module on the machine holding the bus, with the check +// main made before it (novox/hq issue 353, PR 186: is the user list sent to the bus's machine the one composed +// now) kept for the cases the module's word does not settle. +func busGrantIn(open *stores, conn *nats.Conn) grantJudge { + ask := func(ctx context.Context) (string, json.RawMessage, error) { + if open == nil || open.inventory == nil || conn == nil { + return "", nil, errors.New("the mesh's records or the bus are not open") + } + holders, err := seatHolders(ctx, open.inventory) + if err != nil { + return "", nil, err + } + h, held := holders[theBrokerSeat] + if !held || h.Node == "" || h.Module == "" { + return "", nil, nil + } + a, err := link.AskModuleToolOn(ctx, conn, h.Module, busUserCanTool, h.Node, map[string]any{ + "user": broker.ControllerName, "action": "publish", "subject": asks.AskSubject(askerName)}, 15*time.Second) + if err != nil { + return h.Node, nil, err + } + if a.Error != "" { + return h.Node, nil, errors.New(a.Error) + } + return h.Node, a.Result, nil + } + list := func(ctx context.Context) (busListState, error) { + if open == nil || open.inventory == nil { + return busListState{}, errors.New("the mesh's records are not open") + } + s, err := brokerListState(ctx, open, nil) + if err != nil || s.Machine == "" || !s.Behind { + return s, err + } + step, err := busHeld(ctx, open.inventory, []string{s.Machine}) + if err != nil { + return busListState{}, err + } + s.BusStep = step[s.Machine] + return s, nil + } + return busGrantBy(ask, list) +} + +// busListState is the user list on the bus's machine as the controller's record knows it: whether the list +// sent there is behind the one composed now, why it could not be composed, and the bus step a send there waits +// on (that machine cannot be pushed until a person's `bus upgrade`). +type busListState struct { + Machine string + Behind bool + Uncomposed error + BusStep string +} + +// remedy is the one thing that sends the bus's machine the list composed now: the plan's own reason when it +// cannot be composed, the bus upgrade while sends there wait on it, and otherwise a push of that machine. +func (s busListState) remedy(node string) string { + if s.Machine != "" { + node = s.Machine + } + switch { + case s.Uncomposed != nil: + return fmt.Sprintf("the user list for %s cannot be composed (%v); `plan %s` says what to fix first", + node, s.Uncomposed, node) + case s.BusStep != "": + return fmt.Sprintf("sends to %s wait on the bus's planned step, so a push will not carry it: %s", node, s.BusStep) + } + return fmt.Sprintf("`push %s` carries it", node) +} + +// busGrantBy judges the bus's word, and names for each way it is not held the one thing that makes it held: +// +// - **in force**: held; +// - **allowed, from a module older than `in_force`**: held, with the caveat that the reload is not confirmed. +// The file allows the publish, and the bus reloads within about 5 s of a change to it, so the worst case is +// one ask refused in those seconds, which is asked again (issue 369). The list check is not consulted: +// while a bus step is pending the bus's machine cannot be pushed, so any change to the composed list would +// read as behind for good and silence every ask until a person's bus upgrade (the review of PR 188); +// - **allowed, written, not reloaded** (the module gives both times): wait about 5 s; +// - **allowed, in force not known** (the module could not read a time): what is not known, said; +// - **not allowed, or the module cannot answer**: the list check decides — the remedy that sends the list +// when it is behind (busListState.remedy), and, for a module that cannot answer, held while the list sent +// is current, as main judged it. +// +// ask answers the machine holding the bus and its module's answer ("" when nothing holds the bus); list is the +// record's word on the list sent to that machine. +func busGrantBy(ask func(ctx context.Context) (string, json.RawMessage, error), + list func(ctx context.Context) (busListState, error)) grantJudge { + return func(ctx context.Context) (bool, string, error) { + node, raw, askErr := ask(ctx) + if askErr == nil && node == "" { + return false, "no module holds the bus, so nothing says what it enforces", nil + } + var answer busAnswer + if askErr == nil { + if answer, askErr = readBusAnswer(raw); askErr != nil { + askErr = fmt.Errorf("its answer could not be read: %w", askErr) + } + } + if askErr != nil { + if node == "" { + node = "the bus's machine" + } + s, err := list(ctx) + if err != nil { + return false, "", fmt.Errorf("the bus module on %s could not answer (%v), and whether the list sent "+ + "there is the one composed now could not be worked out: %w", node, askErr, err) + } + if s.Behind { + return false, fmt.Sprintf("the bus module on %s could not answer (%v), and the user list sent there is "+ + "behind what the mesh composes; %s", node, askErr, s.remedy(node)), nil + } + return true, fmt.Sprintf("the bus module on %s could not answer (%v); judged by the user list sent there, "+ + "which is the one composed now", node, askErr), nil + } + notAllowed := func(older string) (bool, string, error) { + s, err := list(ctx) + if err != nil { + return false, "", fmt.Errorf("the user list on %s does not allow the controller to ask%s, and whether "+ + "the list sent there is the one composed now could not be worked out: %w", node, older, err) + } + if s.Behind { + return false, fmt.Sprintf("the user list on %s does not allow the controller to ask%s: it is behind "+ + "what the mesh composes; %s", node, older, s.remedy(node)), nil + } + return false, fmt.Sprintf("the user list on %s does not allow the controller to ask%s, though the list "+ + "sent there is the one composed now; %s again", node, older, s.remedy(node)), nil + } + switch { + case answer.InForce == nil && answer.Allowed: + return true, fmt.Sprintf("the user list on %s allows the controller to ask, but its reload is not "+ + "confirmed: the bus module there is older than the answer whether a grant is in force, and the bus "+ + "upgrade (`bus upgrade`) carries one that confirms it", node), nil + case answer.InForce == nil: + return notAllowed(" (the bus module there is older than the answer whether a grant is in force)") + case !answer.Allowed: + return notAllowed("") + case *answer.InForce: + return true, "", nil + case answer.FileWritten != "" && answer.ServerLoaded != "": + return false, fmt.Sprintf("the bus on %s has the controller's grant to ask written (%s) but has not "+ + "reloaded since (last load %s); it reloads in place within about 5 s, so nothing is to be done but wait", + node, answer.FileWritten, answer.ServerLoaded), nil + } + why := answer.Why + if why == "" { + why = "it gives no reason" + } + return false, fmt.Sprintf("the user list on %s allows the controller to ask, but whether the bus enforces it "+ + "is not known: %s", node, why), nil + } +} + +// busUserCanTool is the bus module's answer to whether a user may do something, and whether it is in force. +const busUserCanTool = "nats_user_can" + +// busAnswer is the bus module's answer to busUserCanTool; InForce is nil from a module older than that field, +// and FileWritten and ServerLoaded are given only when it could read both times (mesh-catalog's nats InForce). +type busAnswer struct { + Allowed bool `json:"allowed"` + InForce *bool `json:"in_force"` + Why string `json:"why"` + FileWritten string `json:"file_written"` + ServerLoaded string `json:"server_loaded"` +} + +func readBusAnswer(raw json.RawMessage) (busAnswer, error) { + var answer busAnswer + err := json.Unmarshal(raw, &answer) + return answer, err +} + // grantLookEvery is how often the bus's user list is judged against the one its machine was last sent. const grantLookEvery = 30 * time.Second @@ -302,7 +546,19 @@ func startAsking(ctx context.Context, open *stores, server *link.Server, conn *n fmt.Printf("the operator cannot be asked: %v\n", err) return } - a := &asker{ + a := servingAsker(open, conn, js, keeper) + if err := server.Decides(a); err != nil { + fmt.Printf("the operator's answers cannot be heard, so nothing is asked: %v\n", err) + return + } + askerFrom = a + go a.keep(ctx) +} + +// servingAsker is the serving controller's asker, wired to the mesh: what it reads, publishes, performs and +// records, and the judgements it asks by — the bus's grant to ask among them (novox/hq issue 353). +func servingAsker(open *stores, conn *nats.Conn, js jetstream.JetStream, keeper *conditions.Keeper) *asker { + return &asker{ open: keeper.Open, silence: func(ctx context.Context, key string, d time.Duration, by, why string) error { _, err := keeper.Silence(ctx, key, d, by, why) @@ -322,7 +578,7 @@ func startAsking(ctx context.Context, open *stores, server *link.Server, conn *n }, routerRecord: routerRecordOf(conn, open.inventory), routerHere: routerHereIn(open.inventory), - grantHeld: grantHeldIn(open), + grantHeld: grantHeldIn(open, conn), channels: channelsIn(open.inventory), raise: func(ctx context.Context, obs []conditions.Observation) error { return keeper.Reconcile(ctx, sourceAsker, obs) @@ -330,10 +586,4 @@ func startAsking(ctx context.Context, open *stores, server *link.Server, conn *n now: time.Now, logf: func(format string, args ...any) { fmt.Printf(format+"\n", args...) }, } - if err := server.Decides(a); err != nil { - fmt.Printf("the operator's answers cannot be heard, so nothing is asked: %v\n", err) - return - } - askerFrom = a - go a.keep(ctx) } diff --git a/cmd/mesh-controller/delivery_order_test.go b/cmd/mesh-controller/delivery_order_test.go index 878a2016..e5776e50 100644 --- a/cmd/mesh-controller/delivery_order_test.go +++ b/cmd/mesh-controller/delivery_order_test.go @@ -119,6 +119,13 @@ func TestTheBusMachineIsBehindByItsUserListAlone(t *testing.T) { if userListBehind("", "") { t.Fatal("a machine sent no list reads as behind") } + // A list that could not be composed is behind: not known is never "not behind" (the review of hq issue 353). + if !listBehind("", errors.New("the plan cannot be worked out"), digestOf([]byte(list))) { + t.Fatal("a list nobody could compose reads as current") + } + if listBehind(list, nil, digestOf([]byte(list))) || !listBehind(list, nil, "") { + t.Fatal("listBehind does not read the digest as userListBehind does") + } } // The machine holding the bus goes first: its declaration carries the user list the new grants are diff --git a/cmd/mesh-controller/proposals.go b/cmd/mesh-controller/proposals.go index 124aee3d..6dd4d000 100644 --- a/cmd/mesh-controller/proposals.go +++ b/cmd/mesh-controller/proposals.go @@ -591,7 +591,7 @@ func proposeCommand(ctx context.Context, module, node, valuesArg string, clear, return err }, routerHere: routerHereIn(open.inventory), - grantHeld: grantHeldIn(open), + grantHeld: grantHeldIn(open, conn), routerRecord: func(ctx context.Context, id string) (string, error) { bucket, err := asksRecords(ctx, open.inventory) if err != nil || bucket == "" { diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 723eb7bc..8aef75c2 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -1084,42 +1084,58 @@ func brokerFirst(names []string, holder string, behind bool) []string { // 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) { + s, err := brokerListState(ctx, open, names) + return s.Machine, s.Behind, err +} + +// brokerListState is brokerBehind with why the list could not be composed, when it could not: the plan's own +// reason, which a remedy names rather than a push that would fail on it (the review of hq issue 353). +func brokerListState(ctx context.Context, open *stores, names []string) (busListState, error) { inv := open.inventory holders, err := seatHolders(ctx, inv) if err != nil { - return "", false, err + return busListState{}, err } h, held := holders[theBrokerSeat] if !held || h.Node == "" { - return "", false, nil + return busListState{}, nil } shelf, err := inv.Catalogue(ctx) if err != nil { - return "", false, err + return busListState{}, err } if m, known := shelf[h.Module]; !known || m.BusUsers == "" { // A holder that is sent no user list carries no grant: nothing to send first. - return "", false, nil + return busListState{}, nil } for _, n := range names { if n == h.Node { - return h.Node, false, nil + return busListState{Machine: h.Node}, nil } } + // **What cannot be worked out is behind** (the review of hq issue 353). The list it would be sent cannot be + // composed, so whether it carries the grants a send relies on is not known; it is taken as behind, so a push + // sends that machine first and says why it cannot (`plan` says it too), and a rollout that needs the list + // refuses rather than sending code whose grants the bus may not hold. It said "not behind" until then: a + // send went ahead as if the bus held a list nobody could compose. + list, composeErr := "", error(nil) plan, _, err := planFor(ctx, open, h.Node) if err != nil { - // It cannot be worked out: sending it would refuse the whole send, and `plan` says why. - return h.Node, false, nil - } - list, _, err := busUserList(ctx, inv, plan.Modules) - if err != nil { - return h.Node, false, nil + composeErr = err + } else if list, _, err = busUserList(ctx, inv, plan.Modules); err != nil { + composeErr = err } sent, err := inv.SentBusUsers(ctx, h.Node) if err != nil { - return "", false, err + return busListState{}, err } - return h.Node, userListBehind(list, sent), nil + return busListState{Machine: h.Node, Behind: listBehind(list, composeErr, sent), Uncomposed: composeErr}, nil +} + +// listBehind is whether the bus's machine is behind: the list composed now is not the one last sent, or it +// could not be composed at all — not known is behind, never "not behind". +func listBehind(list string, composeErr error, sentDigest string) bool { + return composeErr != nil || userListBehind(list, sentDigest) } // userListBehind is whether the user list composed now is not the one last sent, by its digest. An