Merge pull request 'Require the service manager's reset-failed, wanted-by and unlink-dangling verbs (issue 332)' (#248) from fix/332-require-service-manager-verbs into main

cmd/mesh-controller/seat_dependencies_test.go

@@ -15,7 +15,8 @@ import (
 func serviceManagerHolder() catalogue.Manifest {
 	return catalogue.Manifest{Module: "systemd", Version: "1",
 		Claims: []catalogue.Claim{{Name: catalogue.ServiceManagerSeat, Scope: catalogue.ScopeNode,
-			Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal"}}},
+			Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal",
+				"reset-failed", "wanted-by", "unlink-dangling"}}},
 		Resources: []map[string]any{{"id": "systemd", "type": "package", "package": "systemd"}}}
 }
 

internal/catalogue/operators_machine_test.go

@@ -68,9 +68,10 @@ func TestFailedIsAnOptionalVerbOfTheServiceManager(t *testing.T) {
 	holder := func(serves ...string) Manifest {
 		return Manifest{Module: "systemd", Version: "1", Claims: []Claim{{Name: ServiceManagerSeat, Scope: ScopeNode, Serves: serves}}}
 	}
-	eight := []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal"}
+	eight := []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal",
+		"reset-failed", "wanted-by", "unlink-dangling"}
 	if err := CanHold(holder(eight...), seat); err != nil {
-		t.Fatalf("today's holder, without failed, is refused: %v", err)
+		t.Fatalf("a holder without failed is refused: %v", err)
 	}
 	if err := CanHold(holder(append(eight, "failed")...), seat); err != nil {
 		t.Fatalf("a holder serving failed is refused: %v", err)
@@ -81,7 +82,7 @@ func TestFailedIsAnOptionalVerbOfTheServiceManager(t *testing.T) {
 	if err := CanHold(holder(eight[1:]...), seat); err == nil || !strings.Contains(err.Error(), "does not serve units") {
 		t.Fatalf("a holder missing a required verb was accepted: %v", err)
 	}
-	optional := map[string]bool{"failed": true, "reset-failed": true, "wanted-by": true, "unlink-dangling": true}
+	optional := map[string]bool{"failed": true}
 	for _, v := range seat.Serves {
 		if v.Optional != optional[v.Name] {
 			t.Errorf("%s optional: %v", v.Name, v.Optional)
@@ -89,26 +90,35 @@ func TestFailedIsAnOptionalVerbOfTheServiceManager(t *testing.T) {
 	}
 }
 
-// **`reset-failed` and `wanted-by` join the seat optional** (novox/hq issue 332, ADR 0246 step 1): the
+// **`reset-failed`, `wanted-by` and `unlink-dangling` are required** (novox/hq issue 332, ADR 0246 step 3):
-// holder running today, serving the nine verbs, still holds the seat; a holder serving all eleven is not
+// every holder serves them, so a holder serving only the nine verbs before them is refused by the verb it
-// refused; each replaces the shell command it names; and read back from the store's row they stay optional.
+// lacks; a holder serving all twelve holds the seat; each replaces the shell command it names; and read
-func TestResetFailedAndWantedByAreOptionalVerbsOfTheServiceManager(t *testing.T) {
+// back from the store's row they stay required.
+func TestResetFailedWantedByAndUnlinkDanglingAreRequiredVerbsOfTheServiceManager(t *testing.T) {
 	defer UseSeats(DefaultSeats())
 	nine := []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal", "failed"}
+	all := append(slices.Clone(nine), "reset-failed", "wanted-by", "unlink-dangling")
 	holder := func(serves ...string) Manifest {
 		return Manifest{Module: "systemd", Version: "1", Claims: []Claim{{Name: ServiceManagerSeat, Scope: ScopeNode, Serves: serves}}}
 	}
+	refusedWithout := func(seat Seat) {
+		t.Helper()
+		for _, verb := range []string{"reset-failed", "wanted-by", "unlink-dangling"} {
+			serves := slices.DeleteFunc(slices.Clone(all), func(v string) bool { return v == verb })
+			if err := CanHold(holder(serves...), seat); err == nil || !strings.Contains(err.Error(), "does not serve "+verb) {
+				t.Errorf("a holder not serving %s holds the seat: %v", verb, err)
+			}
+		}
+	}
 	seat, _ := SeatNamed(ServiceManagerSeat)
-	if err := CanHold(holder(nine...), seat); err != nil {
+	if err := CanHold(holder(nine...), seat); err == nil {
-		t.Fatalf("today's holder, without the two verbs, is refused: %v", err)
+		t.Fatal("a holder serving only the nine verbs before issue 332 holds the seat")
 	}
-	if err := CanHold(holder(append(nine, "reset-failed", "wanted-by")...), seat); err != nil {
+	refusedWithout(seat)
-		t.Fatalf("a holder serving both is refused: %v", err)
+	if err := CanHold(holder(all...), seat); err != nil {
+		t.Fatalf("a holder serving all twelve is refused: %v", err)
 	}
-	if err := CanHold(holder(append(nine, "reset-failed", "wanted-by", "unlink-dangling")...), seat); err != nil {
+	if err := CanHold(holder(append(slices.Clone(all), "why-started")...), seat); err == nil || !strings.Contains(err.Error(), "does not promise") {
-		t.Fatalf("a holder serving unlink-dangling too is refused: %v", err)
-	}
-	if err := CanHold(holder(append(nine, "why-started")...), seat); err == nil || !strings.Contains(err.Error(), "does not promise") {
 		t.Fatalf("a verb the seat does not promise was accepted: %v", err)
 	}
 	replaces := map[string]string{"reset-failed": "systemctl reset-failed", "wanted-by": "systemctl list-dependencies --reverse",
@@ -118,6 +128,9 @@ func TestResetFailedAndWantedByAreOptionalVerbsOfTheServiceManager(t *testing.T)
 		if !ok {
 			continue
 		}
+		if v.Optional {
+			t.Errorf("%s is still optional", v.Name)
+		}
 		if !slices.Contains(v.Replaces, want) {
 			t.Errorf("%s does not say it replaces %q: %v", v.Name, want, v.Replaces)
 		}
@@ -148,8 +161,9 @@ func TestResetFailedAndWantedByAreOptionalVerbsOfTheServiceManager(t *testing.T)
 	}
 	UseSeats(rows)
 	seat, _ = SeatNamed(ServiceManagerSeat)
-	if err := CanHold(holder(nine...), seat); err != nil {
+	refusedWithout(seat)
-		t.Fatalf("read back from the row, the two verbs are required: %v", err)
+	if err := CanHold(holder(all...), seat); err != nil {
+		t.Fatalf("read back from the row, a holder serving all twelve is refused: %v", err)
 	}
 }
 
@@ -169,8 +183,9 @@ func TestAnOptionalVerbStaysOptionalInASetReadFromTheStore(t *testing.T) {
 	UseSeats(rows)
 	seat, _ := SeatNamed(ServiceManagerSeat)
 	holder := Manifest{Module: "systemd", Version: "1", Claims: []Claim{{Name: ServiceManagerSeat, Scope: ScopeNode,
-		Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal"}}}}
+		Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal",
+			"reset-failed", "wanted-by", "unlink-dangling"}}}}
 	if err := CanHold(holder, seat); err != nil {
-		t.Fatalf("a set read from rows refuses today's holder: %v", err)
+		t.Fatalf("a set read from rows refuses a holder without the optional failed: %v", err)
 	}
 }

internal/catalogue/seats.go

@@ -678,13 +678,13 @@ func serviceManagerVerbs() []Verb {
 			Replaces: []string{"systemctl --failed"}},
 		// **Clearing a failed record, and finding what starts a unit** (novox/hq issue 332): a unit whose
 		// file is gone stays failed in its manager until the record is reset, and what keeps asking for it
-		// is a dependency or an enable link no verb could show. Both optional while their holders catch up
+		// is a dependency or an enable link no verb could show. Required since every holder serves them
-		// (ADR 0246 step 1); a later change requires them.
+		// (ADR 0246 step 3).
-		{Name: "reset-failed", Optional: true, Description: "Clear one unit's failed record in its manager — " +
+		{Name: "reset-failed", Description: "Clear one unit's failed record in its manager — " +
 			"its failed state and its start-limit count — and answer its state after. It removes no file and " +
 			"starts nothing.",
 			Input: scoped(unit, []string{"unit"}), Replaces: []string{"systemctl reset-failed"}},
-		{Name: "wanted-by", Optional: true, Description: "What wants, requires or triggers one unit, read-only: the " +
+		{Name: "wanted-by", Description: "What wants, requires or triggers one unit, read-only: the " +
 			"manager's reverse dependencies (WantedBy, RequiredBy, UpheldBy, BoundBy, TriggeredBy and the rest) and " +
 			"every enable link naming it in the scope's configuration directories (*.wants, *.requires, *.upholds), " +
 			"each with where it points and whether it dangles — the account's own directory included in user scope.",
@@ -692,9 +692,9 @@ func serviceManagerVerbs() []Verb {
 			Replaces: []string{"systemctl list-dependencies --reverse", "systemctl show -p WantedBy",
 				"ls ~/.config/systemd/user/*.wants"}},
 		// **A dangling enable link removed on the person's word** (novox/hq issue 332): `systemctl disable`
-		// leaves a link whose unit file is gone, so the manager keeps asking for the unit. Optional until its
+		// leaves a link whose unit file is gone, so the manager keeps asking for the unit. Required since
-		// holders serve it (ADR 0246 step 1).
+		// every holder serves it (ADR 0246 step 3).
-		{Name: "unlink-dangling", Optional: true, Description: "Remove the dangling enable links named for one " +
+		{Name: "unlink-dangling", Description: "Remove the dangling enable links named for one " +
 			"unit — links in a *.wants, *.requires or *.upholds directory of the account's or the machine's own " +
 			"configuration whose target does not exist, which `disable` leaves once the unit's file is gone — on " +
 			"the person's word: `why` is required. Each is recorded (its path, its target, the reason) before it is " +

internal/inventory/seats_test.go

@@ -202,14 +202,15 @@ func TestASeatsVerbGainsTheArgumentsTheBinaryNames(t *testing.T) {
 		t.Fatal("failed was not added to the row")
 	}
 	// The optional mark is not stored; the working set read from the rows takes it from the compiled seat,
-	// so today's holder, which does not serve failed, still holds the seat.
+	// so a holder that does not serve the optional failed still holds the seat.
 	catalogue.UseSeats(seats)
 	defer catalogue.UseSeats(catalogue.DefaultSeats())
 	live, _ := catalogue.SeatNamed(catalogue.ServiceManagerSeat)
 	holder := catalogue.Manifest{Module: "systemd", Version: "1", Claims: []catalogue.Claim{{Name: catalogue.ServiceManagerSeat,
-		Scope: catalogue.ScopeNode, Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal"}}}}
+		Scope: catalogue.ScopeNode, Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal",
+			"reset-failed", "wanted-by", "unlink-dangling"}}}}
 	if err := catalogue.CanHold(holder, live); err != nil {
-		t.Fatalf("the seat read from the store refuses today's holder: %v", err)
+		t.Fatalf("the seat read from the store refuses a holder without the optional failed: %v", err)
 	}
 	// Seeding again changes nothing more.
 	before := verbs["journal"]