Merge pull request 'Keep a machine that does not resolve on the private network and in the names, so it is refused alone (issue 1833)' (#68) from fix/1833-one-machine-alone into main

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,
@@ -233,7 +242,9 @@ func (c *composer) whereEveryoneIs() (map[string]string, error) {
 }
 
 // onTheNetwork is every placed machine that resolves the private network: has an address AND runs what puts it there
-// (novox/hq issue 079). A machine that does not resolve is not counted, and said.
+// (novox/hq issue 079). A machine that does not resolve keeps its place when it holds one (a key and an address), by the
+// same rule as network(), so its name and its address stay where the tunnel to it does (issue 1833); one without is not
+// counted, and said.
 func (c *composer) onTheNetwork() []Node {
 	var out []Node
 	for _, n := range c.in.sorted() {
@@ -247,6 +258,12 @@ func (c *composer) onTheNetwork() []Node {
 			catalogue.Node{Name: n.Name, Site: n.Place.Site, Capabilities: n.Capabilities},
 			catalogue.World{Unchecked: true, Holdings: c.in.Holdings})
 		if err != nil {
+			if n.Place.Key != "" {
+				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",
+					n.Name, n.Place.Address)
+				out = append(out, n)
+				continue
+			}
 			c.say("%s is not counted as on the network: it does not resolve: %v", n.Name, err)
 			continue
 		}

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

go/compose/node_test.go

@@ -387,9 +387,11 @@ func TestAnotherNodesKeptBuildDoesNotChangeTheRestOfTheMeshOrWhoIsOnTheNetwork(t
 
 func overlayName() string { return overlay.Name }
 
-// Round 1, finding 6: the nodes the composition did not count as on the network are returned, said, not lost.
+// Round 1, finding 6: the nodes the composition did not count as on the network are returned, said, not lost. laptop holds no
+// complete place (no key): one that does keeps it on the network (issue 1833) and is counted.
 func TestANodeNotCountedAsOnTheNetworkIsSaidInTheResult(t *testing.T) {
 	in := aMesh(t)
+	nodeOf(t, &in, "laptop").Place.Key = ""
 	in.Catalogue["rival-one"] = catalogue.Manifest{Module: "rival-one", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}}
 	in.Catalogue["rival-two"] = catalogue.Manifest{Module: "rival-two", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}}
 	laptop := nodeOf(t, &in, "laptop")

go/compose/one_machine_alone_test.go

@@ -0,0 +1,167 @@
+package compose
+
+import (
+	"slices"
+	"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")
+	}
+}
+
+// 1833/AC2, a failure the composition's unchecked pass refuses too (two modules claiming one seat on the hub, the 1360/AC3
+// shape): the hub keeps its place in what every other machine is sent besides the tunnel -- its internal name and its address
+// among the sources "from the mesh" -- and another machine still composes.
+func TestAMachineThatDoesNotResolveKeepsItsNameAndItsAddressInTheMesh(t *testing.T) {
+	in := fourOnTheNetwork(t)
+	in.Catalogue["rival-one"] = catalogue.Manifest{Module: "rival-one", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}}
+	in.Catalogue["rival-two"] = catalogue.Manifest{Module: "rival-two", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}}
+	hub := nodeOf(t, &in, "anchor")
+	hub.Assigned = append(hub.Assigned, "rival-one", "rival-two")
+	if _, err := Resolve(in, "anchor"); !IsUnresolvable(err) {
+		t.Fatalf("the hub is meant not to resolve: %v", err)
+	}
+	c := newComposer(in)
+	if got := c.namesInTheMesh()["anchor."+c.suffix()]; got != "10.77.0.1" {
+		t.Errorf("the hub's name is not in the mesh's names: %v", c.namesInTheMesh())
+	}
+	if got := c.onThePrivateNetwork(); !slices.Contains(got, "10.77.0.1") || len(got) != 4 {
+		t.Errorf("the sources from the mesh are %v, want all four machines with the hub", got)
+	}
+	if _, problems, err := Compose(in, "ace"); err != nil || len(problems) != 0 {
+		t.Errorf("another machine does not compose: %v %v", err, problems)
+	}
+}