Merge pull request 'Retire node-resolver-config and the seat need only it used (hq ADR 0223 part 2, step 2 of 2)' (#68) from retire-resolv-conf into main

internal/catalogue/resolver_file_is_the_uplinks_test.go

@@ -0,0 +1,57 @@
+package catalogue
+
+import (
+	"reflect"
+	"testing"
+)
+
+// novox/hq ADR 0223 part 2: /etc/resolv.conf belongs to the module holding node-uplink. The seat that
+// wrote it, node-resolver-config, and ADR 0220's dependency of it on the uplink retire: one owner for
+// the file, and it is the program that would otherwise rewrite it.
+
+func TestTheResolverConfigSeatIsGone(t *testing.T) {
+	if _, known := SeatNamed("node-resolver-config"); known {
+		t.Error("node-resolver-config is still in the mesh's set; the uplink's holder writes the resolver file")
+	}
+	// An uplink's holder needs no seat beside it for the file: it is its own.
+	if got := DependsOn(mod("networkmanager", nil, nil, nil, Claim{Name: "node-uplink"})); len(got) != 0 {
+		t.Errorf("an uplink holder with nothing declared depends on %v", got)
+	}
+}
+
+// The catalogue as it is: nothing claims the retired seat, and every manager the mesh knows holds the
+// uplink and writes the resolver file.
+func TestTheCataloguesUplinksWriteTheResolverFileAndNothingElseDoes(t *testing.T) {
+	cat := map[string]Manifest{}
+	for _, m := range theCatalogue(t) {
+		cat[m.Module] = m
+	}
+	if got := PossibleHolders(cat, "node-uplink"); !reflect.DeepEqual(got, uplinks) {
+		t.Errorf("node-uplink can be held by %v, not %v", got, uplinks)
+	}
+	for name, m := range cat {
+		for _, c := range m.Claims {
+			if c.Name == "node-resolver-config" {
+				t.Errorf("%s still claims node-resolver-config", name)
+			}
+		}
+		_, writes := m.Facts["resolvers"]
+		isUplink := false
+		for _, u := range uplinks {
+			isUplink = isUplink || u == name
+		}
+		for _, f := range m.Facts {
+			if f.Path == "/etc/resolv.conf" && !isUplink {
+				t.Errorf("%s writes /etc/resolv.conf and does not hold the uplink", name)
+			}
+		}
+		for _, r := range m.Resources {
+			if r["path"] == "/etc/resolv.conf" {
+				t.Errorf("%s declares /etc/resolv.conf as a file of its own", name)
+			}
+		}
+		if isUplink && !writes {
+			t.Errorf("%s holds the uplink and does not write the resolver file", name)
+		}
+	}
+}

internal/catalogue/resolver_needs_uplink_test.go

@@ -1,121 +0,0 @@
-package catalogue
-
-import (
-	"errors"
-	"reflect"
-	"strings"
-	"testing"
-)
-
-// Defends novox/hq ADR 0220: what a machine asks for names needs the uplink held beside it.
-//
-// resolv.conf is the mesh's only while the program managing the machine's network is told to leave
-// it alone, and the uplink's holder is what tells it (ADR 0117). Without one, the first connectivity
-// change rewrites the file — so the dependency is checked at assignment, by the same mechanism as a
-// service's on the service manager (ADR 0207), derived from the claim and never stated in a manifest.
-
-// resolverAndUplinks is a resolver-config holder and two uplink holders, with no resources of their
-// own so that nothing but the seats is judged.
-func resolverAndUplinks() map[string]Manifest {
-	return shelf(
-		mod("resolv-conf", nil, nil, nil, Claim{Name: "node-resolver-config"}),
-		mod("networkmanager", nil, nil, nil, Claim{Name: "node-uplink"}),
-		mod("systemd-networkd", nil, nil, nil, Claim{Name: "node-uplink"}),
-	)
-}
-
-func TestTheResolverConfigSeatNeedsTheUplink(t *testing.T) {
-	s, known := SeatNamed("node-resolver-config")
-	if !known {
-		t.Fatal("node-resolver-config is not in the mesh's set")
-	}
-	if !reflect.DeepEqual(s.Needs, []string{"node-uplink"}) {
-		t.Errorf("node-resolver-config needs %v, want [node-uplink] (ADR 0220)", s.Needs)
-	}
-	// Derived from the claim: a module claiming the seat depends on the uplink with nothing written.
-	got := DependsOn(mod("anything", nil, nil, nil, Claim{Name: "node-resolver-config"}))
-	if !reflect.DeepEqual(got, []string{"node-uplink"}) {
-		t.Errorf("a module claiming node-resolver-config depends on %v, want [node-uplink]", got)
-	}
-	// And the uplink's holders need nothing of the kind: the dependency runs one way.
-	if got := DependsOn(mod("networkmanager", nil, nil, nil, Claim{Name: "node-uplink"})); len(got) != 0 {
-		t.Errorf("an uplink holder depends on %v; it needs no seat beside it", got)
-	}
-}
-
-// What the store loads has no column for it, so the compiled value survives a load.
-func TestTheNeedSurvivesTheStoresRows(t *testing.T) {
-	defer UseSeats(DefaultSeats())
-	var rows []Seat
-	for _, s := range DefaultSeats() {
-		rows = append(rows, Seat{Name: s.Name, Scope: s.Scope, Delivers: s.Delivers, Decision: s.Decision})
-	}
-	UseSeats(rows)
-	if s, _ := SeatNamed("node-resolver-config"); !reflect.DeepEqual(s.Needs, []string{"node-uplink"}) {
-		t.Errorf("after loading the store's rows node-resolver-config needs %v", s.Needs)
-	}
-}
-
-func TestAssigningTheResolverConfigWithoutAnUplinkIsRefused(t *testing.T) {
-	cat := resolverAndUplinks()
-	_, err := AssignRefusal(cat, "laptop", nil, []string{"resolv-conf"})
-	var refusal *Refusal
-	if !errors.As(err, &refusal) {
-		t.Fatalf("resolv-conf was assigned to a machine nothing manages the network of: %v", err)
-	}
-	for _, want := range []string{"resolv-conf on laptop depends on node-uplink", "networkmanager", "systemd-networkd"} {
-		if !strings.Contains(err.Error(), want) {
-			t.Errorf("the refusal does not say %q:\n%s", want, err)
-		}
-	}
-	// Beside a holder, or together with one in one act, it is let through.
-	if _, err := AssignRefusal(cat, "laptop", []string{"networkmanager"}, []string{"resolv-conf"}); err != nil {
-		t.Errorf("resolv-conf beside networkmanager was refused: %v", err)
-	}
-	if _, err := AssignRefusal(cat, "anchor", nil, []string{"systemd-networkd", "resolv-conf"}); err != nil {
-		t.Errorf("resolv-conf assigned with systemd-networkd was refused: %v", err)
-	}
-	// And a composition without one is refused once the switch is on.
-	if _, err := Resolve(cat, []string{"resolv-conf"}, workstation(), World{}); err == nil ||
-		!strings.Contains(err.Error(), "node-uplink") {
-		t.Errorf("a node with resolv-conf and no uplink composed: %v", err)
-	}
-}
-
-func TestUnassigningTheUplinkUnderTheResolverConfigIsRefused(t *testing.T) {
-	cat := resolverAndUplinks()
-	err := UnassignRefusal(cat, "laptop", []string{"networkmanager", "resolv-conf"}, []string{"networkmanager"})
-	if err == nil || !strings.Contains(err.Error(), "resolv-conf") || !strings.Contains(err.Error(), "node-uplink") {
-		t.Errorf("taking the uplink from under resolv-conf gave %v", err)
-	}
-}
-
-// The catalogue as it is: the module that writes resolv.conf depends on the uplink, and every manager
-// the mesh knows can meet it — so the refusal always has a remedy to name.
-func TestTheCataloguesResolverConfigHasUplinkHoldersToName(t *testing.T) {
-	cat := map[string]Manifest{}
-	for _, m := range theCatalogue(t) {
-		cat[m.Module] = m
-	}
-	deps := DependsOn(cat["resolv-conf"])
-	found := false
-	for _, d := range deps {
-		found = found || d == "node-uplink"
-	}
-	if !found {
-		t.Errorf("the catalogue's resolv-conf depends on %v, not on node-uplink", deps)
-	}
-	holders := PossibleHolders(cat, "node-uplink")
-	for _, want := range []string{"dhcpcd", "networkmanager", "systemd-networkd"} {
-		in := false
-		for _, h := range holders {
-			in = in || h == want
-		}
-		if !in {
-			t.Errorf("%s does not hold node-uplink in the catalogue; holders: %v", want, holders)
-		}
-	}
-	if got := PossibleHolders(cat, "node-resolver-config"); !reflect.DeepEqual(got, []string{"resolv-conf"}) {
-		t.Errorf("node-resolver-config can be held by %v; resolv-conf alone since ADR 0220", got)
-	}
-}

internal/catalogue/seat_dependencies.go

@@ -6,9 +6,10 @@ import (
 	"strings"
 )
 
-// A module depends on the node seats that apply its resources (novox/hq ADR 0207), on the
+// A module depends on the node seats that apply its resources (novox/hq ADR 0207) and on the
-// seats it contributes to (novox/hq ADR 0210), and on the seats a seat it holds needs beside it
+// seats it contributes to (novox/hq ADR 0210). A third source — what a seat it holds needs beside it
-// (novox/hq ADR 0220).
+// (novox/hq ADR 0220) — had one user, node-resolver-config needing node-uplink, and went with that
+// seat when the resolver file became the uplink's own (novox/hq ADR 0223).
 //
 // Some of what a module declares is applied through software on the machine that is itself a
 // module: a service through the service manager, a package through the package manager, a container
@@ -94,9 +95,6 @@ func DependsOn(m Manifest) []string {
 	for _, seat := range contributedTo(m) {
 		seen[seat] = true
 	}
-	for _, seat := range neededBesideClaims(m) {
-		seen[seat] = true
-	}
 	out := make([]string, 0, len(seen))
 	for s := range seen {
 		out = append(out, s)
@@ -128,20 +126,6 @@ func contributedTo(m Manifest) []string {
 	return out
 }
 
-// neededBesideClaims is every seat a seat the module claims at node scope needs held on the same
-// node (novox/hq ADR 0220): the holder of node-resolver-config is only right while node-uplink's
-// holder keeps the network manager off resolv.conf. Derived from the claim, as a resource's seat is
-// derived from its type, so a module that claims the seat cannot leave the dependency out.
-func neededBesideClaims(m Manifest) []string {
-	var out []string
-	for _, name := range nodeSeatsClaimed(m) {
-		if s, known := SeatNamed(name); known {
-			out = append(out, s.Needs...)
-		}
-	}
-	return out
-}
-
 // claimsSeat is whether a module claims a node seat, by its current name or one it used to have
 // (ADR 0122), so a rename leaves the dependency met.
 func claimsSeat(m Manifest, seat string) bool {

internal/catalogue/seats.go

@@ -28,7 +28,7 @@ type Seat struct {
 	// machine's resolver file lists two that give one answer. Each holder is on record, added by an
 	// act (`seat <name> --add <node>/<module>`), never by being assigned: two claimants with nothing on
 	// record are refused exactly as for any mesh seat. One per machine still — two modules on one
-	// node claiming it are refused. Compiled, never stored, like Needs: it is the mesh's definition of
+	// node claiming it are refused. Compiled, never stored, like Receives: it is the mesh's definition of
 	// the role, and the store's rows carry no column for it.
 	Replicated bool
 	// Delivers is the provision the seat's holder answers for, or empty. A seat that delivers a
@@ -53,13 +53,6 @@ type Seat struct {
 	// ${contribution:<seat>:<kind>}. Compiled, never stored: like the protocol, it is the mesh's
 	// definition of the role, and the store's rows carry no column for it.
 	Receives []Receivable
-	// Needs is every node seat this seat's holder needs held on its own node (novox/hq ADR 0220): a
-	// role whose holder is only right while another role is filled beside it. A module claiming this
-	// seat depends on each, exactly as a module declaring a service depends on the service manager
-	// (ADR 0207) — derived from the claim, never written in a manifest, and judged over the node's
-	// whole set of assignments. Compiled, never stored, like Receives: it is the mesh's definition of
-	// the role, and the store's rows carry no column for it.
-	Needs []string
 	// Decision is the record that made it a seat.
 	Decision string
 }
@@ -260,18 +253,12 @@ var defaultSeats = append([]Seat{
 	// Deferred (novox/hq ADR 0121): renaming to mesh-private-network is a scope + server/client
 	// model change, not a rename, so it stays until that is built.
 	{Name: "the-private-network", Scope: ScopeNode, Decision: "novox/hq ADR 0110"},
-	// What a machine asks for names (novox/hq ADR 0121, ADR 0196): its holder writes resolv.conf.
+	// The program that manages the machine's own network. It delivers nothing: its holder keeps the
-	// **And it needs the uplink held beside it** (novox/hq ADR 0220): resolv.conf stays the mesh's
+	// manager and the mesh from contradicting each other — the private network's interface left
-	// only while the program managing the machine's network is told to keep its hands off it, and
+	// alone — and writes the machine's resolver file itself, because the manager is what would
-	// that is what the uplink's holder says (ADR 0117). Without one, the first connectivity change
+	// otherwise rewrite it (novox/hq ADR 0223, which retired node-resolver-config into this seat).
-	// rewrites the file and every surface of the mesh still reads green — so it is refused at
+	// It never declares a link, an address or a wireless network, because the link is the only
-	// assignment instead.
+	// channel a fix could arrive on. A seat
-	{Name: "node-resolver-config", Scope: ScopeNode, Decision: "novox/hq ADR 0121, ADR 0220",
-		Needs: []string{"node-uplink"}},
-	// The program that manages the machine's own network. It delivers nothing: its holder only
-	// keeps the manager and the mesh from contradicting each other — the resolver file left to the
-	// mesh, the private network's interface left alone — and never declares a link, an address or
-	// a wireless network, because the link is the only channel a fix could arrive on. A seat
 	// rather than a condition in the resolver's module, so a machine running two managers is
 	// refused at assignment instead of found by the resolver being rewritten (novox/hq ADR 0117).
 	{Name: "node-uplink", Scope: ScopeNode, Decision: "novox/hq ADR 0117"},
@@ -326,11 +313,10 @@ func UseSeats(s []Seat) {
 				row.Accepts, row.Emits, row.Serves = d.Accepts, d.Emits, d.Serves
 			}
 		}
-		// What a seat receives, what its holder needs and whether it is replicated are never stored
+		// What a seat receives and whether it is replicated are never stored (novox/hq ADR 0212, ADR
-		// (novox/hq ADR 0212, ADR 0220, ADR 0223), so they are always the compiled ones.
+		// 0223), so they are always the compiled ones.
 		if d, known := byName[row.Name]; known {
 			row.Receives = d.Receives
-			row.Needs = d.Needs
 			row.Replicated = d.Replicated
 		}
 		merged = append(merged, row)

internal/catalogue/seats_test.go

@@ -46,7 +46,8 @@ func TestTheSeatsAreAClosedSetAndEachNamesItsDecision(t *testing.T) {
 			delivered[s.Delivers] = s.Name
 		}
 	}
-	// Thirty-seven since the retired node-dns-resolver went (novox/hq ADR 0220); thirty-eight with
+	// Thirty-six since node-resolver-config retired into node-uplink (novox/hq ADR 0223); thirty-seven
+	// since the retired node-dns-resolver went (novox/hq ADR 0220); thirty-eight with
 	// node-backup (novox/hq ADR 0214); thirty-seven with node-message-bus (novox/hq ADR 0215);
 	// thirty-six with mesh-dns-resolver (novox/hq ADR 0194) and node-hosts-file (ADR 0199); thirty-four
 	// with node-hotkeys (ADR 0212); thirty-three with node-power (ADR 0211); thirty-two since the
@@ -54,8 +55,8 @@ func TestTheSeatsAreAClosedSetAndEachNamesItsDecision(t *testing.T) {
 	// node-container-runtime (ADR 0207); nineteen with node-environment and node-login-shell (ADR 0203,
 	// ADR 0204); seventeen with node-build-agent (ADR 0190). One fewer once the retired
 	// mesh-build-machine row goes, when no registered manifest claims it.
-	if len(Seats()) != 37 {
+	if len(Seats()) != 36 {
-		t.Errorf("the mesh defines %d seats rather than 37; the set is closed, so a change here is "+
+		t.Errorf("the mesh defines %d seats rather than 36; the set is closed, so a change here is "+
 			"a decision (novox/hq ADR 0110): %s", len(Seats()), seatNames())
 	}
 }

internal/inventory/migrations/0063-the-resolver-file-is-the-uplinks.sql

@@ -0,0 +1,17 @@
+-- What a machine asks for names is the uplink's holder's to write (novox/hq ADR 0223, retiring what
+-- ADR 0121 and ADR 0220 decided for node-resolver-config).
+--
+-- `/etc/resolv.conf` is written by the module holding `node-uplink` — the program that would otherwise
+-- rewrite it — so the seat whose holder wrote it, and its need of the uplink beside it, go. Its only
+-- claimant, `resolv-conf`, declared nothing for one release while every machine handed the file to its
+-- uplink module in one apply, and was then unassigned everywhere and forgotten before this runs.
+--
+-- **The compiled defaults no longer carry it, and that alone would not remove it**: seeding adds a
+-- seat a release ships and never takes one away (ADR 0122), as 0060 found for the per-node resolver.
+-- A holding on record goes with it by cascade; a node seat has none. No alias is kept: nothing was
+-- renamed, and a manifest still claiming the old name should be refused at registration, naming it.
+--
+-- Numbered after 0063 (node-hosts-file renamed to node-hostname), which is expected to merge first;
+-- if this one lands first, the two are renumbered so the order they merge in is the order they run.
+delete from seat_alias where seat = 'node-resolver-config' or alias = 'node-resolver-config';
+delete from seat where name = 'node-resolver-config';