diff --git a/cmd/mesh-controller/network.go b/cmd/mesh-controller/network.go index fb93047..dcfccae 100644 --- a/cmd/mesh-controller/network.go +++ b/cmd/mesh-controller/network.go @@ -346,9 +346,16 @@ func whoResolves(ctx context.Context, open *stores, requirement string) ( refused := map[string]string{} for _, n := range nodes { plan, _, err := planFor(ctx, open, n.Name) - if err != nil { + switch { + case unresolvable(err): refused[n.Name] = err.Error() continue + case err != nil: + // Not a node that does not resolve — a question that went unanswered. Recording it as a + // refusal would take the machine off the private network, and the generator that reads + // this would then write a roster and a filter without it (novox/hq 04-ISSUES/152). + return nil, nil, fmt.Errorf("whether %s answers %q cannot be read: %w", + n.Name, requirement, err) } for _, m := range plan.Modules { for _, offered := range m.Offers() { diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index c6f174d..a32a4a2 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -25,7 +25,36 @@ import ( // cheapest next step. That is how novox/hq ADR 0001 records `hal/sdk` reaching 34,636: // nothing in it was wrong, and no one edit was the one that should have been a new file. +// notResolvable marks the one failure in planFor that is a statement about the node: its assigned +// modules do not compose. Every other failure means the mesh could not be *asked* — the store was +// unreachable, a key could not be read — and says nothing about the node at all. +// +// The distinction exists because three callers gather something across every machine and must carry +// on when one machine's set is broken. Each of them read a plain error as "their set does not +// resolve", and so read a store that was briefly unreachable as a machine that runs nothing. On the +// roster of routed names that is not a degraded answer but a false one: it states, to every machine +// at once, that another machine's names do not exist. A control node spent hours replacing every +// container it ran, on a six-minute cycle, because each pass restarted the store this is read from, +// the read failed, one name left the roster, and the roster is part of every container's identity +// (novox/hq 04-ISSUES/152, and 04-ISSUES/151 for why a changed roster is a changed container). +// +// So: skip a node that cannot resolve, and never a node that could not be read. +type notResolvable struct{ err error } + +func (n notResolvable) Error() string { return n.err.Error() } +func (n notResolvable) Unwrap() error { return n.err } + +// unresolvable reports whether err is a node's own set failing to compose, rather than the mesh +// being unable to answer. +func unresolvable(err error) bool { + var n notResolvable + return errors.As(err, &n) +} + // planFor works out everything a node should run, from what was assigned to it. +// +// A failure to compose the node's own modules is wrapped as notResolvable; every other failure is +// returned as it is. Callers gathering across the mesh must tell them apart — see notResolvable. func planFor(ctx context.Context, open *stores, nodeName string) (catalogue.Resolution, catalogue.SettingsBy, error) { inv := open.inventory shelf, err := inv.Catalogue(ctx) @@ -95,7 +124,9 @@ func planFor(ctx context.Context, open *stores, nodeName string) (catalogue.Reso At: onNetwork[nodeName], PublicDomain: publicDomain, Account: who.Account, AccountHome: who.AccountHome}, world) if err != nil { - return catalogue.Resolution{}, nil, err + // The node's own set does not compose. Marked, because this is the only failure here that + // a mesh-wide gatherer may pass over — see notResolvable. + return catalogue.Resolution{}, nil, notResolvable{err} } // The credential for each thing this node takes from elsewhere. Made once and kept, so the @@ -163,8 +194,13 @@ func planFor(ctx context.Context, open *stores, nodeName string) (catalogue.Reso if len(stray) > 0 { // Somebody set something that reaches no file. Said here rather than discovered by the // machine not behaving differently, which is the slowest way there is. - return catalogue.Resolution{}, nil, fmt.Errorf( - "these settings reach nothing:\n - %s", strings.Join(stray, "\n - ")) + // + // Marked like a set that will not compose, and for the same reason: it is a standing fact + // about this node's own configuration, not a question the mesh could not answer. A gatherer + // passes over it as it always did — one node's stray setting must not stop every other node + // being described (novox/hq 04-ISSUES/152). + return catalogue.Resolution{}, nil, notResolvable{fmt.Errorf( + "these settings reach nothing:\n - %s", strings.Join(stray, "\n - "))} } return resolved, settings, nil } @@ -671,11 +707,17 @@ func renderingFor(ctx context.Context, open *stores, node string, // routed name only because it carried a label the mesh composed, never because the mesh knows what // "route" means. A node that does not resolve is skipped, so one machine's broken set does not cost // the rest their names. +// +// **A node that could not be READ is a different matter and is raised.** Skipping one states, to +// every machine at once, that its names do not exist — and since the roster is part of every +// container's identity, that withdraws them and replaces every container (novox/hq 04-ISSUES/152, +// 151). So every failure here says which machine and which read, because the alternative is a +// mesh-wide refusal with nothing named in it. func routeNamesInTheMesh(ctx context.Context, open *stores) (map[string]string, error) { inv := open.inventory places, err := inv.Overlays(ctx) if err != nil { - return nil, err + return nil, fmt.Errorf("where the machines are cannot be read: %w", err) } address := map[string]string{} for _, p := range places { @@ -686,14 +728,22 @@ func routeNamesInTheMesh(ctx context.Context, open *stores) (map[string]string, nodes, err := inv.Nodes(ctx) if err != nil { - return nil, err + return nil, fmt.Errorf("which machines the mesh has cannot be read: %w", err) } out := map[string]string{} for _, n := range nodes { plan, settings, err := planFor(ctx, open, n.Name) - if err != nil { + switch { + case unresolvable(err): + // Their set does not compose, so they serve no names. Passed over, so one machine's + // broken set does not cost the rest theirs. continue + case err != nil: + // The mesh could not be asked. Returning the roster without this machine's names would + // state that they do not exist — to every machine, and indistinguishably from the + // operator having withdrawn them (novox/hq 04-ISSUES/152). + return nil, fmt.Errorf("the names %s serves cannot be read: %w", n.Name, err) } for _, m := range plan.Modules { for to := range m.Contributes { @@ -823,11 +873,17 @@ func grantsFor(ctx context.Context, open *stores, node string) ([]catalogue.Gran out := make([]catalogue.Grant, 0, len(issued)) for _, s := range issued { plan, settings, err := planFor(ctx, open, s.Consumer) - if err != nil { + switch { + case unresolvable(err): // Their set does not resolve. Skipped rather than fatal: this node is not the place // to report another machine's problem, and a grant for something that is not going to // run would have the provider create a user nothing uses. continue + case err != nil: + // The mesh could not be asked what they wanted, which is not the same as their wanting + // nothing — and withholding a grant on that reading takes a consumer's access away + // (novox/hq 04-ISSUES/152). + return nil, fmt.Errorf("what %s asked of %s cannot be read: %w", s.Consumer, s.Name, err) } values, asks, err := plan.ContributionsFrom(s.Name, s.ConsumerModule, settings) if err != nil { diff --git a/cmd/mesh-controller/roster_failure_test.go b/cmd/mesh-controller/roster_failure_test.go new file mode 100644 index 0000000..82b87ac --- /dev/null +++ b/cmd/mesh-controller/roster_failure_test.go @@ -0,0 +1,99 @@ +package main + +import ( + "context" + "strings" + "testing" +) + +// A node's own set failing to compose, and the mesh being unable to answer at all, are different +// things, and only the first may be passed over when something is gathered across every machine +// (novox/hq 04-ISSUES/152). These pin that distinction where the three gatherers rely on it. + +func TestASetThatDoesNotComposeIsMarkedAsTheNodesOwnProblem(t *testing.T) { + open := aMesh(t) + one, two := rivals() + register(t, open, one) + register(t, open, two) + for _, m := range []string{one.Module, two.Module} { + if _, err := open.inventory.Assign(t.Context(), "laptop", m); err != nil { + t.Fatal(err) + } + } + + _, _, err := planFor(t.Context(), open, "laptop") + if err == nil { + t.Fatal("two modules claiming one seat composed anyway") + } + if !unresolvable(err) { + t.Fatalf("a set that cannot compose was not marked as the node's own problem: %v", err) + } +} + +func TestAStoreThatCannotBeReadIsNotANodeThatDoesNotCompose(t *testing.T) { + open := aMesh(t) + + // Nothing is wrong with anchor. The question simply cannot be asked. + stopped, cancel := context.WithCancel(t.Context()) + cancel() + + _, _, err := planFor(stopped, open, "anchor") + if err == nil { + t.Fatal("a plan composed against a store that could not be read") + } + if unresolvable(err) { + t.Fatalf("a question the mesh could not answer was read as a node that runs nothing: %v", err) + } +} + +func TestOneIncoherentNodeDoesNotCostTheRestTheirNames(t *testing.T) { + open := aMesh(t) + one, two := rivals() + register(t, open, one) + register(t, open, two) + for _, m := range []string{one.Module, two.Module} { + if _, err := open.inventory.Assign(t.Context(), "laptop", m); err != nil { + t.Fatal(err) + } + } + + // laptop cannot compose. That is laptop's problem and nobody else's: the roster is still + // answerable, and anchor keeps whatever it serves. + if _, err := routeNamesInTheMesh(t.Context(), open); err != nil { + t.Fatalf("one node's broken set cost the whole mesh its roster: %v", err) + } +} + +func TestARosterIsNeverReturnedWithNamesItCouldNotRead(t *testing.T) { + open := aMesh(t) + + stopped, cancel := context.WithCancel(t.Context()) + cancel() + + names, err := routeNamesInTheMesh(stopped, open) + if err == nil { + t.Fatalf("a roster was composed from a store that could not be read: %v", names) + } + // The failure must be raised, not turned into an absence. A roster missing a machine's names + // is indistinguishable, on every machine that receives it, from the operator withdrawing them — + // and because the roster is part of every container's identity, it replaces all of them. + if names != nil { + t.Fatalf("a partial roster was returned beside the error: %v", names) + } +} + +// Kept so the reason survives the next person reading it: the message the gatherer raises must say +// which machine could not be read, or the operator is left with a mesh-wide failure and no name. +func TestTheRaisedFailureNamesTheMachineItCouldNotRead(t *testing.T) { + open := aMesh(t) + stopped, cancel := context.WithCancel(t.Context()) + cancel() + + _, err := routeNamesInTheMesh(stopped, open) + if err == nil { + t.Fatal("no failure was raised") + } + if !strings.Contains(err.Error(), "cannot be read") { + t.Fatalf("the failure does not say the mesh could not be read: %v", err) + } +}