From a92c11be124d08fbae1cbc4ed19edc3e6f7c80c1 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 8 Sep 2026 18:37:41 +0200 Subject: [PATCH] Resolve: one un-hostable assignment no longer refuses the whole node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A module a person assigns to a machine that cannot host it — its declared capability has no detector there, as fail2ban does on a host with no firewall — made Resolve refuse the entire node, so a whole-node push refused to send the healthy modules beside it too. One module on the wrong machine took down every other module on that node. Assign already keeps such an assignment on purpose (it is what a person meant, and acts.go says so), so the fix is on the resolve/push side: a directly-assigned module the machine cannot host is left out of the closure and reported as un-applied on the Resolution, rather than refusing the set. The healthy modules still resolve, declare, and converge. A module that is *required* by something running here and cannot be hosted still refuses — that set is genuinely incoherent — so the distinction is who wanted it. assign, plan and push now name the un-applied module and the missing capability, via a shared WrongMachine message, so it is neither silently dropped nor fatal. Reconciled two tests that encoded the old whole-node refusal for directly-assigned un-hostable modules; added coverage for the healthy-modules-still-converge case and the required-un-hostable-still-refuses distinction. Claude-Session: https://claude.ai/code/session_01LrgweAeERJYBg88c5cKDzF --- cmd/mesh-control/acts.go | 16 +++++- cmd/mesh-control/plan.go | 7 +++ cmd/mesh-control/push.go | 19 +++++++ internal/catalogue/resolve.go | 83 ++++++++++++++++++++++----- internal/catalogue/resolve_test.go | 90 ++++++++++++++++++++++++++---- 5 files changed, 190 insertions(+), 25 deletions(-) diff --git a/cmd/mesh-control/acts.go b/cmd/mesh-control/acts.go index 1a83690..0586ceb 100644 --- a/cmd/mesh-control/acts.go +++ b/cmd/mesh-control/acts.go @@ -3,6 +3,8 @@ package main import ( "context" "fmt" + + "github.com/novox/mesh-control/internal/catalogue" ) // The things the mesh can be asked to do, separated from how it was asked. @@ -25,10 +27,22 @@ func assign(ctx context.Context, open *stores, node, module string) (string, err return "", err } said := fmt.Sprintf("%s is assigned %s", node, module) - if _, _, err := planFor(ctx, open, node); err != nil { + plan, _, err := planFor(ctx, open, node) + if err != nil { // Kept, and still refused. Both halves are the answer. return said, err } + // Kept, and cannot be hosted here. Said at once rather than discovered at push: a module whose + // capability the machine lacks is on the wrong machine, and the assignment records what a person + // meant while this line says it will not run until it moves. The rest of the node still pushes. + for _, u := range plan.Unhostable { + if u.Module != module { + continue + } + for _, c := range u.Missing { + said += "\n but " + catalogue.WrongMachine(u.Module, c, node) + } + } return said + fmt.Sprintf("\n run `push %s` to send it", node), nil } diff --git a/cmd/mesh-control/plan.go b/cmd/mesh-control/plan.go index e41a1d6..b93dd16 100644 --- a/cmd/mesh-control/plan.go +++ b/cmd/mesh-control/plan.go @@ -611,6 +611,13 @@ func planCommand(ctx context.Context, args []string) error { for _, m := range plan.Modules { fmt.Printf(" %-20s %s\n", m.Module, plan.Because[m.Module]) } + // What was assigned here and cannot run here. Said with the rest rather than as a refusal: it is + // one module on the wrong machine, the others still run, and the remedy is to move this one. + for _, u := range plan.Unhostable { + for _, c := range u.Missing { + fmt.Printf(" %-20s not applied — %s\n", u.Module, catalogue.WrongMachine(u.Module, c, args[0])) + } + } for _, c := range plan.Claims { fmt.Printf(" holds %s, one per %s\n", c.Claim, c.Scope) } diff --git a/cmd/mesh-control/push.go b/cmd/mesh-control/push.go index fa26e0a..5bab629 100644 --- a/cmd/mesh-control/push.go +++ b/cmd/mesh-control/push.go @@ -13,10 +13,24 @@ import ( "time" "github.com/novox/mesh-control/internal/broker" + "github.com/novox/mesh-control/internal/catalogue" "github.com/novox/mesh-control/internal/inventory" "github.com/novox/mesh-control/internal/link" ) +// reportUnhostable says which of a node's assigned modules the machine cannot run, once per push. +// +// A module whose declared capability has no detector on the machine is on the wrong machine. It is +// kept out of what the node is sent — the healthy modules beside it still converge — and named here +// so it is neither silently dropped nor a reason the whole node fails to push. +func reportUnhostable(node string, plan catalogue.Resolution) { + for _, u := range plan.Unhostable { + for _, c := range u.Missing { + fmt.Printf("%s not applied — %s\n", node, catalogue.WrongMachine(u.Module, c, node)) + } + } +} + // sending it, and holding the link that carries it. // // Split out of main.go, which had reached 2,769 lines because appending was always the @@ -262,6 +276,10 @@ func pushCommand(ctx context.Context, args []string) error { refusals = append(refusals, fmt.Sprintf("%s:\n%v", n.Name, err)) continue } + // A module assigned here that this machine cannot host is said and left out, not fatal: the + // healthy modules beside it are still resolved and sent. Reported so it is not silently + // dropped — the remedy is to move it, and until then the rest of the node converges. + reportUnhostable(n.Name, plan) // The private network is in here with everything else. It used to be composed separately // and prepended, which meant every machine with an address was on it and no machine could // be kept off. It is a module now, so it arrives the way a module does. @@ -335,6 +353,7 @@ func sendTo(ctx context.Context, open *stores, names []string) error { refusals = append(refusals, fmt.Sprintf("%s:\n%v", name, err)) continue } + reportUnhostable(name, plan) resources, err := declarationWith(ctx, open, name, plan, settings, gens, Allocating) if err != nil { refusals = append(refusals, fmt.Sprintf("%s:\n%v", name, err)) diff --git a/internal/catalogue/resolve.go b/internal/catalogue/resolve.go index ef0ab9d..ef8d99e 100644 --- a/internal/catalogue/resolve.go +++ b/internal/catalogue/resolve.go @@ -113,6 +113,21 @@ type Resolution struct { // because it is where a credential will have to be handed back once there is a mechanism for // that, and because "what does this machine depend on that is not on it" has no other answer. Needs []Needed + // Unhostable is what a person assigned to this machine that this machine cannot run — a module + // whose declared capability has no detector here. Kept out of Modules rather than refusing the + // whole set: a module put on the wrong machine is that one module's problem, and the healthy + // modules beside it still resolve, declare, and converge. Reported so it is neither silently + // dropped nor fatal to the rest. A module that is *required* by something running here is a + // different case — that set is incoherent and is refused (see checkCapabilities). + Unhostable []Unhostable +} + +// Unhostable is one directly-assigned module the machine cannot run. +type Unhostable struct { + // Module is the assigned module the machine cannot host. + Module string + // Missing is the capabilities it declares that this machine does not have. + Missing []string } // Needed is one thing this node's set takes from elsewhere in the mesh. @@ -212,16 +227,32 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world // how this read when first used. satisfied := map[string]bool{} - // Everything a person assigned goes in first. Those are choices already made, and a - // requirement one of them answers is not a choice to put back to anybody. - queue := append([]string{}, assigned...) + // Everything a person assigned goes in first, except what this machine cannot run. Those are + // choices already made, and a requirement one of them answers is not a choice to put back to + // anybody. + // + // **A module the machine cannot host is left out here rather than refusing the whole set.** + // Assigning fail2ban to a machine whose profile reports no firewall is one module on the wrong + // machine (novox/hq ADR 0009's sibling case): the person meant something, the remedy is theirs, + // and it is a fact about this one module — not a reason the healthy modules beside it should + // fail to resolve and converge. It is reported as un-applied on the resolution, so it is neither + // silently dropped nor fatal to the rest. The distinction is who wanted it: a module *required* + // by something running here that cannot be hosted makes the set itself incoherent, and that is + // still refused, by checkCapabilities, where it stays in the closure. + var unhostable []Unhostable + var queue []string for _, a := range assigned { - because[a] = "assigned" if m, known := catalogue[a]; known { + if missing := missingCapabilities(m, node); len(missing) > 0 { + unhostable = append(unhostable, Unhostable{Module: a, Missing: missing}) + continue + } for _, o := range m.Offers() { satisfied[o] = true } } + because[a] = "assigned" + queue = append(queue, a) } // What has already been complained about. A requirement can be wanted by several modules at @@ -473,7 +504,8 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world } } - resolution := Resolution{Node: node.Name, At: node.At, Because: because, Needs: needs} + resolution := Resolution{Node: node.Name, At: node.At, Because: because, Needs: needs, + Unhostable: unhostable} for _, n := range providersFirst(order, catalogue) { resolution.Modules = append(resolution.Modules, catalogue[n]) } @@ -519,19 +551,42 @@ func isModule(catalogue map[string]Manifest, want string) bool { return ok } -// checkCapabilities refuses a module the machine cannot run. +// missingCapabilities is the capabilities a module declares that this machine does not have. // -// Said as a fact about the machine rather than about the module, because that is what it is and -// because nothing can be installed to change it. +// Only the absent ones: a capability the machine reports having is nothing to say. Empty means the +// machine can host the module as far as its capabilities go. +func missingCapabilities(m Manifest, node Node) []string { + var missing []string + for _, c := range m.Capabilities { + if !node.Capabilities[c] { + missing = append(missing, c) + } + } + return missing +} + +// WrongMachine is why a module cannot run here, said as a fact about the machine. +// +// Said the same whether a module was refused because something running here requires it or reported +// as un-applied because a person assigned it directly: the reason is identical, and nothing can be +// installed to change it. +func WrongMachine(module, capability, node string) string { + return fmt.Sprintf( + "%s needs the capability %q and %s does not have it — this is the wrong machine, "+ + "not a missing module", module, capability, node) +} + +// checkCapabilities refuses a module the machine cannot run that something running here requires. +// +// A directly-assigned module the machine cannot host is left out of the closure before this runs +// and reported as un-applied instead (see Resolve); what reaches here is a module still in the +// closure because something requires it, and that set is genuinely incoherent — the requiring +// module cannot have its requirement met on this machine. func checkCapabilities(modules []Manifest, node Node) []string { var problems []string for _, m := range modules { - for _, c := range m.Capabilities { - if !node.Capabilities[c] { - problems = append(problems, fmt.Sprintf( - "%s needs the capability %q and %s does not have it — this is the wrong "+ - "machine, not a missing module", m.Module, c, node.Name)) - } + for _, c := range missingCapabilities(m, node) { + problems = append(problems, WrongMachine(m.Module, c, node.Name)) } } return problems diff --git a/internal/catalogue/resolve_test.go b/internal/catalogue/resolve_test.go index 5288036..80c4e69 100644 --- a/internal/catalogue/resolve_test.go +++ b/internal/catalogue/resolve_test.go @@ -131,20 +131,85 @@ func TestAThirdModuleNeedsNoChangeToTheOthers(t *testing.T) { } } -func TestAMissingCapabilityIsSaidToBeTheWrongMachine(t *testing.T) { - // The remedy differs from a missing module and the message has to say which. Nothing can be - // installed to give a server a seat. +func TestADirectlyAssignedModuleTheMachineCannotHostIsReportedNotRefused(t *testing.T) { + // A person put a display server on a seatless machine. The remedy differs from a missing module + // and the message has to say which — nothing can be installed to give a server a seat. But it is + // one module on the wrong machine, not a reason the whole node fails to resolve: it is kept out + // of what the node runs and reported as un-applied, so the healthy modules beside it still + // converge. server := Node{Name: "server", Capabilities: map[string]bool{"container-runtime": true}} - _, err := Resolve(shelf(mod("xorg", nil, nil, []string{"seat"}, Claim{Name: "the-seat"})), []string{"xorg"}, server, World{}) + got, err := Resolve(shelf(mod("xorg", nil, nil, []string{"seat"}, Claim{Name: "the-seat"})), []string{"xorg"}, server, World{}) + + if err != nil { + t.Fatalf("one un-hostable assignment refused the whole node: %v", err) + } + if len(got.Modules) != 0 { + t.Errorf("xorg was declared on a machine that cannot run it: %v", names(got)) + } + if len(got.Unhostable) != 1 || got.Unhostable[0].Module != "xorg" { + t.Fatalf("xorg was not reported as un-applied: %+v", got.Unhostable) + } + if len(got.Unhostable[0].Missing) != 1 || got.Unhostable[0].Missing[0] != "seat" { + t.Errorf("the missing capability was not named: %+v", got.Unhostable[0]) + } + if !strings.Contains(WrongMachine("xorg", "seat", "server"), "wrong machine") { + t.Errorf("the report reads like a missing module") + } +} + +func TestAnUnhostableModuleSomethingRequiresRefusesTheNode(t *testing.T) { + // The other side of the distinction. A module the machine cannot host that is *required* by + // something running here makes the set incoherent: the requiring module cannot have its + // requirement met on this machine, so it is refused rather than quietly declared without it. + server := Node{Name: "server", Capabilities: map[string]bool{"container-runtime": true}} + _, err := Resolve(shelf( + mod("desktop", nil, []string{"display-server"}, nil), + mod("xorg", []string{"display-server"}, nil, []string{"seat"}), + ), []string{"desktop"}, server, World{}) if err == nil { - t.Fatal("a display server was assigned to a machine with no seat") + t.Fatal("a node requiring a module the machine cannot host was resolved") } if !strings.Contains(err.Error(), "wrong machine") { t.Errorf("the refusal reads like a missing module: %v", err) } } +func TestOneUnhostableAssignmentDoesNotTakeDownTheHealthyModules(t *testing.T) { + // The bug the whole-mesh dry-run found: fail2ban declares a capability the host lacks, and its + // one un-hostable assignment refused the entire node's resolution — so a push refused to send + // the healthy modules beside it too. The healthy modules must still resolve and converge; the + // un-hostable one is reported as un-applied, neither silently dropped nor fatal to the rest. + server := Node{Name: "server", Capabilities: map[string]bool{"container-runtime": true}} + got, err := Resolve(shelf( + mod("web", nil, nil, nil), + mod("cache", nil, nil, nil), + mod("cron", nil, nil, nil), + // The one on the wrong machine: it needs a firewall the machine's profile does not report. + mod("fail2ban", nil, nil, []string{"firewall"}), + ), []string{"web", "cache", "cron", "fail2ban"}, server, World{}) + + if err != nil { + t.Fatalf("one un-hostable assignment refused the whole node: %v", err) + } + // The healthy three resolved. + if got := names(got); len(got) != 3 { + t.Fatalf("the healthy modules did not all resolve: %v", got) + } + for _, m := range got.Modules { + if m.Module == "fail2ban" { + t.Error("fail2ban was declared on a machine that cannot run it") + } + } + // The un-hostable one is reported, not silently dropped. + if len(got.Unhostable) != 1 || got.Unhostable[0].Module != "fail2ban" { + t.Fatalf("fail2ban was not reported as un-applied: %+v", got.Unhostable) + } + if len(got.Unhostable[0].Missing) != 1 || got.Unhostable[0].Missing[0] != "firewall" { + t.Errorf("the missing capability was not named: %+v", got.Unhostable[0]) + } +} + func TestAMeshWideClaimIsHeldByOneNode(t *testing.T) { // The hub, said as a claim rather than hard-coded. Another node already holds it, so this one // cannot. @@ -242,12 +307,17 @@ func TestWhatAServiceReflectsIsQualifiedToo(t *testing.T) { } func TestEveryReasonIsGivenAtOnce(t *testing.T) { - // Somebody resolving these fixes them in one pass or in four. - server := Node{Name: "server", Capabilities: map[string]bool{}} + // Somebody resolving these fixes them in one pass or in three. Every reason that genuinely + // refuses the set is collected, rather than only the first: a claim two modules both take, a + // requirement nothing answers, and a requirement several answer without anybody choosing. _, err := Resolve(shelf( - mod("xorg", nil, nil, []string{"seat"}, Claim{Name: "the-seat"}), - mod("wayland", nil, nil, []string{"seat"}, Claim{Name: "the-seat"}), - ), []string{"xorg", "wayland"}, server, World{}) + mod("xorg", nil, nil, nil, Claim{Name: "the-seat"}), + mod("wayland", nil, nil, nil, Claim{Name: "the-seat"}), + mod("i3", nil, []string{"compositor"}, nil), + mod("editor", nil, []string{"shell"}, nil), + mod("bash", []string{"shell"}, nil, nil), + mod("zsh", []string{"shell"}, nil, nil), + ), []string{"xorg", "wayland", "i3", "editor"}, workstation(), World{}) if err == nil { t.Fatal("expected refusals") -- 2.54.0