diff --git a/cmd/mesh-controller/adopting_test.go b/cmd/mesh-controller/adopting_test.go index b10b202..ef81c2a 100644 --- a/cmd/mesh-controller/adopting_test.go +++ b/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") + } +} diff --git a/cmd/mesh-controller/adoption.go b/cmd/mesh-controller/adoption.go index de93a6c..0f39abf 100644 --- a/cmd/mesh-controller/adoption.go +++ b/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 { - b.WriteString(" nothing reported\n") + if countReachable(reported) == 0 { + // 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)