From c154bdedd315cfd2f53571e3b22e0ed3346d80a2 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 15:03:17 +0200 Subject: [PATCH] Keep a machine that does not resolve on the private network at its place, so one machine's modules refuse only its own declaration (issue 1833) A machine whose own set could not be resolved was left off the private network; when it was the hub, the network could not be composed and every machine's declaration failed. Its placement (key and address) is its last good place and does not change when its modules fail, so it stays on the network and only its own declaration is refused. A refused machine with no place is left off, and a network that cannot be composed from what remains still fails. --- go/compose/network.go | 13 ++- go/compose/network_test.go | 11 ++- go/compose/one_machine_alone_test.go | 142 +++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 go/compose/one_machine_alone_test.go diff --git a/go/compose/network.go b/go/compose/network.go index 1e66c51..0ea9d56 100644 --- a/go/compose/network.go +++ b/go/compose/network.go @@ -74,7 +74,10 @@ func (c *composer) overlayRange() string { } // network builds the private network over the machines that resolved the module for it. A machine is on it because it -// was given the module, or because it is joining with a key and an address (novox/hq ADR 0169). +// was given the module, or because it is joining with a key and an address (novox/hq ADR 0169), or because its own set +// could not be resolved and it holds a place (a key and an address): that place is its last good one on the network, and +// it does not change because its modules failed, so every other machine keeps its path to it -- the hub above all -- and +// only its own declaration is refused (issue 1833). A machine refused with no place is left off. func (c *composer) network(on map[string]bool, refused map[string]string) (*overlay.Generator, error) { tunnels := c.in.Tunnels carried := c.in.CarriedPeers @@ -85,9 +88,15 @@ func (c *composer) network(on map[string]bool, refused map[string]string) (*over var nodes []overlay.Node for _, node := range c.in.sorted() { p := node.Place - if !on[node.Name] && !(isJoining[node.Name] && p.Key != "" && p.Address != "") { + placed := p.Key != "" && p.Address != "" + _, wasRefused := refused[node.Name] + if !on[node.Name] && !((isJoining[node.Name] || wasRefused) && placed) { continue } + if !on[node.Name] && wasRefused && !isJoining[node.Name] { + c.say("%s could not be resolved and keeps its place on the private network at %s, so the others keep their path to it", + node.Name, p.Address) + } n := overlay.Node{ Name: node.Name, Key: p.Key, Endpoint: p.Endpoint, Site: p.Site, Hub: p.Hub, Address: p.Address, diff --git a/go/compose/network_test.go b/go/compose/network_test.go index 9f5717f..ff73e08 100644 --- a/go/compose/network_test.go +++ b/go/compose/network_test.go @@ -159,9 +159,11 @@ func TestANodeNotOnThePrivateNetworkGetsNoAddressAndNoNetwork(t *testing.T) { } // 1360/AC3: a node whose resolution is refused is said with today's words: the network missing its hub names the -// machine that could not be resolved, which is almost always why. +// machine that could not be resolved, which is almost always why. The hub here holds no complete place (no key): a refused +// machine with one keeps it on the network (issue 1833), so only one without is missing from it. func TestARefusedNodeIsSaidWhenTheNetworkCannotBeComposed(t *testing.T) { in := aMesh(t) + nodeOf(t, &in, "anchor").Place.Key = "" one := catalogue.Manifest{Module: "rival-one", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} two := catalogue.Manifest{Module: "rival-two", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} in.Catalogue["rival-one"], in.Catalogue["rival-two"] = one, two @@ -210,9 +212,10 @@ func TestEachFieldOfTheInputReachesTheGenerator(t *testing.T) { if len(base) != 2 { t.Fatalf("the base mesh has %v", base) } - t.Run("Catalogue: without the module nobody is on the network", func(t *testing.T) { + t.Run("Catalogue: without the module offering it nobody is on the network", func(t *testing.T) { in := aMesh(t) - in.Catalogue = map[string]catalogue.Manifest{} + // The module stays known, so every machine still resolves: a machine that does not resolve keeps its place (issue 1833). + in.Catalogue = map[string]catalogue.Manifest{overlay.Name: {Module: overlay.Name, Version: "1"}} if got := addressesOf(in); len(got) != 0 { t.Errorf("got %v", got) } @@ -290,6 +293,8 @@ func TestEachFieldOfTheInputReachesTheGenerator(t *testing.T) { } laptop := nodeOf(t, &in, "laptop") laptop.Assigned = append(laptop.Assigned, "holder-one", "holder-two") + // Without a key: a machine that does not resolve and holds a complete place keeps it on the network (issue 1833). + laptop.Place.Key = "" if got := addressesOf(in); got["laptop"] != "" { t.Fatalf("a machine whose set claims one seat twice is on the network: %v", got) } diff --git a/go/compose/one_machine_alone_test.go b/go/compose/one_machine_alone_test.go new file mode 100644 index 0000000..3fc89e5 --- /dev/null +++ b/go/compose/one_machine_alone_test.go @@ -0,0 +1,142 @@ +package compose + +import ( + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/overlay" +) + +// fourOnTheNetwork is aMesh and two more machines on the private network at the site `there`: `ace`, dialled at +// ace.example:51820, at 10.77.0.3, and `box`, behind no endpoint, at 10.77.0.4. A module `office` requires +// `document-server`, which nothing in the catalogue provides: the shape of issue 1833 (nextcloud on the hub wanting a +// document server nobody offered). +func fourOnTheNetwork(t *testing.T) Input { + t.Helper() + in := aMesh(t) + in.Catalogue["office"] = catalogue.Manifest{Module: "office", Version: "1", Requires: []string{"document-server"}} + caps := map[string]bool{"container-runtime": true, "wireguard": true, "systemd": true} + in.Nodes = append(in.Nodes, + Node{Name: "ace", Place: Place{Key: keyOf(t, "ace"), Endpoint: "ace.example:51820", Site: "there", Address: "10.77.0.3"}, + Assigned: []string{overlay.Name}, Capabilities: caps}, + Node{Name: "box", Place: Place{Key: keyOf(t, "box"), Site: "there", Address: "10.77.0.4"}, + Assigned: []string{overlay.Name}, Capabilities: caps}) + return in +} + +// unresolvableBy makes one machine's own set fail to resolve: it is also given `office`, whose requirement nothing provides. +func unresolvableBy(t *testing.T, in *Input, name string) { + t.Helper() + n := nodeOf(t, in, name) + n.Assigned = append(n.Assigned, "office") +} + +// networkOf is the generator of an Input's private network, failing the test when it is not composed. +func networkOf(t *testing.T, in Input) *overlay.Generator { + t.Helper() + gens, err := Generators(in) + if err != nil { + t.Fatalf("the private network was not composed: %v", err) + } + return gens[overlay.Name].(*overlay.Generator) +} + +func placesOf(g *overlay.Generator) map[string]overlay.Node { + out := map[string]overlay.Node{} + for _, n := range g.Nodes() { + out[n.Name] = n + } + return out +} + +// 1833/AC1: four machines, the hub's own set does not resolve; the other three compose their declarations, and the hub alone is +// refused, with an error that is its own set failing and names what could not be resolved. +func TestOneMachineThatDoesNotResolveIsRefusedAlone(t *testing.T) { + in := fourOnTheNetwork(t) + unresolvableBy(t, &in, "anchor") + for _, node := range []string{"laptop", "ace", "box"} { + d, problems, err := Compose(in, node) + if err != nil || len(problems) != 0 { + t.Errorf("%s does not compose because another machine does not resolve: %v %v", node, err, problems) + continue + } + if !has(resourceIDs(d), "file:mesh-wireguard.overlay-config") { + t.Errorf("%s is given no private network: %v", node, resourceIDs(d)) + } + } + _, _, err := Compose(in, "anchor") + if err == nil || !IsUnresolvable(err) { + t.Fatalf("the machine that does not resolve is not refused as its own set failing: %v", err) + } + if !strings.Contains(err.Error(), `nothing provides "document-server", wanted by office`) { + t.Errorf("the refusal does not name what could not be resolved: %v", err) + } +} + +// 1833/AC2: the private network is composed with every machine on it: the hub that does not resolve keeps its place (key, +// address, endpoint, site, hub role), so every other machine keeps its path through it. +func TestAMachineThatDoesNotResolveKeepsItsPlaceOnTheNetwork(t *testing.T) { + in := fourOnTheNetwork(t) + unresolvableBy(t, &in, "anchor") + got := placesOf(networkOf(t, in)) + if len(got) != 4 { + t.Fatalf("the network is over %v, want all four machines", got) + } + hub := got["anchor"] + if !hub.Hub || hub.Key != keyOf(t, "anchor") || hub.Address != "10.77.0.1" || hub.Endpoint != "anchor.example:51820" || hub.Site != "here" { + t.Errorf("the hub did not keep its place: %+v", hub) + } + nodes, graph, err := Graph(in) + if err != nil || len(nodes) != 4 { + t.Fatalf("the graph is not shown with every machine: %v %v", nodes, err) + } + for _, node := range []string{"ace", "box"} { + through := false + for _, p := range graph[node] { + through = through || (p.Name == "anchor" && p.Endpoint == "anchor.example:51820") + } + if !through { + t.Errorf("%s has no path through the hub: %+v", node, graph[node]) + } + } +} + +// 1833/AC3: a machine that does not resolve and was never placed (a new machine: no key, no address) is left off the network, +// which is composed from the rest -- here with the hub that does not resolve on its last place -- and that machine alone is refused. +func TestANewMachineThatDoesNotResolveIsLeftOffAndRefusedAlone(t *testing.T) { + in := fourOnTheNetwork(t) + unresolvableBy(t, &in, "anchor") + in.Nodes = append(in.Nodes, Node{Name: "fresh", Place: Place{Site: "there"}, + Assigned: []string{overlay.Name, "office"}, Capabilities: map[string]bool{"wireguard": true, "systemd": true}}) + got := placesOf(networkOf(t, in)) + if _, on := got["fresh"]; on || len(got) != 4 { + t.Fatalf("the network is over %v, want the four placed machines without fresh", got) + } + if _, _, err := Compose(in, "fresh"); err == nil || !IsUnresolvable(err) { + t.Errorf("the new machine is not refused as its own set failing: %v", err) + } + if _, problems, err := Compose(in, "box"); err != nil || len(problems) != 0 { + t.Errorf("another machine does not compose: %v %v", err, problems) + } +} + +// 1833/AC4: a network that truly cannot be composed from what remains still fails: the hub does not resolve and has no +// complete place (no key), so nothing is the hub; the refusal is today's, naming the machine that could not be resolved. +func TestANetworkThatCannotBeComposedFromWhatRemainsStillFails(t *testing.T) { + in := fourOnTheNetwork(t) + unresolvableBy(t, &in, "anchor") + nodeOf(t, &in, "anchor").Place.Key = "" + _, err := Generators(in) + if err == nil { + t.Fatal("a network without a hub was composed") + } + for _, want := range []string{overlay.ErrNoHub.Error(), "and 1 node(s) could not be resolved at all, which is probably why:", " anchor: "} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the refusal does not say %q:\n%v", want, err) + } + } + if _, _, err := Compose(in, "laptop"); err == nil { + t.Error("a declaration was composed over a network that cannot be") + } +}