From 2eb9a22c2402415175f6b91a424bee500907a8d4 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 6 Oct 2026 12:29:18 +0200 Subject: [PATCH] Act under a lease, keep accounts by order, one writer at composition (hq to-be 45 Phase 2) Two controllers could both act (issue 204), a reconcile's report could overtake the apply after it and the digest decided (issue 267), and a grant could make a second writer of a machine's report. - The lease (internal/lease, ADR 0229): mesh-controller_lease key `holder`, 15 s age, renewed every 5 s by compare-and-set; the epoch is the revision it was taken at. The gate is the clock (stops 3 s before expiry); a refused renewal is a loss and the process exits; a holder that stops gives it back. serve takes it before asserting the bus. Epochs kept in the store (migration 0068 controller_epoch) as a floor: a bucket raised from nothing is compacted past it. Unleased (no epoch, S12 urgent) only when nobody holds it and the bus will not let it be written. A shell command acts under the holder's epoch, or its own lease when none. - Declarations carry `epoch` inside the signed envelope, only to a machine whose latest account carried a report_sequence (mesh-host #35); would-send is composed with the epoch last sent. Allot and the send both pass the gate. - Reports: contract in internal/link/order.go (epoch, sequence, report_sequence, older_than, refused_older). Accounts kept by epoch, then sequence, then report sequence; older refused, counted; unordered reports keep the digest rule. Plans by compare-and-set on a revision, with epoch. Conditions and calls carry the epoch and are not written off the lease. - S12 and S13 (naming the writer by epoch) watched, D5 run; reset of the bucket said. Writers table compiled in and enforced in PermissionsFor; the controller no longer publishes mesh.control.>. A contract per consumed kind, and the empty-on-error lint over the repository. - mesh-host pinned to its main with the epoch in the validator (D1 validates the envelope as sent). Needs mesh-host's genesis lock with the lease grant (mesh-host PR) for TestTheInstallersFirstUserListIsWhatTheControllerWouldCompose. --- cmd/mesh-controller/acting.go | 353 +++++++++++++ cmd/mesh-controller/build.go | 2 + cmd/mesh-controller/conditions.go | 4 +- cmd/mesh-controller/doctor.go | 6 +- cmd/mesh-controller/epoch_send_test.go | 115 +++++ cmd/mesh-controller/held_back_test.go | 2 +- cmd/mesh-controller/issue_204_test.go | 8 +- cmd/mesh-controller/lease_command_test.go | 68 +++ cmd/mesh-controller/lease_test.go | 175 +++++++ cmd/mesh-controller/main.go | 3 + cmd/mesh-controller/plan_retry.go | 6 +- .../plans_one_at_a_time_test.go | 2 +- cmd/mesh-controller/probes.go | 81 +++ cmd/mesh-controller/push.go | 119 ++++- cmd/mesh-controller/push_test.go | 4 +- cmd/mesh-controller/queue_test.go | 14 +- cmd/mesh-controller/release_plan.go | 8 +- cmd/mesh-controller/sendable.go | 8 + cmd/mesh-controller/signals.go | 126 ++++- cmd/mesh-controller/signals_test.go | 73 ++- cmd/mesh-controller/source.go | 16 +- cmd/mesh-controller/status_summary.go | 13 +- cmd/mesh-controller/stores.go | 2 + cmd/mesh-controller/supersede_test.go | 2 +- cmd/mesh-controller/upgrades.go | 6 +- cmd/mesh-controller/watchdogs.go | 46 +- go.mod | 2 +- go.sum | 4 +- internal/broker/controller_buckets.go | 35 +- internal/broker/nats.go | 12 +- internal/broker/testdata/composed.conf | 2 +- internal/broker/writers.go | 167 +++++++ internal/broker/writers_test.go | 143 ++++++ internal/conditions/condition.go | 5 +- internal/conditions/store.go | 34 ++ internal/inventory/durations_test.go | 10 +- .../migrations/0068-order-and-one-writer.sql | 40 ++ internal/inventory/nodes.go | 47 +- internal/inventory/order.go | 250 ++++++++++ internal/inventory/order_test.go | 126 +++++ internal/inventory/plans.go | 68 ++- internal/inventory/plans_test.go | 10 +- internal/lease/lease.go | 471 ++++++++++++++++++ internal/lease/lease_test.go | 296 +++++++++++ internal/link/calls.go | 35 +- internal/link/contracts.go | 47 ++ internal/link/contracts_test.go | 84 ++++ internal/link/declare.go | 10 + internal/link/enrolment.go | 58 ++- internal/link/heard_order_test.go | 163 ++++++ internal/link/order.go | 118 +++++ internal/link/order_test.go | 113 +++++ internal/link/protocol.go | 17 + internal/link/serve.go | 5 + internal/link/watched.go | 136 ++++- internal/lint/emptyonerror.go | 237 +++++++++ internal/lint/emptyonerror_test.go | 100 ++++ .../internal/declaration/declaration.go | 22 +- vendor/modules.txt | 4 +- 59 files changed, 3975 insertions(+), 158 deletions(-) create mode 100644 cmd/mesh-controller/acting.go create mode 100644 cmd/mesh-controller/epoch_send_test.go create mode 100644 cmd/mesh-controller/lease_command_test.go create mode 100644 cmd/mesh-controller/lease_test.go create mode 100644 internal/broker/writers.go create mode 100644 internal/broker/writers_test.go create mode 100644 internal/inventory/migrations/0068-order-and-one-writer.sql create mode 100644 internal/inventory/order.go create mode 100644 internal/inventory/order_test.go create mode 100644 internal/lease/lease.go create mode 100644 internal/lease/lease_test.go create mode 100644 internal/link/contracts.go create mode 100644 internal/link/contracts_test.go create mode 100644 internal/link/heard_order_test.go create mode 100644 internal/link/order.go create mode 100644 internal/link/order_test.go create mode 100644 internal/lint/emptyonerror.go create mode 100644 internal/lint/emptyonerror_test.go diff --git a/cmd/mesh-controller/acting.go b/cmd/mesh-controller/acting.go new file mode 100644 index 0000000..5fbb01f --- /dev/null +++ b/cmd/mesh-controller/acting.go @@ -0,0 +1,353 @@ +package main + +import ( + "context" + "errors" + "fmt" + "os" + "sync" + "time" + + "github.com/nats-io/nats.go/jetstream" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/lease" + "github.com/novox/mesh-controller/internal/link" +) + +// Acting under the lease (novox/hq to-be 45 §6, ADR 0227 rule 1). +// +// **Only the instance holding the lease acts**: sends a declaration, writes a plan, a condition or a +// call. Every one of those passes theLease.epoch, which answers the epoch the act carries or why it may +// not happen. Three ways a process stands to the lease: +// +// - **The serving controller** takes it before it does anything else — before it asserts the bus's +// objects, which are the controller's to write — waiting while another holds it, and renews it. +// A renewal refused or failed is the lease lost: the gate closes at once and the process exits, so +// its service manager restarts it as a candidate (serve, in push.go). +// - **A command run at a shell** — `push` in the installer, the lab, a person repairing a mesh whose +// controller is down (issue 201) — acts **under the holder's epoch** when a controller holds the +// lease: it is the same mesh's word, composed and sent under the store's hold of each machine like +// the serving controller's, and the epoch it carries is read at the moment it acts, so a handover +// between makes it stale and refused like any other. **When nobody holds the lease, the command +// takes it** for as long as it runs and gives it back; a controller starting meanwhile waits for +// it, as it would for another controller. +// - **A process with no bus** — a test, a command that only reads — acts with no epoch and is +// refused nothing: there is nothing to order against, and nothing it does reaches a machine. +// +// **Unleased, said and temporary.** A serving controller whose bus refuses it the lease's key — the bus's +// user list is older than this build and does not grant the bucket yet — and that sees no other holder +// serves without one, as every controller did before the lease: declarations carry no epoch, which no +// node-engine refuses. Said once, kept as a condition (S12), and tried again every renewal interval; the +// first push that sends the bus its new user list grants it, and the next try takes it. Refusing to act +// instead would be a controller that can never send the user list that lets it act. + +// actor is this process's standing to the lease. +type actor struct { + mu sync.Mutex + // held is the lease this process holds: the serving controller's, or a command's own. + held *lease.Lease + // unleased is why a serving controller acts without the lease; empty while it holds it or is not + // serving. + unleased string + // serving is a serving controller, which never borrows another's epoch. + serving bool + // kv is the lease bucket, for a command to read the holder's epoch from. + kv jetstream.KeyValue + close func() + // noBus is a process with no bus configured. + noBus bool + // reset is when the lease bucket was found raised again from nothing and its revisions moved past + // the highest epoch issued, and what was said of it; zero when it was not (S12). + reset time.Time + resetSaid string +} + +// theLease is this process's standing to the lease. +var theLease = &actor{} + +// instance names this process among controller instances: its machine, its process and when it +// started. The lease's holder and every call this process keeps carry it. +var instance = func() string { + host, _ := os.Hostname() + return fmt.Sprintf("controller@%s pid %d since %s", host, os.Getpid(), time.Now().UTC().Format(time.RFC3339)) +}() + +// epoch is the gate: the epoch an act carries — zero for none — or why it may not happen. +func (a *actor) epoch(ctx context.Context) (uint64, error) { + a.mu.Lock() + held, serving, unleased, noBus := a.held, a.serving, a.unleased, a.noBus + a.mu.Unlock() + switch { + case held != nil: + return held.Epoch() + case serving && unleased != "": + return 0, nil + case serving: + return 0, lease.ErrNotHeld + case noBus: + return 0, nil + } + return a.forACommand(ctx) +} + +// forACommand is a command's epoch: the holder's, or a lease of its own when nobody holds one. +func (a *actor) forACommand(ctx context.Context) (uint64, error) { + a.mu.Lock() + defer a.mu.Unlock() + if a.held != nil { + return a.held.Epoch() + } + if a.kv == nil { + address, err := broker.BusAddress() + if err != nil { + // No bus: this process reaches no machine, and has nothing to order against. + a.noBus = true + return 0, nil + } + js, err := broker.Dial(address) + if err != nil { + return 0, fmt.Errorf("the bus cannot be reached, so whether a controller holds the lease cannot be "+ + "read and nothing is done: %w", err) + } + api, err := jetstream.New(js.Conn()) + if err != nil { + js.Close() + return 0, err + } + reading, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + if err := broker.EnsureLeaseBucket(reading, api); err != nil { + js.Close() + return 0, err + } + kv, err := api.KeyValue(reading, broker.LeaseBucket) + if err != nil { + js.Close() + return 0, err + } + a.kv, a.close = kv, js.Close + if _, found, err := lease.Current(reading, kv); err == nil && !found { + // Nobody: this command takes it for as long as it runs. + l, err := lease.Open(reading, api, broker.LeaseBucket, lease.Options{Holder: holderOf(instance), + Say: func(format string, args ...any) { fmt.Printf(format+"\n", args...) }}) + if err != nil { + return 0, err + } + epoch, err := l.TryTake(reading) + if err != nil { + return 0, fmt.Errorf("no controller holds the lease and this command could not take it: %w", err) + } + keeping, stop := context.WithCancel(context.Background()) + go l.Keep(keeping) + a.held = l + closeBus := a.close + a.close = func() { + stop() + l.Release(context.Background()) + closeBus() + } + return epoch, nil + } + } + reading, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + holder, found, err := lease.Current(reading, a.kv) + if err != nil { + return 0, fmt.Errorf("who holds the controller lease cannot be read, so nothing is done: %w", err) + } + if !found { + return 0, errors.New("the controller that held the lease while this command ran let go of it; nothing " + + "more is done under an epoch nobody holds — run the command again") + } + return holder.Epoch, nil +} + +// release gives back what this process holds, at its end. +func (a *actor) release() { + a.mu.Lock() + closing := a.close + a.close = nil + a.mu.Unlock() + if closing != nil { + closing() + } +} + +// holderOf is this process as the lease's holder. +func holderOf(instance string) lease.Holder { + host, _ := os.Hostname() + return lease.Holder{Instance: instance, Host: host, Build: version} +} + +// serveUnderTheLease takes the lease for the serving controller, waiting while another holds it, and +// keeps it until ctx ends. Lost is closed when it is lost; the caller exits on it. +func (a *actor) serveUnderTheLease(ctx context.Context, inv *inventory.Inventory, address string) (lost <-chan struct{}, err error) { + a.mu.Lock() + a.serving = true + a.mu.Unlock() + js, err := broker.Dial(address) + if err != nil { + return nil, fmt.Errorf("the mesh is on the bus at %s and this control plane cannot reach it to take the "+ + "lease: %w", broker.BareAddress(address), err) + } + api, err := jetstream.New(js.Conn()) + if err != nil { + js.Close() + return nil, err + } + asserting, cancel := context.WithTimeout(ctx, 10*time.Second) + err = broker.EnsureLeaseBucket(asserting, api) + cancel() + if err != nil { + js.Close() + return nil, err + } + say := func(format string, args ...any) { fmt.Printf(format+"\n", args...) } + l, err := lease.Open(ctx, api, broker.LeaseBucket, lease.Options{Holder: holderOf(instance), + Floor: inv.HighestEpoch, Say: say, Moved: func(was, floor uint64) { + a.mu.Lock() + defer a.mu.Unlock() + a.reset = time.Now() + a.resetSaid = fmt.Sprintf("the lease bucket was at revision %d with epoch %d already issued: it was "+ + "raised again from nothing (a bus whose data was replaced), and its revisions were moved past %d so "+ + "no machine refuses the next epoch", was, floor, floor) + }}) + if err != nil { + js.Close() + return nil, err + } + gone := make(chan struct{}) + epoch, err := l.Take(ctx) + switch { + case ctx.Err() != nil: + js.Close() + return nil, ctx.Err() + case err != nil && !errors.Is(err, lease.ErrUnwritable): + // Whether another controller acts cannot be told: this one does not act, and exits to try again. + js.Close() + return nil, err + case err != nil: + // Nobody holds it and the bus will not let it be written: unleased, said, tried again (see above). + a.mu.Lock() + a.unleased = err.Error() + a.mu.Unlock() + say("this controller serves WITHOUT the lease: %v. Its declarations carry no epoch; it tries again "+ + "every %s, and the first push that sends the bus its user list grants it", err, lease.RenewEvery) + go a.takeWhenGranted(ctx, l, inv, gone) + default: + a.took(ctx, l, inv, epoch, gone) + } + a.mu.Lock() + a.close = func() { + if held, err := l.Epoch(); err == nil { + ending, cancel := context.WithTimeout(context.Background(), 5*time.Second) + if err := inv.EndEpoch(ending, held, inventory.EpochReleased); err != nil { + say("how epoch %d ended could not be recorded: %v", held, err) + } + cancel() + } + l.Release(context.Background()) + js.Close() + } + a.mu.Unlock() + return gone, nil +} + +// took is the lease taken: recorded, earlier epochs nobody gave back ended as expired, kept. +func (a *actor) took(ctx context.Context, l *lease.Lease, inv *inventory.Inventory, epoch uint64, gone chan struct{}) { + a.mu.Lock() + a.held, a.unleased = l, "" + a.mu.Unlock() + h := holderOf(instance) + recording, cancel := context.WithTimeout(ctx, 10*time.Second) + expired, err := inv.TookEpoch(recording, inventory.Epoch{Epoch: epoch, Instance: h.Instance, Host: h.Host, + Build: h.Build, Taken: time.Now()}) + cancel() + if err != nil { + fmt.Printf("epoch %d could not be recorded as taken, so a stale refusal from it will not name it: %v\n", epoch, err) + } + for _, e := range expired { + fmt.Printf("the controller of epoch %d (%s) stopped renewing the lease without giving it back: it is "+ + "taken over at epoch %d\n", e.Epoch, e.Instance, epoch) + } + go l.Keep(ctx) + go func() { + <-l.Lost() + if ctx.Err() == nil { + why := l.LostWhy() + ending, cancel := context.WithTimeout(context.Background(), 5*time.Second) + _ = inv.EndEpoch(ending, epoch, inventory.EpochLost) + cancel() + fmt.Printf("the controller lease was lost (epoch %d): %v — this controller stops and exits, to "+ + "be started again as a candidate\n", epoch, why) + } + close(gone) + }() +} + +// takeWhenGranted tries the lease again every renewal interval while serving unleased, and stops this +// controller if another took it meanwhile: two serving at once is what the lease is for. +func (a *actor) takeWhenGranted(ctx context.Context, l *lease.Lease, inv *inventory.Inventory, gone chan struct{}) { + tick := time.NewTicker(lease.RenewEvery) + defer tick.Stop() + for { + select { + case <-ctx.Done(): + return + case <-tick.C: + } + epoch, err := l.TryTake(ctx) + if errors.Is(err, lease.ErrTaken) { + fmt.Printf("another controller took the lease while this one served without it: %v — this one "+ + "stops and exits\n", err) + close(gone) + return + } + if err != nil { + // Still not written, or not readable this time: unleased, said by S12, tried again. + a.mu.Lock() + a.unleased = err.Error() + a.mu.Unlock() + continue + } + a.took(ctx, l, inv, epoch, gone) + return + } +} + +// standing is what `status` and the self-check say of this process and the lease. +type standing struct { + Epoch uint64 + Held bool + Renewed time.Time + Unleased string + // Reset is when the lease bucket was found raised again from nothing, and ResetSaid what of it. + Reset time.Time + ResetSaid string +} + +func (a *actor) standing() standing { + a.mu.Lock() + held, unleased, reset, resetSaid := a.held, a.unleased, a.reset, a.resetSaid + a.mu.Unlock() + st := standing{Unleased: unleased, Reset: reset, ResetSaid: resetSaid} + if held == nil { + return st + } + epoch, err := held.Epoch() + st.Epoch, st.Held, st.Renewed = epoch, err == nil, held.Renewed() + return st +} + +// The gates, given to what acts: a declaration's send (link) and a plan's write (the inventory). +func init() { + link.ActingGate = func(ctx context.Context) error { + _, err := theLease.epoch(ctx) + if err != nil { + return fmt.Errorf("this controller may not send: %w", err) + } + return nil + } +} diff --git a/cmd/mesh-controller/build.go b/cmd/mesh-controller/build.go index cc587ec..7c9a4b5 100644 --- a/cmd/mesh-controller/build.go +++ b/cmd/mesh-controller/build.go @@ -707,12 +707,14 @@ func heldBy(ctx context.Context) map[string]string { if err != nil { fmt.Fprintf(os.Stderr, "could not read what this mesh has built, so a module naming a "+ "base will be told that base is missing: %v\n", err) + // empty-on-error: said above; a build that names a base is refused by name for want of it return nil } defer open.Close() held, err := open.inventory.Held(ctx) if err != nil { fmt.Fprintf(os.Stderr, "could not read what this mesh has built: %v\n", err) + // empty-on-error: said above; a build that names a base is refused by name for want of it return nil } address, err := whereABuilderReachesTheStore(ctx, open.inventory) diff --git a/cmd/mesh-controller/conditions.go b/cmd/mesh-controller/conditions.go index fc016fb..3351436 100644 --- a/cmd/mesh-controller/conditions.go +++ b/cmd/mesh-controller/conditions.go @@ -46,7 +46,9 @@ func keeperOn(ctx context.Context, conn *nats.Conn) (*conditions.Keeper, error) Say: func(format string, args ...any) { fmt.Fprintf(os.Stderr, format+"\n", args...) }, // What status leads with changed: composed again soon (a nudge outside the serving controller // does nothing). - Changed: statusFrom.nudge}), nil + Changed: statusFrom.nudge, + // Written under the lease, carrying its epoch (novox/hq to-be 45 §6). + Epoch: func() (uint64, error) { return theLease.epoch(context.WithoutCancel(ctx)) }}), nil } // withKeeper runs f with the serving controller's keeper, or one of its own that says everything diff --git a/cmd/mesh-controller/doctor.go b/cmd/mesh-controller/doctor.go index 3baf24c..e705a9b 100644 --- a/cmd/mesh-controller/doctor.go +++ b/cmd/mesh-controller/doctor.go @@ -79,9 +79,9 @@ var probeRegistry = []probe{ "machine that is heard from", From: "issues 208, 218", Kind: "holder-silent", Phase: 1, run: probeHolders}, {ID: "D4", Asserts: "every kept archive is held by a manifest", From: "issue 253", Kind: "archives-unheld", Phase: 1, run: probeArchives}, - {ID: "D5", Asserts: "exactly one lease holder; no message from a stale epoch in the last interval", - From: "issue 204", Kind: "lease-split", Phase: 2, - Deferred: "the lease and epoch are built in Phase 2 (to-be 45 §6): nothing holds one yet"}, + {ID: "D5", Asserts: "exactly one lease holder — the key names this controller at its epoch, and the record " + + "holds no other epoch open; no message from a stale epoch refused in the last interval", + From: "issue 204", Kind: "lease-split", Phase: 2, run: probeLease}, {ID: "D6", Asserts: "every durable consumer the mesh expects exists with its definition, and is near its " + "stream's head", From: "issues 248, 266", Kind: "consumer-wrong", Phase: 1, run: probeConsumers}, {ID: "D7", Asserts: "every stream the controller defines exists with its definition, and its own buckets", diff --git a/cmd/mesh-controller/epoch_send_test.go b/cmd/mesh-controller/epoch_send_test.go new file mode 100644 index 0000000..2c16c9a --- /dev/null +++ b/cmd/mesh-controller/epoch_send_test.go @@ -0,0 +1,115 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/link" +) + +// A declaration carries the lease's epoch (novox/hq to-be 45 §6) — to a machine whose node-engine said +// it reads one, and to no other: an older node-engine refuses a key it does not know, whole. + +// bodiesDelivery records each send as the mesh does, and keeps the bodies. +type bodiesDelivery struct { + recordedDelivery + bodies map[string][]byte +} + +func (b *bodiesDelivery) declare(ctx context.Context, s readyNode, body []byte) (string, error) { + b.bodies[s.node] = body + return b.recordedDelivery.declare(ctx, s, body) +} + +func TestAMachineIsSentTheEpochOnlyOnceItSaysItReadsOne(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + inv := open.inventory + epoch := uint64(57) + was := epochForActs + epochForActs = func(context.Context) (uint64, error) { return epoch, nil } + t.Cleanup(func() { epochForActs = was }) + + anchor, err := inv.NodeByName(ctx, "anchor") + if err != nil { + t.Fatal(err) + } + if err := inv.RecordReadsEpoch(ctx, anchor.ID, true); err != nil { + t.Fatal(err) + } + gens, err := generators(ctx, open) + if err != nil { + t.Fatal(err) + } + d := &bodiesDelivery{recordedDelivery: recordedDelivery{inv: inv}, bodies: map[string][]byte{}} + if _, err := sendRound(ctx, open, []string{"anchor", "laptop"}, composeForPush(open, gens), d, ""); err != nil { + t.Fatal(err) + } + carried := func(node string) (epoch float64, has bool) { + var envelope map[string]any + if err := json.Unmarshal(d.bodies[node], &envelope); err != nil { + t.Fatal(err) + } + epoch, has = envelope["epoch"].(float64) + return epoch, has + } + if e, has := carried("anchor"); !has || e != 57 { + t.Fatalf("the machine that reads an epoch was sent %v: %s", e, d.bodies["anchor"]) + } + if _, has := carried("laptop"); has { + t.Fatalf("a machine that never said it reads an epoch was sent one: %s", d.bodies["laptop"]) + } + + // A new holder of the lease is not a change of the machine: neither reads as behind. + epoch = 58 + would, err := wouldSend(ctx, open, mustNodes(t, open)) + if err != nil { + t.Fatal(err) + } + for _, node := range []string{"anchor", "laptop"} { + sent, err := inv.Outstanding(ctx, node) + if err != nil { + t.Fatal(err) + } + if would[node] != sent { + t.Fatalf("%s reads as behind after the lease changed hands, with nothing else changed", node) + } + } +} + +// A process that may not act composes nothing and sends nothing: its number is not taken. +func TestNothingIsComposedOrSentWithoutTheLease(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + was := epochForActs + epochForActs = func(context.Context) (uint64, error) { return 0, errors.New("this controller lost the lease") } + t.Cleanup(func() { epochForActs = was }) + gens, err := generators(ctx, open) + if err != nil { + t.Fatal(err) + } + d := &bodiesDelivery{recordedDelivery: recordedDelivery{inv: open.inventory}, bodies: map[string][]byte{}} + refused, err := sendRound(ctx, open, []string{"anchor"}, composeForPush(open, gens), d, "") + if err != nil { + t.Fatal(err) + } + if len(d.bodies) != 0 || len(refused) != 1 || !strings.Contains(refused[0], "lost the lease") { + t.Fatalf("a controller without the lease composed %d and refused %v", len(d.bodies), refused) + } + anchor, _ := open.inventory.NodeByName(ctx, "anchor") + if seq, _ := open.inventory.Sequence(ctx, anchor.ID); seq != 0 { + t.Fatalf("a controller without the lease took sequence %d", seq) + } + + // And at the send itself: the gate every declaration passes. + gate := link.ActingGate + link.ActingGate = func(context.Context) error { return errors.New("this controller lost the lease") } + t.Cleanup(func() { link.ActingGate = gate }) + if err := link.Declare(ctx, nil, nil, "anchor", []byte(`{"declaration":1}`), 0); err == nil || + !strings.Contains(err.Error(), "lost the lease") { + t.Fatalf("a declaration was let through the gate: %v", err) + } +} diff --git a/cmd/mesh-controller/held_back_test.go b/cmd/mesh-controller/held_back_test.go index 5733803..483097c 100644 --- a/cmd/mesh-controller/held_back_test.go +++ b/cmd/mesh-controller/held_back_test.go @@ -128,7 +128,7 @@ func (r *recordedDelivery) grant(context.Context, []readyNode) error { return ni func (r *recordedDelivery) declare(ctx context.Context, s readyNode, body []byte) (string, error) { r.declared = append(r.declared, s.node) - return recordSent(ctx, r.inv, s.node, body, s.declared.Builds) + return recordSent(ctx, r.inv, s.node, body, s.declared.Builds, s.declared.Epoch) } // aResolver is a module built from a repository, at a commit, with something on the machine that diff --git a/cmd/mesh-controller/issue_204_test.go b/cmd/mesh-controller/issue_204_test.go index f32af93..feda54d 100644 --- a/cmd/mesh-controller/issue_204_test.go +++ b/cmd/mesh-controller/issue_204_test.go @@ -45,11 +45,11 @@ func TestADeclarationComposedEarlierIsNumberedLowerWhateverOrderItIsSent(t *test // fails leaves nothing composed for that machine, and the others are still composed. func TestTheNumberIsTakenBeforeComposingAndItsFailureIsARefusal(t *testing.T) { calls := 0 - allot := func(node string) (int64, error) { + allot := func(node string) (order, error) { if node == "anchor" { - return 0, context.DeadlineExceeded + return order{}, context.DeadlineExceeded } - return 7, nil + return order{sequence: 7}, nil } sending, refusals := composeEach([]string{"anchor", "laptop"}, allot, func(node string) (sendable, error) { calls++ @@ -77,7 +77,7 @@ func TestASendIsRecordedEvenWhenTheSenderIsBeingCancelled(t *testing.T) { } cancel() // the sender is going away: its context is cancelled between the send and the record body := []byte(`{"declaration":1,"resources":[]}`) - digest, err := recordSent(ctx, inv, "anchor", body, nil) + digest, err := recordSent(ctx, inv, "anchor", body, nil, 0) if err != nil { // NodeByName on the cancelled context may itself refuse; the record must still be possible // through the detached context, so look the node up again on a live one. diff --git a/cmd/mesh-controller/lease_command_test.go b/cmd/mesh-controller/lease_command_test.go new file mode 100644 index 0000000..af20840 --- /dev/null +++ b/cmd/mesh-controller/lease_command_test.go @@ -0,0 +1,68 @@ +package main + +import ( + "testing" + + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/lease" +) + +// A command run at a shell (novox/hq to-be 45 §6): under the holder's epoch while a controller holds the +// lease, read at the moment it acts; under a lease of its own while none does, given back as it ends. +func TestACommandActsUnderTheHoldersEpochOrItsOwn(t *testing.T) { + url, kv := aBusForTheLease(t) + t.Setenv(broker.NATSVar, url) + ctx := t.Context() + + // Nobody holds it: the command takes it, and gives it back. + cmd := &actor{} + own, err := cmd.epoch(ctx) + if err != nil || own == 0 { + t.Fatalf("a command with nobody holding the lease acts as %d (%v)", own, err) + } + if h, found, _ := lease.Current(ctx, kv); !found || h.Epoch != own { + t.Fatalf("the command's lease is not on the bus: %+v", h) + } + cmd.release() + if _, found, _ := lease.Current(ctx, kv); found { + t.Fatal("the command did not give its lease back as it ended") + } + + // A controller holds it: a command acts under that epoch. + l, err := lease.Open(ctx, mustJetStream(t, url), broker.LeaseBucket, lease.Options{Holder: lease.Holder{Instance: "serving"}}) + if err != nil { + t.Fatal(err) + } + held, err := l.TryTake(ctx) + if err != nil { + t.Fatal(err) + } + borrower := &actor{} + defer borrower.release() + if got, err := borrower.epoch(ctx); err != nil || got != held { + t.Fatalf("a command acts as %d (%v), want the holder's %d", got, err, held) + } + // The holder lets go: the command does not go on under an epoch nobody holds. + l.Release(ctx) + if _, err := borrower.epoch(ctx); err == nil { + t.Fatal("a command acted under an epoch nobody holds any more") + } +} + +// mustJetStream is a connection of its own to the test bus. +func mustJetStream(t *testing.T, url string) jetstream.JetStream { + t.Helper() + conn, err := nats.Connect(url) + if err != nil { + t.Fatal(err) + } + t.Cleanup(conn.Close) + js, err := jetstream.New(conn) + if err != nil { + t.Fatal(err) + } + return js +} diff --git a/cmd/mesh-controller/lease_test.go b/cmd/mesh-controller/lease_test.go new file mode 100644 index 0000000..717b8c5 --- /dev/null +++ b/cmd/mesh-controller/lease_test.go @@ -0,0 +1,175 @@ +package main + +import ( + "context" + "errors" + "os" + "testing" + "time" + + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/conditions" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/lease" +) + +// Two controllers at once (novox/hq to-be 45 §6, issue 204; the half of replay R1 that lives here): two +// serving controllers over one store and one bus. The second waits while the first holds the lease; on +// a handover it takes it at a higher epoch, the record says which held what and how each ended; and the +// one that lost it acts no more — no declaration composed, no plan and no condition written — the +// moment it lost it. +// +// MESH_TEST_POSTGRES=… MESH_TEST_NATS=nats://127.0.0.1:14222 go test ./cmd/mesh-controller/ -run Controllers + +// aBusForTheLease is the test bus with the controller's lease bucket new. +func aBusForTheLease(t *testing.T) (string, jetstream.KeyValue) { + t.Helper() + url := os.Getenv("MESH_TEST_NATS") + if url == "" { + t.Skip("MESH_TEST_NATS unset") + } + conn, err := nats.Connect(url) + if err != nil { + t.Fatal(err) + } + t.Cleanup(conn.Close) + js, err := jetstream.New(conn) + if err != nil { + t.Fatal(err) + } + _ = js.DeleteKeyValue(t.Context(), broker.LeaseBucket) + if err := broker.EnsureLeaseBucket(t.Context(), js); err != nil { + t.Fatal(err) + } + kv, err := js.KeyValue(t.Context(), broker.LeaseBucket) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = js.DeleteKeyValue(context.Background(), broker.LeaseBucket) }) + return url, kv +} + +func TestTwoControllersOneActs(t *testing.T) { + url, kv := aBusForTheLease(t) + inv := inventory.ForTest(t) + ctx := t.Context() + + // Controller A takes the lease. + a := &actor{} + aCtx, stopA := context.WithCancel(ctx) + defer stopA() + lostA, err := a.serveUnderTheLease(aCtx, inv, url) + if err != nil { + t.Fatal(err) + } + epochA, err := a.epoch(ctx) + if err != nil || epochA == 0 { + t.Fatalf("A holds no epoch: %d, %v", epochA, err) + } + + // Controller B starts while A holds it, and waits — acting on nothing meanwhile. + b := &actor{} + bCtx, stopB := context.WithCancel(ctx) + defer stopB() + tookB := make(chan (<-chan struct{}), 1) + go func() { + lost, err := b.serveUnderTheLease(bCtx, inv, url) + if err != nil { + t.Errorf("B: %v", err) + close(tookB) + return + } + tookB <- lost + }() + select { + case <-tookB: + t.Fatal("B took the lease while A held it") + case <-time.After(3 * time.Second): + } + if _, err := b.epoch(ctx); !errors.Is(err, lease.ErrNotHeld) { + t.Fatalf("B, waiting, may act: %v", err) + } + + // A hands over, as a controller being replaced does: B takes the lease at once, at a higher epoch. + stopA() + a.release() + var lostB <-chan struct{} + select { + case lostB = <-tookB: + case <-time.After(10 * time.Second): + t.Fatal("B did not take the lease A gave back") + } + select { + case <-lostA: + default: + t.Fatal("A, having given the lease back, is not told it no longer holds it") + } + epochB, err := b.epoch(ctx) + if err != nil || epochB <= epochA { + t.Fatalf("B acts as epoch %d after A's %d (%v): an epoch only grows", epochB, epochA, err) + } + if _, err := a.epoch(ctx); err == nil { + t.Fatal("A acts after giving the lease back") + } + ea, _, _ := inv.EpochOf(ctx, epochA) + eb, _, _ := inv.EpochOf(ctx, epochB) + if ea.How != inventory.EpochReleased || eb.Ended != nil || eb.Instance != instance { + t.Fatalf("the record of the handover reads %+v then %+v", ea, eb) + } + + // Something else writes the lease's key — a third controller on a clock that read it as expired: + // B's next renewal is refused, and B stops acting at once. + if _, err := kv.Put(ctx, lease.Key, []byte(`{"instance":"a third controller","epoch":1}`)); err != nil { + t.Fatal(err) + } + select { + case <-lostB: + case <-time.After(2 * lease.RenewEvery): + t.Fatal("B was not told it lost the lease") + } + if _, err := b.epoch(ctx); !errors.Is(err, lease.ErrNotHeld) { + t.Fatalf("B acts after losing the lease: %v", err) + } + // Nothing B does is written: a declaration's number is not taken, a plan is not saved, a condition + // is not raised. + inv.ActsUnder(b.epoch) + if _, err := inv.AddNode(ctx, "anchor"); err != nil { + t.Fatal(err) + } + saved := inventory.Plan{ID: "plan-after-loss", Repository: "novox/app", Commit: "c0ffee00", Created: time.Now(), + State: inventory.PlanBuilding} + if err := inv.SavePlan(ctx, &saved); err == nil { + t.Fatal("B wrote a plan after losing the lease") + } + store := conditions.NewInMemory() + keeper := conditions.NewKeeper(ctx, conditions.Options{Store: store, History: store, + Epoch: func() (uint64, error) { return b.epoch(ctx) }}) + defer keeper.Close(context.Background()) + if _, err := keeper.Observe(ctx, conditions.Observation{Scope: conditions.ScopeCore, ID: "x", Kind: "x", + Severity: conditions.Warning, Summary: "x", Source: "test"}); err == nil { + t.Fatal("B raised a condition after losing the lease") + } + if ended, _, _ := inv.EpochOf(ctx, epochB); ended.How != inventory.EpochLost { + t.Fatalf("B's epoch does not say it was lost: %+v", ended) + } +} + +// A controller whose bus refuses it the lease's key, with nobody holding it, serves without the lease: +// it acts with no epoch — refused by no node-engine — says so, and takes the lease once it can. +func TestAControllerTheBusRefusesTheLeaseServesUnleasedAndSaysSo(t *testing.T) { + a := &actor{serving: true, unleased: "the bus refused the lease's key"} + if epoch, err := a.epoch(context.Background()); err != nil || epoch != 0 { + t.Fatalf("an unleased controller answers %d, %v: it acts, claiming no epoch", epoch, err) + } + if st := a.standing(); st.Unleased == "" || st.Held { + t.Fatalf("its standing says %+v", st) + } + // One that neither holds nor is unleased — still waiting — acts on nothing. + waiting := &actor{serving: true} + if _, err := waiting.epoch(context.Background()); !errors.Is(err, lease.ErrNotHeld) { + t.Fatalf("a controller waiting for the lease may act: %v", err) + } +} diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index 7a55adc..a7c71ee 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -55,6 +55,9 @@ func run() error { ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() + // Whatever this process holds of the controller's lease is given back as it ends (novox/hq to-be + // 45 §6), so the next controller takes it at once rather than after its age. + defer theLease.release() switch args[0] { case "build": diff --git a/cmd/mesh-controller/plan_retry.go b/cmd/mesh-controller/plan_retry.go index 4bbf851..1e2d1fd 100644 --- a/cmd/mesh-controller/plan_retry.go +++ b/cmd/mesh-controller/plan_retry.go @@ -280,7 +280,7 @@ func retryPlan(ctx context.Context, open *stores, id string) (string, error) { } } resumed(&p, fmt.Sprintf("tier %d retried by hand: %s asked again", p.Tier, strings.Join(failed, ", "))) - if err := inv.SavePlan(ctx, p); err != nil { + if err := inv.SavePlan(ctx, &p); err != nil { return "", err } if p.State != inventory.PlanBuilding { @@ -323,7 +323,7 @@ func joinAPlan(ctx context.Context, open *stores, module string) (bool, string, askModule(ctx, &p, module, byName) s := p.Modules[module] resumed(&p, fmt.Sprintf("tier %d: %s rebuilt by hand", p.Tier, module)) - if err := inv.SavePlan(ctx, p); err != nil { + if err := inv.SavePlan(ctx, &p); err != nil { return false, "", err } if s.State != "asked" { @@ -393,7 +393,7 @@ func retryRollouts(ctx context.Context, open *stores, p *inventory.Plan) (string } p.State = inventory.PlanRolling p.Note = fmt.Sprintf("tier %d retried by hand; sent %s first again", p.Tier, strings.Join(said, "; ")) - if err := open.inventory.SavePlan(ctx, *p); err != nil { + if err := open.inventory.SavePlan(ctx, p); err != nil { return "", err } return fmt.Sprintf("%s retried at tier %d of %d: sent %s first again; the rest follow once it reports it "+ diff --git a/cmd/mesh-controller/plans_one_at_a_time_test.go b/cmd/mesh-controller/plans_one_at_a_time_test.go index d76283b..1eb4ec3 100644 --- a/cmd/mesh-controller/plans_one_at_a_time_test.go +++ b/cmd/mesh-controller/plans_one_at_a_time_test.go @@ -18,7 +18,7 @@ func TestAControllerLeavesThePlansToTheOneHoldingThem(t *testing.T) { plan := inventory.Plan{ID: "plan-213", Repository: "r", Commit: "abc", Created: now, Updated: now, State: inventory.PlanRolling, Tier: 1, Tiers: [][]string{{"app"}}, Modules: map[string]*inventory.PlanModule{"app": {State: "built"}}} - if err := open.inventory.SavePlan(ctx, plan); err != nil { + if err := open.inventory.SavePlan(ctx, &plan); err != nil { t.Fatal(err) } diff --git a/cmd/mesh-controller/probes.go b/cmd/mesh-controller/probes.go index 9146a51..865e539 100644 --- a/cmd/mesh-controller/probes.go +++ b/cmd/mesh-controller/probes.go @@ -14,6 +14,7 @@ import ( "time" "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" "github.com/nats-io/nats.go/micro" "github.com/novox/mesh-host/validate" "golang.org/x/net/dns/dnsmessage" @@ -22,6 +23,7 @@ import ( "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/catalogue" "github.com/novox/mesh-controller/internal/conditions" + "github.com/novox/mesh-controller/internal/lease" "github.com/novox/mesh-controller/internal/link" "github.com/novox/mesh-controller/internal/overlay" ) @@ -61,6 +63,15 @@ func probeDeclarations(ctx context.Context, d *doctor) ([]conditions.Observation if err == nil && gensErr == nil { var declared sendable if declared, err = declarationWith(ctx, open, n.Name, plan, settings, gens, Reading); err == nil { + // With the order it was last sent, so the validator reads the envelope a machine is sent + // — its epoch included (novox/hq to-be 45 §6). + var serr error + if declared.Sequence, serr = open.inventory.Sequence(ctx, n.ID); serr == nil { + declared.Epoch, serr = open.inventory.SentEpoch(ctx, n.ID) + } + if serr != nil { + return nil, serr // the store, not the machine: the probe could not run + } var body []byte if body, err = declared.Body(); err == nil { problems = validate.Declaration(body) @@ -818,3 +829,73 @@ func askSeatTool(ctx context.Context, conn *nats.Conn, seat, verb, node string) // oneLine is a message of several lines said on one, its runs of space made one. func oneLine(s string) string { return strings.Join(strings.Fields(s), " ") } + +// probeLease is D5 (novox/hq to-be 45 §4, §6): exactly one lease holder — the key on the bus names this +// controller at the epoch it acts under, and the mesh's record has that epoch and no other open — and no +// message from a stale epoch refused in the last interval. +func probeLease(ctx context.Context, d *doctor) ([]conditions.Observation, error) { + if d.js == nil { + return nil, errors.New("this controller is not on the bus") + } + st := theLease.standing() + var out []conditions.Observation + split := func(token, summary, said string) { + out = append(out, conditions.Observation{Scope: conditions.ScopeCore, ID: "controller.lease", Token: token, + Machine: d.host, Severity: conditions.Urgent, Summary: summary, Said: said}) + } + if st.Unleased != "" { + split("unheld", "this controller acts without the lease, so nothing keeps another from acting beside it: "+ + st.Unleased, st.Unleased) + return out, nil + } + api, err := jetstream.New(d.js.Conn()) + if err != nil { + return nil, err + } + kv, err := api.KeyValue(ctx, broker.LeaseBucket) + if err != nil { + return nil, fmt.Errorf("the lease bucket cannot be read: %w", err) + } + holder, found, err := lease.Current(ctx, kv) + if err != nil { + return nil, fmt.Errorf("the lease cannot be read: %w", err) + } + switch { + case !found: + split("split", fmt.Sprintf("nobody holds the lease on the bus, and this controller acts as epoch %d", st.Epoch), + "the lease's key is absent") + case holder.Instance != instance || holder.Epoch != st.Epoch: + split("split", fmt.Sprintf("the lease on the bus names %s at epoch %d, and this controller (%s) acts as "+ + "epoch %d: two controllers believe they may act", holder.Instance, holder.Epoch, instance, st.Epoch), + fmt.Sprintf("held by %s, epoch %d", holder.Instance, holder.Epoch)) + } + // The record: one epoch open, this one. + epochs, err := d.open.inventory.EpochsSince(ctx, time.Now().Add(-time.Hour)) + if err != nil { + return nil, fmt.Errorf("the epochs the mesh issued cannot be read: %w", err) + } + var open []string + for _, e := range epochs { + if e.Ended == nil && e.Epoch != st.Epoch { + open = append(open, fmt.Sprintf("epoch %d (%s)", e.Epoch, e.Instance)) + } + } + if len(open) > 0 { + split("open-epochs", fmt.Sprintf("the mesh's record holds %s open beside this controller's epoch %d: a "+ + "holder that neither gave the lease back nor was found expired", strings.Join(open, ", "), st.Epoch), + strings.Join(open, ", ")) + } + // And no message from a stale epoch in the last interval. + for _, w := range link.StaleRefusals.Within(time.Now().Add(-doctorEvery)) { + if w.Epoch <= 0 || uint64(w.Epoch) >= st.Epoch { + continue + } + out = append(out, conditions.Observation{Scope: conditions.ScopeCore, ID: fmt.Sprintf("controller.epoch-%d", w.Epoch), + Token: "stale-epoch", Machine: firstOf(w.Receivers), Severity: conditions.Urgent, + Summary: fmt.Sprintf("%d declaration(s) from epoch %d — older than this controller's %d — reached %s in the "+ + "last %s and were refused: a controller that lost the lease is still sending", w.Count, w.Epoch, st.Epoch, + strings.Join(w.Receivers, ", "), doctorEvery), + Said: fmt.Sprintf("%d refused, the last at %s", w.Count, w.Last.UTC().Format(time.RFC3339))}) + } + return sortedFound(out), nil +} diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 1aa52c1..45a5431 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -63,7 +63,7 @@ func connectLink(ctx context.Context, inv *inventory.Inventory, enroller link.En return link.ConnectNats(js, enroller, listener), nil } -func serve(ctx context.Context) error { +func serve(ctx context.Context) (err error) { // The one process whose log is read over time, so the one that says each change to a node's // unmet seat dependencies once (novox/hq ADR 0207). logUnheldChanges = true @@ -104,6 +104,41 @@ func serve(ctx context.Context) error { // being live is refused, because a mesh half on each is one where a declaration goes out on one // and the report comes back on the other, and every component logs success while it happens. + // **The lease, before anything that acts** (novox/hq to-be 45 §6): asserting the bus's objects is the + // controller's to do, and so is everything after. A controller starting while another holds it waits + // here, said; one that loses it stops: every act's gate closes at once, and ctx ends so the process + // exits and is started again as a candidate. + busAddress, err := broker.BusAddress() + if err != nil { + return err + } + lost, err := theLease.serveUnderTheLease(ctx, inv, busAddress) + if err != nil { + return err + } + // Given back before the store closes, so the epoch is recorded as given back rather than found + // expired by the next holder. + defer theLease.release() + ctx, stopActing := context.WithCancel(ctx) + defer stopActing() + go func() { + select { + case <-lost: + stopActing() + case <-ctx.Done(): + } + }() + defer func() { + select { + case <-lost: + if err == nil { + err = errors.New("the controller lease was lost; this controller stopped acting and exits, to " + + "be started again as a candidate") + } + default: + } + }() + work := link.Enrolment{Inventory: inv, Identity: ident, Broker: known, OnNATS: true} // `status` from a summary kept current here (novox/hq to-be 45 Phase 0): a machine saying @@ -170,6 +205,9 @@ func serve(ctx context.Context) error { givenEvents = bus // Composed now and kept current, before the verb that answers from it is served. go statusFrom.keep(ctx) + // Every call carries the lease's epoch, and its record is written only under the lease (novox/hq + // to-be 45 §6). + link.Calls.UnderLease(func() (uint64, error) { return theLease.epoch(ctx) }) // A call that outlasts its caller's patience is followed by `calls` (novox/hq issue 265). link.Calls.Follow = catalogue.ControllerSeatName + ".calls" // And every call is kept on the bus, so a restart of this process keeps what came of each @@ -178,7 +216,7 @@ func serve(ctx context.Context) error { said := log.New(os.Stdout, "", log.LstdFlags) if keeper, err := link.CallsOnTheBus(ctx, bus.Conn); err != nil { fmt.Printf("calls are kept in memory only, and lost when this controller stops: %v\n", err) - } else if err := link.Calls.Durably(ctx, keeper, controllerProcess(), said); err != nil { + } else if err := link.Calls.Durably(ctx, keeper, instance, said); err != nil { fmt.Printf("calls are kept on the bus from now on; the ones kept before could not be read: %v\n", err) } // What is wrong, kept and said (novox/hq to-be 45 §2): the condition store, the watchdogs of the @@ -257,7 +295,12 @@ func declare(ctx context.Context, args []string) error { // is still what the machine was last told, and status must not read it as current for the one // the mesh would compose. Which builds it carried is recorded as not known (novox/hq issue 259): // the mesh did not compose it, so a push that does not name this machine treats it as held. - if _, err := recordSent(ctx, inv, node, raw, nil); err != nil { + // The epoch it carried, if a person wrote one in, is what the machine heard. + var carried struct { + Epoch uint64 `json:"epoch"` + } + _ = json.Unmarshal(raw, &carried) + if _, err := recordSent(ctx, inv, node, raw, nil, carried.Epoch); err != nil { return err } fmt.Printf("sent %s a signed declaration (%d bytes)\n", node, len(raw)) @@ -586,7 +629,7 @@ type readyNode struct { // // The all-or-nothing rule is kept where it means something — sendTo, which rotates a credential // across two machines that must agree — and dropped here, where it never did. -func composeEach(names []string, allot func(node string) (int64, error), +func composeEach(names []string, allot func(node string) (order, error), compose func(node string) (sendable, error)) ([]readyNode, []string) { var sending []readyNode @@ -600,7 +643,7 @@ func composeEach(names []string, allot func(node string) (int64, error), // took the older content as the newer word: on 2026-10-02 a runtime assigned and applied on // two machines was undone two seconds later by exactly that. Taken here, before the first // read, what was composed earlier is numbered lower whatever order the sends happen in. - seq, err := allot(name) + numbered, err := allot(name) if err != nil { refusals = append(refusals, fmt.Sprintf("%s:\n%v", name, err)) continue @@ -610,7 +653,7 @@ func composeEach(names []string, allot func(node string) (int64, error), refusals = append(refusals, fmt.Sprintf("%s:\n%v", name, err)) continue } - declared.Sequence = seq + declared.Sequence, declared.Epoch = numbered.sequence, numbered.epoch if len(declared.Resources) == 0 { // Sent, not skipped (novox/hq issue 127). A node whose declaration composes to // nothing may have HELD something before — the broker opening a placement gave it, @@ -794,7 +837,7 @@ func (b overTheBus) declare(ctx context.Context, s readyNode, body []byte) (stri } // After it is away, not before. A digest recorded for something that failed to send would make // the machine look current for a declaration it never received. - digest, err := recordSent(ctx, b.open.inventory, s.node, body, s.declared.Builds) + digest, err := recordSent(ctx, b.open.inventory, s.node, body, s.declared.Builds, s.declared.Epoch) if err != nil { return "", err } @@ -947,7 +990,7 @@ func sendToEach(ctx context.Context, open *stores, names []string) ([]string, er var refusals []string for _, name := range names { // Numbered before composing, for the reason composeEach gives (novox/hq issue 204). - seq, err := allot(ctx, inv, name) + numbered, err := allot(ctx, inv, name) if err != nil { refusals = append(refusals, fmt.Sprintf("%s:\n%v", name, err)) continue @@ -963,7 +1006,7 @@ func sendToEach(ctx context.Context, open *stores, names []string) ([]string, er refusals = append(refusals, fmt.Sprintf("%s:\n%v", name, err)) continue } - declared.Sequence = seq + declared.Sequence, declared.Epoch = numbered.sequence, numbered.epoch reportLeftOut(name, declared) sending = append(sending, readyNode{name, declared}) } @@ -1120,6 +1163,11 @@ func wouldSendFrom(ctx context.Context, open *stores, if declared.Sequence, err = open.inventory.Sequence(ctx, n.ID); err != nil { return nil, err } + // And the epoch it was last sent under, for the same reason: a new holder of the lease is not a + // change of the machine (novox/hq to-be 45 §6). + if declared.Epoch, err = open.inventory.SentEpoch(ctx, n.ID); err != nil { + return nil, err + } body, err := declared.Body() if err != nil { return nil, err @@ -1245,17 +1293,46 @@ func seatHolders(ctx context.Context, inv *inventory.Inventory) (map[string]brok // number gives one send the next sequence for its node (novox/hq 04-ISSUES/107). // allotting is allot over one inventory, in the shape composeEach takes. -func allotting(ctx context.Context, inv *inventory.Inventory) func(node string) (int64, error) { - return func(node string) (int64, error) { return allot(ctx, inv, node) } +func allotting(ctx context.Context, inv *inventory.Inventory) func(node string) (order, error) { + return func(node string) (order, error) { return allot(ctx, inv, node) } } -// allot takes the next sequence for a machine — the number its next declaration carries. -func allot(ctx context.Context, inv *inventory.Inventory, node string) (int64, error) { +// epochForActs is the lease's gate as a composition asks it; a variable so a test can act under an epoch +// without a bus. +var epochForActs = func(ctx context.Context) (uint64, error) { return theLease.epoch(ctx) } + +// order is what a declaration carries of its writer's order (link/order.go): its sequence, and the +// epoch of the lease it is composed under — zero for a machine that has not said it reads one. +type order struct { + sequence int64 + epoch uint64 +} + +// allot takes the next sequence for a machine — the number its next declaration carries — under the +// lease: a process that may not act takes none, and composes nothing (novox/hq to-be 45 §6). +func allot(ctx context.Context, inv *inventory.Inventory, node string) (order, error) { + epoch, err := epochForActs(ctx) + if err != nil { + return order{}, fmt.Errorf("nothing was composed for %s: %w", node, err) + } record, err := inv.NodeByName(ctx, node) if err != nil { - return 0, err + return order{}, err } - return inv.NextSequence(ctx, record.ID) + if epoch > 0 { + reads, err := inv.ReadsEpoch(ctx, record.ID) + if err != nil { + return order{}, err + } + if !reads { + epoch = 0 + } + } + seq, err := inv.NextSequence(ctx, record.ID) + if err != nil { + return order{}, err + } + return order{sequence: seq, epoch: epoch}, nil } // recordSent writes down what a machine was just sent, and returns the digest. @@ -1270,7 +1347,7 @@ func allot(ctx context.Context, inv *inventory.Inventory, node string) (int64, e // And the build of each module it carried (novox/hq issue 259, ADR 0221), nil when that is not known: // what tells a machine held back by a policy or a plan from one a push left behind. func recordSent(ctx context.Context, inv *inventory.Inventory, node string, body []byte, - builds map[string]string) (string, error) { + builds map[string]string, epoch uint64) (string, error) { kept, cancel := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Second) defer cancel() record, err := inv.NodeByName(kept, node) @@ -1278,7 +1355,7 @@ func recordSent(ctx context.Context, inv *inventory.Inventory, node string, body return "", err } digest := digestOf(body) - if err := inv.RecordSent(kept, record.ID, digest, builds); err != nil { + if err := inv.RecordSentUnder(kept, record.ID, digest, builds, epoch); err != nil { return "", err } return digest, nil @@ -1304,11 +1381,3 @@ func reportUnheldPushed(w io.Writer, named bool, asked []string, unheld map[stri fmt.Fprintf(w, "%s: %d unmet seat dependenc(ies) — see `status`\n", node, len(lines)) } } - -// controllerProcess names this serving process among controllers: the machine, the process and when -// it started — what a call kept on the bus carries, so the next controller can tell a call this one -// left running from one it is running itself (novox/hq to-be 45 §6). -func controllerProcess() string { - host, _ := os.Hostname() - return fmt.Sprintf("controller@%s pid %d since %s", host, os.Getpid(), time.Now().UTC().Format(time.RFC3339)) -} diff --git a/cmd/mesh-controller/push_test.go b/cmd/mesh-controller/push_test.go index ddaf362..541145c 100644 --- a/cmd/mesh-controller/push_test.go +++ b/cmd/mesh-controller/push_test.go @@ -76,7 +76,7 @@ func TestASkippedMachineIsStillAnError(t *testing.T) { } // numbered is an allotter for tests: one higher per call, as the inventory's is per machine. -func numbered() func(string) (int64, error) { +func numbered() func(string) (order, error) { var n int64 - return func(string) (int64, error) { n++; return n, nil } + return func(string) (order, error) { n++; return order{sequence: n}, nil } } diff --git a/cmd/mesh-controller/queue_test.go b/cmd/mesh-controller/queue_test.go index d78bb67..a157932 100644 --- a/cmd/mesh-controller/queue_test.go +++ b/cmd/mesh-controller/queue_test.go @@ -64,7 +64,7 @@ func TestAFailedPlanIsRetriedAndGoesOnThroughItsLaterTiers(t *testing.T) { Note: "a failed to build in tier 0", Modules: map[string]*inventory.PlanModule{"a": {State: "failed", AskedAt: &before, Build: "build-1", Why: link.KilledByHand}}} - if err := open.inventory.SavePlan(ctx, failed); err != nil { + if err := open.inventory.SavePlan(ctx, &failed); err != nil { t.Fatal(err) } @@ -156,7 +156,7 @@ func TestARebuildJoinsThePlanHoldingTheModule(t *testing.T) { Created: before, State: inventory.PlanFailed, Tiers: [][]string{{"a"}, {"b"}}, Note: "a failed to build in tier 0", Modules: map[string]*inventory.PlanModule{"a": {State: "failed", AskedAt: &before, Build: "build-1", Why: link.CancelledByHand}}} - if err := open.inventory.SavePlan(ctx, failed); err != nil { + if err := open.inventory.SavePlan(ctx, &failed); err != nil { t.Fatal(err) } if err := rebuildCommand(ctx, []string{"a"}); err != nil { @@ -433,7 +433,7 @@ func TestCancelDeletesTheAskAndFailsThePlanThatAskedIt(t *testing.T) { plan := inventory.Plan{ID: "plan-cancel", Repository: "novox/a", Commit: "c0ffee", Created: asked, State: inventory.PlanBuilding, Tiers: [][]string{{"a"}, {"b"}}, Modules: map[string]*inventory.PlanModule{"a": {State: "asked", AskedAt: &asked, Build: id}}} - if err := open.inventory.SavePlan(ctx, plan); err != nil { + if err := open.inventory.SavePlan(ctx, &plan); err != nil { t.Fatal(err) } @@ -631,21 +631,21 @@ func TestAPlanStoppedAtItsFirstMachineIsRetried(t *testing.T) { Note: "a stopped at its first machine in tier 0: laptop refused what it was sent", Modules: map[string]*inventory.PlanModule{"a": {State: "built", BuiltAt: &long, Commit: "c0ffee", First: []string{"laptop"}, FirstAt: &long, Why: "laptop refused what it was sent"}}} - if err := open.inventory.SavePlan(ctx, stopped); err != nil { + if err := open.inventory.SavePlan(ctx, &stopped); err != nil { t.Fatal(err) } // A newer plan holding a refuses it: sending the older build would put it back. newer := inventory.Plan{ID: "plan-newer", Repository: "novox/other", Commit: "d00d", Created: long.Add(time.Hour), State: inventory.PlanDone, Tiers: [][]string{{"a"}}, Modules: map[string]*inventory.PlanModule{}} - if err := open.inventory.SavePlan(ctx, newer); err != nil { + if err := open.inventory.SavePlan(ctx, &newer); err != nil { t.Fatal(err) } if _, err := retryPlan(ctx, open, stopped.ID); err == nil || !strings.Contains(err.Error(), "plan-newer") { t.Fatalf("retried under a newer plan: %v", err) } newer.State = inventory.PlanSuperseded - if err := open.inventory.SavePlan(ctx, newer); err != nil { + if err := open.inventory.SavePlan(ctx, &newer); err != nil { t.Fatal(err) } @@ -683,7 +683,7 @@ func TestAPlanIsAnsweredOnlyByTheBuildItAskedFor(t *testing.T) { plan := inventory.Plan{ID: "plan-own", Repository: "novox/a", Commit: "c0ffee", Created: asked, State: inventory.PlanBuilding, Tiers: [][]string{{"a"}}, Modules: map[string]*inventory.PlanModule{"a": {State: "asked", AskedAt: &asked, Build: "build-own"}}} - if err := open.inventory.SavePlan(ctx, plan); err != nil { + if err := open.inventory.SavePlan(ctx, &plan); err != nil { t.Fatal(err) } planBuilt(ctx, open, "a", "0ldc0mm1t", "", time.Now().UTC(), "build-replay") diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index bc79cf9..ee912d5 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -441,7 +441,7 @@ func planBuilt(ctx context.Context, open *stores, module, commit, failed string, state.BuiltAt = &now state.Commit = commit } - if err := inv.SavePlan(ctx, *p); err != nil { + if err := inv.SavePlan(ctx, p); err != nil { fmt.Printf("%s: cannot keep the plan: %v\n", p.ID, err) continue } @@ -500,7 +500,7 @@ func advanceHeld(ctx context.Context, open *stores) { // Kept in the plan, so `plans` says why it has not moved rather than the log alone; // the state is left as it was and the step is tried again on the next tick. p.Note = "tier " + fmt.Sprint(p.Tier) + ": " + err.Error() + " — tried again" - if err := inv.SavePlan(ctx, *p); err != nil { + if err := inv.SavePlan(ctx, p); err != nil { fmt.Printf("%s: cannot keep the plan: %v\n", p.ID, err) } break @@ -508,7 +508,7 @@ func advanceHeld(ctx context.Context, open *stores) { if p.State == inventory.PlanFailed { sayUnsent(p, rollsOut) } - if err := inv.SavePlan(ctx, *p); err != nil { + if err := inv.SavePlan(ctx, p); err != nil { fmt.Printf("%s: cannot keep the plan: %v\n", p.ID, err) break } @@ -1076,7 +1076,7 @@ func plansCommand(ctx context.Context, args []string) error { u, err := inv.UpgradeOf(ctx, m) return err == nil && u.RollOut }) - if err := inv.SavePlan(ctx, p); err != nil { + if err := inv.SavePlan(ctx, &p); err != nil { return err } fmt.Printf("%s %s at tier %d of %d; what was asked still builds and registers, nothing further is asked\n", diff --git a/cmd/mesh-controller/sendable.go b/cmd/mesh-controller/sendable.go index d4dc0de..70eb1a8 100644 --- a/cmd/mesh-controller/sendable.go +++ b/cmd/mesh-controller/sendable.go @@ -22,6 +22,11 @@ type sendable struct { // under the node's hold just before the body is made (novox/hq 04-ISSUES/107). Zero is not sent // at all, which a host reads as "no order claimed" — the shape of every declaration before this. Sequence int64 + // Epoch is the controller lease's epoch it was composed under (novox/hq to-be 45 §6): a machine that + // heard a later epoch refuses it. Zero is not sent at all — every machine whose node-engine has not + // said it reads one is sent none, because an older node-engine refuses a key it does not know, whole + // (link/order.go, the contract). + Epoch uint64 // Adoption is nil for a converged node, and then the body is byte for byte what it was before // adoption existed: an older host parses the envelope strictly and would refuse the key. Adoption *adoptionEnvelope @@ -70,6 +75,9 @@ func (s sendable) Body() ([]byte, error) { if s.Sequence > 0 { envelope["sequence"] = s.Sequence } + if s.Epoch > 0 { + envelope["epoch"] = s.Epoch + } if len(s.LeftOut) > 0 { envelope["left_out"] = s.LeftOut } diff --git a/cmd/mesh-controller/signals.go b/cmd/mesh-controller/signals.go index 48a3367..e29b00d 100644 --- a/cmd/mesh-controller/signals.go +++ b/cmd/mesh-controller/signals.go @@ -5,7 +5,9 @@ import ( "strings" "time" + "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/conditions" + "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" ) @@ -70,6 +72,8 @@ const ( // staleRefusalsAllowed in staleRefusalsWithin are what S13 lets pass from one writer. staleRefusalsAllowed = 5 staleRefusalsWithin = 5 * time.Minute + // leaseBound is how long the lease may go unrenewed (S12): the key's age. + leaseBound = broker.LeaseTTL ) // callBounds are the verbs that may run longer than callDefault, and how long (S7). @@ -164,13 +168,20 @@ var signalsTable = []signalRow{ return newestOf(f.machines, func(m machineFacts) time.Time { return m.toolsHeard }) }}, {Row: "S12", Signal: "the controller lease renewed", Emitter: "controller", Trigger: "every 5 s", - Bound: "15 s", Kind: "lease-lost", Severity: conditions.Urgent, Phase: 2, - Deferred: "the lease is built in Phase 2 (to-be 45 §6): there is nothing renewed to watch yet, and a " + - "second controller is caught today by its consumers being bound (standingBy)"}, + Bound: "15 s (the key's age); a holder that lost the lease, or stopped renewing and was taken over, and a " + + "lease bucket found raised again from nothing, are said for an hour after; a controller serving without " + + "the lease, for as long as it does", + Kind: "lease-lost", Severity: conditions.Urgent, Phase: 2, + needs: func(f *signalFacts) error { return f.leaseErr }, watch: watchLease, + newest: func(f *signalFacts) time.Time { return f.lease.renewed }}, {Row: "S13", Signal: "stale refusals", Emitter: "every receiver (rule 2)", Trigger: "each refusal", - Bound: "more than 5 from one machine in 5 min", Kind: "stale-writer", Severity: conditions.Warning, Phase: 1, + Bound: "more than 5 from one writer in 5 min: a controller epoch, a controller that claimed none, or a " + + "machine's node-engine whose accounts the controller refused", + Kind: "stale-writer", Severity: conditions.Warning, Phase: 2, needs: func(*signalFacts) error { return nil }, watch: watchStaleRefusals, - newest: func(f *signalFacts) time.Time { return time.Time{} }}, + newest: func(f *signalFacts) time.Time { + return newestOf(f.staleRefusals, func(w link.WriterRefusals) time.Time { return w.Last }) + }}, {Row: "S14", Signal: "facts snapshot exported", Emitter: "controller", Trigger: "daily", Bound: "2 days", Kind: "facts-stale", Severity: conditions.Warning, Phase: 5, Deferred: "the facts snapshot is built in Phase 5 (to-be 45 §9): nothing exports one yet"}, @@ -450,16 +461,105 @@ func watchTools(f *signalFacts) []conditions.Observation { func watchStaleRefusals(f *signalFacts) []conditions.Observation { var out []conditions.Observation - for node, n := range f.staleRefusals { - if n <= staleRefusalsAllowed { + for _, w := range f.staleRefusals { + if w.Count <= staleRefusalsAllowed { continue } - out = append(out, conditions.Observation{Scope: conditions.ScopeMachine, ID: node, Kind: "stale-writer", - Machine: node, Severity: conditions.Warning, - Summary: fmt.Sprintf("%s refused %d declarations in %s as older than the one it holds: a controller "+ - "is sending what it has moved past (which one is said once declarations carry an epoch, Phase 2)", - node, n, staleRefusalsWithin), - Said: fmt.Sprintf("%d stale refusals in %s", n, staleRefusalsWithin)}) + scope, id, machine := conditions.ScopeCore, "controller.unnamed", "" + named := w.Writer + switch { + case w.Epoch > 0: + id = fmt.Sprintf("controller.epoch-%d", w.Epoch) + if e, ok := f.epochs[w.Epoch]; ok { + named = fmt.Sprintf("the controller of epoch %d (%s%s)", w.Epoch, e.Instance, endedWords(e)) + } + case w.Writer == link.WriterNodeEngine(firstOf(w.Receivers)) || strings.HasPrefix(w.Writer, "the node-engine on "): + node := strings.TrimPrefix(w.Writer, "the node-engine on ") + scope, id, machine = conditions.ScopeMachine, node, node + } + if machine == "" && len(w.Receivers) > 0 { + machine = w.Receivers[0] + } + var also []string + for _, r := range w.Receivers { + if r != machine && r != "controller" { + also = append(also, r) + } + } + out = append(out, conditions.Observation{Scope: scope, ID: id, Kind: "stale-writer", Token: "stale-writer", + Machine: machine, Also: also, Severity: conditions.Warning, + Summary: fmt.Sprintf("%s was refused %d time(s) in %s as older than what its receivers hold (%s): a "+ + "writer is sending what the mesh has moved past", named, w.Count, staleRefusalsWithin, + strings.Join(w.Receivers, ", ")), + Said: fmt.Sprintf("%d stale refusals in %s, the last at %s", w.Count, staleRefusalsWithin, + w.Last.UTC().Format(time.RFC3339))}) + } + return out +} + +// firstOf is a list's first, empty for none. +func firstOf(list []string) string { + if len(list) == 0 { + return "" + } + return list[0] +} + +// endedWords is how an epoch ended, for a sentence naming it; nothing while it is held. +func endedWords(e inventory.Epoch) string { + if e.Ended == nil { + return ", still holding the lease" + } + return fmt.Sprintf(", %s at %s", e.How, e.Ended.UTC().Format("15:04:05 MST")) +} + +// watchLease is S12: the lease held and renewed by this controller, and every holder that lost it. +func watchLease(f *signalFacts) []conditions.Observation { + var out []conditions.Observation + l := f.lease + if l.unleased != "" { + out = append(out, conditions.Observation{Scope: conditions.ScopeCore, ID: "controller.lease", Token: "unleased", + Kind: "lease-lost", Machine: f.host, Severity: conditions.Urgent, + Summary: "the controller serves WITHOUT the lease: nothing keeps a second controller from acting " + + "beside it, and its declarations carry no epoch. A bus whose user list is older than this " + + "controller does not grant it the lease's bucket: a push of the machine holding the bus sends " + + "the list that does, and the controller takes the lease within five seconds — " + l.unleased, + Said: l.unleased}) + } else if l.held && !l.renewed.IsZero() && f.now.Sub(l.renewed) > leaseBound { + out = append(out, conditions.Observation{Scope: conditions.ScopeCore, ID: "controller.lease", Token: "late", + Kind: "lease-lost", Machine: f.host, Severity: conditions.Urgent, + Summary: fmt.Sprintf("the controller has not renewed its lease (epoch %d) since %s, past the key's age "+ + "of %s: another controller may take it", l.epoch, l.renewed.UTC().Format(time.RFC3339), leaseBound), + Said: fmt.Sprintf("not renewed for %s", ago(f.now.Sub(l.renewed)))}) + } + if !l.reset.IsZero() && f.now.Sub(l.reset) <= advisoryQuiet { + out = append(out, conditions.Observation{Scope: conditions.ScopeCore, ID: "controller.lease", Token: "reset", + Kind: "lease-lost", Machine: f.host, Severity: conditions.Urgent, + Summary: "the controller lease's bucket was raised again from nothing: " + l.resetSaid, + Said: l.resetSaid}) + } + var lost []string + newest := inventory.Epoch{} + for _, e := range l.ended { + if e.Ended == nil || e.How == inventory.EpochReleased || f.now.Sub(*e.Ended) > advisoryQuiet { + continue + } + lost = append(lost, fmt.Sprintf("epoch %d (%s) %s at %s", e.Epoch, e.Instance, e.How, + e.Ended.UTC().Format("15:04:05 MST"))) + if newest.Ended == nil || e.Ended.After(*newest.Ended) { + newest = e + } + } + if len(lost) > 0 { + how := "lost it: its renewal was refused or could not be made" + if newest.How == inventory.EpochExpired { + how = "stopped renewing it without giving it back, and was taken over" + } + out = append(out, conditions.Observation{Scope: conditions.ScopeCore, ID: "controller.lease", Token: "lost", + Kind: "lease-lost", Machine: newest.Host, Severity: conditions.Urgent, + Summary: fmt.Sprintf("the controller of epoch %d (%s) %s; the controller of epoch %d acts now", newest.Epoch, + newest.Instance, how, l.epoch), + Said: strings.Join(lost, "; ")}) } return out } diff --git a/cmd/mesh-controller/signals_test.go b/cmd/mesh-controller/signals_test.go index c53529e..4a60cc0 100644 --- a/cmd/mesh-controller/signals_test.go +++ b/cmd/mesh-controller/signals_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/novox/mesh-controller/internal/conditions" + "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" ) @@ -31,10 +32,17 @@ func calm(now time.Time) *signalFacts { loop: loopFacts{took: now.Add(-time.Second), pending: 1}, mergesPassed: now.Add(-time.Minute), selfCheck: selfCheckFacts{last: now.Add(-time.Minute), every: 5 * time.Minute}, - lostConsumers: map[string]bool{}, staleRefusals: map[string]int{}, + lostConsumers: map[string]bool{}, epochs: map[int64]inventory.Epoch{}, + lease: leaseFacts{held: true, epoch: 57, renewed: now.Add(-2 * time.Second)}, } } +// refusedBy is n refusals of one writer, the last a moment ago. +func refusedBy(now time.Time, epoch int64, n int) []link.WriterRefusals { + return []link.WriterRefusals{{Writer: link.WriterEpoch(epoch), Epoch: epoch, Count: n, + Receivers: []string{"anchor", "laptop"}, Last: now.Add(-time.Second)}} +} + // suppression is one row's signal held back: inside its bound, and past it. type suppression struct{ inside, past func(f *signalFacts) } @@ -106,9 +114,13 @@ var suppressions = map[string]suppression{ inside: func(f *signalFacts) { f.machines[0].toolsHeard = f.now.Add(-179 * time.Second) }, past: func(f *signalFacts) { f.machines[0].toolsHeard = f.now.Add(-181 * time.Second) }, }, + "S12": { + inside: func(f *signalFacts) { f.lease.renewed = f.now.Add(-15 * time.Second) }, + past: func(f *signalFacts) { f.lease.renewed = f.now.Add(-16 * time.Second) }, + }, "S13": { - inside: func(f *signalFacts) { f.staleRefusals = map[string]int{"anchor": 5} }, - past: func(f *signalFacts) { f.staleRefusals = map[string]int{"anchor": 6} }, + inside: func(f *signalFacts) { f.staleRefusals = refusedBy(f.now, 41, 5) }, + past: func(f *signalFacts) { f.staleRefusals = refusedBy(f.now, 41, 6) }, }, } @@ -273,3 +285,58 @@ func TestAControllerStandingBySaysNothing(t *testing.T) { t.Fatalf("%+v", open) } } + +// **A stale writer is named** (novox/hq to-be 45 §3, S13): by the controller instance that held the epoch +// its refused declarations claimed, and how that epoch ended — the question issue 204 could not answer. +func TestAStaleWriterIsNamedByItsEpoch(t *testing.T) { + now := time.Now() + f := calm(now) + ended := now.Add(-time.Minute) + f.staleRefusals = refusedBy(now, 41, 6) + f.epochs[41] = inventory.Epoch{Epoch: 41, Instance: "controller@anchor pid 7 since 2026-10-06T10:00:00Z", + Host: "anchor", Ended: &ended, How: inventory.EpochExpired} + got := watchStaleRefusals(f) + if len(got) != 1 || got[0].Key() != "core.controller.epoch-41.stale-writer" || + !strings.Contains(got[0].Summary, "pid 7") || !strings.Contains(got[0].Summary, "expired") || + got[0].Machine != "anchor" || !slices.Equal(got[0].Also, []string{"laptop"}) { + t.Fatalf("the stale writer is not named: %+v", got) + } + // An account the controller refused names the machine whose node-engine sent it. + f.staleRefusals = []link.WriterRefusals{{Writer: link.WriterNodeEngine("laptop"), Count: 6, + Receivers: []string{"controller"}, Last: now}} + got = watchStaleRefusals(f) + if len(got) != 1 || got[0].Key() != "machine.laptop.stale-writer" || got[0].Machine != "laptop" { + t.Fatalf("a node-engine sending older accounts is not named: %+v", got) + } +} + +// **The lease lost is said for an hour, and serving without it for as long as it lasts** (S12). +func TestALeaseLostOrMissingIsSaid(t *testing.T) { + now := time.Now() + f := calm(now) + expired := now.Add(-59 * time.Minute) + f.lease.ended = []inventory.Epoch{{Epoch: 41, Instance: "controller@anchor pid 7", Host: "anchor", + Ended: &expired, How: inventory.EpochExpired}} + got := watchLease(f) + if len(got) != 1 || got[0].Key() != "core.controller.lease.lost" || !strings.Contains(got[0].Summary, "epoch 41") || + got[0].Severity != conditions.Urgent { + t.Fatalf("a holder that stopped renewing was not said: %+v", got) + } + long := now.Add(-61 * time.Minute) + f.lease.ended[0].Ended = &long + if got := watchLease(f); len(got) != 0 { + t.Fatalf("a loss an hour old is still said: %+v", got) + } + f.lease.ended[0].How, f.lease.ended[0].Ended = inventory.EpochReleased, &expired + if got := watchLease(f); len(got) != 0 { + t.Fatalf("a lease given back is said as lost: %+v", got) + } + f.lease = leaseFacts{held: true, epoch: 501, renewed: now, reset: now.Add(-time.Minute), resetSaid: "moved past 500"} + if got := watchLease(f); len(got) != 1 || got[0].Key() != "core.controller.lease.reset" { + t.Fatalf("a lease bucket raised again from nothing was not said: %+v", got) + } + f.lease = leaseFacts{unleased: "the bus refused the key"} + if got := watchLease(f); len(got) != 1 || got[0].Key() != "core.controller.lease.unleased" { + t.Fatalf("serving without the lease was not said: %+v", got) + } +} diff --git a/cmd/mesh-controller/source.go b/cmd/mesh-controller/source.go index c5b7be9..c919aba 100644 --- a/cmd/mesh-controller/source.go +++ b/cmd/mesh-controller/source.go @@ -3,6 +3,7 @@ package main import ( "context" "fmt" + "os" "strconv" "strings" @@ -134,19 +135,28 @@ func seatBase(world catalogue.World, seatName string) (string, error) { // seatBases is the clone base of every seat a recipe's context may name, for a build request // (novox/hq ADR 0155). A seat nobody holds is left out rather than refused here: the build may not // name it at all, and if it does the builder refuses with the seat's name. +// +// **What cannot be read is said, not passed over as no seats** (novox/hq to-be 45 Phase 2, the +// empty-on-error lint): the build still goes ahead — one that names no seat needs none — and one that +// does is refused naming it, but the reason is the store, and that is said here where it is known. func seatBases(ctx context.Context) map[string]string { + unread := func(what string, err error) map[string]string { + fmt.Fprintf(os.Stderr, "could not read %s, so a build naming a seat's clone base will be told that "+ + "seat is not held: %v\n", what, err) + return nil + } open, err := openStores(ctx) if err != nil { - return nil + return unread("the mesh's store", err) } defer open.Close() shelf, err := open.inventory.Catalogue(ctx) if err != nil { - return nil + return unread("the catalogue", err) } world, err := theRestOfTheMesh(ctx, open.inventory, shelf, "") if err != nil { - return nil + return unread("who holds which seat", err) } bases := map[string]string{} for _, seatName := range []string{gitSeat} { diff --git a/cmd/mesh-controller/status_summary.go b/cmd/mesh-controller/status_summary.go index b9a1b8a..ff91fe6 100644 --- a/cmd/mesh-controller/status_summary.go +++ b/cmd/mesh-controller/status_summary.go @@ -184,9 +184,16 @@ type nudgingListener struct { } func (l nudgingListener) Heard(ctx context.Context, report link.Report) (bool, error) { - // A declaration refused as older than the one the machine holds is counted (novox/hq to-be 45 S13). - if link.IsStaleRefusal(report.Refused) { - link.StaleRefusals.Refused(report.Node, time.Now()) + // A declaration refused as older than the one the machine holds is counted by its writer — the + // controller epoch it claimed (novox/hq to-be 45 §6, S13) — and so is what the machine's own count + // says it refused beyond the refusals heard. + now := time.Now() + if report.StaleRefusalOf() { + link.StaleRefusals.Refused(link.Refusal{Writer: link.WriterEpoch(report.Epoch), Epoch: report.Epoch, + Receiver: report.Node, At: now}) + } + if report.Ordered() { + link.StaleRefusals.Lifetime(report.Node, report.RefusedOlder, now) } news, err := l.Enrolment.Heard(ctx, report) if news { diff --git a/cmd/mesh-controller/stores.go b/cmd/mesh-controller/stores.go index 1fa2045..64219d9 100644 --- a/cmd/mesh-controller/stores.go +++ b/cmd/mesh-controller/stores.go @@ -94,6 +94,8 @@ func openInventory(ctx context.Context) (*inventory.Inventory, error) { inv.Close() return nil, err } + // A plan is written only under the lease, carrying its epoch (novox/hq to-be 45 §6). + inv.ActsUnder(theLease.epoch) // Load the seat set from the store, so the control plane reads the set as data rather than as // the slice it was compiled with (novox/hq ADR 0122). A store not yet seeded — or one whose // seat table a migration has not reached — returns nothing, and UseSeats leaves the compiled diff --git a/cmd/mesh-controller/supersede_test.go b/cmd/mesh-controller/supersede_test.go index 824aaca..6664d08 100644 --- a/cmd/mesh-controller/supersede_test.go +++ b/cmd/mesh-controller/supersede_test.go @@ -72,7 +72,7 @@ func TestAPersonClosesAStuckPlan(t *testing.T) { stuck := inventory.Plan{ID: "plan-97b1b2b", Repository: "novox/mesh-catalog", Commit: "97b1b2b", Created: time.Now().UTC(), State: inventory.PlanRolling, Tier: 1, Tiers: [][]string{{"a"}, {"b"}}, Modules: map[string]*inventory.PlanModule{"a": {State: "built"}, "b": {}}} - if err := open.inventory.SavePlan(ctx, stuck); err != nil { + if err := open.inventory.SavePlan(ctx, &stuck); err != nil { t.Fatal(err) } if err := plansCommand(ctx, []string{"close", stuck.ID}); err == nil || !strings.Contains(err.Error(), "--why") { diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index 74e433a..267126c 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -383,13 +383,13 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { fmt.Printf(" the last tier depends on itself: %s — built together, in no order\n", strings.Join(plan.Tiers[len(plan.Tiers)-1], ", ")) } - if err := inv.SavePlan(ctx, plan); err != nil { + if err := inv.SavePlan(ctx, &plan); err != nil { return notNow(err) } // Closed after the newer plan is kept, never before: a controller replaced between the two leaves // both open, which the next merge settles, rather than neither. for _, old := range superseded { - if err := inv.SavePlan(ctx, old); err != nil { + if err := inv.SavePlan(ctx, &old); err != nil { return notNow(err) } fmt.Printf(" %s (%s at %s) is %s\n", old.ID, old.Repository, short(old.Commit), old.Note) @@ -411,7 +411,7 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { if err := askTier(ctx, inv, &plan); err != nil { return notNow(err) } - if err := inv.SavePlan(ctx, plan); err != nil { + if err := inv.SavePlan(ctx, &plan); err != nil { return notNow(err) } return nil diff --git a/cmd/mesh-controller/watchdogs.go b/cmd/mesh-controller/watchdogs.go index 0510715..5213b5b 100644 --- a/cmd/mesh-controller/watchdogs.go +++ b/cmd/mesh-controller/watchdogs.go @@ -73,7 +73,25 @@ type signalFacts struct { selfCheck selfCheckFacts - staleRefusals map[string]int + // staleRefusals are the writers refused as older lately, and epochs the mesh's record of each epoch + // they name (novox/hq to-be 45 §6, S13). + staleRefusals []link.WriterRefusals + epochs map[int64]inventory.Epoch + + // lease is this controller's standing to the lease, and the epochs that ended lately (S12). + lease leaseFacts + leaseErr error +} + +type leaseFacts struct { + held bool + epoch uint64 + renewed time.Time + unleased string + ended []inventory.Epoch + // reset is when the lease bucket was found raised again from nothing; resetSaid what of it. + reset time.Time + resetSaid string } type machineFacts struct { @@ -246,12 +264,23 @@ func blindRow(row signalRow, err error) conditions.Observation { func (w *watchdogs) gather(ctx context.Context) *signalFacts { now := time.Now() f := &signalFacts{now: now, started: w.started, toolsHeardFrom: link.ToolsBeats.Started(), calls: link.Calls.Running(), - staleRefusals: link.StaleRefusals.Within(now.Add(-staleRefusalsWithin)), lostConsumers: map[string]bool{}} + staleRefusals: link.StaleRefusals.Within(now.Add(-staleRefusalsWithin)), lostConsumers: map[string]bool{}, + epochs: map[int64]inventory.Epoch{}} if w.doctor != nil { f.selfCheck = selfCheckFacts{last: w.doctor.lastRunEnded(), every: doctorEvery} } inv := w.open.inventory f.host = controlHost(ctx, inv) + f.lease, f.leaseErr = gatherLease(ctx, inv, now) + for _, r := range f.staleRefusals { + if r.Epoch <= 0 { + continue + } + // Named where the record has it; a writer the record cannot name is still said by its epoch. + if e, found, err := inv.EpochOf(ctx, uint64(r.Epoch)); err == nil && found { + f.epochs[r.Epoch] = e + } + } f.machines, f.machinesErr = w.gatherMachines(ctx, inv, now) f.plans, f.plansErr = gatherPlans(ctx, inv, now) f.loop, f.loopErr = w.gatherLoop() @@ -270,6 +299,19 @@ func (w *watchdogs) gather(ctx context.Context) *signalFacts { return f } +// gatherLease is this controller's standing to the lease and the epochs that ended within the hour. +func gatherLease(ctx context.Context, inv *inventory.Inventory, now time.Time) (leaseFacts, error) { + st := theLease.standing() + f := leaseFacts{held: st.Held, epoch: st.Epoch, renewed: st.Renewed, unleased: st.Unleased, reset: st.Reset, + resetSaid: st.ResetSaid} + ended, err := inv.EpochsSince(ctx, now.Add(-advisoryQuiet)) + if err != nil { + return f, fmt.Errorf("the epochs the mesh issued cannot be read: %w", err) + } + f.ended = ended + return f, nil +} + // controlHost is the machine running the controller, as the mesh names it: the one the controller // module is assigned to, or this process's host name where that is not one machine. func controlHost(ctx context.Context, inv *inventory.Inventory) string { diff --git a/go.mod b/go.mod index 590d289..d93a1f7 100644 --- a/go.mod +++ b/go.mod @@ -28,4 +28,4 @@ require ( // committed. Every build (the build agent's `go build`, the Dockerfile) compiles from vendor/ and // fetches nothing; go refuses to build when vendor/ and this file disagree, so a pin moved without // `go mod vendor` fails loudly, at once, everywhere. -replace github.com/novox/mesh-host => git.novox.be/novox/mesh-host v0.0.0-20261006081854-6953b5bafdb2 +replace github.com/novox/mesh-host => git.novox.be/novox/mesh-host v0.0.0-20261006095519-3e80b7ae325e diff --git a/go.sum b/go.sum index 5eaa821..f909fc3 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-host v0.0.0-20261006081854-6953b5bafdb2 h1:b4+F4tTfrPkuJTST+rkwIHpEb4mkVJR9158hr6nnc4g= -git.novox.be/novox/mesh-host v0.0.0-20261006081854-6953b5bafdb2/go.mod h1:VlilMCRZ5yyNXg7SNigNBLr0Gt32jrGw5KSNq5JAVYs= +git.novox.be/novox/mesh-host v0.0.0-20261006095519-3e80b7ae325e h1:g9h4QRaAMg5yaJLwqtb0FoOs23DVGUYpW6qvnQ3oY5A= +git.novox.be/novox/mesh-host v0.0.0-20261006095519-3e80b7ae325e/go.mod h1:VlilMCRZ5yyNXg7SNigNBLr0Gt32jrGw5KSNq5JAVYs= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/internal/broker/controller_buckets.go b/internal/broker/controller_buckets.go index e863767..12850d5 100644 --- a/internal/broker/controller_buckets.go +++ b/internal/broker/controller_buckets.go @@ -31,8 +31,15 @@ var ( HandActsBucket = BucketName(ControllerSeat, "hand-acts") ConditionsBucket = BucketName(ControllerSeat, "conditions") ConditionHistoryBucket = BucketName(ControllerSeat, "condition-history") + // LeaseBucket holds the controller's lease (to-be 45 §6): one key, `holder`, which the instance + // allowed to act writes by compare-and-set and renews; its revision when taken is the epoch. + LeaseBucket = BucketName(ControllerSeat, "lease") ) +// LeaseTTL is how long the lease's key lives unrenewed (to-be 45 §6): fifteen seconds, renewed +// every five. The bucket's age, so the bus forgets a holder that stopped renewing. +const LeaseTTL = 15 * time.Second + // The bounds to-be 45 §6 sets for calls: the last thousand, or fourteen days, whichever is fewer. // A call is two keys — its record, and its answer apart so a listing does not read every answer — // so the stream holds twice as many messages as it keeps calls. @@ -52,12 +59,12 @@ const ( // IsControllerBucket says a bucket is the controller's own, not a module's state nothing declares. func IsControllerBucket(bucket string) bool { return bucket == CallsBucket || bucket == HandActsBucket || bucket == ConditionsBucket || - bucket == ConditionHistoryBucket + bucket == ConditionHistoryBucket || bucket == LeaseBucket } // ControllerBuckets are the controller's own buckets, in the order they are asserted. func ControllerBuckets() []string { - return []string{CallsBucket, HandActsBucket, ConditionsBucket, ConditionHistoryBucket} + return []string{LeaseBucket, CallsBucket, HandActsBucket, ConditionsBucket, ConditionHistoryBucket} } // ControllerBucketsAsserter is what raising the controller's buckets needs of a connection. @@ -74,6 +81,9 @@ func (j *JetStream) EnsureControllerBuckets() error { } ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() + if err := EnsureLeaseBucket(ctx, js); err != nil { + return err + } if _, err := js.CreateOrUpdateKeyValue(ctx, jetstream.KeyValueConfig{ Bucket: CallsBucket, Description: "the calls of the mesh's own verbs and what came of each (novox/hq to-be 45 §6, issue " + @@ -141,3 +151,24 @@ func (j *JetStream) EnsureControllerBuckets() error { } return nil } + +// EnsureLeaseBucket creates the lease's bucket if absent and brings its options to match (to-be 45 +// §6). **Before the lease is taken, by any candidate**: it is the lease's own precondition, and asserting +// a bucket that exists changes nothing. File storage, so its revisions — the epochs — outlive a restart of +// the bus; one raised again from nothing is moved past the highest epoch issued when the lease is taken. +func EnsureLeaseBucket(ctx context.Context, js jetstream.JetStream) error { + if _, err := js.CreateOrUpdateKeyValue(ctx, jetstream.KeyValueConfig{ + Bucket: LeaseBucket, + Description: "the controller's lease (novox/hq to-be 45 §6): the key `holder`, written by compare-and-set " + + "by the one controller instance that may act and renewed every five seconds; its revision when taken " + + "is the epoch every declaration and plan write carries", + History: 1, + TTL: LeaseTTL, + MaxValueSize: 4 << 10, + MaxBytes: 1 << 20, + Storage: jetstream.FileStorage, + }); err != nil { + return fmt.Errorf("asserting bucket %s: %w", LeaseBucket, err) + } + return nil +} diff --git a/internal/broker/nats.go b/internal/broker/nats.go index 00e9703..2738b1e 100644 --- a/internal/broker/nats.go +++ b/internal/broker/nats.go @@ -229,7 +229,12 @@ func PermissionsFor(p Principal) (Permissions, error) { // stream definitions (design 25 §3), so it alone reaches the JetStream API — and it alone // issues memberships (novox/hq ADR 0160), which it publishes into the assignments stream // after each push; refused by the server on 2026-10-01 until this line named them. - pub = []string{"mesh.control.>", "mesh.node.>", "mesh.assignment.>", "$JS.API.>"} + // + // **Not what the machines say** (novox/hq to-be 45 §1): `mesh.control.>` is subscribed, never + // published — a machine's report has one writer, its node-engine, and the controller granted + // that subject would be a second (the writers table refuses it). It was granted from the first + // composition and nothing ever published under it. + pub = []string{"mesh.node.>", "mesh.assignment.>", "$JS.API.>"} // **And where its consumers deliver.** A push consumer delivers on `_DELIVER.`, // and a client bound to it subscribes exactly that; the server refused it for every // principal the first time one bound a consumer (2026-09-28). Each kind below is granted @@ -604,6 +609,11 @@ func PermissionsFor(p Principal) (Permissions, error) { sort.Strings(pub) sort.Strings(sub) + // One writer per piece of state (novox/hq to-be 45 §1): a grant that would make a second is + // refused here, at composition, naming the state and its writer. + if err := CheckWriters(p, pub); err != nil { + return Permissions{}, err + } return Permissions{ Publish: pub, Subscribe: sub, diff --git a/internal/broker/testdata/composed.conf b/internal/broker/testdata/composed.conf index e130cc0..7b9caed 100644 --- a/internal/broker/testdata/composed.conf +++ b/internal/broker/testdata/composed.conf @@ -24,7 +24,7 @@ accounts { jetstream: enabled users = [ { user: "controller", password: "$2a$11$cccccccccccccccccccccc", permissions: { - publish: { allow: ["$JS.ACK.CONTROL.controller.>", "$JS.ACK.EVENTS.controller.>", "$JS.API.>", "$KV.SEAT_MESH_BUILD_MACHINE_cancelled.>", "$KV.SEAT_NODE_BUILD_AGENT_cancelled.>", "$KV.mesh-controller_calls.>", "$KV.mesh-controller_condition-history.>", "$KV.mesh-controller_conditions.>", "$KV.mesh-controller_hand-acts.>", "$SRV.INFO", "_INBOX.enrol.>", "mesh.assignment.>", "mesh.control.>", "mesh.mod.*.tool.>", "mesh.node.>", "mesh.seat.mesh-build-machine.accept.>", "mesh.seat.mesh-build-machine.tool.>", "mesh.seat.mesh-controller.event.applied", "mesh.seat.mesh-controller.event.built-before", "mesh.seat.mesh-controller.event.condition-changed", "mesh.seat.mesh-controller.event.condition-cleared", "mesh.seat.mesh-controller.event.condition-raised", "mesh.seat.mesh-controller.event.doctor-heartbeat", "mesh.seat.mesh-controller.event.refused", "mesh.seat.mesh-controller.event.secret-replaced", "mesh.seat.node-build-agent.accept.>", "mesh.seat.node-build-agent.tool.>", "mesh.seat.node-intrusion-prevention.tool.banned.*"] } + publish: { allow: ["$JS.ACK.CONTROL.controller.>", "$JS.ACK.EVENTS.controller.>", "$JS.API.>", "$KV.SEAT_MESH_BUILD_MACHINE_cancelled.>", "$KV.SEAT_NODE_BUILD_AGENT_cancelled.>", "$KV.mesh-controller_calls.>", "$KV.mesh-controller_condition-history.>", "$KV.mesh-controller_conditions.>", "$KV.mesh-controller_hand-acts.>", "$KV.mesh-controller_lease.>", "$SRV.INFO", "_INBOX.enrol.>", "mesh.assignment.>", "mesh.mod.*.tool.>", "mesh.node.>", "mesh.seat.mesh-build-machine.accept.>", "mesh.seat.mesh-build-machine.tool.>", "mesh.seat.mesh-controller.event.applied", "mesh.seat.mesh-controller.event.built-before", "mesh.seat.mesh-controller.event.condition-changed", "mesh.seat.mesh-controller.event.condition-cleared", "mesh.seat.mesh-controller.event.condition-raised", "mesh.seat.mesh-controller.event.doctor-heartbeat", "mesh.seat.mesh-controller.event.refused", "mesh.seat.mesh-controller.event.secret-replaced", "mesh.seat.node-build-agent.accept.>", "mesh.seat.node-build-agent.tool.>", "mesh.seat.node-intrusion-prevention.tool.banned.*"] } subscribe: { allow: ["$JS.API.>", "$JS.EVENT.ADVISORY.CONSUMER.DELETED.>", "$JS.EVENT.ADVISORY.CONSUMER.MAX_DELIVERIES.>", "$SRV.INFO", "$SRV.INFO.mesh-controller", "$SRV.INFO.mesh-controller.>", "$SRV.PING", "$SRV.PING.mesh-controller", "$SRV.PING.mesh-controller.>", "$SRV.STATS", "$SRV.STATS.mesh-controller", "$SRV.STATS.mesh-controller.>", "_DELIVER.controller", "_DELIVER.controller.>", "_INBOX.controller.>", "mesh.control.>", "mesh.mod.*.event.provisioner.failing", "mesh.mod.*.event.provisioner.recovered", "mesh.mod.gitea.event.pull.merged", "mesh.mod.mesh-catalog.event.catching-up", "mesh.mod.mesh-catalog.event.upgraded", "mesh.seat.mesh-build-machine.event.built", "mesh.seat.mesh-controller.tool.>", "mesh.seat.node-build-agent.event.built"] } allow_responses: { max: 1, ttl: "1m" } } } diff --git a/internal/broker/writers.go b/internal/broker/writers.go new file mode 100644 index 0000000..1638a93 --- /dev/null +++ b/internal/broker/writers.go @@ -0,0 +1,167 @@ +package broker + +import ( + "fmt" + "slices" + "strings" +) + +// The writers table (novox/hq to-be 45 §1, ADR 0227 rule 1), compiled in. +// +// **Every kind of state the core keeps has one writer; anyone else asks it.** The table is the design's, +// row for row, with what each row writes on the bus where it writes there. Two things read it: the +// composition of every principal's grants (PermissionsFor), which **refuses a grant that lets a +// principal publish on a subject another writes** — so a second writer cannot be granted by accident, +// and the composition says which state and whose — and the test beside it, which walks the rows +// against the design's list and the composition of a whole mesh. Changing a writer is a change to +// this table, through a decision. + +// WriterRow is one row of the writers table. +type WriterRow struct { + State string + Writer string + KeptIn string + Others string + // Subjects are the bus subjects a write of this state is a publish to; none for state kept off the + // bus (the controller's store, a module's own) or not built yet. + Subjects []string + // Writes says a principal granted a publish pattern overlapping Subjects is this state's writer — + // given the pattern, because a machine writes its own report and no other's. + Writes func(p Principal, pattern string) bool + // Shared says why more than one principal may publish here; empty for one writer. + Shared string +} + +// isController, and the other writers' tests, are who a row's writer is as a principal. +func isController(p Principal, _ string) bool { return p.Kind == KindController } + +// ownMachine is a node writing a subject whose third token is its own name, and no wildcard there. +func ownMachine(p Principal, pattern string) bool { + tokens := strings.Split(pattern, ".") + return p.Kind == KindNode && len(tokens) > 2 && tokens[2] == p.Node +} + +// holdsSeatOf is a principal holding the seat a `mesh.seat..…` pattern names: its module's own +// principal, or the node tools carrying a module that holds it (ADR 0175, the runtime is its modules). +func holdsSeatOf(p Principal, pattern string) bool { + tokens := strings.Split(pattern, ".") + if len(tokens) < 3 { + return false + } + seat := tokens[2] + holds := func(seats []Seat) bool { + return slices.ContainsFunc(seats, func(s Seat) bool { return s.Name == seat }) + } + switch p.Kind { + case KindModule: + return holds(p.Holds) + case KindNodeTools: + return slices.ContainsFunc(p.Carries, func(d Declared) bool { return holds(d.Holds) }) + } + return false +} + +// ownModule is a module publishing under its own name, or the node tools carrying it. +func ownModule(p Principal, pattern string) bool { + tokens := strings.Split(pattern, ".") + if len(tokens) < 3 { + return false + } + module := tokens[2] + switch p.Kind { + case KindModule: + return p.Module == module + case KindNodeTools: + return slices.ContainsFunc(p.Carries, func(d Declared) bool { return d.Module == module }) + } + return false +} + +// kvOf is a bucket's write subjects. +func kvOf(bucket string) []string { return []string{"$KV." + bucket + ".>"} } + +// WritersTable is to-be 45 §1, in its order. +var WritersTable = []WriterRow{ + {State: "a machine's declaration", Writer: "controller (lease holder)", KeptIn: "the bus, last per subject", + Others: "read", Subjects: []string{"mesh.node.*.declare"}, Writes: isController}, + {State: "a machine's applied state and its report", Writer: "the node-engine's apply queue", + KeptIn: "the machine; the report on the bus", Others: "the reconcile and a delivery enqueue, never apply", + Subjects: []string{"mesh.control.*.report"}, Writes: ownMachine}, + {State: "the controller lease", Writer: "the controller instance holding it", KeptIn: "key-value " + LeaseBucket, + Others: "a candidate waits", Subjects: kvOf(LeaseBucket), Writes: isController}, + {State: "plans and their tiers", Writer: "controller (lease holder), compare-and-set on the plan's revision", + KeptIn: "the controller's store", Others: "read through plans"}, + {State: "conditions", Writer: "controller", KeptIn: "key-value " + ConditionsBucket + " (and its history, " + + ConditionHistoryBucket + ")", Others: "raise or clear only through observations the controller reads", + Subjects: append(kvOf(ConditionsBucket), kvOf(ConditionHistoryBucket)...), Writes: isController}, + {State: "calls and their outcomes", Writer: "controller", KeptIn: "key-value " + CallsBucket, + Others: "read by id", Subjects: kvOf(CallsBucket), Writes: isController}, + {State: "the hand-act log", Writer: "controller, through the verbs that act", KeptIn: "key-value " + HandActsBucket, + Others: "—", Subjects: kvOf(HandActsBucket), Writes: isController}, + {State: "stream definitions and bus permissions", Writer: "controller", KeptIn: "the bus", Others: "—", + // A stream's definition, and a durable consumer's by the API that names it so. Not every + // consumer create: a module watching its own bucket makes and deletes an ordered consumer on the + // bucket's stream (ADR 0201), which defines nothing the mesh keeps. + Subjects: []string{"$JS.API.STREAM.CREATE.>", "$JS.API.STREAM.UPDATE.>", "$JS.API.STREAM.DELETE.>", + "$JS.API.CONSUMER.DURABLE.CREATE.>"}, + Writes: isController}, + {State: "builds and their outcomes", Writer: "the build seat's holder", KeptIn: "its own state", + Others: "the controller asks", + Subjects: []string{"mesh.seat.node-build-agent.event.built", "mesh.seat.mesh-build-machine.event.built"}, + Writes: holdsSeatOf, + Shared: "every machine holding the build seat answers the asks it took; each outcome names its ask"}, + {State: "a merge announced", Writer: "one announcer per forge (the hook, or the poll when the hook is absent — never both)", + KeptIn: "the bus", Others: "—", Subjects: []string{"mesh.mod.*.event.pull.merged"}, Writes: ownModule}, + {State: "a provider's standing", Writer: "the provider", KeptIn: "the provider's events", + Others: "the controller keeps the newest word as a condition", + Subjects: []string{"mesh.mod.*.event.provisioner.failing", "mesh.mod.*.event.provisioner.recovered"}, + Writes: ownModule}, + {State: "the operator-channel's open messages", Writer: "the seat's holder", KeptIn: "its own key-value state", + Others: "—"}, + {State: "the facts snapshot", Writer: "controller", KeptIn: "the artifact store, facts/latest", + Others: "the build seat reads"}, +} + +// CheckWriters refuses a grant that lets a principal publish on a subject the writers table gives +// another writer (to-be 45 §1): which state, whose, and the pattern that would make a second writer. +func CheckWriters(p Principal, publish []string) error { + var problems []string + for _, pattern := range publish { + for _, row := range WritersTable { + if row.Writes == nil { + continue + } + for _, subject := range row.Subjects { + if !SubjectsOverlap(pattern, subject) || row.Writes(p, pattern) { + continue + } + problems = append(problems, fmt.Sprintf("%s may publish %s, which writes %s (%s), whose writer is %s", + p.Username(), pattern, row.State, subject, row.Writer)) + } + } + } + if len(problems) == 0 { + return nil + } + return fmt.Errorf("a second writer would be granted (novox/hq to-be 45 §1, one writer per piece of state):\n %s", + strings.Join(problems, "\n ")) +} + +// SubjectsOverlap says some subject matches both patterns: `*` is one token, `>` one or more to the end. +func SubjectsOverlap(a, b string) bool { + x, y := strings.Split(a, "."), strings.Split(b, ".") + for i := 0; ; i++ { + switch { + case i == len(x) && i == len(y): + return true + case i == len(x) || i == len(y): + return false + case x[i] == ">" || y[i] == ">": + return true + case x[i] == "*" || y[i] == "*" || x[i] == y[i]: + continue + default: + return false + } + } +} diff --git a/internal/broker/writers_test.go b/internal/broker/writers_test.go new file mode 100644 index 0000000..d1e3972 --- /dev/null +++ b/internal/broker/writers_test.go @@ -0,0 +1,143 @@ +package broker + +import ( + "slices" + "strings" + "testing" +) + +// The writers table, checked (novox/hq to-be 45 §1, ADR 0227 rule 1, "how it is checked"): it is the +// design's table row for row; every row that writes on the bus names its writer; a whole mesh composes +// with one writer per piece of state; and a grant that would make a second writer is refused, naming +// the state and whose it is. + +// designRows are the states of to-be 45 §1, in its order. A row added to the design is added here and +// to the table; one dropped from either fails. +var designRows = []string{ + "a machine's declaration", + "a machine's applied state and its report", + "the controller lease", + "plans and their tiers", + "conditions", + "calls and their outcomes", + "the hand-act log", + "stream definitions and bus permissions", + "builds and their outcomes", + "a merge announced", + "a provider's standing", + "the operator-channel's open messages", + "the facts snapshot", +} + +func TestTheWritersTableIsTheDesigns(t *testing.T) { + var states []string + for _, row := range WritersTable { + states = append(states, row.State) + if row.Writer == "" || row.KeptIn == "" || row.Others == "" { + t.Errorf("%q does not say who writes it, where it is kept and what others do", row.State) + } + if len(row.Subjects) > 0 && row.Writes == nil { + t.Errorf("%q is written on the bus and names no writer among the principals", row.State) + } + } + if !slices.Equal(states, designRows) { + t.Fatalf("the writers table is not to-be 45 §1's:\n have %q\n want %q", states, designRows) + } +} + +// A mesh of every kind of principal composes: nobody is granted a second writer's subject. +func TestAWholeMeshComposesWithOneWriterPerState(t *testing.T) { + builder := Seat{Name: "node-build-agent", Scope: "node", Accepts: []string{"build"}, Emits: []string{"built", "started"}} + principals := []Principal{ + {Kind: KindController, PasswordHash: "x"}, + {Kind: KindNode, Node: "one", PasswordHash: "x"}, + {Kind: KindEnrolment, Node: "two", PasswordHash: "x"}, + {Kind: KindPerson, Module: "jochen", Invokes: []string{"*"}, PasswordHash: "x"}, + {Kind: KindModule, Node: "one", Module: "gitea", Emits: []string{"pull.merged"}, PasswordHash: "x"}, + {Kind: KindModule, Node: "one", Module: "postgres", Emits: []string{"provisioner.failing", "provisioner.recovered"}, + PasswordHash: "x"}, + {Kind: KindModule, Node: "one", Module: "build-agent", Holds: []Seat{builder}, PasswordHash: "x"}, + {Kind: KindNodeTools, Node: "one", Module: RuntimeModule, PasswordHash: "x", Carries: []Declared{ + {Module: "gitea", Emits: []string{"pull.merged"}}, + {Module: "build-agent", Holds: []Seat{builder}}}}, + } + for _, p := range principals { + if _, err := PermissionsFor(p); err != nil { + t.Errorf("%s does not compose: %v", p.Username(), err) + } + } + if _, err := ComposeAccounts(principals); err != nil { + t.Fatalf("the mesh does not compose: %v", err) + } +} + +func TestASecondWriterIsRefusedAtComposition(t *testing.T) { + for _, c := range []struct { + name string + p Principal + publish []string + state string + }{ + {"the controller publishing what machines say", Principal{Kind: KindController}, + []string{"mesh.control.>"}, "a machine's applied state and its report"}, + {"a machine publishing another's report", Principal{Kind: KindNode, Node: "one"}, + []string{"mesh.control.two.report"}, "a machine's applied state and its report"}, + {"a machine publishing every machine's", Principal{Kind: KindNode, Node: "one"}, + []string{"mesh.control.*.>"}, "a machine's applied state and its report"}, + {"a module writing the lease", Principal{Kind: KindModule, Module: "shop"}, + []string{"$KV.mesh-controller_lease.>"}, "the controller lease"}, + {"a module sending a declaration", Principal{Kind: KindModule, Module: "shop"}, + []string{"mesh.node.one.declare"}, "a machine's declaration"}, + {"a module defining a stream", Principal{Kind: KindModule, Module: "shop"}, + []string{"$JS.API.>"}, "stream definitions and bus permissions"}, + {"a module announcing another forge's merge", Principal{Kind: KindModule, Module: "shop"}, + []string{"mesh.mod.gitea.event.pull.merged"}, "a merge announced"}, + {"a module saying a build it did not do", Principal{Kind: KindModule, Module: "shop"}, + []string{"mesh.seat.node-build-agent.event.built"}, "builds and their outcomes"}, + } { + t.Run(c.name, func(t *testing.T) { + err := CheckWriters(c.p, c.publish) + if err == nil { + t.Fatalf("%v granted to %s was not refused", c.publish, c.p.Username()) + } + if !strings.Contains(err.Error(), c.state) { + t.Fatalf("the refusal does not name %q: %v", c.state, err) + } + }) + } + // And the writers themselves are not refused. + for _, ok := range []struct { + p Principal + publish []string + }{ + {Principal{Kind: KindNode, Node: "one"}, []string{"mesh.control.one.>"}}, + {Principal{Kind: KindController}, []string{"mesh.node.>", "$JS.API.>", "$KV.mesh-controller_lease.>"}}, + {Principal{Kind: KindModule, Module: "gitea"}, []string{"mesh.mod.gitea.event.pull.merged"}}, + {Principal{Kind: KindModule, Module: "shop"}, []string{"$JS.API.CONSUMER.CREATE.KV_shop_carts.>"}}, + } { + if err := CheckWriters(ok.p, ok.publish); err != nil { + t.Errorf("a writer was refused its own: %v", err) + } + } +} + +func TestSubjectsOverlap(t *testing.T) { + for _, c := range []struct { + a, b string + want bool + }{ + {"mesh.control.>", "mesh.control.*.report", true}, + {"mesh.control.one.>", "mesh.control.*.report", true}, + {"mesh.control.one.alive", "mesh.control.*.report", false}, + {"mesh.control.*", "mesh.control.*.report", false}, + {"$JS.API.>", "$JS.API.STREAM.CREATE.>", true}, + {"$JS.API.CONSUMER.CREATE.KV_x.>", "$JS.API.STREAM.CREATE.>", false}, + {"a.b", "a.b", true}, + {"a.b", "a.b.c", false}, + {"a.>", "a", false}, + } { + if got := SubjectsOverlap(c.a, c.b); got != c.want || SubjectsOverlap(c.b, c.a) != c.want { + t.Errorf("%s ~ %s: %v, want %v", c.a, c.b, got, c.want) + } + } +} diff --git a/internal/conditions/condition.go b/internal/conditions/condition.go index a09301a..81864f9 100644 --- a/internal/conditions/condition.go +++ b/internal/conditions/condition.go @@ -129,9 +129,8 @@ type Condition struct { Resolver string `json:"resolver"` // Silenced is null when no silence is in force: said, not left out, so a reader need not guess. Silenced *Silence `json:"silenced"` - // Epoch is the controller lease epoch that last wrote it. Zero until the lease exists (to-be 45 - // Phase 2): no controller holds an epoch yet, and a number invented here would be one nobody - // could compare. + // Epoch is the controller lease epoch that last wrote it (to-be 45 §6). Zero where it was written + // by a controller serving without the lease, or before the lease existed. Epoch uint64 `json:"epoch"` } diff --git a/internal/conditions/store.go b/internal/conditions/store.go index 77aff2d..4c89690 100644 --- a/internal/conditions/store.go +++ b/internal/conditions/store.go @@ -56,6 +56,7 @@ type Keeper struct { now func() time.Time say func(format string, args ...any) changed func() + epoch func() (uint64, error) mu sync.Mutex // cleared is when each recently cleared condition cleared and how often it had been raised, so @@ -88,6 +89,9 @@ type Options struct { Say func(format string, args ...any) // Changed is told of every transition, at once — for `status`, which leads with what is open. Changed func() + // Epoch is the controller lease this keeper writes under (to-be 45 §6): every write carries its + // epoch, and none is made while it is not held. Nil writes with no epoch (a test, the memory store). + Epoch func() (uint64, error) } // TellFor is how long one transition is offered to the bus before it is said lost. @@ -97,6 +101,7 @@ var TellFor = 10 * time.Minute // that cleared just before this controller started and is raised again now is a reopening. func NewKeeper(ctx context.Context, o Options) *Keeper { k := &Keeper{store: o.Store, history: o.History, teller: o.Teller, now: o.Now, say: o.Say, changed: o.Changed, + epoch: o.Epoch, cleared: map[string]clearing{}, out: make(chan Event, 1024), drained: make(chan struct{})} if k.now == nil { k.now = time.Now @@ -137,6 +142,20 @@ func (k *Keeper) Unsaid() int { return k.unsaid } +// stamp is the gate every write passes: the epoch it carries, set on the condition, or why this +// keeper may not write — a controller that lost the lease raises, observes and clears nothing. +func (k *Keeper) stamp(c *Condition) error { + if k.epoch == nil { + return nil + } + epoch, err := k.epoch() + if err != nil { + return fmt.Errorf("the condition %s is not written: %w", c.Key, err) + } + c.Epoch = epoch + return nil +} + // tries bounds one compare-and-set: two writers rarely race more than once. const tries = 8 @@ -174,6 +193,9 @@ func (k *Keeper) Observe(ctx context.Context, o Observation) (Condition, error) } } k.mu.Unlock() + if err := k.stamp(&c); err != nil { + return Condition{}, err + } body, err := json.Marshal(c) if err != nil { return Condition{}, err @@ -214,6 +236,9 @@ func (k *Keeper) Observe(ctx context.Context, o Observation) (Condition, error) if len(c.Evidence) > KeptEvidence { c.Evidence = c.Evidence[:KeptEvidence] } + if err := k.stamp(&c); err != nil { + return Condition{}, err + } body, err := json.Marshal(c) if err != nil { return Condition{}, err @@ -247,6 +272,9 @@ func (k *Keeper) Clear(ctx context.Context, key, why string) (bool, error) { // Unreadable is not resolved: kept, and said, rather than removed unread. return false, fmt.Errorf("the condition %s on the bus cannot be read, so it is not cleared: %w", key, err) } + if err := k.stamp(&c); err != nil { + return false, err + } if err := k.store.Delete(ctx, key, entry.Revision); errors.Is(err, ErrMoved) { continue } else if err != nil { @@ -318,6 +346,9 @@ func (k *Keeper) Silence(ctx context.Context, key string, d time.Duration, by, w } now := k.now().UTC() c.Silenced = &Silence{Until: now.Add(d), By: by, Why: strings.TrimSpace(why), Since: now} + if err := k.stamp(&c); err != nil { + return Condition{}, err + } body, err := json.Marshal(c) if err != nil { return Condition{}, err @@ -362,6 +393,9 @@ func (k *Keeper) EndSilences(ctx context.Context) error { } was := held.Silenced.Why held.Silenced = nil + if err := k.stamp(&held); err != nil { + return err + } body, err := json.Marshal(held) if err != nil { return err diff --git a/internal/inventory/durations_test.go b/internal/inventory/durations_test.go index c377ac4..f3c8477 100644 --- a/internal/inventory/durations_test.go +++ b/internal/inventory/durations_test.go @@ -68,25 +68,25 @@ func TestAPlansTierIsMeasuredWhenItIsLeft(t *testing.T) { p := Plan{ID: "plan-1", Repository: "novox/mesh-tools", Commit: "abc", Created: time.Now().UTC(), State: PlanBuilding, Tiers: [][]string{{"mesh-tools"}, {"builder"}}, Modules: map[string]*PlanModule{"mesh-tools": {}, "builder": {}}} - if err := inv.SavePlan(ctx, p); err != nil { + if err := inv.SavePlan(ctx, &p); err != nil { t.Fatal(err) } p.State = PlanRolling // the same tier, saved again - if err := inv.SavePlan(ctx, p); err != nil { + if err := inv.SavePlan(ctx, &p); err != nil { t.Fatal(err) } if ds, _ := inv.Durations(ctx, DurationPlanTier, time.Now().Add(-time.Hour)); len(ds) != 0 { t.Fatalf("a tier not left was measured: %+v", ds) } p.Tier = 1 - if err := inv.SavePlan(ctx, p); err != nil { + if err := inv.SavePlan(ctx, &p); err != nil { t.Fatal(err) } p.State = PlanDone - if err := inv.SavePlan(ctx, p); err != nil { + if err := inv.SavePlan(ctx, &p); err != nil { t.Fatal(err) } - if err := inv.SavePlan(ctx, p); err != nil { // saved again once ended: nothing more to measure + if err := inv.SavePlan(ctx, &p); err != nil { // saved again once ended: nothing more to measure t.Fatal(err) } ds, err := inv.Durations(ctx, DurationPlanTier, time.Now().Add(-time.Hour)) diff --git a/internal/inventory/migrations/0068-order-and-one-writer.sql b/internal/inventory/migrations/0068-order-and-one-writer.sql new file mode 100644 index 0000000..9f39565 --- /dev/null +++ b/internal/inventory/migrations/0068-order-and-one-writer.sql @@ -0,0 +1,40 @@ +-- Order and one writer (novox/hq to-be 45 §6, Phase 2). +-- +-- The controller acts only while it holds its lease, and every act carries the lease's epoch: a +-- declaration, a plan write. A report carries the order of the declaration it accounts for, and the +-- controller keeps the highest per machine, refusing an older account rather than letting the last +-- one written win (issue 267). These are the records each of those needs. + +-- Every epoch the mesh issued: which controller instance held it, from when, and how it ended — +-- given back, or lost (its key expired, or a renewal was refused). The highest one is the floor no +-- new epoch may be at or under, even when the lease's bucket was raised again from nothing; and a +-- stale refusal names its writer from here. +create table controller_epoch ( + epoch bigint primary key, + instance text not null, + host text not null default '', + build text not null default '', + taken timestamptz not null default now(), + ended timestamptz, + -- how: 'released' (given back), 'lost' (its own renewal was refused or failed), 'expired' + -- (found expired by the next holder: it stopped renewing without saying so). + how text not null default '' +); + +-- The epoch a machine was last sent, so what the mesh WOULD send is composed with it and reads as +-- byte for byte what it DID send when nothing else changed — a new holder's epoch is not a change +-- of the machine. Null for a declaration sent without one. +alter table node add column sent_epoch bigint; +-- Whether the machine's node-engine said it reads an epoch in a declaration (its report's `reads`): +-- until it has, it is sent none, because an older node-engine refuses a key it does not know, whole. +alter table node add column reads_epoch boolean not null default false; + +-- The order of the account kept for each machine: the declaration's sequence and epoch it is about +-- and the node-engine's own report sequence. Null where the kept account carried none. +alter table node_report add column reported_sequence bigint; +alter table node_report add column reported_epoch bigint; +alter table node_report add column report_sequence bigint; + +-- A plan is written by compare-and-set on its revision, and says the epoch that wrote it last. +alter table release_plan add column revision bigint not null default 0; +alter table release_plan add column epoch bigint; diff --git a/internal/inventory/nodes.go b/internal/inventory/nodes.go index e35990a..11e8328 100644 --- a/internal/inventory/nodes.go +++ b/internal/inventory/nodes.go @@ -14,12 +14,17 @@ import ( "time" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/store" ) // Inventory is this context, holding the store it exclusively owns. -type Inventory struct{ store *store.Store } +type Inventory struct { + store *store.Store + // acting is the gate a write that acts passes (ActsUnder, novox/hq to-be 45 §6); nil passes all. + acting func(ctx context.Context) (uint64, error) +} // Open connects to the inventory store. func Open(ctx context.Context) (*Inventory, error) { @@ -763,14 +768,35 @@ func sameFailure(a, b Doing) bool { // ADR 0134). A machine reconciles continuously and reports each time; the same outcome about the same // declaration is the same state said again, and a fact per report would be a fact per minute per // machine that tells nobody anything. Read here because the previous row is read here anyway. +// +// **An account that claims no order clears the order kept** (novox/hq to-be 45 §6): the account kept is +// then one the next ordered report has nothing to compare against, and it is taken as it was before +// reports carried an order. RecordOrderedDoing keeps an ordered one. func (i *Inventory) RecordDoing(ctx context.Context, node string, d Doing) (news bool, err error) { + news, err = recordDoing(ctx, i.store.Pool(), node, d) + if err != nil { + return false, err + } + _, err = i.store.Pool().Exec(ctx, + `update node_report set reported_sequence = null, reported_epoch = null, report_sequence = null + where node = $1`, node) + return news, err +} + +// queries is what recordDoing writes through: the pool, or a transaction an ordered account holds. +type queries interface { + QueryRow(ctx context.Context, sql string, args ...any) pgx.Row + Exec(ctx context.Context, sql string, args ...any) (pgconn.CommandTag, error) +} + +func recordDoing(ctx context.Context, q queries, node string, d Doing) (news bool, err error) { failed, err := json.Marshal(d.Failed) if err != nil { return false, err } var before Doing var beforeFailed []byte - found := i.store.Pool().QueryRow(ctx, + found := q.QueryRow(ctx, `select outcome, refused, failed, failing_since, failures, coalesce(declared,'') from node_report where node = $1`, node).Scan(&before.Outcome, &before.Refused, &beforeFailed, &before.Since, &before.Times, @@ -795,7 +821,7 @@ func (i *Inventory) RecordDoing(ctx context.Context, node string, d Doing) (news since, times = before.Since, before.Times+1 } } - _, err = i.store.Pool().Exec(ctx, + _, err = q.Exec(ctx, `insert into node_report (node, outcome, refused, failed, applied, at, declared, failing_since, failures) values ($1, $2, $3, $4, $5, now(), $6, $7, $8) @@ -923,6 +949,18 @@ func (i *Inventory) LastReports(ctx context.Context) ([]Reported, error) { // is a push's to send to a machine it did not name. Nil records that it is not known, as for a // declaration sent by hand. func (i *Inventory) RecordSent(ctx context.Context, node, digest string, builds map[string]string) error { + return i.RecordSentUnder(ctx, node, digest, builds, 0) +} + +// RecordSentUnder is RecordSent for a declaration that carried an epoch (novox/hq to-be 45 §6): kept +// beside the digest, so what the mesh would send is composed with it. Zero is one that carried none. +func (i *Inventory) RecordSentUnder(ctx context.Context, node, digest string, builds map[string]string, + epoch uint64) error { + var sentEpoch *int64 + if epoch > 0 { + e := int64(epoch) + sentEpoch = &e + } var carried *string if builds != nil { raw, err := json.Marshal(builds) @@ -933,7 +971,8 @@ func (i *Inventory) RecordSent(ctx context.Context, node, digest string, builds carried = &text } _, err := i.store.Pool().Exec(ctx, - `update node set sent = $2, sent_at = now(), sent_builds = $3::jsonb where id = $1`, node, digest, carried) + `update node set sent = $2, sent_at = now(), sent_builds = $3::jsonb, sent_epoch = $4 where id = $1`, + node, digest, carried, sentEpoch) return err } diff --git a/internal/inventory/order.go b/internal/inventory/order.go new file mode 100644 index 0000000..33834b3 --- /dev/null +++ b/internal/inventory/order.go @@ -0,0 +1,250 @@ +package inventory + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/jackc/pgx/v5" +) + +// Order and one writer, as this context keeps them (novox/hq to-be 45 §6, Phase 2): the epochs the +// mesh issued, the epoch each machine was sent, whether its node-engine reads one, and the order of +// the account kept for it. + +// ActsUnder gives this inventory the gate every write that acts passes (a plan's): the epoch of the +// controller lease this process acts under, or why it may not act. Nil — a test, a command reading — +// writes with no epoch and refuses nothing. +func (i *Inventory) ActsUnder(epoch func(ctx context.Context) (uint64, error)) { i.acting = epoch } + +// actingEpoch is the epoch a write carries, nil when there is no gate or it claims none. +func (i *Inventory) actingEpoch(ctx context.Context) (*int64, error) { + if i.acting == nil { + return nil, nil + } + epoch, err := i.acting(ctx) + if err != nil { + return nil, err + } + if epoch == 0 { + return nil, nil + } + e := int64(epoch) + return &e, nil +} + +// Epoch is one epoch the mesh issued. +type Epoch struct { + Epoch uint64 + Instance string + Host string + Build string + Taken time.Time + // Ended is when it ended, nil while it is held; How is how: EpochReleased, EpochLost, EpochExpired. + Ended *time.Time + How string +} + +// How an epoch ends. +const ( + // EpochReleased is a holder that gave the lease back. + EpochReleased = "released" + // EpochLost is a holder whose own renewal was refused or failed, and which said so. + EpochLost = "lost" + // EpochExpired is a holder the next one found gone: it stopped renewing without saying anything. + EpochExpired = "expired" +) + +// HighestEpoch is the highest epoch the mesh has issued, zero before the first: the floor no new one +// may be at or under. +func (i *Inventory) HighestEpoch(ctx context.Context) (uint64, error) { + var highest int64 + if err := i.store.Pool().QueryRow(ctx, `select coalesce(max(epoch), 0) from controller_epoch`).Scan(&highest); err != nil { + return 0, fmt.Errorf("reading the highest epoch issued: %w", err) + } + return uint64(highest), nil +} + +// TookEpoch records an epoch taken, and ends every earlier one still open as found expired: the lease +// was free to take, so whoever held it last neither holds it nor said it let go. Answers the epochs it +// ended that way, newest first. +func (i *Inventory) TookEpoch(ctx context.Context, e Epoch) ([]Epoch, error) { + tx, err := i.store.Pool().Begin(ctx) + if err != nil { + return nil, err + } + defer func() { _ = tx.Rollback(ctx) }() + rows, err := tx.Query(ctx, + `update controller_epoch set ended = now(), how = $2 + where ended is null and epoch < $1 + returning epoch, instance, host, build, taken, ended, how`, int64(e.Epoch), EpochExpired) + if err != nil { + return nil, err + } + expired, err := scanEpochs(rows) + if err != nil { + return nil, err + } + if _, err := tx.Exec(ctx, + `insert into controller_epoch (epoch, instance, host, build, taken) values ($1, $2, $3, $4, $5) + on conflict (epoch) do nothing`, + int64(e.Epoch), e.Instance, e.Host, e.Build, e.Taken); err != nil { + return nil, err + } + return expired, tx.Commit(ctx) +} + +// EndEpoch records how an epoch ended. One already ended keeps its first word. +func (i *Inventory) EndEpoch(ctx context.Context, epoch uint64, how string) error { + _, err := i.store.Pool().Exec(ctx, + `update controller_epoch set ended = now(), how = $2 where epoch = $1 and ended is null`, int64(epoch), how) + return err +} + +// EpochOf is the epoch issued at a number; false when the mesh issued none there. +func (i *Inventory) EpochOf(ctx context.Context, epoch uint64) (Epoch, bool, error) { + rows, err := i.store.Pool().Query(ctx, + `select epoch, instance, host, build, taken, ended, how from controller_epoch where epoch = $1`, int64(epoch)) + if err != nil { + return Epoch{}, false, err + } + found, err := scanEpochs(rows) + if err != nil || len(found) == 0 { + return Epoch{}, false, err + } + return found[0], true, nil +} + +// EpochsSince is every epoch taken or ended since a moment, newest first. +func (i *Inventory) EpochsSince(ctx context.Context, since time.Time) ([]Epoch, error) { + rows, err := i.store.Pool().Query(ctx, + `select epoch, instance, host, build, taken, ended, how from controller_epoch + where taken >= $1 or ended >= $1 or ended is null order by epoch desc`, since) + if err != nil { + return nil, err + } + return scanEpochs(rows) +} + +func scanEpochs(rows pgx.Rows) ([]Epoch, error) { + defer rows.Close() + var out []Epoch + for rows.Next() { + var e Epoch + var epoch int64 + if err := rows.Scan(&epoch, &e.Instance, &e.Host, &e.Build, &e.Taken, &e.Ended, &e.How); err != nil { + return nil, err + } + e.Epoch = uint64(epoch) + out = append(out, e) + } + return out, rows.Err() +} + +// SentEpoch is the epoch a machine was last sent, by its id; zero for one sent without. +func (i *Inventory) SentEpoch(ctx context.Context, id string) (uint64, error) { + var epoch *int64 + if err := i.store.Pool().QueryRow(ctx, `select sent_epoch from node where id = $1`, id).Scan(&epoch); err != nil { + return 0, fmt.Errorf("reading the epoch %s was last sent: %w", id, err) + } + if epoch == nil { + return 0, nil + } + return uint64(*epoch), nil +} + +// ReadsEpoch says a machine's node-engine said it reads an epoch in a declaration, by its id. +func (i *Inventory) ReadsEpoch(ctx context.Context, id string) (bool, error) { + var reads bool + if err := i.store.Pool().QueryRow(ctx, `select reads_epoch from node where id = $1`, id).Scan(&reads); err != nil { + return false, fmt.Errorf("reading whether %s reads an epoch: %w", id, err) + } + return reads, nil +} + +// RecordReadsEpoch keeps what a machine's latest report said of reading an epoch. Every report of a +// node-engine that orders its reports says it, so a node-engine rolled back says it no longer does. +func (i *Inventory) RecordReadsEpoch(ctx context.Context, id string, reads bool) error { + _, err := i.store.Pool().Exec(ctx, `update node set reads_epoch = $2 where id = $1`, id, reads) + return err +} + +// ReportOrder is the order of an account: the declaration's epoch and sequence it is about, and the +// node-engine's own report sequence. Zero in any claims none. +type ReportOrder struct { + Epoch int64 + Sequence int64 + ReportSequence int64 +} + +// ErrOlderAccount is an account refused because the one kept is newer. +var ErrOlderAccount = errors.New("an older account than the one kept") + +// KeptOrder is the order of the account kept for a machine, by its id; zero when it carried none. +func (i *Inventory) KeptOrder(ctx context.Context, id string) (ReportOrder, error) { + o, err := keptOrder(ctx, i.store.Pool(), id, "") + if errors.Is(err, pgx.ErrNoRows) { + return ReportOrder{}, nil + } + return o, err +} + +func keptOrder(ctx context.Context, q queries, id, lock string) (ReportOrder, error) { + var epoch, seq, rseq *int64 + err := q.QueryRow(ctx, + `select reported_epoch, reported_sequence, report_sequence from node_report where node = $1`+lock, id). + Scan(&epoch, &seq, &rseq) + if err != nil { + return ReportOrder{}, err + } + var o ReportOrder + for _, f := range []struct { + from *int64 + to *int64 + }{{epoch, &o.Epoch}, {seq, &o.Sequence}, {rseq, &o.ReportSequence}} { + if f.from != nil { + *f.to = *f.from + } + } + return o, nil +} + +// RecordOrderedDoing is RecordDoing for an account that carries its order (novox/hq to-be 45 §6): +// written only when it is not older than the account kept, as olderThan judges — the rule is the +// link's (link.Account), stated once — and in one transaction with the row locked, so two accounts +// arriving together cannot both win. ErrOlderAccount when it is older; nothing is written then. +func (i *Inventory) RecordOrderedDoing(ctx context.Context, id string, d Doing, o ReportOrder, + olderThan func(kept ReportOrder) bool) (news bool, err error) { + tx, err := i.store.Pool().Begin(ctx) + if err != nil { + return false, err + } + defer func() { _ = tx.Rollback(ctx) }() + kept, err := keptOrder(ctx, tx, id, " for update") + switch { + case errors.Is(err, pgx.ErrNoRows): + case err != nil: + return false, err + case olderThan(kept): + return false, ErrOlderAccount + } + news, err = recordDoing(ctx, tx, id, d) + if err != nil { + return false, err + } + if _, err := tx.Exec(ctx, + `update node_report set reported_epoch = $2, reported_sequence = $3, report_sequence = $4 where node = $1`, + id, nullIfZero(o.Epoch), nullIfZero(o.Sequence), nullIfZero(o.ReportSequence)); err != nil { + return false, err + } + return news, tx.Commit(ctx) +} + +// nullIfZero is a claimed order or none. +func nullIfZero(n int64) *int64 { + if n == 0 { + return nil + } + return &n +} diff --git a/internal/inventory/order_test.go b/internal/inventory/order_test.go new file mode 100644 index 0000000..bd09d19 --- /dev/null +++ b/internal/inventory/order_test.go @@ -0,0 +1,126 @@ +package inventory + +import ( + "context" + "errors" + "testing" + "time" +) + +// Order and one writer, as the store keeps them (novox/hq to-be 45 §6). + +// **A plan is written by compare-and-set on its revision, carrying the epoch**: a write against a plan +// another writer moved since is refused, and nothing is written by a process that may not act. +func TestAPlanIsWrittenByCompareAndSetUnderTheLease(t *testing.T) { + inv := ForTest(t) + ctx := context.Background() + epoch := uint64(57) + inv.ActsUnder(func(context.Context) (uint64, error) { return epoch, nil }) + + p := Plan{ID: "plan-cas", Repository: "novox/app", Commit: "c0ffee00", Created: time.Now(), State: PlanBuilding, + Tiers: [][]string{{"app"}}, Modules: map[string]*PlanModule{"app": {}}} + if err := inv.SavePlan(ctx, &p); err != nil { + t.Fatal(err) + } + if p.Revision != 1 || p.Epoch != 57 { + t.Fatalf("a new plan was written at revision %d, epoch %d", p.Revision, p.Epoch) + } + // Two readers of revision 1: the first write wins, the second is refused and writes nothing. + first, err := inv.PlanByID(ctx, "plan-cas") + if err != nil { + t.Fatal(err) + } + second := first + first.Note = "the newer word" + if err := inv.SavePlan(ctx, &first); err != nil { + t.Fatal(err) + } + second.State = PlanFailed + if err := inv.SavePlan(ctx, &second); !errors.Is(err, ErrPlanMoved) { + t.Fatalf("a write against a plan moved since it was read was not refused: %v", err) + } + kept, _ := inv.PlanByID(ctx, "plan-cas") + if kept.State != PlanBuilding || kept.Note != "the newer word" || kept.Revision != 2 { + t.Fatalf("the refused write reached the plan: %+v", kept) + } + // The writer saves its own plan again without reading it: its revision moved with its write. + first.Tier = 0 + if err := inv.SavePlan(ctx, &first); err != nil { + t.Fatalf("a writer could not save its own plan twice: %v", err) + } + // A new plan under an id already written is refused, not laid over it. + again := Plan{ID: "plan-cas", Repository: "novox/app", Commit: "deadbeef", Created: time.Now(), State: PlanBuilding} + if err := inv.SavePlan(ctx, &again); !errors.Is(err, ErrPlanMoved) { + t.Fatalf("a second plan under one id was written: %v", err) + } + // And a process that does not hold the lease writes nothing. + inv.ActsUnder(func(context.Context) (uint64, error) { return 0, errors.New("this controller lost the lease") }) + first.Note = "from a controller that lost the lease" + if err := inv.SavePlan(ctx, &first); err == nil { + t.Fatal("a plan was written by a controller that does not hold the lease") + } + if kept, _ := inv.PlanByID(ctx, "plan-cas"); kept.Note != "the newer word" || kept.Epoch != 57 { + t.Fatalf("a controller without the lease wrote the plan: %+v", kept) + } +} + +// The epochs the mesh issued: the highest is the floor; taking one ends every earlier one nobody gave +// back as found expired, and says which; one given back says so and is not said again. +func TestTheEpochsIssuedAreKept(t *testing.T) { + inv := ForTest(t) + ctx := context.Background() + if highest, err := inv.HighestEpoch(ctx); err != nil || highest != 0 { + t.Fatalf("a mesh that issued none has %d (%v)", highest, err) + } + if _, err := inv.TookEpoch(ctx, Epoch{Epoch: 41, Instance: "a", Taken: time.Now()}); err != nil { + t.Fatal(err) + } + // a stopped renewing and said nothing; b takes the lease. + expired, err := inv.TookEpoch(ctx, Epoch{Epoch: 57, Instance: "b", Taken: time.Now()}) + if err != nil || len(expired) != 1 || expired[0].Epoch != 41 || expired[0].How != EpochExpired { + t.Fatalf("taking 57 ended %+v (%v), want 41 found expired", expired, err) + } + if err := inv.EndEpoch(ctx, 57, EpochReleased); err != nil { + t.Fatal(err) + } + expired, err = inv.TookEpoch(ctx, Epoch{Epoch: 60, Instance: "c", Taken: time.Now()}) + if err != nil || len(expired) != 0 { + t.Fatalf("a lease given back was found expired: %+v (%v)", expired, err) + } + if highest, _ := inv.HighestEpoch(ctx); highest != 60 { + t.Fatalf("the highest epoch issued is %d, want 60", highest) + } + e, found, err := inv.EpochOf(ctx, 57) + if err != nil || !found || e.Instance != "b" || e.How != EpochReleased || e.Ended == nil { + t.Fatalf("epoch 57 reads %+v (%v, %v)", e, found, err) + } + recent, err := inv.EpochsSince(ctx, time.Now().Add(-time.Hour)) + if err != nil || len(recent) != 3 || recent[0].Epoch != 60 || recent[0].Ended != nil { + t.Fatalf("the epochs of the last hour read %+v (%v)", recent, err) + } +} + +// What a machine was sent under, and whether it reads an epoch. +func TestTheEpochAMachineWasSentIsKept(t *testing.T) { + inv := ForTest(t) + ctx := context.Background() + node, err := inv.AddNode(ctx, "anchor") + if err != nil { + t.Fatal(err) + } + if err := inv.RecordSentUnder(ctx, node.ID, "d1", nil, 57); err != nil { + t.Fatal(err) + } + if e, err := inv.SentEpoch(ctx, node.ID); err != nil || e != 57 { + t.Fatalf("sent under %d (%v)", e, err) + } + if err := inv.RecordSent(ctx, node.ID, "d2", nil); err != nil { + t.Fatal(err) + } + if e, _ := inv.SentEpoch(ctx, node.ID); e != 0 { + t.Fatalf("a declaration sent without an epoch left %d kept", e) + } + if reads, _ := inv.ReadsEpoch(ctx, node.ID); reads { + t.Fatal("a machine that never said so reads an epoch") + } +} diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index 0d63308..1a1261d 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -32,8 +32,17 @@ type Plan struct { // never written from here: a save measures the tier it leaves and stamps the next. What the // watchdog of a plan's progress (S3) reads. TierEntered time.Time `json:"tier_entered,omitempty"` + // Revision is the plan's as it was read, and the one a save must find (novox/hq to-be 45 §6): a + // plan is written by compare-and-set, so a write against a plan another writer moved since is + // refused rather than laid over it. Zero is a plan never saved. SavePlan moves it. + Revision int64 `json:"revision"` + // Epoch is the controller lease epoch that wrote it last; zero for a write that claimed none. + Epoch uint64 `json:"epoch,omitempty"` } +// ErrPlanMoved is a save against a plan written by somebody else since it was read. +var ErrPlanMoved = errors.New("the plan was written by somebody else since it was read") + // PlanModule is one module's state within a plan. type PlanModule struct { // State: asked, built, failed; empty for a module whose tier has not been asked yet. @@ -75,7 +84,16 @@ const ( func (p Plan) Open() bool { return p.State == PlanBuilding || p.State == PlanRolling } // SavePlan writes a plan, new or changed, whole: the plan is small and read as one thing. -func (i *Inventory) SavePlan(ctx context.Context, p Plan) error { +// +// **By compare-and-set on its revision, carrying the epoch** (novox/hq to-be 45 §6): written only if +// the plan is still at the revision it was read at — a new one only if it does not exist — and refused +// with ErrPlanMoved otherwise; and only by a process that may act (ActsUnder), whose epoch it records. +// On success p's revision and epoch are the ones written, so the caller may save it again. +func (i *Inventory) SavePlan(ctx context.Context, p *Plan) error { + epoch, err := i.actingEpoch(ctx) + if err != nil { + return fmt.Errorf("the plan for %s %s is not written: %w", p.Repository, p.Commit, err) + } tiers, err := json.Marshal(p.Tiers) if err != nil { return err @@ -92,21 +110,49 @@ func (i *Inventory) SavePlan(ctx context.Context, p Plan) error { return err } defer func() { _ = tx.Rollback(ctx) }() - entered, err := planTierLeft(ctx, tx, p, time.Now()) + entered, err := planTierLeft(ctx, tx, *p, time.Now()) if err != nil { return err } - _, err = tx.Exec(ctx, - `insert into release_plan (id, repository, commit_hash, created, updated, state, tier, tiers, modules, note, branch, tier_entered) - values ($1, $2, $3, $4, now(), $5, $6, $7, $8, $9, $10, $11) + var revision int64 + err = tx.QueryRow(ctx, + `insert into release_plan (id, repository, commit_hash, created, updated, state, tier, tiers, modules, note, + branch, tier_entered, revision, epoch) + values ($1, $2, $3, $4, now(), $5, $6, $7, $8, $9, $10, $11, 1, $13) on conflict (id) do update set updated = now(), state = excluded.state, tier = excluded.tier, tiers = excluded.tiers, modules = excluded.modules, note = excluded.note, branch = excluded.branch, - tier_entered = excluded.tier_entered`, - p.ID, p.Repository, p.Commit, p.Created, p.State, p.Tier, tiers, modules, p.Note, p.Branch, entered) + tier_entered = excluded.tier_entered, revision = release_plan.revision + 1, epoch = excluded.epoch + where release_plan.revision = $12 + returning revision`, + p.ID, p.Repository, p.Commit, p.Created, p.State, p.Tier, tiers, modules, p.Note, p.Branch, entered, + p.Revision, epoch).Scan(&revision) + if errors.Is(err, pgx.ErrNoRows) { + // The row is there and at another revision — moved since this was read, or there already + // when this one is new: either way not this writer's to overwrite. (A plan saved before plans + // had revisions is at zero, and its first save here is from a read at zero.) + return fmt.Errorf("the plan for %s %s (%s) is not written: %w", p.Repository, short(p.Commit), p.ID, ErrPlanMoved) + } if err != nil { return err } - return tx.Commit(ctx) + if err := tx.Commit(ctx); err != nil { + return err + } + p.Revision, p.TierEntered = revision, entered + if epoch != nil { + p.Epoch = uint64(*epoch) + } else { + p.Epoch = 0 + } + return nil +} + +// short is a commit as a person reads it. +func short(commit string) string { + if len(commit) > 8 { + return commit[:8] + } + return commit } // OpenPlans is every plan still being worked, oldest first. @@ -134,7 +180,7 @@ func (i *Inventory) PlanByID(ctx context.Context, id string) (Plan, error) { func (i *Inventory) plans(ctx context.Context, tail string) ([]Plan, error) { rows, err := i.store.Pool().Query(ctx, `select id, repository, commit_hash, created, updated, state, tier, tiers, modules, note, branch, - coalesce(tier_entered, created) + coalesce(tier_entered, created), revision, coalesce(epoch, 0) from release_plan `+tail) if err != nil { return nil, err @@ -144,10 +190,12 @@ func (i *Inventory) plans(ctx context.Context, tail string) ([]Plan, error) { for rows.Next() { var p Plan var tiers, modules []byte + var epoch int64 if err := rows.Scan(&p.ID, &p.Repository, &p.Commit, &p.Created, &p.Updated, &p.State, - &p.Tier, &tiers, &modules, &p.Note, &p.Branch, &p.TierEntered); err != nil { + &p.Tier, &tiers, &modules, &p.Note, &p.Branch, &p.TierEntered, &p.Revision, &epoch); err != nil { return nil, err } + p.Epoch = uint64(epoch) if err := json.Unmarshal(tiers, &p.Tiers); err != nil { return nil, err } diff --git a/internal/inventory/plans_test.go b/internal/inventory/plans_test.go index e4804a7..a1cfff4 100644 --- a/internal/inventory/plans_test.go +++ b/internal/inventory/plans_test.go @@ -13,7 +13,7 @@ func TestAPlanIsKeptAdvancedAndResumedFromTheStore(t *testing.T) { p := Plan{ID: "plan-1", Repository: "novox/mesh-tools", Commit: "abc", Created: time.Now().UTC(), State: PlanBuilding, Tiers: [][]string{{"mesh-tools"}, {"builder"}, {"shop"}}, Modules: map[string]*PlanModule{"mesh-tools": {}, "builder": {}, "shop": {}}} - if err := inv.SavePlan(ctx, p); err != nil { + if err := inv.SavePlan(ctx, &p); err != nil { t.Fatal(err) } open, err := inv.OpenPlans(ctx) @@ -28,7 +28,7 @@ func TestAPlanIsKeptAdvancedAndResumedFromTheStore(t *testing.T) { resumed.Modules["mesh-tools"].BuiltAt = &now resumed.State = PlanRolling resumed.Note = "tier 0 built; waiting for builder on anchor to be applied" - if err := inv.SavePlan(ctx, resumed); err != nil { + if err := inv.SavePlan(ctx, &resumed); err != nil { t.Fatal(err) } again, err := inv.PlanByID(ctx, "plan-1") @@ -36,7 +36,7 @@ func TestAPlanIsKeptAdvancedAndResumedFromTheStore(t *testing.T) { t.Fatalf("the advanced plan did not come back as left: %v %+v", err, again) } again.State = PlanDone - if err := inv.SavePlan(ctx, again); err != nil { + if err := inv.SavePlan(ctx, &again); err != nil { t.Fatal(err) } if open, _ = inv.OpenPlans(ctx); len(open) != 0 { @@ -54,7 +54,7 @@ func TestASupersededPlanIsNotOpen(t *testing.T) { p := Plan{ID: "plan-1", Repository: "novox/mesh-catalog", Branch: "main", Commit: "abc", Created: time.Now().UTC(), State: PlanBuilding, Tiers: [][]string{{"gitea"}}, Modules: map[string]*PlanModule{"gitea": {}}} - if err := inv.SavePlan(ctx, p); err != nil { + if err := inv.SavePlan(ctx, &p); err != nil { t.Fatal(err) } kept, err := inv.PlanByID(ctx, "plan-1") @@ -63,7 +63,7 @@ func TestASupersededPlanIsNotOpen(t *testing.T) { } kept.State = PlanSuperseded kept.Note = "superseded at tier 0 by plan-2" - if err := inv.SavePlan(ctx, kept); err != nil { + if err := inv.SavePlan(ctx, &kept); err != nil { t.Fatal(err) } if open, err := inv.OpenPlans(ctx); err != nil || len(open) != 0 { diff --git a/internal/lease/lease.go b/internal/lease/lease.go new file mode 100644 index 0000000..cd0bf71 --- /dev/null +++ b/internal/lease/lease.go @@ -0,0 +1,471 @@ +// Package lease is the controller's lease (novox/hq to-be 45 §6, ADR 0227 rule 1): the one key that +// says which controller instance may act, and the epoch every act of it carries. +// +// **A controller instance acts only while it holds the key `holder`** in its lease bucket — sends a +// declaration, writes a plan, a condition or a call. The key is written by compare-and-set, lives a +// bucket's age (fifteen seconds) unless renewed, and is renewed every five. The **epoch** is the +// bucket's revision at which the instance took it: a later holder's is higher, so a machine that has +// heard from epoch 57 can refuse anything still arriving from 41 (issue 204). +// +// controller A (epoch 41) ──renew──renew──╳ (renewal refused)──► stops acting, exits +// controller B ──wait──────────────take (epoch 57)──► acts +// +// **Three ways an instance stops acting, all at once.** A renewal refused — somebody else wrote the key +// — is a loss, said and final: Lost closes and the process exits, so its service manager restarts it as +// a candidate. A renewal that cannot be made in time is the same, because past its time the key may be +// somebody else's. And Epoch, the gate every act passes, answers an error from the moment the last +// renewal is older than the key's age less a margin — **before** another instance could have taken +// it, whatever the renewing goroutine is doing — so a send in flight at the moment of loss is refused +// by the clock, not by a goroutine that may be stuck. +package lease + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "sync" + "time" + + "github.com/nats-io/nats.go/jetstream" +) + +// Key is the one key of the lease bucket. +const Key = "holder" + +// Holder is who holds the lease, as the key says it. +type Holder struct { + // Instance names one controller process: its machine, its process and when it started. Two + // instances on one machine — the handover of issue 213 — are two names. + Instance string `json:"instance"` + Host string `json:"host,omitempty"` + Build string `json:"build,omitempty"` + // Epoch is the revision the key was taken at. Zero in the value written to take it, because the + // revision is known only once written; every renewal says it, and Current reads the revision of a + // key never renewed. + Epoch uint64 `json:"epoch,omitempty"` + Taken time.Time `json:"taken"` + Renewed time.Time `json:"renewed"` +} + +// Defaults, as to-be 45 §6 sets them. The age is the bucket's, read from it (Open), so the bucket +// and the lease cannot disagree about it. +const ( + RenewEvery = 5 * time.Second + // Margin is how long before the key could expire this instance stops acting on it: the gap + // between its own clock and the bus's, and a send already on its way. + Margin = 3 * time.Second + // Poll is how often a candidate looks again at a key somebody else holds. + Poll = time.Second +) + +// ErrNotHeld is an act asked of an instance that does not hold the lease. +var ErrNotHeld = errors.New("this controller does not hold the lease") + +// ErrUnwritable is a lease nobody holds that the bus would not let this instance write: the one case a +// caller may serve without it, since nothing else holds it either. Every other failure to take it — +// the holder unreadable, the floor unreadable — says nothing about whether another instance acts. +var ErrUnwritable = errors.New("nobody holds the lease, and the bus would not let this controller write it") + +// Options are what a Lease is made with. +type Options struct { + Holder Holder + // RenewEvery, Margin and Poll default to the package's. + RenewEvery, Margin, Poll time.Duration + // Floor is the highest epoch this mesh has issued, from the controller's own store. **An epoch is + // never issued twice**: a lease bucket raised again from nothing — a bus whose data was replaced — + // starts its revisions over, and every machine that heard epoch 57 would refuse the next controller + // for ever. Below the floor, the bucket's revisions are moved past it before the key is taken. + // Nil is no floor. + Floor func(ctx context.Context) (uint64, error) + // Say is where what the lease does is said: waiting, taking, losing. + Say func(format string, args ...any) + // Moved is told when the bucket's revisions were moved past the floor: the bucket was raised again + // from nothing, which the caller says as a condition. Nil tells nobody. + Moved func(was, floor uint64) + // Now is the clock; nil is time.Now. + Now func() time.Time +} + +// Lease is one instance's hold, or its wait for one. +type Lease struct { + kv jetstream.KeyValue + stream jetstream.Stream + ttl time.Duration + o Options + + mu sync.Mutex + // held is whether this instance holds the key; epoch the revision it took it at; revision the + // revision of its last write, which the next renewal must find. + held bool + epoch uint64 + revision uint64 + // validUntil is when this instance stops acting unless it renews: the moment before its last + // renewal was sent, plus the key's age, less the margin. + validUntil time.Time + // renewed is when this instance last took or renewed the key. + renewed time.Time + lostWhy error + lost chan struct{} + taken time.Time +} + +// Open is a lease over the bucket, read for its age. The bucket is the caller's to assert. +func Open(ctx context.Context, js jetstream.JetStream, bucket string, o Options) (*Lease, error) { + kv, err := js.KeyValue(ctx, bucket) + if err != nil { + return nil, fmt.Errorf("the lease bucket %s is not on the bus: %w", bucket, err) + } + status, err := kv.Status(ctx) + if err != nil { + return nil, fmt.Errorf("the lease bucket %s cannot be read: %w", bucket, err) + } + if status.TTL() <= 0 { + // A key that never expires is a lease a dead controller holds for ever. + return nil, fmt.Errorf("the lease bucket %s keeps its keys for ever, so a controller that died "+ + "holding it would hold it for ever: it must have an age", bucket) + } + stream, err := js.Stream(ctx, "KV_"+bucket) + if err != nil { + return nil, fmt.Errorf("the stream under the lease bucket %s cannot be read: %w", bucket, err) + } + if o.RenewEvery <= 0 { + o.RenewEvery = RenewEvery + } + if o.Margin <= 0 { + o.Margin = Margin + } + if o.Poll <= 0 { + o.Poll = Poll + } + if o.Say == nil { + o.Say = func(string, ...any) {} + } + if o.Now == nil { + o.Now = time.Now + } + if o.RenewEvery+o.Margin >= status.TTL() { + return nil, fmt.Errorf("a lease renewed every %s with a margin of %s does not fit a key that lives %s", + o.RenewEvery, o.Margin, status.TTL()) + } + return &Lease{kv: kv, stream: stream, ttl: status.TTL(), o: o, lost: make(chan struct{})}, nil +} + +// TTL is how long the key lives unrenewed. +func (l *Lease) TTL() time.Duration { return l.ttl } + +// Current is who holds the lease now, and false when nobody does. An error is an error: never "nobody". +func Current(ctx context.Context, kv jetstream.KeyValue) (Holder, bool, error) { + entry, err := kv.Get(ctx, Key) + if errors.Is(err, jetstream.ErrKeyNotFound) { + return Holder{}, false, nil + } + if err != nil { + return Holder{}, false, err + } + var h Holder + if err := json.Unmarshal(entry.Value(), &h); err != nil { + return Holder{}, false, fmt.Errorf("the lease's key holds something that is not a holder: %w", err) + } + if h.Epoch == 0 { + // Taken and not yet renewed: the revision it was taken at is the one it has. + h.Epoch = entry.Revision() + } + return h, true, nil +} + +// Current is who holds this lease now. +func (l *Lease) Current(ctx context.Context) (Holder, bool, error) { return Current(ctx, l.kv) } + +// ErrTaken is a take that found the key held by somebody else; Take waits on it, TryTake answers it. +var ErrTaken = errors.New("another controller holds the lease") + +// Take waits until the key is absent or expired and takes it, answering the epoch. A key somebody +// else holds is waited on and said once; an error reading or writing the bucket is answered, because +// a candidate that cannot see the key cannot tell whether it may act. +func (l *Lease) Take(ctx context.Context) (uint64, error) { + said := "" + for { + epoch, err := l.TryTake(ctx) + if err == nil { + return epoch, nil + } + var held *heldBy + if !errors.As(err, &held) { + return 0, err + } + if held.h.Instance != said { + l.o.Say("another controller holds the lease (%s, epoch %d, taken %s); waiting until it lets go "+ + "or stops renewing", held.h.Instance, held.h.Epoch, held.h.Taken.UTC().Format(time.RFC3339)) + said = held.h.Instance + } + select { + case <-ctx.Done(): + return 0, ctx.Err() + case <-time.After(l.o.Poll): + } + } +} + +// heldBy is ErrTaken naming the holder. +type heldBy struct{ h Holder } + +func (e *heldBy) Error() string { + return fmt.Sprintf("%v: %s, epoch %d", ErrTaken, e.h.Instance, e.h.Epoch) +} +func (e *heldBy) Unwrap() error { return ErrTaken } + +// TryTake takes the key if nobody holds it, once, without waiting: the epoch, or ErrTaken naming the +// holder, or what the bus said. +func (l *Lease) TryTake(ctx context.Context) (uint64, error) { + l.mu.Lock() + switch { + case l.lostWhy != nil: + // A lease lost is not taken again by the instance that lost it: that instance exits, and its + // successor is a new candidate with a new name. + defer l.mu.Unlock() + return 0, fmt.Errorf("%w: it lost it — %v", ErrNotHeld, l.lostWhy) + case l.held: + defer l.mu.Unlock() + return l.epoch, nil + } + l.mu.Unlock() + if h, found, err := l.Current(ctx); err != nil { + return 0, fmt.Errorf("who holds the lease cannot be read: %w", err) + } else if found { + return 0, &heldBy{h} + } + if err := l.aboveTheFloor(ctx); err != nil { + return 0, err + } + now := l.o.Now() + h := l.o.Holder + h.Taken, h.Renewed, h.Epoch = now, now, 0 + value, err := json.Marshal(h) + if err != nil { + return 0, err + } + anchor := l.o.Now() + revision, err := l.kv.Create(ctx, Key, value) + if errors.Is(err, jetstream.ErrKeyExists) { + // Somebody took it between the read and the write: theirs. + if h, found, rerr := l.Current(ctx); rerr == nil && found { + return 0, &heldBy{h} + } + return 0, ErrTaken + } + if err != nil { + return 0, fmt.Errorf("%w: %w", ErrUnwritable, err) + } + if floor, err := l.floor(ctx); err != nil { + _ = l.kv.Delete(ctx, Key, jetstream.LastRevision(revision)) + return 0, err + } else if revision <= floor { + // The bucket moved under the floor between the check and the write (another candidate raising + // it again): never an epoch already issued. Given back, and taken again on the next try. + _ = l.kv.Delete(ctx, Key, jetstream.LastRevision(revision)) + return 0, fmt.Errorf("the lease was taken at revision %d, not above the highest epoch issued (%d); given "+ + "back to be taken again", revision, floor) + } + l.mu.Lock() + l.held, l.epoch, l.revision, l.taken, l.renewed = true, revision, revision, now, anchor + l.validUntil = anchor.Add(l.ttl - l.o.Margin) + l.mu.Unlock() + l.o.Say("took the controller lease at epoch %d (%s)", revision, h.Instance) + return revision, nil +} + +// floor is the highest epoch issued, zero without a floor. +func (l *Lease) floor(ctx context.Context) (uint64, error) { + if l.o.Floor == nil { + return 0, nil + } + floor, err := l.o.Floor(ctx) + if err != nil { + return 0, fmt.Errorf("the highest epoch this mesh has issued cannot be read, so no epoch can be "+ + "issued that is surely higher: %w", err) + } + return floor, nil +} + +// aboveTheFloor moves the bucket's revisions past the highest epoch issued, when a bucket raised again +// from nothing is behind it. Only while nobody holds the key: the stream is compacted to the floor, which +// sets the next revision above it. +func (l *Lease) aboveTheFloor(ctx context.Context) error { + floor, err := l.floor(ctx) + if err != nil || floor == 0 { + return err + } + info, err := l.stream.Info(ctx) + if err != nil { + return fmt.Errorf("the lease bucket's revisions cannot be read: %w", err) + } + if info.State.LastSeq > floor { + return nil + } + l.o.Say("the lease bucket is at revision %d and this mesh has issued epoch %d: the bucket was raised "+ + "again from nothing, so its revisions are moved past %d before the lease is taken", info.State.LastSeq, + floor, floor) + if err := l.stream.Purge(ctx, jetstream.WithPurgeSequence(floor+1)); err != nil { + return fmt.Errorf("the lease bucket's revisions could not be moved past epoch %d: %w", floor, err) + } + if l.o.Moved != nil { + l.o.Moved(info.State.LastSeq, floor) + } + return nil +} + +// Epoch is the gate every act passes: this instance's epoch while it holds the lease and its last +// renewal is in time, and an error otherwise — not held, lost, or a renewal overdue. +func (l *Lease) Epoch() (uint64, error) { + l.mu.Lock() + defer l.mu.Unlock() + switch { + case l.lostWhy != nil: + return 0, fmt.Errorf("%w: it lost it — %v", ErrNotHeld, l.lostWhy) + case !l.held: + return 0, ErrNotHeld + case l.o.Now().After(l.validUntil): + return 0, fmt.Errorf("%w in time: its last renewal was due by %s", ErrNotHeld, + l.validUntil.UTC().Format(time.RFC3339)) + } + return l.epoch, nil +} + +// Renewed is when this instance last took or renewed the key; zero before it took it. +func (l *Lease) Renewed() time.Time { + l.mu.Lock() + defer l.mu.Unlock() + return l.renewed +} + +// Held says this instance holds the lease now, as Epoch's gate does. +func (l *Lease) Held() bool { + _, err := l.Epoch() + return err == nil +} + +// Lost is closed when this instance loses the lease it held. A caller exits on it. +func (l *Lease) Lost() <-chan struct{} { return l.lost } + +// LostWhy is why it was lost, nil while it was not. +func (l *Lease) LostWhy() error { + l.mu.Lock() + defer l.mu.Unlock() + return l.lostWhy +} + +// Keep renews the lease until ctx ends or a renewal fails; on a failure it is lost. +func (l *Lease) Keep(ctx context.Context) { + tick := time.NewTicker(l.o.RenewEvery) + defer tick.Stop() + for { + select { + case <-ctx.Done(): + return + case <-tick.C: + } + if err := l.Renew(ctx); err != nil { + return // lost, said by lose; or stopping + } + } +} + +// Renew writes the key once more at the revision this instance last wrote. A refusal or a failure is +// the lease lost: past this renewal's time the key may be somebody else's, and an instance that is not +// sure it holds the lease does not act on it. +func (l *Lease) Renew(ctx context.Context) error { + l.mu.Lock() + if !l.held || l.lostWhy != nil { + defer l.mu.Unlock() + if l.lostWhy != nil { + return l.lostWhy + } + return ErrNotHeld + } + revision, epoch, taken := l.revision, l.epoch, l.taken + l.mu.Unlock() + + h := l.o.Holder + h.Epoch, h.Taken = epoch, taken + anchor := l.o.Now() + h.Renewed = anchor + value, err := json.Marshal(h) + if err != nil { + return l.lose(err) + } + renewing, cancel := context.WithTimeout(ctx, l.o.RenewEvery) + defer cancel() + next, err := l.kv.Update(renewing, Key, value, revision) + if err != nil { + if ctx.Err() != nil { + // Stopping, not losing: Release gives it back. + return ctx.Err() + } + why := fmt.Errorf("the renewal at revision %d was refused: %w", revision, err) + if !errors.Is(err, jetstream.ErrKeyExists) && !isWrongLast(err) { + why = fmt.Errorf("the renewal could not be made: %w", err) + } + return l.lose(why) + } + l.mu.Lock() + l.revision, l.renewed = next, anchor + l.validUntil = anchor.Add(l.ttl - l.o.Margin) + l.mu.Unlock() + return nil +} + +// isWrongLast is the bus refusing a compare-and-set because the key moved. +func isWrongLast(err error) bool { + var apiErr *jetstream.APIError + return errors.As(err, &apiErr) && apiErr.ErrorCode == jetstream.JSErrCodeStreamWrongLastSequence +} + +// lose marks the lease lost, once, and says why. +func (l *Lease) lose(why error) error { + l.mu.Lock() + defer l.mu.Unlock() + if l.lostWhy == nil { + l.lostWhy = why + l.held = false + close(l.lost) + l.o.Say("LOST the controller lease (epoch %d): %v — this controller stops acting at once", l.epoch, why) + } + return l.lostWhy +} + +// Release gives the lease back, so the next candidate takes it at once rather than after its age. +// Only the key this instance last wrote is deleted: one that moved is somebody else's. +func (l *Lease) Release(ctx context.Context) { + l.mu.Lock() + held, revision, epoch := l.held && l.lostWhy == nil, l.revision, l.epoch + l.held = false + if l.lostWhy == nil { + l.lostWhy = errors.New("given back") + close(l.lost) + } + l.mu.Unlock() + if !held { + return + } + releasing, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second) + defer cancel() + err := l.kv.Delete(releasing, Key, jetstream.LastRevision(revision)) + if err != nil && isWrongLast(err) { + // A renewal was in flight as this began and landed first: the key moved, and is still this + // instance's if it still names it at this epoch. Deleted at the revision it has now. + if entry, gerr := l.kv.Get(releasing, Key); gerr == nil { + var h Holder + if json.Unmarshal(entry.Value(), &h) == nil && h.Instance == l.o.Holder.Instance && + (h.Epoch == epoch || h.Epoch == 0 && entry.Revision() == epoch) { + err = l.kv.Delete(releasing, Key, jetstream.LastRevision(entry.Revision())) + } + } + } + if err != nil { + l.o.Say("the controller lease (epoch %d) could not be given back, so the next controller waits for "+ + "it to expire: %v", epoch, err) + return + } + l.o.Say("gave the controller lease back (epoch %d)", epoch) +} diff --git a/internal/lease/lease_test.go b/internal/lease/lease_test.go new file mode 100644 index 0000000..317f0ae --- /dev/null +++ b/internal/lease/lease_test.go @@ -0,0 +1,296 @@ +package lease + +import ( + "context" + "errors" + "fmt" + "os" + "sync" + "testing" + "time" + + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" +) + +// Two controllers at once, on a real bus (novox/hq to-be 45 §6, replay R1's half that lives here): +// one takes the lease and the other waits; a handover gives the second a higher epoch; a holder that +// stops renewing is taken over once its key expires, and has stopped acting before then; a renewal +// refused is a loss at once; and a bucket raised again from nothing never issues an epoch twice. +// +// MESH_TEST_NATS=nats://127.0.0.1:14222 go test ./internal/lease/ + +// The bounds in these tests are the design's divided by five, so the cases run in seconds. +const ( + testTTL = 3 * time.Second + testRenew = time.Second + testPoll = 100 * time.Millisecond +) + +// aBucket is a fresh lease bucket on the test bus, with the test's age. +func aBucket(t *testing.T) (jetstream.JetStream, string) { + t.Helper() + url := os.Getenv("MESH_TEST_NATS") + if url == "" { + t.Skip("MESH_TEST_NATS unset") + } + conn, err := nats.Connect(url) + if err != nil { + t.Fatal(err) + } + t.Cleanup(conn.Close) + js, err := jetstream.New(conn) + if err != nil { + t.Fatal(err) + } + bucket := fmt.Sprintf("lease-test-%d", time.Now().UnixNano()) + if _, err := js.CreateKeyValue(t.Context(), jetstream.KeyValueConfig{Bucket: bucket, History: 1, + TTL: testTTL, Storage: jetstream.FileStorage}); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = js.DeleteKeyValue(context.Background(), bucket) }) + return js, bucket +} + +// aController is one controller instance's lease over the bucket, on a connection of its own. +func aController(t *testing.T, bucket, name string, floor func(context.Context) (uint64, error)) *Lease { + t.Helper() + conn, err := nats.Connect(os.Getenv("MESH_TEST_NATS")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(conn.Close) + js, err := jetstream.New(conn) + if err != nil { + t.Fatal(err) + } + var said sync.Mutex + l, err := Open(t.Context(), js, bucket, Options{Holder: Holder{Instance: name, Host: name}, + RenewEvery: testRenew, Margin: testRenew / 2, Poll: testPoll, Floor: floor, + Say: func(format string, args ...any) { + said.Lock() + defer said.Unlock() + t.Logf(name+": "+format, args...) + }}) + if err != nil { + t.Fatal(err) + } + return l +} + +// takeWithin takes the lease in a goroutine and answers when it did, or fails past the bound. +func takeWithin(t *testing.T, l *Lease) <-chan uint64 { + t.Helper() + took := make(chan uint64, 1) + go func() { + epoch, err := l.Take(t.Context()) + if err != nil { + t.Errorf("taking: %v", err) + close(took) + return + } + took <- epoch + }() + return took +} + +func TestTheSecondControllerWaitsAndTakesAHigherEpochOnHandover(t *testing.T) { + _, bucket := aBucket(t) + a, b := aController(t, bucket, "a", nil), aController(t, bucket, "b", nil) + + epochA, err := a.Take(t.Context()) + if err != nil { + t.Fatal(err) + } + keeping, stop := context.WithCancel(t.Context()) + defer stop() + go a.Keep(keeping) + + // b waits while a renews — across more than one age of the key, so it is the renewals holding it. + took := takeWithin(t, b) + select { + case epoch := <-took: + t.Fatalf("b took the lease (epoch %d) while a held it and renewed", epoch) + case <-time.After(testTTL + testRenew): + } + if _, err := b.Epoch(); !errors.Is(err, ErrNotHeld) { + t.Fatalf("b, waiting, may act: %v", err) + } + if e, err := a.Epoch(); err != nil || e != epochA { + t.Fatalf("a, holding, answers %d, %v", e, err) + } + holder, found, err := b.Current(t.Context()) + if err != nil || !found || holder.Instance != "a" || holder.Epoch != epochA { + t.Fatalf("b reads the holder as %+v (%v, %v), want a at epoch %d", holder, found, err, epochA) + } + + // a gives it back: b takes it at once, not after the key's age, at a higher epoch. + stop() + handedOver := time.Now() + a.Release(t.Context()) + select { + case epochB := <-took: + if epochB <= epochA { + t.Fatalf("b took epoch %d after a's %d: an epoch must only grow", epochB, epochA) + } + if waited := time.Since(handedOver); waited > testTTL { + t.Fatalf("b took the lease %s after a gave it back: it waited out the key's age", waited) + } + case <-time.After(2 * testTTL): + t.Fatal("b never took the lease a gave back") + } + if _, err := a.Epoch(); !errors.Is(err, ErrNotHeld) { + t.Fatalf("a, having given it back, may still act: %v", err) + } + if _, err := a.TryTake(t.Context()); err == nil { + t.Fatal("a took the lease again after giving it back: its successor is a new candidate") + } +} + +func TestAHolderThatStopsRenewingStopsActingBeforeItIsTakenOver(t *testing.T) { + _, bucket := aBucket(t) + a, b := aController(t, bucket, "a", nil), aController(t, bucket, "b", nil) + epochA, err := a.Take(t.Context()) + if err != nil { + t.Fatal(err) + } + // a is stuck: it never renews (a process stopped, a goroutine wedged). It was not told anything. + took := takeWithin(t, b) + + // Its gate closes by its own clock, before the key can expire. + closed := time.Now() + for a.Held() { + if time.Since(closed) > testTTL { + t.Fatal("a still may act past its key's age without renewing") + } + time.Sleep(20 * time.Millisecond) + } + select { + case epoch := <-took: + t.Fatalf("b took the lease (epoch %d) before a stopped acting", epoch) + default: + } + select { + case epochB := <-took: + if epochB <= epochA { + t.Fatalf("b took epoch %d after a's %d", epochB, epochA) + } + // And a, the moment it tries to renew, knows it lost: the key moved. + if err := a.Renew(t.Context()); err == nil { + t.Fatal("a renewed a lease b holds") + } + select { + case <-a.Lost(): + default: + t.Fatal("a's renewal was refused and a was not told it lost the lease") + } + if _, err := a.Epoch(); !errors.Is(err, ErrNotHeld) { + t.Fatalf("a, having lost the lease, may act: %v", err) + } + case <-time.After(3 * testTTL): + t.Fatal("b never took over a lease nobody renewed") + } +} + +func TestARenewalRefusedIsALossAtOnce(t *testing.T) { + js, bucket := aBucket(t) + a := aController(t, bucket, "a", nil) + if _, err := a.Take(t.Context()); err != nil { + t.Fatal(err) + } + // Somebody else writes the key — a second controller that read it as expired on a skewed clock, + // or a person — so a's next renewal finds a revision it did not write. + kv, err := js.KeyValue(t.Context(), bucket) + if err != nil { + t.Fatal(err) + } + if _, err := kv.Put(t.Context(), Key, []byte(`{"instance":"intruder"}`)); err != nil { + t.Fatal(err) + } + if err := a.Renew(t.Context()); err == nil { + t.Fatal("a renewed over a key somebody else wrote") + } + select { + case <-a.Lost(): + case <-time.After(time.Second): + t.Fatal("a was not told it lost the lease") + } + if _, err := a.Epoch(); !errors.Is(err, ErrNotHeld) { + t.Fatalf("a acts after its renewal was refused: %v", err) + } + // And giving back a lease it lost deletes nothing of the one who holds it now. + a.Release(t.Context()) + if h, found, err := Current(t.Context(), kv); err != nil || !found || h.Instance != "intruder" { + t.Fatalf("after a gave back what it lost, the key holds %+v (%v, %v)", h, found, err) + } +} + +func TestKeepingRenewsAcrossManyAges(t *testing.T) { + _, bucket := aBucket(t) + a := aController(t, bucket, "a", nil) + epoch, err := a.Take(t.Context()) + if err != nil { + t.Fatal(err) + } + keeping, stop := context.WithCancel(t.Context()) + defer stop() + go a.Keep(keeping) + until := time.Now().Add(3 * testTTL) + for time.Now().Before(until) { + if e, err := a.Epoch(); err != nil || e != epoch { + t.Fatalf("a, renewing, answers %d, %v", e, err) + } + time.Sleep(200 * time.Millisecond) + } + h, found, err := a.Current(t.Context()) + if err != nil || !found || h.Epoch != epoch || h.Instance != "a" { + t.Fatalf("the key says %+v (%v, %v)", h, found, err) + } +} + +func TestAnEpochIsNeverIssuedTwiceWhenTheBucketStartsOver(t *testing.T) { + _, bucket := aBucket(t) + // The mesh has issued epoch 500 before: its store says so. The bucket is new — a bus whose data + // was replaced — and its revisions start at one. + floor := func(context.Context) (uint64, error) { return 500, nil } + a := aController(t, bucket, "a", floor) + moved := false + a.o.Moved = func(was, floor uint64) { moved = was < floor && floor == 500 } + epoch, err := a.Take(t.Context()) + if err != nil { + t.Fatal(err) + } + if epoch <= 500 { + t.Fatalf("a took epoch %d with 500 already issued: every machine that heard 500 would refuse it", epoch) + } + if !moved { + t.Fatal("the bucket's revisions were moved past the floor and nobody was told") + } + // A floor that cannot be read takes nothing: an epoch that may not be higher is not issued. + _, other := aBucket(t) + b := aController(t, other, "b", func(context.Context) (uint64, error) { return 0, errors.New("store away") }) + if _, err := b.TryTake(t.Context()); err == nil { + t.Fatal("b took a lease without knowing the highest epoch issued") + } +} + +func TestALeaseBucketWithoutAnAgeIsRefused(t *testing.T) { + url := os.Getenv("MESH_TEST_NATS") + if url == "" { + t.Skip("MESH_TEST_NATS unset") + } + conn, err := nats.Connect(url) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + js, _ := jetstream.New(conn) + bucket := fmt.Sprintf("lease-ageless-%d", time.Now().UnixNano()) + if _, err := js.CreateKeyValue(t.Context(), jetstream.KeyValueConfig{Bucket: bucket, History: 1}); err != nil { + t.Fatal(err) + } + defer func() { _ = js.DeleteKeyValue(context.Background(), bucket) }() + if _, err := Open(t.Context(), js, bucket, Options{Holder: Holder{Instance: "a"}}); err == nil { + t.Fatal("a lease over keys that never expire was opened: a controller that died holding it would hold it for ever") + } +} diff --git a/internal/link/calls.go b/internal/link/calls.go index 5926843..a122c70 100644 --- a/internal/link/calls.go +++ b/internal/link/calls.go @@ -78,6 +78,8 @@ type Call struct { // Holder is the controller process that served it, so one starting can tell its own running // calls from those a stopped one left. Holder string `json:"holder,omitempty"` + // Epoch is the controller lease epoch it was served under (novox/hq to-be 45 §6); zero for none. + Epoch uint64 `json:"epoch,omitempty"` reply string // the subject the answer went to, which the bus names when it refuses it } @@ -111,6 +113,18 @@ type CallLog struct { logger *log.Logger lost int // writes the keeper could not take, said once each keepErr error + // epoch is the lease this process serves under (UnderLease): a call carries its epoch, and its + // record is written only while the lease is held. Nil writes every record with no epoch. + epoch func() (uint64, error) +} + +// UnderLease makes every call carry the controller lease's epoch, and every write of a call's record +// pass the lease (novox/hq to-be 45 §6): a controller that lost the lease writes no finish over a call +// the next holder has marked abandoned — that mark is the record's last word. +func (l *CallLog) UnderLease(epoch func() (uint64, error)) { + l.mu.Lock() + defer l.mu.Unlock() + l.epoch = epoch } // Calls is this process's log: one holder process serves its seats on one connection. @@ -150,9 +164,9 @@ func (l *CallLog) Durably(ctx context.Context, keeper CallKeeper, holder string, return fmt.Errorf("marking %s abandoned: %w", c.ID, err) } if logger != nil { - logger.Printf("%s (%s.%s, asked %s by %s) was running under %s, which stopped: marked abandoned — "+ - "it may have done part of what it was asked, and nothing will finish it", c.ID, c.Seat, c.Verb, - c.Started.Format(time.RFC3339), orSomebody(c.Caller), orSomebody(c.Holder)) + logger.Printf("%s (%s.%s, asked %s by %s) was running under %s (epoch %d), which stopped: marked "+ + "abandoned — it may have done part of what it was asked, and nothing will finish it", c.ID, c.Seat, + c.Verb, c.Started.Format(time.RFC3339), orSomebody(c.Caller), orSomebody(c.Holder), c.Epoch) } } return nil @@ -184,6 +198,18 @@ func (l *CallLog) keep(c Call) { func (l *CallLog) keepWrites() { for c := range l.writes { + l.mu.Lock() + gate := l.epoch + l.mu.Unlock() + if gate != nil { + if _, err := gate(); err != nil { + if l.logger != nil { + l.logger.Printf("%s (%s.%s, %s) is not kept on the bus: %v — the controller holding the lease "+ + "marks it abandoned", c.ID, c.Seat, c.Verb, c.State, err) + } + continue + } + } var err error for try := 0; try < keepTries; try++ { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) @@ -237,6 +263,9 @@ func (l *CallLog) begin(seat, verb string, args json.RawMessage, reply string) * c := &Call{ID: "call-" + strconv.FormatInt(l.now().UnixNano(), 10) + "-" + strconv.FormatUint(l.next, 10), Seat: seat, Verb: verb, Args: args, Started: l.now(), State: CallRunning, reply: reply, Caller: callerOf(reply), Holder: l.holder} + if l.epoch != nil { + c.Epoch, _ = l.epoch() // zero when not held: its record is then not written either + } l.calls = append(l.calls, c) if len(l.calls) > KeptCalls { l.calls = l.calls[len(l.calls)-KeptCalls:] diff --git a/internal/link/contracts.go b/internal/link/contracts.go new file mode 100644 index 0000000..bcf44cd --- /dev/null +++ b/internal/link/contracts.go @@ -0,0 +1,47 @@ +package link + +// The contract of every message kind the controller consumes (novox/hq to-be 45 Phase 2, ADR 0227 rule +// 2 "how it is checked": a contract test per consumed message kind in the receiver's repository, and a +// check listing every consumed subject against the tests that name it). +// +// **Each kind says how an older one is told from a newer, and the tests that deliver n, then n−1.** A +// kind that carries no order says why none is needed — a word whose newest is simply the latest heard, +// a request answered once. The test beside it fails a kind the controller can be handed without an +// entry here, and an entry naming a test that does not exist. + +// Contract is one consumed kind's order. +type Contract struct { + // Ordered is how an older message of this kind is refused; empty for a kind that carries no order. + Ordered string + // Unordered is why a kind needs no order; empty for an ordered one. + Unordered string + // Tests are the tests that deliver the newer and then the older, and assert the older refused. + Tests []string +} + +// Contracts are every kind the controller consumes, by kind. +var Contracts = map[string]Contract{ + KindReport: {Ordered: "by the epoch and sequence of the declaration it is about, then the node-engine's " + + "report sequence (order.go); an older account is refused and counted. A report from a node-engine " + + "that orders nothing is judged by the digest it names (issue 267)", + Tests: []string{"TestAnAccountOfAnOlderDeclarationIsRefusedByItsSequence", + "TestAnOlderReportOfTheSameDeclarationIsRefused", "TestAnAccountOfAnOlderDeclarationDoesNotReplaceTheNewer", + "TestAnAccountIsOlderByEpochThenSequenceThenReportSequence"}}, + KindBuilt: {Ordered: "by the build's ask: an older ask finishing later is recorded and not registered (issue 219)", + Tests: []string{"TestAnOlderBuildHeardLaterDoesNotReplaceTheNewer"}}, + KindSourceMoved: {Ordered: "by the merge's commit against what was built from it: a merge already acted on " + + "or older than the last look is history, not a second plan (issues 250, 266)", + Tests: []string{"TestAMergeOlderThanTheLastLookIsHistory", "TestAMergeMatchesTheSourcesBuiltFromIt"}}, + KindHeartbeat: {Unordered: "a word that the machine is there: the newest heard is the newest said, and one " + + "lost is the next one"}, + KindToolsHeartbeat: {Unordered: "a word that the node tools are there, as a machine's heartbeat"}, + KindEnrolment: {Unordered: "a request answered once, under a token spent once: a second presentation is " + + "refused by the token, not by an order (issue 083)", + Tests: []string{"TestAnEnrolmentMetByAHeldTokenIsAskedToTryAgain"}}, + KindModuleMoved: {Unordered: "the catalogue saying a module's current build moved: acted on by reading the " + + "catalogue's record, which is the order, so a late one reads the same record"}, + KindCatchUp: {Unordered: "a catalogue asking what it missed: answered from the record, whenever asked"}, + KindProvisioner: {Unordered: "a provider's newest word about a consumer, said again every fifteen minutes " + + "while it holds (ADR 0224): the condition keeps the last observed, and S8 says when the words stop", + Tests: []string{"TestAProviderFailingAConsumerBreaksAllWellUntilItRecovers"}}, +} diff --git a/internal/link/contracts_test.go b/internal/link/contracts_test.go new file mode 100644 index 0000000..c2995fd --- /dev/null +++ b/internal/link/contracts_test.go @@ -0,0 +1,84 @@ +package link + +import ( + "go/parser" + "go/token" + "io/fs" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/broker" +) + +// Every subject the controller consumes is a kind with a contract, and every test a contract names +// exists (novox/hq to-be 45 Phase 2, ADR 0227 rule 2). +func TestEveryConsumedKindHasAContract(t *testing.T) { + // What the controller can be handed, from the subjects it is granted and the ones it derives. + subjects := []string{EnrolSubject, BuiltSubject, ReportSubject("anchor"), AliveSubject("anchor"), + ToolsAliveSubject("anchor"), "mesh.mod.postgres.event.provisioner.failing"} + subjects = append(subjects, broker.ControllerFollows...) + kinds := map[string]bool{} + for _, s := range subjects { + kind, known := kindOfSubject(s) + if !known { + if strings.Contains(s, "*") { + continue // a pattern among the follows; its concrete subject is listed above + } + t.Errorf("the controller follows %s and has no kind for it", s) + continue + } + kinds[kind] = true + } + for kind := range kinds { + c, ok := Contracts[kind] + switch { + case !ok: + t.Errorf("the controller consumes %s and no contract says how an older one is told from a newer", kind) + case (c.Ordered == "") == (c.Unordered == ""): + t.Errorf("%s's contract says neither, or both, how it is ordered and why it need not be: %+v", kind, c) + case c.Ordered != "" && len(c.Tests) == 0: + t.Errorf("%s is ordered and no test delivers the newer and then the older", kind) + } + } + for kind := range Contracts { + if !kinds[kind] { + t.Errorf("a contract for %s, which the controller does not consume", kind) + } + } + + // Every test named exists in this repository. + exists := map[string]bool{} + fset := token.NewFileSet() + err := filepath.WalkDir("../..", func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() && (d.Name() == "vendor" || d.Name() == ".git") { + return filepath.SkipDir + } + if d.IsDir() || !strings.HasSuffix(path, "_test.go") { + return nil + } + f, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + return err + } + for name, obj := range f.Scope.Objects { + if obj.Kind.String() == "func" && strings.HasPrefix(name, "Test") { + exists[name] = true + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } + for kind, c := range Contracts { + for _, name := range c.Tests { + if !exists[name] { + t.Errorf("%s's contract names %s, which no test in this repository is", kind, name) + } + } + } +} diff --git a/internal/link/declare.go b/internal/link/declare.go index 665c810..f873037 100644 --- a/internal/link/declare.go +++ b/internal/link/declare.go @@ -12,6 +12,10 @@ type Signer interface { Sign(ctx context.Context, message []byte) ([]byte, error) } +// ActingGate is what every send passes before it is made (novox/hq to-be 45 §6): whether this process +// may act under the controller's lease. Set by the controller; nil passes every send — a test, a tool. +var ActingGate func(ctx context.Context) error + // Declare sends a node what it should be, signed. // // The signature is over the declaration exactly as it is published — the same bytes the node @@ -25,6 +29,12 @@ func Declare(ctx context.Context, bus Bus, signer Signer, node string, if !json.Valid(declaration) { return fmt.Errorf("refusing to send %s something that is not a declaration", node) } + // **At the send, not only where it was composed**: a lease lost between the two stops this one. + if ActingGate != nil { + if err := ActingGate(ctx); err != nil { + return fmt.Errorf("%s was not sent its declaration: %w", node, err) + } + } signature, err := signer.Sign(ctx, declaration) if err != nil { diff --git a/internal/link/enrolment.go b/internal/link/enrolment.go index 463fbe5..45d00ab 100644 --- a/internal/link/enrolment.go +++ b/internal/link/enrolment.go @@ -11,6 +11,7 @@ import ( "fmt" "log" "sort" + "time" "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/identity" @@ -421,23 +422,54 @@ func (e Enrolment) Heard(ctx context.Context, report Report) (news bool, err err // the newer one arrived reported *after* the newer apply's report, and stored last, it read as the // machine never having applied what it was sent — the plan waited until somebody pushed by hand. // One row per node means the last write wins, so the order of arrival must not decide it. - if report.Declared != "" { - sent, err := e.Inventory.Outstanding(ctx, report.Node) + // + // **By order, from a node-engine that orders its reports** (novox/hq to-be 45 §6, the contract in + // order.go): the account is kept only if it is not older than the one kept — by the epoch and + // sequence of the declaration it is about, then the node-engine's own report sequence — and refused + // otherwise, said and counted (rule 2). One rule, whatever the account is about and whenever it + // arrives; the digest the mesh last sent decides nothing for it. **By digest, from an older + // node-engine**: its report claims no order, and the rule of issue 267 stands for it. + // + // And whether the node-engine reads an epoch in a declaration, from every account it gives: the + // mesh sends one only to a machine that said so, and one rolled back says so no longer. + if err := e.Inventory.RecordReadsEpoch(ctx, node.ID, report.ReadsEpoch()); err != nil { + return false, err + } + if report.Ordered() { + account := AccountOf(report) + news, err = e.Inventory.RecordOrderedDoing(ctx, node.ID, doing, + inventory.ReportOrder{Epoch: report.Epoch, Sequence: report.Sequence, ReportSequence: report.ReportSequence}, + func(kept inventory.ReportOrder) bool { + return account.OlderThan(Account{Order: Order{Epoch: kept.Epoch, Sequence: kept.Sequence}, + ReportSequence: kept.ReportSequence}) + }) + if errors.Is(err, inventory.ErrOlderAccount) { + log.Printf("kept what %s says about the machine, and refused its account of declaration %s (%s, report %d): "+ + "the account kept is newer", report.Node, short(report.Declared), report.Order.Words(), report.ReportSequence) + StaleRefusals.Refused(Refusal{Writer: WriterNodeEngine(report.Node), Receiver: "controller", At: time.Now()}) + return false, e.Inventory.Seen(ctx, node.ID) + } if err != nil { return false, err } - if Superseded(report.Declared, sent) { - log.Printf("kept what %s says about the machine, and not its account of declaration %s: "+ - "the mesh has sent it %s since", report.Node, short(report.Declared), short(sent)) - return false, e.Inventory.Seen(ctx, node.ID) + } else { + if report.Declared != "" { + sent, err := e.Inventory.Outstanding(ctx, report.Node) + if err != nil { + return false, err + } + if Superseded(report.Declared, sent) { + log.Printf("kept what %s says about the machine, and not its account of declaration %s: "+ + "the mesh has sent it %s since", report.Node, short(report.Declared), short(sent)) + return false, e.Inventory.Seen(ctx, node.ID) + } + } + // **Whether this is news** is the store's answer: it holds the previous report, and a machine + // that reconciles every minute says the same thing until something changes (novox/hq ADR 0134). + news, err = e.Inventory.RecordDoing(ctx, node.ID, doing) + if err != nil { + return false, err } - } - - // **Whether this is news** is the store's answer: it holds the previous report, and a machine - // that reconciles every minute says the same thing until something changes (novox/hq ADR 0134). - news, err = e.Inventory.RecordDoing(ctx, node.ID, doing) - if err != nil { - return false, err } // And how long the machine took, from the send to this first account of it (novox/hq to-be 45 // Phase 0): what the sent-not-reported bound will be set from. Said if lost, never a failure. diff --git a/internal/link/heard_order_test.go b/internal/link/heard_order_test.go new file mode 100644 index 0000000..b00ab0c --- /dev/null +++ b/internal/link/heard_order_test.go @@ -0,0 +1,163 @@ +package link_test + +import ( + "context" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// A report kept by its order (novox/hq to-be 45 §6, ADR 0227 rule 2): the contract test of the report, +// the message kind the controller consumes from every machine. Deliver n, then n−1: refused and counted. +// Of the same declaration, report r then r−1: refused. The digest the mesh last sent decides nothing +// for an ordered report; an unordered one, from an older node-engine, is judged by it as before. + +func anOrderedMachine(t *testing.T) (*inventory.Inventory, link.Enrolment, string) { + t.Helper() + inv := inventory.ForTest(t) + node, err := inv.AddNode(context.Background(), "home-server") + if err != nil { + t.Fatal(err) + } + return inv, link.Enrolment{Inventory: inv}, node.ID +} + +func ordered(declared string, epoch, sequence, report int64, applied ...string) link.Report { + return link.Report{Node: "home-server", Declared: declared, Applied: applied, + Order: link.Order{Epoch: epoch, Sequence: sequence}, ReportSequence: report} +} + +// Issue 267, ordered: a reconcile's account of declaration 11 reaches the mesh after the apply's of 12. +func TestAnAccountOfAnOlderDeclarationIsRefusedByItsSequence(t *testing.T) { + inv, heard, id := anOrderedMachine(t) + ctx := context.Background() + before := countFor(link.WriterNodeEngine("home-server")) + if _, err := heard.Heard(ctx, ordered("d12", 57, 12, 41, "a", "b")); err != nil { + t.Fatal(err) + } + if _, err := heard.Heard(ctx, ordered("d11", 57, 11, 40, "a")); err != nil { + t.Fatal(err) + } + doing, _, err := inv.DoingOf(ctx, "home-server") + if err != nil || doing.Declared != "d12" || doing.Applied != 2 { + t.Fatalf("the older account replaced the newer: %+v (%v)", doing, err) + } + if got := countFor(link.WriterNodeEngine("home-server")) - before; got != 1 { + t.Fatalf("the refusal was counted %d times, want once", got) + } + kept, err := inv.KeptOrder(ctx, id) + if err != nil || kept != (inventory.ReportOrder{Epoch: 57, Sequence: 12, ReportSequence: 41}) { + t.Fatalf("the order kept is %+v (%v)", kept, err) + } +} + +func TestAnOlderReportOfTheSameDeclarationIsRefused(t *testing.T) { + inv, heard, _ := anOrderedMachine(t) + ctx := context.Background() + if _, err := heard.Heard(ctx, ordered("d12", 57, 12, 41, "a", "b")); err != nil { + t.Fatal(err) + } + failed := ordered("d12", 57, 12, 40) + failed.Failed = map[string]string{"b": "it failed a moment before it applied"} + if _, err := heard.Heard(ctx, failed); err != nil { + t.Fatal(err) + } + doing, _, _ := inv.DoingOf(ctx, "home-server") + if doing.Outcome != inventory.OutcomeApplied { + t.Fatalf("an older report of the same declaration replaced the newer: %+v", doing) + } + // A newer report of it — the next reconcile — is kept. + again := ordered("d12", 57, 12, 42) + again.Failed = map[string]string{"b": "it failed since"} + if _, err := heard.Heard(ctx, again); err != nil { + t.Fatal(err) + } + if doing, _, _ := inv.DoingOf(ctx, "home-server"); doing.Outcome != inventory.OutcomeFailed { + t.Fatalf("a newer report of the same declaration was not kept: %+v", doing) + } +} + +// An ordered account is judged by its order, not by the digest the mesh last sent: the account of 12 +// is kept although the mesh has sent 13 since — it is still the newest account of the machine. +func TestAnOrderedAccountIsNotJudgedByTheDigestSent(t *testing.T) { + inv, heard, id := anOrderedMachine(t) + ctx := context.Background() + if err := inv.RecordSent(ctx, id, "d13", nil); err != nil { + t.Fatal(err) + } + if _, err := heard.Heard(ctx, ordered("d12", 57, 12, 41, "a")); err != nil { + t.Fatal(err) + } + if doing, said, _ := inv.DoingOf(ctx, "home-server"); !said || doing.Declared != "d12" { + t.Fatalf("the newest account the machine gave was set aside by the digest: %+v", doing) + } + // And the machine reads an epoch: it orders its reports. + if reads, err := inv.ReadsEpoch(ctx, id); err != nil || !reads { + t.Fatalf("a machine whose reports carry a report sequence is not recorded as reading an epoch: %v", err) + } +} + +// A node-engine rolled back to one that orders nothing: its account is taken by the digest rule, the +// order kept is cleared, and it is no longer sent an epoch. +func TestAnUnorderedAccountClearsTheOrderAndTheEpoch(t *testing.T) { + inv, heard, id := anOrderedMachine(t) + ctx := context.Background() + if _, err := heard.Heard(ctx, ordered("d12", 57, 12, 41, "a")); err != nil { + t.Fatal(err) + } + if err := inv.RecordSent(ctx, id, "d13", nil); err != nil { + t.Fatal(err) + } + if _, err := heard.Heard(ctx, link.Report{Node: "home-server", Declared: "d13", Applied: []string{"a", "b"}}); err != nil { + t.Fatal(err) + } + if doing, _, _ := inv.DoingOf(ctx, "home-server"); doing.Declared != "d13" { + t.Fatalf("an older node-engine's account of what was sent was not kept: %+v", doing) + } + if kept, _ := inv.KeptOrder(ctx, id); kept != (inventory.ReportOrder{}) { + t.Fatalf("an unordered account left the order kept: %+v", kept) + } + if reads, _ := inv.ReadsEpoch(ctx, id); reads { + t.Fatal("a node-engine that orders nothing is still sent an epoch") + } + // The next ordered account, whatever its numbers, has nothing to be older than. + if _, err := heard.Heard(ctx, ordered("d14", 58, 14, 1, "a")); err != nil { + t.Fatal(err) + } + if doing, _, _ := inv.DoingOf(ctx, "home-server"); doing.Declared != "d14" { + t.Fatalf("the first ordered account after an unordered one was refused: %+v", doing) + } +} + +// A stale refusal names its writer by the epoch the refused declaration claimed, and a refusal whose +// own report was lost is still counted from the machine's own tally. +func TestAStaleRefusalIsCountedByItsWriter(t *testing.T) { + count := link.NewRefusalCount() + now := time.Now() + refusal := link.Report{Node: "anchor", Refused: "older", Order: link.Order{Epoch: 41, Sequence: 11}, + OlderThan: &link.Order{Epoch: 57, Sequence: 12}, ReportSequence: 9, RefusedOlder: 3} + if !refusal.StaleRefusalOf() { + t.Fatal("a refusal naming the order it holds is not stale") + } + count.Lifetime("anchor", 2, now) // the machine's tally, as the controller first heard it + count.Refused(link.Refusal{Writer: link.WriterEpoch(refusal.Epoch), Epoch: refusal.Epoch, Receiver: "anchor", At: now}) + count.Lifetime("anchor", refusal.RefusedOlder, now) + count.Lifetime("anchor", 5, now) // two more refused, their reports lost + got := count.Within(now.Add(-time.Minute)) + if len(got) != 2 || got[0].Count != 2 || got[0].Epoch != 0 || got[1].Writer != "the controller of epoch 41" || + got[1].Count != 1 || got[1].Receivers[0] != "anchor" { + t.Fatalf("the refusals by writer are %+v", got) + } +} + +// countFor is how many refusals of a writer this process has heard in the last minute. +func countFor(writer string) int { + for _, w := range link.StaleRefusals.Within(time.Now().Add(-time.Minute)) { + if w.Writer == writer { + return w.Count + } + } + return 0 +} diff --git a/internal/link/order.go b/internal/link/order.go new file mode 100644 index 0000000..311bd0f --- /dev/null +++ b/internal/link/order.go @@ -0,0 +1,118 @@ +package link + +import "strconv" + +// The order a declaration carries, and the order a report about one carries back (novox/hq to-be 45 +// §6, ADR 0227 rule 2). +// +// **This file is the controller's side of the contract with the node-engine for Phase 2**, and the one +// place it is written here. It mirrors mesh-host internal/link/messages.go (`Order`, and the report's +// `report_sequence`, `older_than`, `refused_older`) field for field and rule for rule, as Report has +// always mirrored the host's report: a key added on one side and not the other is a key the other side +// does not know exists. +// +// **The wire.** +// +// - A declaration's order is two top-level keys of its signed envelope, beside "declaration" and +// "resources": `sequence` (since issue 107) and `epoch` (new). Inside the signed bytes, so a message +// cannot be given a newer order than the controller gave it. +// - A report carries the order of the declaration it is about as the same two top-level keys, beside +// "declared"; `report_sequence`, the node-engine's own number for the report; `older_than`, on a +// refusal of a declaration older than one it applied, the order of the one it holds; and +// `refused_older`, how many it has refused so, ever, on every report. +// +// Every key is optional and absent when zero, and zero is "no order claimed", never "first". So each +// side reads the other's older shape: +// +// - **An older node-engine with this controller.** It decodes a declaration strictly and refuses a key +// it does not know, whole — so `epoch` is sent only to a machine whose latest report carried a +// `report_sequence`, which a node-engine that reads the epoch always does. Until then it is sent the +// sequence alone, as today; its reports carry no report sequence and are judged by the digest they +// name, as today (issue 267). +// - **A newer node-engine with an older controller.** It is sent no epoch, which claims none: it +// refuses nothing by epoch. The keys it adds to a report are fields the older controller ignores. +// - **A controller rolled back to a build without the lease** sends no epoch again and is not refused +// for it: a node-engine refuses by epoch only when both declarations claim one. That gives up the +// epoch's protection while such a build runs — the price of a rollback that cannot strand every +// machine. +// +// **Epoch** is the controller lease's epoch (to-be 45 §6): the lease bucket's revision at which the +// instance that composed the declaration took the lease. It only grows: a later holder's is higher, and +// the controller never issues one at or under the highest it has issued (internal/lease, the floor). +// **Sequence** is the declaration's number for that machine, one higher every send, taken from the +// controller's store before the declaration is composed (issues 107, 204). + +// Order is where a declaration stands among everything the mesh has sent a machine: the lease epoch it +// was sent under and its sequence. Zero in either claims none. +type Order struct { + Epoch int64 `json:"epoch,omitempty"` + Sequence int64 `json:"sequence,omitempty"` +} + +// Older is the node-engine's rule, as mesh-host states it: a declaration of this order is older than +// one of order `than` the machine applied **only when both claim an epoch** — and then when its epoch +// is lower, or its epoch is the same and its sequence lower (both claimed). A higher epoch is a new lease +// holder and is never older, whatever its sequence. Here so the controller's tests state what the +// machines will do with what it sends. +func (o Order) Older(than Order) bool { + if o.Epoch <= 0 || than.Epoch <= 0 { + return false + } + if o.Epoch != than.Epoch { + return o.Epoch < than.Epoch + } + return o.Sequence > 0 && than.Sequence > 0 && o.Sequence < than.Sequence +} + +// Words is an order as a person reads it in a log line. Not String: Report embeds Order, and a Stringer +// promoted onto every report would print each one as its order alone. +func (o Order) Words() string { + switch { + case o.Epoch > 0: + return "epoch " + strconv.FormatInt(o.Epoch, 10) + ", sequence " + strconv.FormatInt(o.Sequence, 10) + case o.Sequence > 0: + return "sequence " + strconv.FormatInt(o.Sequence, 10) + ", no epoch" + } + return "no order" +} + +// Account is the order of one account of a machine — a report about a declaration: that declaration's +// order and the node-engine's own number for the report. +type Account struct { + Order + ReportSequence int64 +} + +// AccountOf is a report's account order. +func AccountOf(r Report) Account { return Account{Order: r.Order, ReportSequence: r.ReportSequence} } + +// Ordered says a report comes from a node-engine that orders its reports: it carries a report sequence. +// Such a report is judged by its order (OlderThan); one without is judged by the digest it names, as +// before (issue 267). +func (r Report) Ordered() bool { return r.ReportSequence > 0 } + +// ReadsEpoch says the node-engine that made the report takes an epoch in a declaration: one that orders +// its reports does (mesh-host: "its presence also says this host reads a declaration's epoch"). +func (r Report) ReadsEpoch() bool { return r.ReportSequence > 0 } + +// StaleRefusalOf says a report refuses a declaration older than one the machine applied: the node-engine +// names the order it holds (`older_than`), or, from a node-engine older than that, says so in the words +// of its sequence refusal. +func (r Report) StaleRefusalOf() bool { + return r.OlderThan != nil || (r.Refused != "" && IsStaleRefusal(r.Refused)) +} + +// OlderThan is the controller's rule (to-be 45 §6): whether this account is older than the one kept for +// the machine — **by epoch, then sequence, then report sequence**, each compared only where both claim +// one. An account of a declaration of an older epoch is refused; of an older sequence, refused; an older +// report of the same declaration (a reconcile's, overtaken by the apply's — issue 267), refused. Equal is +// the same report again, redelivered, and is not refused. +func (a Account) OlderThan(kept Account) bool { + if a.Epoch > 0 && kept.Epoch > 0 && a.Epoch != kept.Epoch { + return a.Epoch < kept.Epoch + } + if a.Sequence > 0 && kept.Sequence > 0 && a.Sequence != kept.Sequence { + return a.Sequence < kept.Sequence + } + return a.ReportSequence > 0 && kept.ReportSequence > 0 && a.ReportSequence < kept.ReportSequence +} diff --git a/internal/link/order_test.go b/internal/link/order_test.go new file mode 100644 index 0000000..a215ba0 --- /dev/null +++ b/internal/link/order_test.go @@ -0,0 +1,113 @@ +package link + +import ( + "encoding/json" + "testing" +) + +// The contract of order.go, case by case: what a node-engine refuses of a declaration (the rule +// mesh-host states, restated so the controller's tests say what the machines do with what it sends), +// what the controller refuses of a report, and the bytes on the wire both ways. + +func TestADeclarationIsOlderOnlyWhenBothClaimAnEpoch(t *testing.T) { + for _, c := range []struct { + name string + arriving Order + held Order + older bool + }{ + {"a newer sequence of the same epoch", Order{57, 12}, Order{57, 11}, false}, + {"the same declaration again (a reconcile)", Order{57, 12}, Order{57, 12}, false}, + {"a lower sequence of the same epoch", Order{57, 11}, Order{57, 12}, true}, + // Issue 204: a controller that lost its lease goes on sending. + {"an older epoch, whatever its sequence", Order{41, 99}, Order{57, 12}, true}, + {"a newer epoch, whatever its sequence", Order{57, 3}, Order{41, 99}, false}, + {"no epoch arriving: a rolled-back controller is not stranded", Order{0, 11}, Order{57, 12}, false}, + {"no epoch held: what the machine held before any lease", Order{57, 11}, Order{0, 12}, false}, + {"no order at all", Order{}, Order{57, 12}, false}, + } { + t.Run(c.name, func(t *testing.T) { + if got := c.arriving.Older(c.held); got != c.older { + t.Fatalf("%s against %s: older %v, want %v", c.arriving.Words(), c.held.Words(), got, c.older) + } + }) + } +} + +func TestAnAccountIsOlderByEpochThenSequenceThenReportSequence(t *testing.T) { + for _, c := range []struct { + name string + arriving Account + kept Account + older bool + }{ + // Issue 267: a reconcile's account of declaration 11 arrives after the apply's of 12. + {"an account of an older declaration", Account{Order{57, 11}, 40}, Account{Order{57, 12}, 41}, true}, + {"an older report of the same declaration", Account{Order{57, 12}, 40}, Account{Order{57, 12}, 41}, true}, + {"a newer report of the same declaration (a reconcile after the apply)", + Account{Order{57, 12}, 42}, Account{Order{57, 12}, 41}, false}, + {"the same report again (redelivered)", Account{Order{57, 12}, 41}, Account{Order{57, 12}, 41}, false}, + {"an account of a newer declaration, whatever its report sequence", + Account{Order{57, 13}, 1}, Account{Order{57, 12}, 41}, false}, + {"an account of an older epoch", Account{Order{41, 99}, 50}, Account{Order{57, 12}, 41}, true}, + {"an account of a newer epoch", Account{Order{58, 1}, 2}, Account{Order{57, 12}, 41}, false}, + {"no epoch either side: the sequences", Account{Order{0, 11}, 50}, Account{Order{0, 12}, 41}, true}, + {"no report sequence: the declaration's alone", Account{Order{57, 12}, 0}, Account{Order{57, 12}, 41}, false}, + {"an unordered declaration: the report sequences", Account{Order{}, 40}, Account{Order{}, 41}, true}, + {"nothing kept yet", Account{Order{57, 3}, 1}, Account{}, false}, + } { + t.Run(c.name, func(t *testing.T) { + if got := c.arriving.OlderThan(c.kept); got != c.older { + t.Fatalf("%+v against %+v: older %v, want %v", c.arriving, c.kept, got, c.older) + } + }) + } +} + +// The bytes, as mesh-host's report writes them: every key optional, absent when zero. +func TestTheOrderOnTheWire(t *testing.T) { + // A stale refusal, as the node-engine says it: the refused declaration's order at the top, the one + // it holds in older_than, and how many it has refused ever. + raw := []byte(`{"node":"anchor","refused":"older","declared":"d1","epoch":41,"sequence":11,` + + `"report_sequence":7,"older_than":{"epoch":57,"sequence":12},"refused_older":3}`) + var r Report + if err := json.Unmarshal(raw, &r); err != nil { + t.Fatal(err) + } + if r.Order != (Order{41, 11}) || r.ReportSequence != 7 || r.OlderThan == nil || *r.OlderThan != (Order{57, 12}) || + r.RefusedOlder != 3 { + t.Fatalf("a node-engine's stale refusal reads as %+v", r) + } + if !r.Ordered() || !r.ReadsEpoch() || !r.StaleRefusalOf() { + t.Fatalf("a node-engine that orders its reports reads as one that does not: %+v", r) + } + + // A report from a node-engine older than the contract says none of it. + var older Report + if err := json.Unmarshal([]byte(`{"node":"anchor","applied":["a"],"declared":"d1"}`), &older); err != nil { + t.Fatal(err) + } + if older.Ordered() || older.ReadsEpoch() || older.StaleRefusalOf() { + t.Fatalf("an older node-engine's report reads as ordered: %+v", older) + } + // Its stale refusal, in the words it has always used, is still one. + older.Refused = "this declaration " + StaleRefusal + ": it is sequence 3" + if !older.StaleRefusalOf() { + t.Fatal("an older node-engine's refusal of an older sequence does not read as stale") + } + + // And a report with no order puts no order key on the wire. + raw, err := json.Marshal(Report{Node: "anchor", Declared: "d1"}) + if err != nil { + t.Fatal(err) + } + var keys map[string]any + if err := json.Unmarshal(raw, &keys); err != nil { + t.Fatal(err) + } + for _, key := range []string{"epoch", "sequence", "report_sequence", "older_than", "refused_older"} { + if _, there := keys[key]; there { + t.Fatalf("an unordered report carries %s: %s", key, raw) + } + } +} diff --git a/internal/link/protocol.go b/internal/link/protocol.go index 3a08390..ec1a9c8 100644 --- a/internal/link/protocol.go +++ b/internal/link/protocol.go @@ -178,6 +178,23 @@ type Report struct { // the same way, as the `sent` digest the mesh recorded. Which declaration, not when. Declared string `json:"declared,omitempty"` + // Order is where the declaration this report is about stands — its `epoch` and `sequence` as the + // declaration carried them — so the mesh keeps accounts by what they are about rather than by when + // they arrived (novox/hq to-be 45 §6; the contract is order.go). Two top-level keys; absent for a + // declaration that claimed no order, and from a node-engine older than the contract. + Order + // ReportSequence is the node-engine's own number for this report: one higher for every report it + // makes, kept on disk across restarts and self-updates. Zero claims none — every report an older + // node-engine makes. Its presence also says the node-engine reads a declaration's epoch. + ReportSequence int64 `json:"report_sequence,omitempty"` + // OlderThan is set on a report refusing a declaration older than one the machine applied: the order + // of the one it holds. The refused declaration is the report's own Declared and Order — whose epoch + // names the controller that sent it (S13). + OlderThan *Order `json:"older_than,omitempty"` + // RefusedOlder is how many declarations the node-engine has refused as older, ever, on every report: + // a refusal whose own report was lost is still counted from the next. + RefusedOlder int64 `json:"refused_older,omitempty"` + // Held is what an adopted node found and is keeping as it was until its module is taken // (novox/hq ADR 0100). Without it an adopted node reads as converged. Held []Held `json:"held,omitempty"` diff --git a/internal/link/serve.go b/internal/link/serve.go index e63f02f..6094329 100644 --- a/internal/link/serve.go +++ b/internal/link/serve.go @@ -317,6 +317,11 @@ func (s *Server) reported(ctx context.Context, m Control) { // the server's, it comes back after the newer was applied whatever the controller does. outstanding := s.outstanding(ctx, report.Node) declaredIn := staleAgainst(report) + if report.Ordered() { + // Kept by its order, not by the digest the mesh last sent (novox/hq to-be 45 §6): the + // listener refuses an older account under the one rule, whatever it is about. + declaredIn = "" + } if Superseded(declaredIn, outstanding) { s.log.Printf("set aside %s: the mesh has moved past that declaration", what) _ = m.Took() diff --git a/internal/link/watched.go b/internal/link/watched.go index f93a680..f21206b 100644 --- a/internal/link/watched.go +++ b/internal/link/watched.go @@ -5,6 +5,8 @@ import ( "encoding/json" "errors" "fmt" + "slices" + "sort" "strings" "sync" "sync/atomic" @@ -176,48 +178,130 @@ const StaleRefusal = "is older than what the mesh last said to this node" // IsStaleRefusal says a report's refusal is the node-engine refusing a declaration older than it holds. func IsStaleRefusal(refused string) bool { return strings.Contains(refused, StaleRefusal) } -// RefusalCount keeps when each machine refused a stale declaration, for S13 (novox/hq to-be 45 §3): -// more than five from one writer in five minutes is a writer sending what it has moved past. +// A Refusal is one receiver refusing something older than what it holds (novox/hq to-be 45 §6, ADR 0227 +// rule 2): a machine refusing a declaration, or this controller refusing an account. +type Refusal struct { + // Writer is who sent what was refused, in the mesh's words: WriterEpoch for a declaration, a + // machine's node-engine for an account. + Writer string + // Epoch is the lease epoch the refused declaration claimed, for a controller to be named by; zero + // for a declaration that claimed none, and for an account. + Epoch int64 + // Receiver is the machine, or "controller", that refused it. + Receiver string + At time.Time +} + +// WriterEpoch is how a refused declaration's writer is said before it is named: by the epoch it +// claimed, or as a controller that claimed none. +func WriterEpoch(epoch int64) string { + if epoch > 0 { + return fmt.Sprintf("the controller of epoch %d", epoch) + } + return "a controller that claimed no epoch" +} + +// WriterNodeEngine is a machine's node-engine as the writer of an account the controller refused. +func WriterNodeEngine(node string) string { return "the node-engine on " + node } + +// RefusalCount keeps every stale refusal heard, for S13 (novox/hq to-be 45 §3): more than five from +// one writer in five minutes is a writer sending what it — or the mesh — has moved past. type RefusalCount struct { - mu sync.Mutex - per map[string][]time.Time + mu sync.Mutex + list []Refusal + // lifetime is each machine's own count of declarations it refused as older, as its last report + // said it (`refused_older`), and named how many of those this process heard as refusal reports + // since: what the count rose by beyond them is refusals whose reports never arrived. + lifetime map[string]int64 + named map[string]int64 } // StaleRefusals is this process's count. -var StaleRefusals = &RefusalCount{per: map[string][]time.Time{}} +var StaleRefusals = NewRefusalCount() -// Refused records one refusal by a machine. -func (r *RefusalCount) Refused(node string, at time.Time) { - r.mu.Lock() - defer r.mu.Unlock() - r.per[node] = append(r.per[node], at) +// NewRefusalCount is an empty count. +func NewRefusalCount() *RefusalCount { + return &RefusalCount{lifetime: map[string]int64{}, named: map[string]int64{}} } -// Within is how many refusals each machine made since a moment; older ones are forgotten. -func (r *RefusalCount) Within(since time.Time) map[string]int { +// Refused records one refusal. +func (r *RefusalCount) Refused(f Refusal) { r.mu.Lock() defer r.mu.Unlock() - out := map[string]int{} - for node, times := range r.per { - var kept []time.Time - for _, t := range times { - if !t.Before(since) { - kept = append(kept, t) - } - } - if len(kept) == 0 { - delete(r.per, node) + r.list = append(r.list, f) + if f.Writer != WriterNodeEngine(f.Receiver) { + r.named[f.Receiver]++ + } +} + +// Lifetime reads a machine's own count of declarations it refused as older, from any report: what it +// rose by since the last, beyond the refusals heard by name, is recorded as refused by a writer the +// mesh never heard named — the refusal's report was lost, and the count is still a fact. +func (r *RefusalCount) Lifetime(node string, total int64, at time.Time) { + r.mu.Lock() + defer r.mu.Unlock() + before, known := r.lifetime[node] + r.lifetime[node] = total + named := r.named[node] + r.named[node] = 0 + if !known || total <= before { + return // the first word since this controller started, or a node-engine that started over + } + for missing := total - before - named; missing > 0; missing-- { + r.list = append(r.list, Refusal{Writer: "a writer whose refused declaration was never reported", + Receiver: node, At: at}) + } +} + +// WriterRefusals is one writer's refusals within a window. +type WriterRefusals struct { + Writer string + Epoch int64 + Count int + Receivers []string + Last time.Time +} + +// Within is every writer's refusals since a moment, most first; older ones are forgotten. +func (r *RefusalCount) Within(since time.Time) []WriterRefusals { + r.mu.Lock() + defer r.mu.Unlock() + kept := r.list[:0] + by := map[string]*WriterRefusals{} + var order []string + for _, f := range r.list { + if f.At.Before(since) { continue } - r.per[node] = kept - out[node] = len(kept) + kept = append(kept, f) + w, ok := by[f.Writer] + if !ok { + w = &WriterRefusals{Writer: f.Writer, Epoch: f.Epoch} + by[f.Writer] = w + order = append(order, f.Writer) + } + w.Count++ + if !slices.Contains(w.Receivers, f.Receiver) { + w.Receivers = append(w.Receivers, f.Receiver) + } + if f.At.After(w.Last) { + w.Last = f.At + } } + r.list = kept + out := make([]WriterRefusals, 0, len(order)) + for _, writer := range order { + w := by[writer] + sort.Strings(w.Receivers) + out = append(out, *w) + } + sort.SliceStable(out, func(i, j int) bool { return out[i].Count > out[j].Count }) return out } // holding is whether this process holds the controller's consumer of what nodes say: the controller -// acting, not one standing by for another (issue 213). Until the lease (to-be 45 §6) it is how a -// watchdog knows it is the one that hears. +// acting, not one standing by for another (issue 213). Beside the lease (to-be 45 §6), which decides +// who may act, it is how a watchdog knows this process is the one that hears. var holding atomic.Bool // Holding says this process is the controller acting now. diff --git a/internal/lint/emptyonerror.go b/internal/lint/emptyonerror.go new file mode 100644 index 0000000..c3d7c0b --- /dev/null +++ b/internal/lint/emptyonerror.go @@ -0,0 +1,237 @@ +// Package lint holds the checks this repository runs over its own source (novox/hq to-be 45 Phase 2). +// +// **Empty on error** (ADR 0227 rule 4, "nothing is dropped silently"): a reader that cannot read and +// answers an empty collection is a reader whose caller cannot tell *nothing is there* from *I could not +// tell* — issue 241's unreadable contributions file read as "nobody asks", and seven databases were +// dropped. EmptyOnError finds every error branch that answers an empty collection with no error; the +// test beside it fails the build on each one that does not say, where it does it, why the empty answer +// is the truth (a `// empty-on-error: ` comment on the return or the line above). +package lint + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "io/fs" + "path/filepath" + "sort" + "strings" +) + +// Allow is the comment that says, at the return, why an empty answer to an error is the truth. +const Allow = "empty-on-error:" + +// Finding is one error branch answering an empty collection. +type Finding struct { + File string + Line int + Func string + // Allowed is the reason given where it was allowed; empty for a finding that is a failure. + Allowed string +} + +func (f Finding) String() string { return fmt.Sprintf("%s:%d (%s)", f.File, f.Line, f.Func) } + +// EmptyOnError walks every Go file under root but tests, vendor/ and testdata/, and answers every +// return inside an `if != nil` branch that answers an empty collection — nil, or a literal with no +// elements, where the function answers a slice or a map — and no error: a nil error, or none declared. +func EmptyOnError(root string) ([]Finding, error) { + var out []Finding + fset := token.NewFileSet() + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + switch d.Name() { + case "vendor", "testdata", ".git", "node_modules": + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + file, err := parser.ParseFile(fset, path, nil, parser.ParseComments) + if err != nil { + return fmt.Errorf("%s cannot be read: %w", path, err) + } + rel, _ := filepath.Rel(root, path) + out = append(out, inFile(fset, file, rel)...) + return nil + }) + sort.Slice(out, func(i, j int) bool { + if out[i].File != out[j].File { + return out[i].File < out[j].File + } + return out[i].Line < out[j].Line + }) + return out, err +} + +// inFile is every finding in one file. +func inFile(fset *token.FileSet, file *ast.File, rel string) []Finding { + // Every comment's text by the line it ends on, so an allowance is read on the return's line or the + // line above it. + comments := map[int]string{} + for _, group := range file.Comments { + for _, c := range group.List { + comments[fset.Position(c.End()).Line] += c.Text + } + } + var out []Finding + var walk func(name string, results *ast.FieldList, body *ast.BlockStmt) + walk = func(name string, results *ast.FieldList, body *ast.BlockStmt) { + if body == nil { + return + } + ast.Inspect(body, func(n ast.Node) bool { + switch n := n.(type) { + case *ast.FuncLit: + // Its returns are its own, judged against its own results. + walk(name+" (a function inside it)", n.Type.Results, n.Body) + return false + case *ast.IfStmt: + if !errNotNil(n.Cond) { + return true + } + for _, ret := range returnsIn(n.Body) { + if !emptyOnError(results, ret) { + continue + } + line := fset.Position(ret.Pos()).Line + f := Finding{File: rel, Line: line, Func: name} + for _, l := range []int{line, line - 1} { + if text, ok := comments[l]; ok { + if _, why, found := strings.Cut(text, Allow); found && strings.TrimSpace(why) != "" { + f.Allowed = strings.TrimSpace(why) + } + } + } + out = append(out, f) + } + } + return true + }) + } + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok { + continue + } + name := fn.Name.Name + if fn.Recv != nil && len(fn.Recv.List) > 0 { + name = "(" + exprString(fn.Recv.List[0].Type) + ")." + name + } + walk(name, fn.Type.Results, fn.Body) + } + return out +} + +// errNotNil is a condition ` != nil` where x is an error by its name: err, or one ending in Err. +func errNotNil(cond ast.Expr) bool { + found := false + ast.Inspect(cond, func(n ast.Node) bool { + b, ok := n.(*ast.BinaryExpr) + if !ok || b.Op != token.NEQ { + return true + } + x, xok := b.X.(*ast.Ident) + y, yok := b.Y.(*ast.Ident) + if xok && yok && y.Name == "nil" && (x.Name == "err" || strings.HasSuffix(x.Name, "Err")) { + found = true + } + return !found + }) + return found +} + +// returnsIn is the returns of a branch, not those of a function literal inside it. +func returnsIn(body *ast.BlockStmt) []*ast.ReturnStmt { + var out []*ast.ReturnStmt + ast.Inspect(body, func(n ast.Node) bool { + switch n := n.(type) { + case *ast.FuncLit: + return false + case *ast.ReturnStmt: + out = append(out, n) + } + return true + }) + return out +} + +// emptyOnError says a return answers an empty collection and no error, for a function's results. +func emptyOnError(results *ast.FieldList, ret *ast.ReturnStmt) bool { + if results == nil { + return false + } + var types []ast.Expr + for _, f := range results.List { + n := len(f.Names) + if n == 0 { + n = 1 + } + for i := 0; i < n; i++ { + types = append(types, f.Type) + } + } + if len(ret.Results) != len(types) { + return false // a bare return of named results, or a call answering them: not judged here + } + empty := false + for i, t := range types { + value := ret.Results[i] + if isError(t) { + if !isNil(value) { + return false // an error is answered: said, not dropped + } + continue + } + if isCollection(t) && isEmpty(value) { + empty = true + } + } + return empty +} + +func isError(t ast.Expr) bool { + id, ok := t.(*ast.Ident) + return ok && id.Name == "error" +} + +func isCollection(t ast.Expr) bool { + switch t := t.(type) { + case *ast.ArrayType: + return t.Len == nil + case *ast.MapType: + return true + } + return false +} + +func isNil(e ast.Expr) bool { + id, ok := e.(*ast.Ident) + return ok && id.Name == "nil" +} + +func isEmpty(e ast.Expr) bool { + if isNil(e) { + return true + } + lit, ok := e.(*ast.CompositeLit) + return ok && len(lit.Elts) == 0 +} + +func exprString(e ast.Expr) string { + switch e := e.(type) { + case *ast.StarExpr: + return "*" + exprString(e.X) + case *ast.Ident: + return e.Name + case *ast.IndexExpr: + return exprString(e.X) + } + return "?" +} diff --git a/internal/lint/emptyonerror_test.go b/internal/lint/emptyonerror_test.go new file mode 100644 index 0000000..2b71f83 --- /dev/null +++ b/internal/lint/emptyonerror_test.go @@ -0,0 +1,100 @@ +package lint + +import ( + "go/parser" + "go/token" + "strings" + "testing" +) + +// The lint, over this repository (novox/hq to-be 45 Phase 2, ADR 0227 rule 4 "how it is checked"): every +// error branch that answers an empty collection says why that is the truth, or the build fails naming it. +func TestNoReaderAnswersEmptyForAnError(t *testing.T) { + found, err := EmptyOnError("../..") + if err != nil { + t.Fatal(err) + } + allowed := 0 + for _, f := range found { + if f.Allowed != "" { + allowed++ + continue + } + t.Errorf("%s answers an empty collection and no error when it could not read: refuse by name "+ + "(return the error), or say at the return why empty is the truth (// %s )", f, Allow) + } + t.Logf("%d error branch(es) answer empty, each saying why", allowed) +} + +// The lint itself: what it finds, and what it does not. +func TestTheLintFindsEmptyOnError(t *testing.T) { + const src = `package x + +func readContributions() []string { + raw, err := read() + if err != nil { + return nil + } + return parse(raw) +} + +func consumers() ([]string, error) { + if err := load(); err != nil { + return []string{}, nil + } + return nil, nil +} + +func byName() (map[string]int, error) { + if err := load(); err != nil { + return nil, err + } + return map[string]int{}, nil +} + +func allowed() ([]string, error) { + if err := load(); err != nil { + // empty-on-error: a store not yet seeded holds no seats, and the caller keeps its defaults + return nil, nil + } + return nil, nil +} + +func notFound() ([]string, error) { + if errors.Is(err, ErrNoRows) { + return nil, nil + } + return nil, nil +} + +func inside() { + f := func() []int { + if readErr != nil { + return nil + } + return []int{1} + } + _ = f +} +` + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "x.go", src, parser.ParseComments) + if err != nil { + t.Fatal(err) + } + var failing, allowedFns []string + for _, f := range inFile(fset, file, "x.go") { + if f.Allowed != "" { + allowedFns = append(allowedFns, f.Func) + continue + } + failing = append(failing, f.Func) + } + want := "readContributions consumers inside (a function inside it)" + if strings.Join(failing, " ") != want { + t.Fatalf("the lint found %q, want %q", failing, want) + } + if strings.Join(allowedFns, " ") != "allowed" { + t.Fatalf("the allowance was not read: %q", allowedFns) + } +} diff --git a/vendor/github.com/novox/mesh-host/internal/declaration/declaration.go b/vendor/github.com/novox/mesh-host/internal/declaration/declaration.go index f64c02d..dc908cb 100644 --- a/vendor/github.com/novox/mesh-host/internal/declaration/declaration.go +++ b/vendor/github.com/novox/mesh-host/internal/declaration/declaration.go @@ -1297,6 +1297,16 @@ type Declaration struct { // superseded. Sequence int64 + // Epoch is the controller's lease epoch this declaration was sent under (novox/hq to-be 45 §6): + // the revision at which the sending controller took the lease. A controller that lost its lease + // and goes on sending sends an older epoch than the holder's, and the node-engine refuses what + // is older than what it applied. Zero is a declaration from a controller without a lease — every + // one sent before the lease existed — and carries no claim. + // + // Inside what is signed, beside the sequence, so a message cannot be given a newer epoch than + // the controller gave it. + Epoch int64 + // LeftOut names the modules of this machine's set the mesh left out of this declaration, // because a setting stored for one cannot compose with its definition (novox/hq ADR 0163, // rule 6). A machine is told everything or nothing about what it IS told; this is what it is @@ -1462,6 +1472,10 @@ type envelope struct { // Sequence is optional on the wire, so a controller that does not send one is still // understood: absent reads as zero, which is "no ordering claimed" rather than "first". Sequence int64 `json:"sequence,omitempty"` + // Epoch is optional on the wire as the sequence is: absent is a controller without a lease. + // **An older host refuses this key**, decoding strictly; a controller sends it only to a host + // whose reports carry a report sequence, which a host that reads it does. + Epoch int64 `json:"epoch,omitempty"` // LeftOut is optional on the wire too, and absent when nothing was left out (ADR 0163). LeftOut []string `json:"left_out,omitempty"` } @@ -1481,8 +1495,14 @@ func parse(raw []byte, allowActions bool) (*Declaration, error) { } d := &Declaration{Version: env.Version, For: env.For, Adoption: env.Adoption, Sequence: env.Sequence, - LeftOut: env.LeftOut} + Epoch: env.Epoch, LeftOut: env.LeftOut} var problems []string + if env.Sequence < 0 || env.Epoch < 0 { + // Below zero is no order any controller assigns, and read as "none claimed" it would let the + // declaration past every refusal of what is older. + problems = append(problems, fmt.Sprintf("an order below zero (epoch %d, sequence %d) is not "+ + "one the mesh assigns", env.Epoch, env.Sequence)) + } if len(env.LeftOut) > 0 && allowActions { // The bundle is carried with the binary and leaves nothing out: which module a setting // stopped composing for is the mesh's record (ADR 0163). diff --git a/vendor/modules.txt b/vendor/modules.txt index 56650f0..54ce9ae 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -41,7 +41,7 @@ github.com/nats-io/nkeys # github.com/nats-io/nuid v1.0.1 ## explicit github.com/nats-io/nuid -# github.com/novox/mesh-host v0.0.0 => git.novox.be/novox/mesh-host v0.0.0-20261006081854-6953b5bafdb2 +# github.com/novox/mesh-host v0.0.0 => git.novox.be/novox/mesh-host v0.0.0-20261006095519-3e80b7ae325e ## explicit; go 1.26.0 github.com/novox/mesh-host/internal/declaration github.com/novox/mesh-host/validate @@ -83,4 +83,4 @@ golang.org/x/text/transform golang.org/x/text/unicode/bidi golang.org/x/text/unicode/norm golang.org/x/text/width -# github.com/novox/mesh-host => git.novox.be/novox/mesh-host v0.0.0-20261006081854-6953b5bafdb2 +# github.com/novox/mesh-host => git.novox.be/novox/mesh-host v0.0.0-20261006095519-3e80b7ae325e