diff --git a/internal/link/held_note_test.go b/internal/link/held_note_test.go new file mode 100644 index 0000000..43b9953 --- /dev/null +++ b/internal/link/held_note_test.go @@ -0,0 +1,46 @@ +package link + +import ( + "strings" + "testing" +) + +// The count that did not add up was the only symptom sixteen held resources had, and reading it meant +// opening the node's state file by hand (novox/hq 04-ISSUES/125). The line that says what an apply did +// says what it did not, too. + +func TestTheApplyLineSaysWhatItHeldAndForWhichModule(t *testing.T) { + got := heldNote([]Held{ + {ID: "ca", Module: "route-proxy", Kind: "directory"}, + {ID: "certs", Module: "route-proxy", Kind: "directory"}, + {ID: "server", Module: "route-proxy", Kind: "container"}, + {ID: "mail", Module: "mailu", Kind: "container"}, + }) + if !strings.Contains(got, "4 held") { + t.Fatalf("the count of what was held is not in the line: %q", got) + } + // The module is the thing an operator can act on: `take` takes a module. + if !strings.Contains(got, "route-proxy: 3") || !strings.Contains(got, "mailu: 1") { + t.Fatalf("the line does not break the holds down by module: %q", got) + } + // Ordered, so two machines holding the same things read the same and a diff of two reports is + // about what changed. + if strings.Index(got, "mailu") > strings.Index(got, "route-proxy") { + t.Fatalf("modules are not in a stated order: %q", got) + } + // It says why, because "held" alone reads as a failure and this is correct behaviour. + if !strings.Contains(got, "taken") { + t.Fatalf("the line does not say a hold ends when the module is taken: %q", got) + } +} + +func TestAnApplyThatHeldNothingSaysNothingExtra(t *testing.T) { + // A converged machine holds nothing, which is most applies. Reporting "0 held" on every one of + // them is how a line stops being read. + if got := heldNote(nil); got != "" { + t.Fatalf("an apply with no holds added %q to its line", got) + } + if got := heldNote([]Held{}); got != "" { + t.Fatalf("an apply with no holds added %q to its line", got) + } +} diff --git a/internal/link/run.go b/internal/link/run.go index 47c5edf..fb28c00 100644 --- a/internal/link/run.go +++ b/internal/link/run.go @@ -6,6 +6,8 @@ import ( "encoding/json" "errors" "fmt" + "sort" + "strings" "time" ) @@ -250,9 +252,11 @@ func Run(ctx context.Context, m Membership, apply Applier, say Announce, timeout case report.Refused != "": say("refused a declaration: " + report.Refused) case len(report.Failed) > 0: - say(fmt.Sprintf("applied %d and failed: %v", len(report.Applied), report.Failed)) + say(fmt.Sprintf("applied %d and failed: %v%s", + len(report.Applied), report.Failed, heldNote(report.Held))) default: - say(fmt.Sprintf("applied %d resource(s)", len(report.Applied))) + say(fmt.Sprintf("applied %d resource(s)%s", + len(report.Applied), heldNote(report.Held))) } publishReport(ctx, link, m, report, say, timeout) // Settled after the report is published. A node that dies between applying and @@ -392,3 +396,36 @@ func publishAlive(ctx context.Context, bus Bus, m Membership, say Announce, say("could not tell the mesh this node is here: " + err.Error()) } } + +// heldNote is what this apply did NOT do, for the line that says what it did. +// +// **A count that does not add up is the only symptom a held resource had** (novox/hq 04-ISSUES/125). +// An adopted node keeps what it found until its module is taken (ADR 0100), and that is correct — but +// it was recorded only in the node's own state file. On the edge cut-over the mesh sent 346 resources, +// the journal said it applied 330, and nothing anywhere said which sixteen or why. Reading it took +// opening state.json by hand; not reading it took every public name on the machine down, because the +// operator had four green surfaces and a discrepancy nobody could interpret. +// +// So the line that reports the apply carries it. Grouped by module and ordered by name, because the +// sentence an operator needs is "route-proxy is assigned and not taken", and the module is the thing +// they can act on — `take` is the verb, and it takes a module. +func heldNote(held []Held) string { + if len(held) == 0 { + return "" + } + byModule := map[string]int{} + for _, h := range held { + byModule[h.Module]++ + } + names := make([]string, 0, len(byModule)) + for name := range byModule { + names = append(names, name) + } + sort.Strings(names) + parts := make([]string, 0, len(names)) + for _, name := range names { + parts = append(parts, fmt.Sprintf("%s: %d", name, byModule[name])) + } + return fmt.Sprintf(", %d held until their module is taken (%s)", + len(held), strings.Join(parts, ", ")) +}