cmd/mesh-controller/network.go

@@ -348,11 +348,28 @@ func whereEveryoneIs(ctx context.Context, inv *inventory.Inventory,
 		return nil, errors.New(
 			"asked where everyone is without the catalogue, which cannot be answered")
 	}
-	places, err := inv.Overlays(ctx)
+	places, err := onTheNetwork(ctx, inv, shelf)
 	if err != nil {
 		return nil, err
 	}
 	out := map[string]string{}
+	for _, p := range places {
+		out[p.Name] = overlay.InternalName(p.Name)
+	}
+	return out, nil
+}
+
+// onTheNetwork is every placed machine that resolves the private network — has an address AND
+// runs what puts it there. "Has an address" alone was true of every placed machine and told you
+// nothing about whether anything could reach it; a name written for such a machine resolves to
+// an address that does not answer, and a connection to it hangs (novox/hq issue 079).
+func onTheNetwork(ctx context.Context, inv *inventory.Inventory,
+	shelf map[string]catalogue.Manifest) ([]inventory.Overlay, error) {
+	places, err := inv.Overlays(ctx)
+	if err != nil {
+		return nil, err
+	}
+	var out []inventory.Overlay
 	for _, p := range places {
 		if p.Address == "" {
 			continue
@@ -371,7 +388,7 @@ func whereEveryoneIs(ctx context.Context, inv *inventory.Inventory,
 		for _, m := range got.Modules {
 			for _, offered := range m.Offers() {
 				if offered == overlay.Requirement {
-					out[p.Name] = overlay.InternalName(p.Name)
+					out = append(out, p)
 				}
 			}
 		}
@@ -400,22 +417,21 @@ func onThePrivateNetwork(ctx context.Context, inv *inventory.Inventory) ([]strin
 	return out, nil
 }
 
-// namesInTheMesh is every machine's internal name and the address behind it.
+// namesInTheMesh is every machine's internal name and the address behind it — every machine
+// that is on the private network, the same set the resolver means by that.
 //
-// A machine with no address has no name: writing one that resolves to nothing is worse than not
+// A machine that is not has no name: writing one that resolves to nothing is worse than not
 // writing it, because a connection to an address that does not answer hangs where a name that
-// does not resolve fails at once and says so. That is the rule the hosts file already follows,
-// and this is the same set read the same way.
-func namesInTheMesh(ctx context.Context, inv *inventory.Inventory) (map[string]string, error) {
-	places, err := inv.Overlays(ctx)
+// does not resolve fails at once and says so. A machine placed on the overlay but not running
+// the module that puts it there is exactly that (novox/hq issue 079).
+func namesInTheMesh(ctx context.Context, inv *inventory.Inventory,
+	shelf map[string]catalogue.Manifest) (map[string]string, error) {
+	places, err := onTheNetwork(ctx, inv, shelf)
 	if err != nil {
 		return nil, err
 	}
 	out := map[string]string{}
 	for _, p := range places {
-		if strings.TrimSpace(p.Address) == "" {
-			continue
-		}
 		out[overlay.InternalName(p.Name)] = p.Address
 	}
 	return out, nil

cmd/mesh-controller/network_test.go

@@ -76,3 +76,34 @@ func TestAPlacementAndNothingTogetherIsRefused(t *testing.T) {
 		t.Fatal("a placement and --nothing together was accepted")
 	}
 }
+
+// A machine is named for the others only while it is on the private network — placed AND running
+// what puts it there — the same set the resolver means by "on the private network". A machine
+// that has an address and no networking is not named: a name resolving to an address that does
+// not answer hangs where an unknown name fails at once (novox/hq issue 079).
+func TestOnlyAMachineOnThePrivateNetworkIsNamed(t *testing.T) {
+	open := aMesh(t) // two placed machines, both assigned what puts them on the private network
+	ctx := t.Context()
+	shelf, err := open.inventory.Catalogue(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	names, err := namesInTheMesh(ctx, open.inventory, shelf)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if names["anchor.internal"] != "10.77.0.1" || names["laptop.internal"] != "10.77.0.2" {
+		t.Fatalf("two machines on the network are not both named: %v", names)
+	}
+	// The laptop keeps its place and its address, and stops running the network.
+	if err := open.inventory.Unassign(ctx, "laptop", "mesh-wireguard"); err != nil {
+		t.Fatal(err)
+	}
+	names, err = namesInTheMesh(ctx, open.inventory, shelf)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, still := names["laptop.internal"]; still || names["anchor.internal"] != "10.77.0.1" {
+		t.Fatalf("a machine that left the network is still named, or the one that stayed is not: %v", names)
+	}
+}

cmd/mesh-controller/plan.go

@@ -434,7 +434,11 @@ func declarationWith(ctx context.Context, open *stores, node string,
 	// And every machine's name, so a container can reach one. The same set that writes the
 	// machine's own hosts file — one reading, so a container and its machine cannot disagree
 	// about where another machine is.
-	names, err := namesInTheMesh(ctx, inv)
+	shelf, err := inv.Catalogue(ctx)
+	if err != nil {
+		return nil, err
+	}
+	names, err := namesInTheMesh(ctx, inv, shelf)
 	if err != nil {
 		return nil, err
 	}