From cbf50185d02f67781a2012ba81cb23271c06029a Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 30 Sep 2026 08:46:19 +0200 Subject: [PATCH] An apply says what it held, not only what it applied MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit novox/hq 04-ISSUES/125. An adopted node keeps what it found until its module is taken, which is correct and 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 meant opening state.json by hand, and not reading it took every public name on the machine down. The line that reports the apply now 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 `take` acts on. An apply that held nothing says nothing extra: a line that reports "0 held" on every converged apply is a line that stops being read. --- internal/link/held_note_test.go | 46 +++++++++++++++++++++++++++++++++ internal/link/run.go | 41 +++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 internal/link/held_note_test.go 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, ", ")) +}