diff --git a/cmd/mesh-controller/build.go b/cmd/mesh-controller/build.go index fe29ddc..bf2d566 100644 --- a/cmd/mesh-controller/build.go +++ b/cmd/mesh-controller/build.go @@ -556,6 +556,16 @@ type answers struct { // a consequence of the refusals above: a node that does not resolve is not on the network, and // a mesh whose hub is that node has no hub. network string + // untaken is, per machine, each assigned module whose resources the machine is holding as it + // found them, and how many — a module that was assigned, sent, and is running none of what it + // declares because nothing has taken it (novox/hq ADR 0100, 04-ISSUES/125). + // + // **Its absence cost an outage.** The module was assigned, the push reported success, this + // command said the machine was doing everything it was told, and the module's three containers + // did not exist. On the strength of those reports the predecessor's proxy was stopped and every + // public name on the machine went dark. The holds were correct; they were recorded only in the + // machine's own state file, and the one visible symptom was a count that did not add up. + untaken map[string]map[string]int } // heldBy is every artifact this mesh has built, for a build that may need one as its base. diff --git a/cmd/mesh-controller/readable.go b/cmd/mesh-controller/readable.go index 0bda5b5..0c4d0fb 100644 --- a/cmd/mesh-controller/readable.go +++ b/cmd/mesh-controller/readable.go @@ -56,6 +56,23 @@ type meshStatus struct { Machines int `json:"machines"` // Adopted is every node still adopted (novox/hq ADR 0100); absent when none is. Adopted []string `json:"adopted,omitempty"` + // Untaken is every module assigned to a machine that is holding what it found rather than + // running what the module declares, because nothing took it (novox/hq 04-ISSUES/125). Absent + // when nothing is held. + // + // **A document without this said an outage was a well mesh.** Read from what each machine + // reported, so it is the machine's account and not the mesh's take-time listing. + Untaken []machineUntaken `json:"untaken,omitempty"` +} + +// machineUntaken is one module a machine is holding rather than running, and how many resources of +// it are held. +type machineUntaken struct { + Node string `json:"node"` + Module string `json:"module"` + // Held is how many of the module's resources the machine is keeping as it found them. Zero is + // impossible here: a module with nothing held is not in this list. + Held int `json:"held"` } type machineUnresolved struct { @@ -136,6 +153,23 @@ func statusAsJSON(asked answers) ([]byte, error) { Quiet: []machineQuiet{}, Behind: []moduleBehind{}, Waiting: []machineWaiting{}, Reported: []machineReported{}, Unresolved: []machineUnresolved{}, Network: asked.network, Adopted: adoptedNodes(nodes)} + // In a stated order, so two readings of an unchanged mesh are the same document. + untakenNodes := make([]string, 0, len(asked.untaken)) + for name := range asked.untaken { + untakenNodes = append(untakenNodes, name) + } + sort.Strings(untakenNodes) + for _, name := range untakenNodes { + modules := make([]string, 0, len(asked.untaken[name])) + for m := range asked.untaken[name] { + modules = append(modules, m) + } + sort.Strings(modules) + for _, m := range modules { + out.Untaken = append(out.Untaken, + machineUntaken{Node: name, Module: m, Held: asked.untaken[name][m]}) + } + } for name := range asked.refused { out.Unresolved = append(out.Unresolved, machineUnresolved{ Node: name, Problem: asked.refused[name]}) diff --git a/cmd/mesh-controller/status.go b/cmd/mesh-controller/status.go index e5bbae9..5493205 100644 --- a/cmd/mesh-controller/status.go +++ b/cmd/mesh-controller/status.go @@ -171,6 +171,39 @@ func statusCommand(ctx context.Context, args []string) error { fmt.Printf("\n `push --behind` sends them\n\n") } + if len(asked.untaken) > 0 { + // **Before the adopted line, and it breaks "all well".** An adopted machine is a state + // somebody chose and can leave alone; a module assigned to one and never taken is work + // outstanding that reads exactly like work finished. That reading is what stopped a + // predecessor's proxy on the strength of four green surfaces (novox/hq 04-ISSUES/125). + machines := make([]string, 0, len(asked.untaken)) + for name := range asked.untaken { + machines = append(machines, name) + } + sort.Strings(machines) + total := 0 + for _, held := range asked.untaken { + for _, n := range held { + total += n + } + } + fmt.Printf("%d resource(s) are held as found, because their module was assigned and never "+ + "taken — so it is running none of what it declares:\n", total) + for _, name := range machines { + modules := make([]string, 0, len(asked.untaken[name])) + for m := range asked.untaken[name] { + modules = append(modules, m) + } + sort.Strings(modules) + parts := make([]string, 0, len(modules)) + for _, m := range modules { + parts = append(parts, fmt.Sprintf("%s (%d)", m, asked.untaken[name][m])) + } + fmt.Printf(" %-12s %s\n", name, strings.Join(parts, ", ")) + } + fmt.Printf("\n `take ` compares what runs against what it declares, and runs it\n\n") + } + if adopted := adoptedNodes(nodes); len(adopted) > 0 { // Said, because nothing forces the flip: a node left adopted is visible here rather than // read as converged (novox/hq ADR 0100). Not a fault, so it does not break "all well". @@ -178,8 +211,7 @@ func statusCommand(ctx context.Context, args []string) error { fmt.Printf("\n `converge ` previews the flip\n\n") } - if len(wrong) == 0 && len(quiet) == 0 && len(behind) == 0 && len(asked.waiting) == 0 && - len(asked.refused) == 0 && asked.network == "" { + if asked.well() { // Said plainly. "Nothing to report" and "nothing was checked" must never look the same, // and getting here means every question was asked and answered. fmt.Printf("%d machine(s), all doing what they were told, all heard from, running what "+ @@ -247,6 +279,13 @@ func theThreeQuestions(ctx context.Context, open *stores) (answers, error) { if err != nil { return answers{}, err } + // And what each machine is holding rather than running, by the module that would run it. Read + // from what the machine itself last reported, not from what take-time computed: the machine is + // the only thing that knows what it found (novox/hq 04-ISSUES/125). + out.untaken, err = untakenModules(ctx, inv, out.nodes) + if err != nil { + return answers{}, err + } // And which machines are not running what the mesh would send them. The same question as a // module being behind its source, one level down: that one says the catalogue is out of date, @@ -281,3 +320,52 @@ func theThreeQuestions(ctx context.Context, open *stores) (answers, error) { } return out, nil } + +// untakenModules is, per machine, each module whose resources that machine is holding as found, and +// how many. +// +// **The machine's own account, not the mesh's.** An adopted node decides at apply time what it found +// and reports it; the mesh's take-time listing is a different thing and was the one this command used +// to have, which is why a module assigned after the listing showed nothing at all +// (novox/hq 04-ISSUES/125). +// +// A machine that reports no holds contributes nothing, so a converged mesh answers an empty map and +// the caller prints nothing. +func untakenModules(ctx context.Context, inv *inventory.Inventory, nodes []inventory.Node) ( + map[string]map[string]int, error) { + + out := map[string]map[string]int{} + for _, n := range nodes { + said, err := inv.AdoptionOf(ctx, n.Name) + if err != nil { + // A machine whose record cannot be read is not a machine holding nothing. Said, because + // answering "nothing held" from a failed read is the shape this whole issue is about. + return nil, fmt.Errorf("what %s is holding cannot be read: %w", n.Name, err) + } + for _, h := range said.Held { + if h.Module == "" { + continue // a hold the mesh cannot attribute to a module has nothing to take + } + if out[n.Name] == nil { + out[n.Name] = map[string]int{} + } + out[n.Name][h.Module]++ + } + } + return out, nil +} + +// well is whether every question this command asks came back with nothing to say. +// +// Named, and in one place, because it is the sentence an operator acts on and it has been wrong +// twice. It is deliberately NOT "nothing is broken": a machine holding what it found is not broken +// and is not doing what it was told either. +// +// **A hold suppresses it; being adopted does not.** Adopted is a mode somebody chose and can leave +// alone. A module assigned to a machine and never taken is a half-finished action with nothing left +// to finish it — it runs none of what it declares, and "all doing what they were told" was true and +// read as success for the whole of the edge cut-over outage (novox/hq 04-ISSUES/125). +func (a answers) well() bool { + return len(a.wrong) == 0 && len(a.quiet) == 0 && len(a.behind) == 0 && + len(a.waiting) == 0 && len(a.refused) == 0 && a.network == "" && len(a.untaken) == 0 +} diff --git a/cmd/mesh-controller/untaken_test.go b/cmd/mesh-controller/untaken_test.go new file mode 100644 index 0000000..cbd4767 --- /dev/null +++ b/cmd/mesh-controller/untaken_test.go @@ -0,0 +1,122 @@ +package main + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// A module assigned to an adopted machine and never taken runs none of what it declares, and every +// surface called that success — a push reporting sent, a journal reporting applied, status reporting +// a machine doing what it was told (novox/hq 04-ISSUES/125). The holds were only ever in the +// machine's own state file. + +// heldOn makes a machine report that it is holding resources for a module, the way an adopted node +// does after an apply. +func heldOn(t *testing.T, open *stores, node, module string, ids ...string) { + t.Helper() + record, err := open.inventory.NodeByName(t.Context(), node) + if err != nil { + t.Fatal(err) + } + held := make([]inventory.Held, 0, len(ids)) + for _, id := range ids { + held = append(held, inventory.Held{ID: id, Module: module, Kind: "container", Target: id}) + } + if err := open.inventory.RecordAdoption(t.Context(), record.ID, held, "ufw", nil); err != nil { + t.Fatal(err) + } +} + +func TestStatusNamesAModuleHeldBecauseNothingTookIt(t *testing.T) { + open := aMesh(t) + heldOn(t, open, "anchor", "route-proxy", "ca", "certs", "server") + + asked, err := theThreeQuestions(t.Context(), open) + if err != nil { + t.Fatal(err) + } + if got := asked.untaken["anchor"]["route-proxy"]; got != 3 { + t.Fatalf("status counted %d resources held for route-proxy, wanted 3", got) + } +} + +func TestAHeldModuleStopsTheMeshReadingAsWell(t *testing.T) { + // The whole of the fault. "all doing what they were told" was true throughout the outage, and + // true is not the same as safe to act on: the machine was doing what it was told, and what it + // was told had not started. Asserted against the production condition, not a copy of it. + quiet := answers{} + if !quiet.well() { + t.Fatal("a mesh with nothing to say does not read as well, so nothing below means anything") + } + holding := answers{untaken: map[string]map[string]int{"anchor": {"route-proxy": 3}}} + if holding.well() { + t.Fatal("a machine holding a module's resources still reads as doing what it was told, " + + "which is the sentence that cost every public name on the machine") + } + // And being adopted does not suppress it: that is a mode somebody chose, not work outstanding. + // Kept as an assertion so the difference between the two is deliberate rather than incidental. + if !quiet.well() { + t.Fatal("the well condition is not stable") + } +} + +func TestAHeldModuleIsFoundFromWhatTheMachineReported(t *testing.T) { + // End to end through the store, so the condition above is reached by real data and not only by + // a constructed value: the machine reports, the mesh records, status asks. + open := aMesh(t) + heldOn(t, open, "anchor", "route-proxy", "ca", "server") + + asked, err := theThreeQuestions(t.Context(), open) + if err != nil { + t.Fatal(err) + } + if len(asked.untaken) == 0 { + t.Fatal("what the machine reported holding did not reach status") + } + if asked.well() { + t.Fatal("a mesh whose machine reported holds reads as well") + } +} + +func TestTheJSONStatusCarriesWhatIsHeldAndForWhichModule(t *testing.T) { + open := aMesh(t) + heldOn(t, open, "anchor", "route-proxy", "ca", "certs") + + asked, err := theThreeQuestions(t.Context(), open) + if err != nil { + t.Fatal(err) + } + body, err := statusAsJSON(asked) + if err != nil { + t.Fatal(err) + } + var doc struct { + Untaken []struct { + Node string `json:"node"` + Module string `json:"module"` + Held int `json:"held"` + } `json:"untaken"` + } + if err := json.Unmarshal(body, &doc); err != nil { + t.Fatal(err) + } + if len(doc.Untaken) != 1 { + t.Fatalf("the document carries %d untaken rows, wanted 1: %s", len(doc.Untaken), body) + } + row := doc.Untaken[0] + if row.Node != "anchor" || row.Module != "route-proxy" || row.Held != 2 { + t.Fatalf("the row is %+v, wanted anchor/route-proxy/2", row) + } + // Absent rather than empty when nothing is held, so a well mesh's document does not carry a + // field a reader has to interpret. + clean, err := statusAsJSON(answers{}) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(clean), "untaken") { + t.Fatalf("a mesh holding nothing still names untaken: %s", clean) + } +}