From ac42d89391be2e113f472ace7984be925bf0f9eb Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 5 Oct 2026 15:32:28 +0200 Subject: [PATCH] No change to a machine takes effect unseen (hq ADR 0217) Three guards, each silent when nothing is at stake: - settings: show reads a layer; set says what it adds, changes and removes, refuses a removal without --replace, and keeps the layer it replaced (settings_history, migration 0057). - push: plan --diff compares with what the machine was last sent, now kept as a summary that holds no file content; push with no machine needs --all. - a running container whose mount would point at another directory holds that machine's push until --move names the module; the other machines go ahead. --- cmd/mesh-controller/licence.go | 8 +- cmd/mesh-controller/modules.go | 92 ++++- cmd/mesh-controller/plan.go | 16 +- cmd/mesh-controller/push.go | 50 ++- cmd/mesh-controller/seatverbs.go | 18 +- cmd/mesh-controller/seatverbs_test.go | 6 +- cmd/mesh-controller/unseen.go | 335 ++++++++++++++++++ cmd/mesh-controller/unseen_test.go | 170 +++++++++ internal/catalogue/verbs.go | 27 +- internal/inventory/catalogue.go | 16 + .../0057-what-a-change-replaces.sql | 26 ++ internal/inventory/unseen.go | 120 +++++++ internal/inventory/unseen_test.go | 90 +++++ 13 files changed, 946 insertions(+), 28 deletions(-) create mode 100644 cmd/mesh-controller/unseen.go create mode 100644 cmd/mesh-controller/unseen_test.go create mode 100644 internal/inventory/migrations/0057-what-a-change-replaces.sql create mode 100644 internal/inventory/unseen.go create mode 100644 internal/inventory/unseen_test.go diff --git a/cmd/mesh-controller/licence.go b/cmd/mesh-controller/licence.go index 25d12ef..0b93e9f 100644 --- a/cmd/mesh-controller/licence.go +++ b/cmd/mesh-controller/licence.go @@ -232,7 +232,7 @@ func licenceKey(ctx context.Context, args []string) error { // and printing the value here would put the one copy that matters on a terminal. fmt.Printf("sealed to %d holder(s). The mesh has discarded the key and cannot read it back\n", sealed) - fmt.Printf(" run `push` to deliver it\n") + fmt.Printf(" run `push --behind` to deliver it\n") return nil } @@ -340,7 +340,7 @@ func licenceSetGrant(ctx context.Context, args []string) error { return err } fmt.Printf("%s now holds a refresh token for %s, sealed to that node's key and readable by it "+ - "alone.\n the control plane stored the box without opening it; run `push` to deliver it\n", + "alone.\n the control plane stored the box without opening it; run `push --behind` to deliver it\n", "the manager", name) return nil } @@ -423,7 +423,7 @@ func licenceSubmitRefresh(ctx context.Context, args []string) error { "manager node alone" } fmt.Printf("submitted a refresh for %s: a new access token sealed to %d holder(s), and %s.\n"+ - " run `push` to deliver it\n", name, sealed, rotatedNote) + " run `push --behind` to deliver it\n", name, sealed, rotatedNote) return nil } @@ -456,7 +456,7 @@ func licenceRefresh(ctx context.Context, args []string) error { return err } fmt.Printf("refreshed %s: a new access token sealed to %d holder(s), and the refresh token left "+ - "with its manager.\n run `push` to deliver it\n", name, sealed) + "with its manager.\n run `push --behind` to deliver it\n", name, sealed) return nil } diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 2042c6a..cf10b77 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -8,7 +8,6 @@ import ( "fmt" "net" "os" - "sort" "strings" "github.com/novox/mesh-controller/internal/broker" @@ -364,7 +363,8 @@ func assignCommand(ctx context.Context, verb string, args []string) error { func settingsCommand(ctx context.Context, args []string) error { if len(args) == 0 { - return errors.New("settings set [--node ], or settings clear [--node ]") + return errors.New("settings show [--node ] [--history], settings set " + + "[--node ] [--replace], or settings clear [--node ]") } open, err := openStores(ctx) if err != nil { @@ -375,6 +375,11 @@ func settingsCommand(ctx context.Context, args []string) error { set := flag.NewFlagSet("settings", flag.ContinueOnError) node := set.String("node", "", "one machine, rather than the whole mesh") + // **What a set removes is refused unless meant** (novox/hq ADR 0217). A layer is replaced whole, + // and on 2026-10-05 setting one placement dropped a machine's whole layer for a module without a + // word (issue 246). Adding and changing keys needs nothing; removing one needs this. + replace := set.Bool("replace", false, "for set: remove the keys the new layer does not name") + history := set.Bool("history", false, "for show: the layers this one replaced, the latest first") positionals, err := parseAround(set, args[1:]) if err != nil { return err @@ -402,17 +407,76 @@ func settingsCommand(ctx context.Context, args []string) error { if err := json.Unmarshal(raw, &values); err != nil { return fmt.Errorf("%s is not a settings file: %w", positionals[1], err) } + before, _, err := inv.Layer(ctx, *node, positionals[0]) + if err != nil { + return err + } + added, changed, removed := settingsChange(before, values) + if len(removed) > 0 && !*replace { + return fmt.Errorf("%s on %s: this layer would no longer set %s. A layer is replaced whole; "+ + "read it with `settings show %s%s` and include what should stay, or add --replace if the "+ + "removal is meant (novox/hq ADR 0217). Nothing was changed", + positionals[0], where, strings.Join(removed, ", "), positionals[0], nodeFlag(*node)) + } if err := inv.SetSettings(ctx, *node, positionals[0], values); err != nil { return err } - - var keys []string - for k := range values { - keys = append(keys, k) + fmt.Printf("%s on %s:\n", positionals[0], where) + if len(added)+len(changed)+len(removed) == 0 { + fmt.Println(" nothing changed") } - sort.Strings(keys) - fmt.Printf("%s on %s: %s\n", positionals[0], where, strings.Join(keys, ", ")) - fmt.Println(" run `push` to send it") + for _, p := range added { + fmt.Printf(" + %s\n", p) + } + for _, p := range changed { + fmt.Printf(" ~ %s\n", p) + } + for _, p := range removed { + fmt.Printf(" - %s\n", p) + } + if before != nil { + fmt.Printf(" the layer it replaced is kept: settings show %s%s --history\n", positionals[0], nodeFlag(*node)) + } + if *node != "" { + fmt.Printf(" run `plan %s --diff` to see what it changes, `push %s` to send it\n", *node, *node) + } else { + fmt.Println(" run `push --behind` to send it to the machines running it") + } + return nil + + case "show": + if len(positionals) != 1 { + return errors.New("settings show [--node ] [--history]") + } + if *history { + past, err := inv.SettingsHistory(ctx, *node, positionals[0]) + if err != nil { + return err + } + if len(past) == 0 { + fmt.Printf("%s on %s: no layer has been replaced\n", positionals[0], where) + return nil + } + for _, p := range past { + shown, _ := json.MarshalIndent(p.Values, " ", " ") + fmt.Printf("%s on %s, until %s (%s):\n %s\n", positionals[0], where, + p.ReplacedAt.Local().Format("2006-01-02 15:04:05"), p.ReplacedBy, shown) + } + return nil + } + values, has, err := inv.Layer(ctx, *node, positionals[0]) + if err != nil { + return err + } + if !has { + fmt.Printf("%s on %s: no layer — the module's definition says\n", positionals[0], where) + return nil + } + shown, err := json.MarshalIndent(values, "", " ") + if err != nil { + return err + } + fmt.Println(string(shown)) return nil case "clear": @@ -426,10 +490,18 @@ func settingsCommand(ctx context.Context, args []string) error { return nil default: - return fmt.Errorf("settings has no %q; it has set and clear", args[0]) + return fmt.Errorf("settings has no %q; it has show, set and clear", args[0]) } } +// nodeFlag is ` --node ` for a machine's layer, nothing for the whole mesh's. +func nodeFlag(node string) string { + if node == "" { + return "" + } + return " --node " + node +} + // describeOffers says what a module provides, and marks the ones answered from anywhere in the // mesh — because "provides a database" and "provides a shell" are read the same way and mean // entirely different things about where the answer has to be. diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index a5b773d..c3f51a8 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -980,12 +980,15 @@ func planCommand(ctx context.Context, args []string) error { // checking it against the host's own parser, most usefully, which is the only way to know // that what the control plane emits is what the host accepts. asJSON := set.Bool("json", false, "print the declaration this node would be sent") + // What a push would change, against what the machine was last sent (novox/hq ADR 0217): read + // before sending, so a change that removes, moves or recreates something is seen first. + asDiff := set.Bool("diff", false, "what a push would change on this node, against what it was last sent") positionals, err := parseAround(set, args) if err != nil { return err } if len(positionals) != 1 { - return errors.New("plan [--files] [--json]") + return errors.New("plan [--files] [--json] [--diff]") } args = positionals open, err := openStores(ctx) @@ -1002,6 +1005,17 @@ func planCommand(ctx context.Context, args []string) error { fmt.Printf("%s is assigned nothing\n", args[0]) return nil } + if *asDiff { + declared, err := declarationFor(ctx, open, args[0], plan, settings) + if err != nil { + return err + } + body, err := declared.Body() + if err != nil { + return err + } + return writePlanDiff(ctx, open.inventory, args[0], body) + } if *asJSON { declared, err := declarationFor(ctx, open, args[0], plan, settings) if err != nil { diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 380f627..95dea38 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -249,14 +249,31 @@ func pushCommand(ctx context.Context, args []string) error { // (novox/hq ADR 0010). 0 waits for nothing, which is the old fire-and-forget. wait := set.Duration("wait", 0, "for a named node, how long to wait for it to report applying what it was sent (0: do not wait)") + // Every machine at once is said, not defaulted to (novox/hq ADR 0217): a bare `push` sent the + // whole mesh on 2026-10-05 when its usage was wanted. + all := set.Bool("all", false, "every machine") + // A running module's data moving is sent only when said, per module (novox/hq ADR 0217). + move := set.String("move", "", "modules whose data may move with this push, comma-separated") positionals, err := parseAround(set, args) if err != nil { return err } args = positionals if len(args) > 1 { - return errors.New("push [] [--behind] — one node, or all of them") + return errors.New("push | push --behind | push --all — one machine, the ones behind, or all of them") } + if len(args) == 0 && !*behind && !*all { + return errors.New("push names a machine, or says --behind (the ones not running what they " + + "should) or --all (every machine) — a push to the whole mesh is not the default (novox/hq ADR 0217)") + } + if *all && (*behind || len(args) == 1) { + return errors.New("push --all is every machine; it takes no machine and no --behind") + } + movable := map[string]bool{} + for _, m := range splitModules(*move) { + movable[m] = true + } + var heldBack []string if len(args) == 1 && *behind { // Naming a machine and asking for the ones that need it are two different requests, and // guessing which was meant would sometimes push to a machine somebody did not name. @@ -396,6 +413,14 @@ func pushCommand(ctx context.Context, args []string) error { if err != nil { return err } + // A running module's data moving holds this machine, and only this one (novox/hq ADR 0217). + if moves, err := heldMoves(ctx, inv, s.node, body, movable); err != nil { + return err + } else if len(moves) > 0 { + sayHeld(os.Stdout, s.node, moves) + heldBack = append(heldBack, s.node) + continue + } if err := link.Declare(ctx, server.Bus(), ident, s.node, body, 15*time.Second); err != nil { return err } @@ -485,6 +510,14 @@ func pushCommand(ctx context.Context, args []string) error { return declared, err }, func(s readyNode, body []byte) error { + // The cascade is a push too, and held the same way (novox/hq ADR 0217). + if moves, err := heldMoves(ctx, inv, s.node, body, movable); err != nil { + return err + } else if len(moves) > 0 { + sayHeld(os.Stdout, s.node, moves) + heldBack = append(heldBack, s.node) + return nil + } if err := link.Declare(ctx, server.Bus(), ident, s.node, body, 15*time.Second); err != nil { return err @@ -516,6 +549,11 @@ func pushCommand(ctx context.Context, args []string) error { return err } } + if len(heldBack) > 0 { + sort.Strings(heldBack) + return fmt.Errorf("held %s: a running module's data would move — see above; nothing was sent "+ + "there (novox/hq ADR 0217)", strings.Join(heldBack, ", ")) + } return couldNotBeResolved(refusals, len(sending)) } @@ -1025,6 +1063,16 @@ func recordSent(ctx context.Context, inv *inventory.Inventory, node string, body if err := inv.RecordSent(kept, record.ID, digest); err != nil { return "", err } + // And what it was, summarised, so the next push can be compared with it (novox/hq ADR 0217). + // Never a reason to fail a send that is already away: a summary that cannot be kept means the + // next comparison has nothing to hold against, which is how every machine starts. + if summary, err := summarize(body); err == nil { + if raw, err := json.Marshal(summary); err == nil { + if err := inv.RecordSentSummary(kept, record.ID, raw); err != nil { + fmt.Fprintf(os.Stderr, "%s: what it was sent is not kept for comparison: %v\n", node, err) + } + } + } return digest, nil } diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 9d2bb65..96666b5 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -131,7 +131,12 @@ func argvFor(verb string, args map[string]any) ([]string, error) { // what a person at a shell does too. A tool call that blocked for a push's whole apply would // time out on every machine that takes a minute, and say nothing about the ones that did not. if n := str("node"); n != "" { - return []string{"push", n, "--wait", "0"}, nil + argv := []string{"push", n, "--wait", "0"} + // A running module's data moving is sent only when said (novox/hq ADR 0217). + if m := str("move"); m != "" { + argv = append(argv, "--move", m) + } + return argv, nil } return []string{"push", "--behind", "--wait", "0"}, nil case "rotate": @@ -160,6 +165,17 @@ func argvFor(verb string, args map[string]any) ([]string, error) { argv = []string{"settings", "clear", str("module")} case str("values") != "": argv = append(argv, str("values")) + // What a set removes is refused unless meant (novox/hq ADR 0217). + if str("replace") == "true" { + argv = append(argv, "--replace") + } + default: + // Neither values nor clear: the layer as it stands, which is what a caller reads before + // replacing it (novox/hq ADR 0217) — and with history, the layers it replaced. + argv = []string{"settings", "show", str("module")} + if str("history") == "true" { + argv = append(argv, "--history") + } } // Neither values nor clear: the command says its usage, which names both, and that is the // answer the caller needs — the same as `rotate` given half of either shape. diff --git a/cmd/mesh-controller/seatverbs_test.go b/cmd/mesh-controller/seatverbs_test.go index edfdb8e..dc07be3 100644 --- a/cmd/mesh-controller/seatverbs_test.go +++ b/cmd/mesh-controller/seatverbs_test.go @@ -86,9 +86,11 @@ func TestSettingsSetsOrClearsALayer(t *testing.T) { if strings.Join(argv, " ") != "settings clear dnsmasq" { t.Fatalf("clear for the mesh: %v", argv) } + // Neither values nor clear reads the layer as it stands (novox/hq ADR 0217): what a caller reads + // before replacing it, where it used to fall to the command's usage. argv, _ = argvFor("settings", map[string]any{"module": "dnsmasq"}) - if strings.Join(argv, " ") != "settings set dnsmasq" { - t.Fatalf("a set with no values falls to the command's usage: %v", argv) + if strings.Join(argv, " ") != "settings show dnsmasq" { + t.Fatalf("a call with no values reads the layer: %v", argv) } } diff --git a/cmd/mesh-controller/unseen.go b/cmd/mesh-controller/unseen.go new file mode 100644 index 0000000..32b4beb --- /dev/null +++ b/cmd/mesh-controller/unseen.go @@ -0,0 +1,335 @@ +package main + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "os" + "reflect" + "sort" + "strings" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// No change to a machine takes effect unseen (novox/hq ADR 0217, to-be 44). +// +// Two incidents in two days had one shape: a change took effect that nobody saw before it did. A +// provisioner read an unreadable file as "nobody asks" and dropped seven databases (issue 241); one +// placement set for one module on one machine replaced its whole settings layer, and the plan then +// ran the module on an empty directory (issue 246). Here are the three guards: what a settings layer +// loses is said and refused unless meant; what a push will change can be read before it is sent; and +// a running container's data moving holds that machine's push until the operator says so. Each is +// silent when nothing is at stake, so an ordinary change goes exactly as before. + +// ---- 1. what a settings layer loses ------------------------------------------------------------ + +// settingsChange is what replacing one layer with another adds, changes and removes, by dotted path +// to each leaf — `places.config`, not only `places` — because a layer that keeps a key and loses one +// of its entries has lost something just the same: on 2026-10-05 a node's `places` kept its name and +// lost three of its four directories. +func settingsChange(before, after map[string]any) (added, changed, removed []string) { + was, now := leaves(before, ""), leaves(after, "") + for path, v := range now { + old, had := was[path] + switch { + case !had: + added = append(added, path) + case !reflect.DeepEqual(old, v): + changed = append(changed, path) + } + } + for path := range was { + if _, kept := now[path]; !kept { + removed = append(removed, path) + } + } + sort.Strings(added) + sort.Strings(changed) + sort.Strings(removed) + return added, changed, removed +} + +// leaves flattens a settings layer to its leaves by dotted path; a list is a leaf, compared whole. +func leaves(m map[string]any, prefix string) map[string]any { + out := map[string]any{} + for k, v := range m { + path := k + if prefix != "" { + path = prefix + "." + k + } + if inner, ok := v.(map[string]any); ok && len(inner) > 0 { + for p, leaf := range leaves(inner, path) { + out[p] = leaf + } + continue + } + out[path] = v + } + return out +} + +// ---- 2. what a push will change ----------------------------------------------------------------- + +// sentResource is what is kept of one resource a machine was sent: enough to say what changed and +// whether a container's data moved, and nothing that could carry a secret — a file's content is +// kept as a digest, never as text (the record lands in the store's own backups). +type sentResource struct { + ID string `json:"id"` + Type string `json:"type"` + // Digest is of the whole resource as it was sent. + Digest string `json:"digest"` + // Fields is each field's digest, so a change can be named by field. + Fields map[string]string `json:"fields"` + // Volumes is a container's mounts as sent — paths, which are not secrets, and what a moved + // data directory is read from. + Volumes []string `json:"volumes,omitempty"` +} + +// summarize is what is kept of a declaration body: its resources, in the order sent. +func summarize(body []byte) ([]sentResource, error) { + var envelope struct { + Resources []map[string]any `json:"resources"` + } + if err := json.Unmarshal(body, &envelope); err != nil { + return nil, fmt.Errorf("a declaration that is not one: %w", err) + } + out := make([]sentResource, 0, len(envelope.Resources)) + for _, r := range envelope.Resources { + s := sentResource{ID: fmt.Sprint(r["id"]), Type: fmt.Sprint(r["type"]), Digest: digestValue(r), + Fields: map[string]string{}} + for k, v := range r { + s.Fields[k] = digestValue(v) + } + if s.Type == "container" { + if vols, ok := r["volumes"].([]any); ok { + for _, v := range vols { + s.Volumes = append(s.Volumes, fmt.Sprint(v)) + } + } + } + out = append(out, s) + } + return out, nil +} + +func digestValue(v any) string { + raw, _ := json.Marshal(v) + sum := sha256.Sum256(raw) + return hex.EncodeToString(sum[:8]) +} + +// changedResource is one resource sent differently, with the fields that differ. +type changedResource struct { + ID, Type string + Fields []string +} + +// sentDiff is what would be sent now against what was sent last, by resource id. +type sentDiff struct { + Added, Removed []string + Changed []changedResource +} + +func (d sentDiff) empty() bool { + return len(d.Added) == 0 && len(d.Removed) == 0 && len(d.Changed) == 0 +} + +func diffSent(before, after []sentResource) sentDiff { + was := map[string]sentResource{} + for _, r := range before { + was[r.ID] = r + } + var d sentDiff + seen := map[string]bool{} + for _, r := range after { + seen[r.ID] = true + old, had := was[r.ID] + if !had { + d.Added = append(d.Added, r.ID) + continue + } + if old.Digest == r.Digest { + continue + } + c := changedResource{ID: r.ID, Type: r.Type} + for k, v := range r.Fields { + if old.Fields[k] != v { + c.Fields = append(c.Fields, k) + } + } + for k := range old.Fields { + if _, kept := r.Fields[k]; !kept { + c.Fields = append(c.Fields, k) + } + } + sort.Strings(c.Fields) + d.Changed = append(d.Changed, c) + } + for _, r := range before { + if !seen[r.ID] { + d.Removed = append(d.Removed, r.ID) + } + } + sort.Strings(d.Added) + sort.Strings(d.Removed) + sort.Slice(d.Changed, func(a, b int) bool { return d.Changed[a].ID < d.Changed[b].ID }) + return d +} + +// writeDiff says a diff the way a person reads one: what is added, removed and changed, and a +// changed container's fields — a container sent differently is a container recreated. +func writeDiff(w io.Writer, node string, d sentDiff, moves []dataMove) { + if d.empty() { + fmt.Fprintf(w, "%s: nothing would change — it was last sent exactly this\n", node) + return + } + fmt.Fprintf(w, "%s would change:\n", node) + for _, id := range d.Added { + fmt.Fprintf(w, " + %s\n", id) + } + for _, id := range d.Removed { + fmt.Fprintf(w, " - %s\n", id) + } + for _, c := range d.Changed { + what := "changed" + if c.Type == "container" { + what = "recreated" + } + fmt.Fprintf(w, " ~ %s %s: %s\n", c.ID, what, strings.Join(c.Fields, ", ")) + } + writeMoves(w, node, moves) +} + +// ---- 3. a running module's data moving ------------------------------------------------------------ + +// dataMove is a container keeping a mount at the same place inside it with a different directory +// on the machine behind it: its data moving — or, as on 2026-10-05, a module about to start on an +// empty directory because the placement that pointed at its data was lost. +type dataMove struct { + Container, Inside, From, To string +} + +// Module is the module the container belongs to: resource ids are `.`. +func (m dataMove) Module() string { + module, _, _ := strings.Cut(m.Container, ".") + return module +} + +// movesIn is every data move between what a machine was sent and what it would be sent. A new +// container, a mount added or dropped and a changed image are not moves. +func movesIn(before, after []sentResource) []dataMove { + was := map[string]sentResource{} + for _, r := range before { + if r.Type == "container" { + was[r.ID] = r + } + } + var out []dataMove + for _, r := range after { + old, had := was[r.ID] + if r.Type != "container" || !had { + continue + } + from := mountSources(old.Volumes) + for inside, to := range mountSources(r.Volumes) { + if prev, mounted := from[inside]; mounted && prev != to { + out = append(out, dataMove{Container: r.ID, Inside: inside, From: prev, To: to}) + } + } + } + sort.Slice(out, func(a, b int) bool { + if out[a].Container != out[b].Container { + return out[a].Container < out[b].Container + } + return out[a].Inside < out[b].Inside + }) + return out +} + +// mountSources is a container's mounts by the place inside it: `source:inside[:options]`. +func mountSources(volumes []string) map[string]string { + out := map[string]string{} + for _, v := range volumes { + parts := strings.SplitN(v, ":", 3) + if len(parts) < 2 { + continue + } + out[parts[1]] = parts[0] + } + return out +} + +func writeMoves(w io.Writer, node string, moves []dataMove) { + for _, m := range moves { + fmt.Fprintf(w, " ! %s: %s moves from %s to %s\n", m.Container, m.Inside, m.From, m.To) + } +} + +// heldMoves is the moves in a push to one machine that the operator has not said to send (`--move +// `); empty when there is nothing to hold, including a machine never sent anything before. +func heldMoves(ctx context.Context, inv *inventory.Inventory, node string, body []byte, allowed map[string]bool) ([]dataMove, error) { + prev, err := inv.SentSummary(ctx, node) + if err != nil || len(prev) == 0 { + return nil, err + } + var before []sentResource + if err := json.Unmarshal(prev, &before); err != nil { + // A record this version cannot read compares with nothing: holding every push for it would + // stop the mesh on a format, which is not what is at stake. + return nil, nil + } + after, err := summarize(body) + if err != nil { + return nil, err + } + var held []dataMove + for _, m := range movesIn(before, after) { + if !allowed[m.Module()] { + held = append(held, m) + } + } + return held, nil +} + +// sayHeld tells the operator why a machine was not sent, and how to send it. +func sayHeld(w io.Writer, node string, moves []dataMove) { + modules := map[string]bool{} + for _, m := range moves { + modules[m.Module()] = true + } + var names []string + for m := range modules { + names = append(names, m) + } + sort.Strings(names) + fmt.Fprintf(w, "held %s: a running module's data would move (novox/hq ADR 0217)\n", node) + writeMoves(w, node, moves) + fmt.Fprintf(w, " if that is meant: push %s --move %s\n", node, strings.Join(names, ",")) +} + +// writePlanDiff says what sending body to a machine would change against what it was last sent. +func writePlanDiff(ctx context.Context, inv *inventory.Inventory, node string, body []byte) error { + after, err := summarize(body) + if err != nil { + return err + } + prev, err := inv.SentSummary(ctx, node) + if err != nil { + return err + } + if len(prev) == 0 { + fmt.Printf("%s: what it was last sent is not kept yet — the first push from this version keeps "+ + "it; %d resource(s) would be sent\n", node, len(after)) + return nil + } + var before []sentResource + if err := json.Unmarshal(prev, &before); err != nil { + return fmt.Errorf("%s: what it was last sent is kept in a form this version does not read: %w", node, err) + } + writeDiff(os.Stdout, node, diffSent(before, after), movesIn(before, after)) + return nil +} diff --git a/cmd/mesh-controller/unseen_test.go b/cmd/mesh-controller/unseen_test.go new file mode 100644 index 0000000..95fc245 --- /dev/null +++ b/cmd/mesh-controller/unseen_test.go @@ -0,0 +1,170 @@ +package main + +import ( + "bytes" + "context" + "reflect" + "strings" + "testing" +) + +// novox/hq ADR 0217: no change to a machine takes effect unseen. + +// The layer of 2026-10-05: a media server's node layer, and the one that replaced it, which kept +// `places` and lost three of its four directories and every other key. +var before = map[string]any{ + "puid": 1000.0, "pgid": 1000.0, + "expose": map[string]any{"32400": "anywhere"}, + "places": map[string]any{ + "config": map[string]any{"path": "/srv/media/config", "owner": "1000:1000"}, + "data": map[string]any{"path": "/srv/media/data", "owner": "1000:1000"}, + "transcode": map[string]any{"path": "/srv/media/temp", "owner": "1000:1000"}, + }, +} + +func TestASettingThatKeepsAKeyAndLosesItsEntriesIsSaidToRemoveThem(t *testing.T) { + after := map[string]any{"places": map[string]any{ + "previews": map[string]any{"path": "/srv/pool/previews", "owner": "1000:1000"}, + }} + added, changed, removed := settingsChange(before, after) + if !reflect.DeepEqual(added, []string{"places.previews.owner", "places.previews.path"}) { + t.Errorf("added %v", added) + } + if len(changed) != 0 { + t.Errorf("changed %v", changed) + } + for _, lost := range []string{"expose.32400", "pgid", "places.config.path", "places.data.owner", "puid"} { + found := false + for _, r := range removed { + found = found || r == lost + } + if !found { + t.Errorf("removing %s was not said: %v", lost, removed) + } + } +} + +func TestASettingThatOnlyAddsOrChangesRemovesNothing(t *testing.T) { + after := map[string]any{ + "puid": 1001.0, "pgid": 1000.0, + "expose": map[string]any{"32400": "anywhere"}, + "places": map[string]any{ + "config": map[string]any{"path": "/srv/media/config", "owner": "1000:1000"}, + "data": map[string]any{"path": "/srv/media/data", "owner": "1000:1000"}, + "transcode": map[string]any{"path": "/srv/media/temp", "owner": "1000:1000"}, + "previews": map[string]any{"path": "/srv/pool/previews", "owner": "1000:1000"}, + }, + } + _, changed, removed := settingsChange(before, after) + if len(removed) != 0 { + t.Errorf("an additive set was said to remove %v", removed) + } + if !reflect.DeepEqual(changed, []string{"puid"}) { + t.Errorf("changed %v", changed) + } +} + +// What was sent, as a push would send it: a media server's container and a file with a secret. +func declaration(configFrom string, extra ...string) []byte { + vols := `"` + configFrom + `:/config", "/srv/media/data:/data"` + for _, e := range extra { + vols += `, "` + e + `"` + } + return []byte(`{"declaration":1,"sequence":7,"resources":[ + {"id":"plex.server","type":"container","image":"plex@sha256:aa","volumes":[` + vols + `]}, + {"id":"plex.env","type":"file","path":"/srv/media/env","content":"TOKEN=the-secret-itself"}]}`) +} + +func summarized(t *testing.T, body []byte) []sentResource { + t.Helper() + s, err := summarize(body) + if err != nil { + t.Fatal(err) + } + return s +} + +func TestWhatIsKeptOfASendHoldsNoFileContent(t *testing.T) { + s := summarized(t, declaration("/srv/media/config")) + var kept bytes.Buffer + for _, r := range s { + kept.WriteString(r.ID + r.Type + r.Digest + strings.Join(r.Volumes, ",")) + for k, v := range r.Fields { + kept.WriteString(k + v) + } + } + if strings.Contains(kept.String(), "the-secret-itself") { + t.Fatal("a file's content was kept in the record of what was sent") + } + if len(s) != 2 || s[0].Volumes[0] != "/srv/media/config:/config" { + t.Fatalf("summary %+v", s) + } +} + +func TestADiffOfAnUnchangedMachineIsEmptyAndAChangedContainerNamesItsField(t *testing.T) { + was := summarized(t, declaration("/srv/media/config")) + if d := diffSent(was, summarized(t, declaration("/srv/media/config"))); !d.empty() { + t.Fatalf("an unchanged machine differs: %+v", d) + } + d := diffSent(was, summarized(t, declaration("/var/lib/plex/config"))) + if len(d.Changed) != 1 || d.Changed[0].ID != "plex.server" || !reflect.DeepEqual(d.Changed[0].Fields, []string{"volumes"}) { + t.Fatalf("diff %+v", d) + } + var out bytes.Buffer + writeDiff(&out, "home", d, movesIn(was, summarized(t, declaration("/var/lib/plex/config")))) + for _, want := range []string{"~ plex.server recreated: volumes", "! plex.server: /config moves from /srv/media/config to /var/lib/plex/config"} { + if !strings.Contains(out.String(), want) { + t.Errorf("diff does not say %q:\n%s", want, out.String()) + } + } +} + +// The incident: the configuration mount would move to an empty default directory. +func TestARunningContainersDataMovingIsAMoveAndAnAddedMountIsNot(t *testing.T) { + was := summarized(t, declaration("/srv/media/config")) + moves := movesIn(was, summarized(t, declaration("/var/lib/plex/config"))) + want := []dataMove{{Container: "plex.server", Inside: "/config", From: "/srv/media/config", To: "/var/lib/plex/config"}} + if !reflect.DeepEqual(moves, want) { + t.Fatalf("moves %+v", moves) + } + if moves[0].Module() != "plex" { + t.Errorf("module %q", moves[0].Module()) + } + // A mount added — the previews of 2026-10-05 — is not a move. + if m := movesIn(was, summarized(t, declaration("/srv/media/config", "/srv/pool/previews:/config/Media"))); len(m) != 0 { + t.Errorf("an added mount was held as a move: %+v", m) + } + // Nor is a container the machine never ran. + if m := movesIn(nil, was); len(m) != 0 { + t.Errorf("a first send was held as a move: %+v", m) + } +} + +func TestAPushNamingNoMachineIsRefusedUnlessItSaysAll(t *testing.T) { + err := pushCommand(context.Background(), nil) + if err == nil || !strings.Contains(err.Error(), "--all") { + t.Fatalf("a bare push was not refused: %v", err) + } + if err := pushCommand(context.Background(), []string{"--all", "--behind"}); err == nil { + t.Fatal("--all with --behind was accepted") + } +} + +func TestTheVerbsCarryReadReplaceAndMove(t *testing.T) { + for _, c := range []struct { + verb string + args map[string]any + want []string + }{ + {"settings", map[string]any{"module": "plex", "node": "home"}, []string{"settings", "show", "plex", "--node", "home"}}, + {"settings", map[string]any{"module": "plex", "history": "true"}, []string{"settings", "show", "plex", "--history"}}, + {"settings", map[string]any{"module": "plex", "values": "{}", "replace": "true"}, []string{"settings", "set", "plex", "{}", "--replace"}}, + {"push", map[string]any{"node": "home", "move": "plex"}, []string{"push", "home", "--wait", "0", "--move", "plex"}}, + {"push", map[string]any{}, []string{"push", "--behind", "--wait", "0"}}, + } { + got, err := argvFor(c.verb, c.args) + if err != nil || !reflect.DeepEqual(got, c.want) { + t.Errorf("%s %v: %v %v, want %v", c.verb, c.args, got, err, c.want) + } + } +} diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index f4a1352..c52d527 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -119,8 +119,13 @@ var ControllerVerbs = []Verb{ }, []string{"node", "provision", "from", "module"})}, {Name: "unpin", Description: "Take that choice back, putting the question to the mesh again.", Input: schema(map[string]string{"node": "the machine's name", "provision": "the provision"}, []string{"node", "provision"})}, - {Name: "push", Description: "Send a machine everything it should be — or every machine that is behind, when no machine is named.", - Input: schema(map[string]string{"node": "the machine's name; every machine behind when absent"}, nil)}, + {Name: "push", Description: "Send a machine everything it should be — or every machine that is behind, when no machine is named. " + + "A push that would move a running module's data to another directory is held for that machine and says so; " + + "name the module in move to send it.", + Input: schema(map[string]string{ + "node": "the machine's name; every machine behind when absent", + "move": "modules whose data may move with this push, comma-separated (optional)", + }, nil)}, {Name: "rotate", Description: "Replace a credential. A pair credential, by provision (and a consuming machine, " + "else every holder): both ends are re-sent together. Or a module's own secret, by machine, module and " + "name: made anew and the machine sent, so the module starts again on it — only for a secret its " + @@ -139,14 +144,18 @@ var ControllerVerbs = []Verb{ "node": "the machine that runs the module", "module": "the module's name", }, []string{"node", "module"})}, - {Name: "settings", Description: "Set what an assignment is configured with: a module's settings for the whole mesh, " + - "or for one machine. Replaces that layer whole — what it does not name, it no longer sets — and takes effect " + - "at the next push. With clear, removes the layer and the module is back to what its definition says.", + {Name: "settings", Description: "Read or set what an assignment is configured with: a module's settings for the whole mesh, " + + "or for one machine. Without values or clear, answers the layer as it stands — read it before setting it. " + + "Setting replaces that layer whole and says what it adds, changes and removes; a set that would remove a key " + + "is refused unless replace is \"true\". The replaced layer is kept (history). Takes effect at the next push. " + + "With clear, removes the layer and the module is back to what its definition says.", Input: schema(map[string]string{ - "module": "the module's name", - "values": "the settings as a JSON object, for set", - "node": "one machine; the whole mesh when absent", - "clear": "\"true\" to remove the layer instead of setting it", + "module": "the module's name", + "values": "the settings as a JSON object, for set", + "node": "one machine; the whole mesh when absent", + "clear": "\"true\" to remove the layer instead of setting it", + "replace": "\"true\" when the set is meant to remove keys the layer had", + "history": "\"true\", without values: the layers this one replaced, latest first", }, []string{"module"})}, {Name: "command", Description: "Run one command line of the controller's own, as you would type it at its " + "shell — `node account g14 jochen`, `node show ace`, `module list` — and answer what it printed. The " + diff --git a/internal/inventory/catalogue.go b/internal/inventory/catalogue.go index 342bb0b..e98954d 100644 --- a/internal/inventory/catalogue.go +++ b/internal/inventory/catalogue.go @@ -691,6 +691,11 @@ func (i *Inventory) SetSettings(ctx context.Context, nodeName, module string, va return fmt.Errorf("%s: %s is given per node — a port is a fact about one machine; "+ "set it with --node", module, catalogue.PortsSetting) } + // The layer it replaces is kept (novox/hq ADR 0217): a previous value is one command + // away, not in a backup. + if err := i.keepReplaced(ctx, nil, module, "set"); err != nil { + return err + } _, err = i.store.Pool().Exec(ctx, `insert into settings (node, module, values) values (null, $1, $2) on conflict (module) where node is null @@ -715,6 +720,10 @@ func (i *Inventory) SetSettings(ctx context.Context, nodeName, module string, va return err } } + // The layer it replaces is kept (novox/hq ADR 0217), in the same transaction as the write. + if _, err := tx.Exec(ctx, keepReplacedSQL, module, node.ID, "set"); err != nil { + return err + } _, err = tx.Exec(ctx, `insert into settings (node, module, values) values ($1, $2, $3) on conflict (node, module) where node is not null @@ -908,6 +917,10 @@ func wrapModule(err error, module string) error { // ClearSettings removes a layer. func (i *Inventory) ClearSettings(ctx context.Context, nodeName, module string) error { if nodeName == "" { + // The layer it removes is kept (novox/hq ADR 0217). + if err := i.keepReplaced(ctx, nil, module, "clear"); err != nil { + return err + } _, err := i.store.Pool().Exec(ctx, `delete from settings where module = $1 and node is null`, module) return err @@ -916,6 +929,9 @@ func (i *Inventory) ClearSettings(ctx context.Context, nodeName, module string) if err != nil { return err } + if err := i.keepReplaced(ctx, &node.ID, module, "clear"); err != nil { + return err + } _, err = i.store.Pool().Exec(ctx, `delete from settings where module = $1 and node = $2`, module, node.ID) return err diff --git a/internal/inventory/migrations/0057-what-a-change-replaces.sql b/internal/inventory/migrations/0057-what-a-change-replaces.sql new file mode 100644 index 0000000..ce74e05 --- /dev/null +++ b/internal/inventory/migrations/0057-what-a-change-replaces.sql @@ -0,0 +1,26 @@ +-- What a change replaces (novox/hq ADR 0217, to-be 44). +-- +-- Two records the mesh did not keep, and each time a change took effect unseen it was the one +-- missing. A settings layer is replaced whole, and the layer it replaced was nowhere: on 2026-10-05 +-- one placement set for one module on one machine dropped that machine's whole layer for it, and +-- the old one was read back from a database backup (issue 246). And of what a machine was sent the +-- mesh kept only a digest — enough to say *whether* it changed, never *what*. + +-- Every layer that was replaced or cleared, with when it had been set and when it went. Not a +-- foreign key to settings: the row it was is the row being replaced. +create table settings_history ( + node uuid references node(id) on delete cascade, + module text not null, + values jsonb not null, + set_at timestamptz, + replaced_at timestamptz not null default now(), + -- set · clear + replaced_by text not null +); +create index settings_history_by_layer on settings_history (module, node, replaced_at desc); + +-- What a machine was last sent, summarised: per resource its id, type, a digest of it and of each +-- field, and a container's mounts. Not the declaration: a file's content may carry a secret, and +-- this lands in the store's backups. Read only to compare a plan with what was sent; what a machine +-- *should* be is composed from the mesh's records every time, as before (migration 0014). +alter table node add column sent_summary jsonb; diff --git a/internal/inventory/unseen.go b/internal/inventory/unseen.go new file mode 100644 index 0000000..b28b60c --- /dev/null +++ b/internal/inventory/unseen.go @@ -0,0 +1,120 @@ +package inventory + +import ( + "context" + "encoding/json" + "errors" + "time" + + "github.com/jackc/pgx/v5" +) + +// What a change replaces (novox/hq ADR 0217, to-be 44): the settings layer a set or a clear replaced, +// and a summary of what a machine was last sent. Both were missing when a change took effect unseen. + +// Layer is one settings layer as it stands — the whole mesh's when nodeName is empty — and whether +// there is one. A layer is replaced whole when set; reading it first is how one key is changed +// without losing the others. +func (i *Inventory) Layer(ctx context.Context, nodeName, module string) (map[string]any, bool, error) { + nodeID, err := i.layerNode(ctx, nodeName) + if err != nil { + return nil, false, err + } + var raw []byte + err = i.store.Pool().QueryRow(ctx, + `select values from settings where module = $1 and node is not distinct from $2::uuid`, + module, nodeID).Scan(&raw) + if errors.Is(err, pgx.ErrNoRows) { + return nil, false, nil + } + if err != nil { + return nil, false, err + } + values := map[string]any{} + if err := json.Unmarshal(raw, &values); err != nil { + return nil, false, err + } + return values, true, nil +} + +// PastLayer is a layer that was replaced or cleared. +type PastLayer struct { + Values map[string]any + SetAt *time.Time + ReplacedAt time.Time + // ReplacedBy is "set" or "clear". + ReplacedBy string +} + +// SettingsHistory is every layer of one module on one machine — the whole mesh's when nodeName is +// empty — that was replaced or cleared, the latest first. +func (i *Inventory) SettingsHistory(ctx context.Context, nodeName, module string) ([]PastLayer, error) { + nodeID, err := i.layerNode(ctx, nodeName) + if err != nil { + return nil, err + } + rows, err := i.store.Pool().Query(ctx, + `select values, set_at, replaced_at, replaced_by from settings_history + where module = $1 and node is not distinct from $2::uuid order by replaced_at desc`, + module, nodeID) + if err != nil { + return nil, err + } + defer rows.Close() + var out []PastLayer + for rows.Next() { + var raw []byte + var p PastLayer + if err := rows.Scan(&raw, &p.SetAt, &p.ReplacedAt, &p.ReplacedBy); err != nil { + return nil, err + } + if err := json.Unmarshal(raw, &p.Values); err != nil { + return nil, err + } + out = append(out, p) + } + return out, rows.Err() +} + +// keepReplaced writes down the layer about to be replaced or cleared, if there is one. +func (i *Inventory) keepReplaced(ctx context.Context, nodeID *string, module, how string) error { + _, err := i.store.Pool().Exec(ctx, keepReplacedSQL, module, nodeID, how) + return err +} + +// keepReplacedSQL copies a layer into the history before it is replaced or cleared; run in the same +// transaction as the write where there is one, so a write that fails leaves no history of it. +const keepReplacedSQL = `insert into settings_history (node, module, values, set_at, replaced_by) + select node, module, values, set_at, $3 from settings + where module = $1 and node is not distinct from $2::uuid` + +// layerNode is the node id of a layer, nil for the whole mesh's. +func (i *Inventory) layerNode(ctx context.Context, nodeName string) (*string, error) { + if nodeName == "" { + return nil, nil + } + n, err := i.NodeByName(ctx, nodeName) + if err != nil { + return nil, err + } + return &n.ID, nil +} + +// RecordSentSummary keeps what a machine was last sent, summarised (migration 0057): read only to +// compare what would be sent with it, never as what the machine should be. +func (i *Inventory) RecordSentSummary(ctx context.Context, node string, summary []byte) error { + _, err := i.store.Pool().Exec(ctx, `update node set sent_summary = $2 where id = $1`, node, summary) + return err +} + +// SentSummary is what a machine was last sent, summarised, by its name; empty for a machine sent +// nothing since the summary was first kept. +func (i *Inventory) SentSummary(ctx context.Context, name string) ([]byte, error) { + var raw []byte + err := i.store.Pool().QueryRow(ctx, + `select sent_summary from node where name = $1`, name).Scan(&raw) + if errors.Is(err, pgx.ErrNoRows) { + return nil, nil + } + return raw, err +} diff --git a/internal/inventory/unseen_test.go b/internal/inventory/unseen_test.go new file mode 100644 index 0000000..dc8ae2a --- /dev/null +++ b/internal/inventory/unseen_test.go @@ -0,0 +1,90 @@ +package inventory + +import ( + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// novox/hq ADR 0217: a settings layer can be read, and the one a set or a clear replaced is kept, so +// a previous value is one command away rather than in a database backup (issue 246). +func TestTheLayerASetReplacesIsKeptAndReadable(t *testing.T) { + inv := fresh(t) + ctx := t.Context() + if _, err := inv.AddNode(ctx, "anchor"); err != nil { + t.Fatal(err) + } + web := catalogue.Manifest{Module: "web", Version: "1", + Resources: []map[string]any{ + {"id": "server", "type": "container", "name": "web", "image": "x"}, + {"id": "conf", "type": "file", "path": "/etc/web.json", "content": "{}", "merge": "json"}, + }} + if err := inv.RegisterModule(ctx, web, Source{}); err != nil { + t.Fatal(err) + } + + if _, has, err := inv.Layer(ctx, "anchor", "web"); err != nil || has { + t.Fatalf("a layer nobody set: %v %v", has, err) + } + first := map[string]any{"colour": "blue", "size": "large"} + if err := inv.SetSettings(ctx, "anchor", "web", first); err != nil { + t.Fatal(err) + } + got, has, err := inv.Layer(ctx, "anchor", "web") + if err != nil || !has || got["colour"] != "blue" || got["size"] != "large" { + t.Fatalf("the layer read back: %v %v %v", got, has, err) + } + if past, err := inv.SettingsHistory(ctx, "anchor", "web"); err != nil || len(past) != 0 { + t.Fatalf("a first set replaced something: %v %v", past, err) + } + + if err := inv.SetSettings(ctx, "anchor", "web", map[string]any{"colour": "red"}); err != nil { + t.Fatal(err) + } + if err := inv.ClearSettings(ctx, "anchor", "web"); err != nil { + t.Fatal(err) + } + past, err := inv.SettingsHistory(ctx, "anchor", "web") + if err != nil || len(past) != 2 { + t.Fatalf("history %v %v", past, err) + } + // The latest first: the clear took the red layer, the set took the first. + if past[0].ReplacedBy != "clear" || past[0].Values["colour"] != "red" { + t.Errorf("the cleared layer: %+v", past[0]) + } + if past[1].ReplacedBy != "set" || past[1].Values["size"] != "large" { + t.Errorf("the replaced layer still has what the set dropped: %+v", past[1]) + } + // The mesh-wide layer keeps its own history, apart from the node's. + if err := inv.SetSettings(ctx, "", "web", map[string]any{"colour": "green"}); err != nil { + t.Fatal(err) + } + if err := inv.SetSettings(ctx, "", "web", map[string]any{"colour": "grey"}); err != nil { + t.Fatal(err) + } + mesh, err := inv.SettingsHistory(ctx, "", "web") + if err != nil || len(mesh) != 1 || mesh[0].Values["colour"] != "green" { + t.Fatalf("the mesh-wide history %v %v", mesh, err) + } +} + +// What a machine was last sent is kept, summarised, and read back by its name; a machine sent +// nothing since has nothing to compare with. +func TestWhatAMachineWasSentIsKeptForComparison(t *testing.T) { + inv := fresh(t) + ctx := t.Context() + n, err := inv.AddNode(ctx, "anchor") + if err != nil { + t.Fatal(err) + } + if got, err := inv.SentSummary(ctx, "anchor"); err != nil || len(got) != 0 { + t.Fatalf("a machine never sent anything: %s %v", got, err) + } + if err := inv.RecordSentSummary(ctx, n.ID, []byte(`[{"id":"web.server","type":"container"}]`)); err != nil { + t.Fatal(err) + } + got, err := inv.SentSummary(ctx, "anchor") + if err != nil || len(got) == 0 { + t.Fatalf("read back: %s %v", got, err) + } +}