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") + } +}