From 0350b4e222764d9dbed7b799a61eb4c7c2441c9f Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 14 Oct 2026 11:06:56 +0200 Subject: [PATCH] Judge a mesh of one by the inventory, not by who reads as on the network (issue 701, review 275) The network reading leaves out a machine whose assignments cannot be read or whose modules do not resolve, so a bad reading could make a mesh of several look like one and send its machines to loopback. Count the nodes the inventory knows, and every recorded holding and assigned claimant of the resolver seat, before any address filter. --- cmd/mesh-controller/addresses_test.go | 47 ++++++++++++++++++++++++ cmd/mesh-controller/network.go | 53 ++++++++++++++++++++------- cmd/mesh-controller/plan.go | 2 +- testdata/store-tests | 1 + 4 files changed, 89 insertions(+), 14 deletions(-) diff --git a/cmd/mesh-controller/addresses_test.go b/cmd/mesh-controller/addresses_test.go index a4025239..e4071e8b 100644 --- a/cmd/mesh-controller/addresses_test.go +++ b/cmd/mesh-controller/addresses_test.go @@ -460,3 +460,50 @@ func TestAMeshOfOneIsHandedNoNameUntilAResolverAnswersIt(t *testing.T) { t.Fatalf("with a resolver, the builder publishes to %q", got) } } + +// **A mesh of two in which one machine does not resolve is still a mesh of two** (review 275). The reading of +// who is on the network leaves the unresolvable laptop out, which made it look like a mesh of one; the +// store's own node must keep its internal name, and its builder keep publishing to it. +func TestAMeshOfTwoWithAnUnresolvableMachineKeepsItsNames(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, aStore()) + register(t, open, catalogue.Manifest{Module: "builder", Version: "1", + Requires: []string{catalogue.ArtifactStoreProvision}, + Resources: []map[string]any{{"id": "env", "type": "file", "path": "/etc/mesh-builder.env", + "content": "MESH_REGISTRY=${bound:" + catalogue.ArtifactStoreProvision + ":at}:${bound:" + + catalogue.ArtifactStoreProvision + ":port}\n"}}}) + for _, module := range []string{"distribution", "builder"} { + if _, err := assign(ctx, open, "anchor", module); err != nil { + t.Fatal(err) + } + } + if err := open.inventory.SetSettings(ctx, "anchor", "distribution", + map[string]any{catalogue.PortsSetting: map[string]any{"5000": 5101}}); err != nil { + t.Fatal(err) + } + // The laptop does not resolve: two modules claim one node seat there, straight into the store. + twoRivalsAndANote(t, open) + for _, m := range []string{"rival-one", "rival-two"} { + if _, err := open.inventory.Assign(ctx, "laptop", m); err != nil { + t.Fatal(err) + } + } + shelf, err := open.inventory.Catalogue(ctx) + if err != nil { + t.Fatal(err) + } + onNetwork, err := whereEveryoneIs(ctx, open.inventory, shelf) + if err != nil || len(onNetwork) != 1 { + t.Fatalf("the unresolvable laptop is still read as on the network: %v %v", onNetwork, err) + } + + if got, err := artifactStoreAddress(ctx, open.inventory, shelf, "anchor"); err != nil || got != "anchor.internal:5101" { + t.Fatalf("a mesh of two read as one: the store's own node is handed %q (%v)", got, err) + } + for _, r := range composed(t, open, "anchor").Resources { + if r["path"] == "/etc/mesh-builder.env" && r["content"] != "MESH_REGISTRY=anchor.internal:5101\n" { + t.Fatalf("a mesh of two read as one: the builder publishes to %v", r["content"]) + } + } +} diff --git a/cmd/mesh-controller/network.go b/cmd/mesh-controller/network.go index 51944230..e9c32ae9 100644 --- a/cmd/mesh-controller/network.go +++ b/cmd/mesh-controller/network.go @@ -729,7 +729,7 @@ func artifactStoreAddress(ctx context.Context, inv *inventory.Inventory, // A mesh of more than one node is answered as before: loopback is no answer for the others, and // one name for every node is what a mesh of several without a resolver is still put together to. if node == forNode { - unanswered, err := aLoneNameNothingAnswers(ctx, inv, shelf, len(on)) + unanswered, err := aLoneNameNothingAnswers(ctx, inv) if err != nil { return "", err } @@ -746,21 +746,48 @@ func artifactStoreAddress(ctx context.Context, inv *inventory.Inventory, return "127.0.0.1:" + port, nil } -// aLoneNameNothingAnswers is whether the mesh is one node on its private network and nothing answers that -// node's internal name: no holder of the mesh's resolver seat (novox/hq ADR 0194, ADR 0223), as a machine's -// resolver file lists them (replicatedHolders). Genesis is exactly that from its network step until a -// resolver is installed (novox/hq issue 701): the node then reaches what runs on it by loopback, as it did -// before the network, and is not composed a name no lookup can answer. -func aLoneNameNothingAnswers(ctx context.Context, inv *inventory.Inventory, shelf map[string]catalogue.Manifest, - onNetwork int) (bool, error) { - if onNetwork != 1 { - return false, nil - } - holders, err := replicatedHolders(ctx, inv, shelf) +// aLoneNameNothingAnswers is whether the mesh is one node and nothing will answer that node's internal +// name: the inventory knows exactly one node, and no module holds or claims the mesh's resolver seat +// anywhere (novox/hq ADR 0194, ADR 0223). Genesis is exactly that from its network step until a resolver is +// installed (novox/hq issue 701): the node then reaches what runs on it by loopback, as it did before the +// network, and is not composed a name no lookup can answer. +// +// **Read from the mesh, never from who is on the network** (review 275): that reading leaves out a machine +// whose assignments cannot be read or whose modules do not resolve, and a bad reading must never make a +// mesh of several look like a mesh of one and send its machines to loopback. Every recorded holding and +// every assignment of a claimant counts, wherever it is and whether or not its machine has an address. +func aLoneNameNothingAnswers(ctx context.Context, inv *inventory.Inventory) (bool, error) { + nodes, err := inv.Nodes(ctx) if err != nil { return false, err } - return len(holders[resolverSeat]) == 0, nil + if len(nodes) != 1 { + return false, nil + } + holdings, err := inv.Holdings(ctx) + if err != nil { + return false, err + } + for _, h := range holdings { + if s, ok := catalogue.SeatNamed(h.Claim); ok && s.Name == resolverSeat { + return false, nil + } + } + entries, err := inv.Catalogued(ctx) + if err != nil { + return false, err + } + for _, e := range entries { + if len(e.On) == 0 { + continue + } + for _, c := range e.Manifest.Claims { + if s, ok := catalogue.SeatNamed(c.Name); ok && s.Name == resolverSeat { + return false, nil + } + } + } + return true, nil } // artifactStoreHolder is whichever node is assigned a module offering the artifact store, on or diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index 3d11f00d..e0deab4c 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -108,7 +108,7 @@ func planFor(ctx context.Context, open *stores, nodeName string) (catalogue.Reso // loopback, as before the network, rather than by a name no lookup on the machine can answer. ownName := onNetwork[nodeName] if ownName != "" { - unanswered, err := aLoneNameNothingAnswers(ctx, inv, shelf, len(onNetwork)) + unanswered, err := aLoneNameNothingAnswers(ctx, inv) if err != nil { return catalogue.Resolution{}, nil, err } diff --git a/testdata/store-tests b/testdata/store-tests index e0952edc..2b7fe261 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -19,6 +19,7 @@ ./internal/inventory TestAWalksStartIsKeptAndARefusedSaveLeavesItUnsaid ./cmd/mesh-controller TestAWalkLetGoAndStoppedByVerbsIsSaidStartedAndEnded ./cmd/mesh-controller TestAMeshOfOneIsHandedNoNameUntilAResolverAnswersIt +./cmd/mesh-controller TestAMeshOfTwoWithAnUnresolvableMachineKeepsItsNames ./cmd/mesh-controller TestAWalkStoppedOrClosedByAPersonIsSaidEndedStopped ./cmd/mesh-controller TestAWalkRetriedByAPersonIsSaidStartedAgain ./cmd/mesh-controller TestABacklogReleasedByAPersonIsSaidStarted