From 0a0f5b4a5ab7233a9f885ed418457d4bdfe36f8a Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 12:03:37 +0200 Subject: [PATCH 1/2] Require the service manager's reset-failed, wanted-by and unlink-dangling verbs now every holder serves them (issue 332) ADR 0246 step 3: the systemd module, the seat's only holder, serves all three on every node, so a holder that stops serving one is refused rather than leaving a node without them. --- internal/catalogue/operators_machine_test.go | 53 +++++++++++++------- internal/catalogue/seats.go | 14 +++--- 2 files changed, 41 insertions(+), 26 deletions(-) diff --git a/internal/catalogue/operators_machine_test.go b/internal/catalogue/operators_machine_test.go index 3bcc9086..9dfa7b37 100644 --- a/internal/catalogue/operators_machine_test.go +++ b/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 -// holder running today, serving the nine verbs, still holds the seat; a holder serving all eleven is not -// refused; each replaces the shell command it names; and read back from the store's row they stay optional. -func TestResetFailedAndWantedByAreOptionalVerbsOfTheServiceManager(t *testing.T) { +// **`reset-failed`, `wanted-by` and `unlink-dangling` are required** (novox/hq issue 332, ADR 0246 step 3): +// every holder serves them, so a holder serving only the nine verbs before them is refused by the verb it +// lacks; a holder serving all twelve holds the seat; each replaces the shell command it names; and read +// 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 { - t.Fatalf("today's holder, without the two verbs, is refused: %v", err) + if err := CanHold(holder(nine...), seat); err == nil { + 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 { - t.Fatalf("a holder serving both is refused: %v", err) + refusedWithout(seat) + 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 { - 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") { + if err := CanHold(holder(append(slices.Clone(all), "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 { - t.Fatalf("read back from the row, the two verbs are required: %v", err) + refusedWithout(seat) + 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) } } diff --git a/internal/catalogue/seats.go b/internal/catalogue/seats.go index c83aca05..ec9a9f50 100644 --- a/internal/catalogue/seats.go +++ b/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 - // (ADR 0246 step 1); a later change requires them. - {Name: "reset-failed", Optional: true, Description: "Clear one unit's failed record in its manager — " + + // is a dependency or an enable link no verb could show. Required since every holder serves them + // (ADR 0246 step 3). + {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 - // holders serve it (ADR 0246 step 1). - {Name: "unlink-dangling", Optional: true, Description: "Remove the dangling enable links named for one " + + // leaves a link whose unit file is gone, so the manager keeps asking for the unit. Required since + // every holder serves it (ADR 0246 step 3). + {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 " + From 61d00a234af7abf7a06385d38cf2728ad1330efb Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 12:25:05 +0200 Subject: [PATCH 2/2] Give the store tests' service manager holders the three verbs it now must serve (issue 332) They named only the eight verbs from before issue 332 and were refused once the seat required reset-failed, wanted-by and unlink-dangling. --- cmd/mesh-controller/seat_dependencies_test.go | 3 ++- internal/inventory/seats_test.go | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/mesh-controller/seat_dependencies_test.go b/cmd/mesh-controller/seat_dependencies_test.go index bea72bef..9d47f4af 100644 --- a/cmd/mesh-controller/seat_dependencies_test.go +++ b/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"}}} } diff --git a/internal/inventory/seats_test.go b/internal/inventory/seats_test.go index 916be433..1dfac4b2 100644 --- a/internal/inventory/seats_test.go +++ b/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"]