Refuse the flip on an account naming nothing reachable, and mark such an account partial in the preview (hq ADR 0100)

cmd/mesh-controller/adopting_test.go

@@ -473,3 +473,32 @@ func TestThePreviewNamesEveryHeldKind(t *testing.T) {
 		t.Fatal("the digest does not change with what is held")
 	}
 }
+
+// novox/hq ADR 0100: the flip acts on what the node said is reachable, so an account naming nothing
+// is refused. Every machine that is up answers on ssh; nothing reported means the host's collectors
+// did not, and flipping would close ports the preview never named.
+func TestTheFlipIsRefusedOnAnAccountNamingNothingReachable(t *testing.T) {
+	open, sent := anAdoptedAnchor(t)
+	ctx := t.Context()
+	if _, err := take(ctx, open, "anchor", "hello-web"); err != nil {
+		t.Fatal(err)
+	}
+	// Only a loopback listener: nothing off the machine, which is the same silence.
+	reportsReaching(t, open, []link.Reach{
+		{Protocol: "tcp", Address: "127.0.0.1", Port: 15672, By: "mesh-broker"},
+	}, heldFile)
+	preview, err := converge(ctx, open, "anchor", false, "", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(preview, "this account looks partial") {
+		t.Errorf("the preview does not mark a partial account:\n%s", preview)
+	}
+	_, err = converge(ctx, open, "anchor", true, digestIn(t, preview), "")
+	if err == nil || !strings.Contains(err.Error(), "says nothing is reachable on it") {
+		t.Fatalf("the flip was not refused on an account naming nothing: %v", err)
+	}
+	if n, _ := open.inventory.NodeByName(ctx, "anchor"); !n.Adopted || len(*sent) != 0 {
+		t.Fatal("a refused flip changed something")
+	}
+}

cmd/mesh-controller/adoption.go

@@ -189,6 +189,10 @@ func converge(ctx context.Context, open *stores, node string, yes bool, digest s
 			"converge once it has applied", node, node)
 	}
 
+	assignedWhenPreviewed, err := inv.Assigned(ctx, node)
+	if err != nil {
+		return "", err
+	}
 	plan, settings, err := planFor(ctx, open, node)
 	if err != nil {
 		return "", err
@@ -248,6 +252,16 @@ func converge(ctx context.Context, open *stores, node string, yes bool, digest s
 		return preview + fmt.Sprintf("\n\nNothing has changed. Run `converge %s --yes %s` to do "+
 			"it.", node, saw), nil
 	}
+	// An account naming nothing reachable is not an account of a machine: every machine answers
+	// on ssh, and the host's collectors failing — `ss` refusing, or the container runtime not
+	// answering, which drops every published port at once — leaves exactly this. Flipping on it
+	// would close ports the preview never named.
+	if yes && countReachable(reported) == 0 {
+		return preview, fmt.Errorf("%s says nothing is reachable on it, which no machine that is "+
+			"up ever is: its account looks partial — whatever reads what is listening, or what "+
+			"the container runtime publishes, did not answer. Fix that on the machine and run "+
+			"`push %s --wait 2m`, then preview again", node, node)
+	}
 	if age := time.Since(reported.At); age > reportFreshFor {
 		return preview, fmt.Errorf("%s last said what is reachable on it %s ago, and the flip acts "+
 			"only on an account newer than %s: wait for its next report, or run `push %s --wait 2m`, "+
@@ -269,6 +283,13 @@ func converge(ctx context.Context, open *stores, node string, yes bool, digest s
 	if err != nil {
 		return "", err
 	}
+	// And nothing assigned since the preview was composed: the flip takes every module the node
+	// runs, and one assigned in between would be taken without ever having been previewed.
+	if !slices.Equal(assigned, assignedWhenPreviewed) {
+		return preview, fmt.Errorf("what %s runs changed while this was converging (it is now %s): "+
+			"the flip takes every module on the node, so read the preview again", node,
+			strings.Join(assigned, ", "))
+	}
 	if !slices.Contains(assigned, filter) {
 		if err := inv.Assign(ctx, node, filter); err != nil {
 			return "", err
@@ -317,8 +338,12 @@ func previewOf(node string, reported inventory.Adoption, derived derivedFilter,
 		fmt.Fprintf(&b, "    %-44s %s\n", what, fate)
 		said = append(said, fmt.Sprintf("reach %s %s %s", r.Address, what, fate))
 	}
-	if len(reported.Reachable) == 0 {
+	if countReachable(reported) == 0 {
-		b.WriteString("    nothing reported\n")
+		// Said as what it is: no machine that is up is reachable on nothing, so this is an
+		// account that did not come back, not a machine with nothing on it.
+		b.WriteString("    nothing reported — this account looks partial, and the flip is " +
+			"refused on it\n")
+		said = append(said, "reach nothing reported")
 	}
 	// What the machine routes for others is not a listener and not a published port, so nothing
 	// above can show it; the derived filter's forward chain drops it all the same.
@@ -431,6 +456,19 @@ func (d derivedFilter) fate(r inventory.Reach) string {
 	return "WILL CLOSE — no module assigned here declares it"
 }
 
+// countReachable is how much of a node's account of itself names something off the machine.
+// Loopback is left out for the same reason the preview leaves it out: nothing outside reaches it,
+// so a report of loopback alone says nothing about what the filter would close.
+func countReachable(reported inventory.Adoption) int {
+	n := 0
+	for _, r := range reported.Reachable {
+		if !loopback(r.Address) {
+			n++
+		}
+	}
+	return n
+}
+
 // heldLine is one thing a node holds as found, as take and the converge preview both say it.
 func heldLine(h inventory.Held) string {
 	return fmt.Sprintf("%s %s (%s)", h.Kind, h.Target, h.ID)