diff --git a/cmd/mesh-control/push.go b/cmd/mesh-control/push.go index 5bab629..0459a0a 100644 --- a/cmd/mesh-control/push.go +++ b/cmd/mesh-control/push.go @@ -239,16 +239,8 @@ func pushCommand(ctx context.Context, args []string) error { } defer server.Close() - // Every node is resolved before anything is sent. A push that configured three nodes and then - // refused on the fourth would leave the mesh in a state nobody asked for, and the fourth is - // exactly where a claim collision shows up. - type ready struct { - node string - resources []map[string]any - } - var sending []ready - var refusals []string - + // Which machines this push is about, before any of them is worked out. + var asked []string for _, n := range nodes { if len(args) == 1 && n.Name != args[0] { continue @@ -271,34 +263,23 @@ func pushCommand(ctx context.Context, args []string) error { n.Name, doing.Outcome, doing.At.Local().Format("2006-01-02 15:04")) } } - plan, settings, err := planFor(ctx, open, n.Name) + asked = append(asked, n.Name) + } + + sending, refusals := composeEach(asked, func(node string) ([]map[string]any, error) { + plan, settings, err := planFor(ctx, open, node) if err != nil { - refusals = append(refusals, fmt.Sprintf("%s:\n%v", n.Name, err)) - continue + return nil, err } // 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) + reportUnhostable(node, 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. - resources, err := declarationWith(ctx, open, n.Name, plan, settings, gens, Allocating) - if err != nil { - refusals = append(refusals, fmt.Sprintf("%s:\n%v", n.Name, err)) - continue - } - if len(resources) == 0 { - fmt.Printf("%s is assigned nothing — skipped\n", n.Name) - continue - } - sending = append(sending, ready{n.Name, resources}) - } - - if len(refusals) > 0 { - return fmt.Errorf("nothing was sent. %d node(s) could not be resolved:\n\n%s", - len(refusals), strings.Join(refusals, "\n\n")) - } + return declarationWith(ctx, open, node, plan, settings, gens, Allocating) + }) for _, s := range sending { body, err := json.Marshal(map[string]any{"declaration": 1, "resources": s.resources}) @@ -320,12 +301,71 @@ func pushCommand(ctx context.Context, args []string) error { fmt.Printf("sent %s %d resource(s)\n", s.node, len(s.resources)) } fmt.Printf("\n%d node(s) told\n", len(sending)) - return nil + return couldNotBeResolved(refusals, len(sending)) +} + +// readyNode is one machine and the declaration it would be sent. +type readyNode struct { + node string + resources []map[string]any +} + +// composeEach works out what each named machine should be, and never lets one machine's answer +// decide another's. +// +// **A machine whose set cannot be worked out is that machine's problem** (novox/hq ADR 0056). A +// whole-mesh push used to refuse outright when any one node failed to resolve, so a single +// unanswerable requirement on a single machine — one module requiring a provision nobody had +// assigned a provider for — left every other machine in the mesh unconverged, including machines +// with no relation to it at all. Nothing was sent anywhere, and the machines that could not be sent +// were the ones with nothing wrong with them. +// +// It is the same rule a92c11b established one level down, where an un-hostable module stopped +// taking down the healthy modules beside it, applied one level up: **the blast radius of a fault is +// the thing that has it.** What could not be worked out is named and returned, so a push still ends +// with a non-zero outcome and nobody mistakes a partial convergence for a whole one. +// +// The all-or-nothing rule is kept where it means something — sendTo, which rotates a credential +// across two machines that must agree — and dropped here, where it never did. +func composeEach(names []string, + compose func(node string) ([]map[string]any, error)) ([]readyNode, []string) { + + var sending []readyNode + var refusals []string + for _, name := range names { + resources, err := compose(name) + if err != nil { + refusals = append(refusals, fmt.Sprintf("%s:\n%v", name, err)) + continue + } + if len(resources) == 0 { + fmt.Printf("%s is assigned nothing — skipped\n", name) + continue + } + sending = append(sending, readyNode{name, resources}) + } + return sending, refusals +} + +// couldNotBeResolved is what a push ends with when some machines could not be worked out. +// +// **After the rest have been sent, never instead of sending them.** It is still an error, because +// the mesh is not in the state somebody asked for and a command that exits cleanly having skipped a +// machine is a command that lies. What it must not do is decide anything about the machines beside +// it, which is why it says how many were sent. +func couldNotBeResolved(refusals []string, sent int) error { + if len(refusals) == 0 { + return nil + } + return fmt.Errorf( + "%d node(s) could not be resolved and were not sent. %d other node(s) were:\n\n%s", + len(refusals), sent, strings.Join(refusals, "\n\n")) } // sendTo resolves and sends to exactly the machines named, or refuses without sending anything. // -// The same all-or-nothing rule push follows, and for the same reason: a rotation that reached the +// The all-or-nothing rule push deliberately does NOT follow, and for a reason that holds here and +// not there: a rotation that reached the // consumer and refused on the provider would leave one end holding a credential the other has // never heard of — which is the state this whole mechanism exists to make impossible. func sendTo(ctx context.Context, open *stores, names []string) error { diff --git a/cmd/mesh-control/push_test.go b/cmd/mesh-control/push_test.go new file mode 100644 index 0000000..8448169 --- /dev/null +++ b/cmd/mesh-control/push_test.go @@ -0,0 +1,74 @@ +package main + +import ( + "errors" + "strings" + "testing" +) + +// One machine that cannot be worked out is not a reason to leave the mesh unconverged. +// +// A whole-mesh push refused outright the moment any single node failed to resolve, so a module on +// the anchor requiring a provision nobody had assigned a provider for stopped every OTHER machine +// from being sent anything — machines with no relation to the fault, and nothing wrong with them. +// The failure and the punishment were on different machines. +// +// It is the same rule an un-hostable module already follows one level down (a92c11b: one module on +// the wrong machine no longer refuses the whole node), applied one level up. +func TestOneUnresolvableNodeStillLetsTheRestBeSent(t *testing.T) { + sending, refusals := composeEach( + []string{"anchor", "home-server", "laptop"}, + func(node string) ([]map[string]any, error) { + if node == "anchor" { + return nil, errors.New(`nothing provides "acme-ca", wanted by route-proxy`) + } + return []map[string]any{{"id": node + ".thing"}}, nil + }) + + var told []string + for _, s := range sending { + told = append(told, s.node) + } + if strings.Join(told, ",") != "home-server,laptop" { + t.Errorf("a machine with nothing wrong with it was not sent: %v", told) + } + if len(refusals) != 1 || !strings.Contains(refusals[0], "anchor") || + !strings.Contains(refusals[0], "acme-ca") { + t.Errorf("the machine that could not be worked out was not named with its reason: %v", + refusals) + } +} + +// And a machine assigned nothing is neither sent nor a refusal — it is nothing to say. +func TestAMachineAssignedNothingIsNotARefusal(t *testing.T) { + sending, refusals := composeEach([]string{"spare"}, + func(string) ([]map[string]any, error) { return nil, nil }) + if len(sending) != 0 || len(refusals) != 0 { + t.Errorf("a machine assigned nothing was treated as something: %v / %v", sending, refusals) + } +} + +// A push that skipped a machine still ends badly, and says what was sent. +// +// **Skipping is not succeeding.** The mesh is not in the state somebody asked for, so the command +// exits non-zero — but it says how many machines it did reach, because the old message ("nothing +// was sent") was the very claim that had become untrue. +func TestASkippedMachineIsStillAnError(t *testing.T) { + if err := couldNotBeResolved(nil, 3); err != nil { + t.Fatalf("a push that resolved every machine reported a problem: %v", err) + } + + err := couldNotBeResolved([]string{"anchor:\nnothing provides \"acme-ca\""}, 2) + if err == nil { + t.Fatal("a push that could not work out a machine reported success") + } + said := err.Error() + if strings.Contains(said, "nothing was sent") { + t.Errorf("the push says nothing was sent, and it sent two machines: %q", said) + } + for _, want := range []string{"anchor", "acme-ca", "2 other node(s) were"} { + if !strings.Contains(said, want) { + t.Errorf("the refusal does not say %q: %q", want, said) + } + } +}