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
 }
 

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)
 	}

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))

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

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")