Hold a unit an administrator installed whatever its state, and a packaged unit only when the machine uses it (hq ADR 0103)

internal/apply/hold.go

@@ -105,14 +105,28 @@ func lookBefore(ctx context.Context, sys system.System, d *declaration.Declarati
 			if known.Recorded(string(declaration.TypeService), res.Unit) {
 				continue
 			}
-			// Found is what the machine runs: a unit that is running or starts at boot. A unit
+			// **Found is a unit somebody put on this machine, or one the machine uses.**
-			// file a package merely ships — a template instance nothing ever started, say the
+			//
-			// private network's own wg-quick@mesh0 — is not a predecessor's service, and holding
+			// Where it comes from first: a unit the service manager loads from outside /usr —
-			// it kept the private network from ever coming up (found by the adoption bed).
+			// /etc/systemd/system or /run/systemd/system — was installed by an administrator, so
+			// it is a predecessor's whatever state it is in, and one deliberately stopped and
+			// disabled must stay that way (novox/hq ADR 0103).
+			//
+			// A unit a package ships, under /usr, is not held by its mere presence: the private
+			// network's own wg-quick@mesh0 is an instance of a template the tunnel package ships,
+			// nothing had ever run it, and holding it kept the private network from ever coming up
+			// (found by the adoption bed). Such a unit is held only if the machine actually uses
+			// it — running, or started at boot.
 			state, err := sys.ServiceState(ctx, run, res.Unit)
 			if err != nil {
 				continue
 			}
+			if from, ok := sys.(unitFiles); ok {
+				if path, err := from.ServiceUnitFile(ctx, run, res.Unit); err == nil && installedByHand(path) {
+					seen.is["unit:"+res.Unit] = true
+					continue
+				}
+			}
 			boot, _ := sys.ServiceBoot(ctx, run, res.Unit)
 			if state == "running" || boot == "enabled" {
 				seen.is["unit:"+res.Unit] = true
@@ -150,6 +164,20 @@ func lookBefore(ctx context.Context, sys system.System, d *declaration.Declarati
 	return seen
 }
 
+// unitFiles is a service manager that can say where it loads a unit from.
+type unitFiles interface {
+	ServiceUnitFile(ctx context.Context, run Runner, unit string) (string, error)
+}
+
+// installedByHand is whether a unit file is one somebody put on this machine rather than one a
+// package ships: anywhere but /usr, where distributions keep what they install.
+func installedByHand(path string) bool {
+	if path == "" {
+		return false
+	}
+	return !strings.HasPrefix(filepath.Clean(path), "/usr/")
+}
+
 func present(path string) bool {
 	_, err := os.Lstat(path)
 	return err == nil

internal/apply/hold_test.go

@@ -29,6 +29,9 @@ type machine struct {
 
 type fakeUnit struct {
 	active, enabled string
+	// fragment is where systemd loads the unit from; empty means /etc/systemd/system, where an
+	// administrator installs one.
+	fragment string
 }
 
 // systemctl answers as systemd does for the units the machine has, and "not-found" for any other.
@@ -41,8 +44,18 @@ func (m *machine) systemctl(args []string) (string, error) {
 	switch args[0] {
 	case "show":
 		if !ok {
+			if len(args) > 2 && strings.Contains(args[2], "FragmentPath") {
+				return "FragmentPath=\n", nil
+			}
 			return "LoadState=not-found\nActiveState=inactive\nType=simple\n", nil
 		}
+		if len(args) > 2 && strings.Contains(args[2], "FragmentPath") {
+			from := u.fragment
+			if from == "" {
+				from = "/etc/systemd/system/" + unit
+			}
+			return "FragmentPath=" + from + "\n", nil
+		}
 		return "LoadState=loaded\nActiveState=" + u.active + "\nType=simple\nRemainAfterExit=no\n", nil
 	case "is-enabled":
 		if !ok {
@@ -617,7 +630,8 @@ func TestAUnitAPackageOnlyShipsIsNotFound(t *testing.T) {
 	// the private network never came up. Found is what the machine runs.
 	dir := t.TempDir()
 	m := &machine{containers: map[string]*fakeContainer{},
-		units: map[string]*fakeUnit{"wg-quick@mesh0.service": {active: "inactive", enabled: "disabled"}}}
+		units: map[string]*fakeUnit{"wg-quick@mesh0.service": {active: "inactive", enabled: "disabled",
+			fragment: "/usr/lib/systemd/system/wg-quick@.service"}}}
 	report, state := applyAdopted(t, adopted(t, untaken("mesh-wireguard.overlay-up"),
 		`{"id":"mesh-wireguard.overlay-up","type":"service","unit":"wg-quick@mesh0.service","state":"running","boot":"enabled"}`),
 		store.State{}, m, dir)
@@ -904,3 +918,43 @@ func TestAVolumeTheRuntimeCannotBeAskedAboutStopsTheContainer(t *testing.T) {
 		t.Fatalf("a volume the runtime could not be asked about did not stop the container: %v", err)
 	}
 }
+
+func TestAUnitSomebodyInstalledIsHeldWhateverStateItIsIn(t *testing.T) {
+	// A predecessor's unit under /etc, deliberately stopped and disabled: starting it would put
+	// back a service somebody took down on purpose (novox/hq ADR 0103).
+	dir := t.TempDir()
+	m := &machine{containers: map[string]*fakeContainer{},
+		units: map[string]*fakeUnit{"hello.service": {active: "inactive", enabled: "disabled",
+			fragment: "/etc/systemd/system/hello.service"}}}
+	report, state := applyAdopted(t, adopted(t, untaken("hello-web.unit"),
+		`{"id":"hello-web.unit","type":"service","unit":"hello.service","state":"running","boot":"enabled"}`),
+		store.State{}, m, dir)
+	if o := outcomeOf(report, "hello-web.unit"); o.Action != "held" {
+		t.Fatalf("a unit an administrator installed was not held: %+v", o)
+	}
+	if m.did("systemctl start") || m.did("systemctl enable") {
+		t.Errorf("a unit somebody had stopped and disabled was started: %v", m.asked)
+	}
+	if _, ok := state.HeldAt("hello-web.unit"); !ok {
+		t.Error("the hold was not recorded")
+	}
+}
+
+func TestAPackagedUnitTheMachineUsesIsStillHeld(t *testing.T) {
+	// The predecessor's own service from a package, running: not the mesh's to restart.
+	dir := t.TempDir()
+	conf := filepath.Join(dir, "hello.conf")
+	m := &machine{containers: map[string]*fakeContainer{},
+		units: map[string]*fakeUnit{"nginx.service": {active: "active", enabled: "enabled",
+			fragment: "/usr/lib/systemd/system/nginx.service"}}}
+	report, _ := applyAdopted(t, adopted(t, untaken("hello-web.conf", "hello-web.unit"),
+		`{"id":"hello-web.conf","type":"file","path":"`+conf+`","content":"x\n"},
+		 {"id":"hello-web.unit","type":"service","unit":"nginx.service","state":"running","boot":"enabled",
+		  "restart-on":["hello-web.conf"]}`), store.State{}, m, dir)
+	if o := outcomeOf(report, "hello-web.unit"); o.Action != "held" {
+		t.Fatalf("a packaged unit the machine runs was not held: %+v", o)
+	}
+	if m.did("systemctl stop") || m.did("systemctl restart") {
+		t.Errorf("the predecessor's service was restarted: %v", m.asked)
+	}
+}

internal/system/arch.go

@@ -247,6 +247,22 @@ func (arch) AddUserToGroup(ctx context.Context, run Runner, name, group string)
 	return nil
 }
 
+// ServiceUnitFile says where the service manager loads a unit from — systemd's FragmentPath. It
+// is how the host tells a unit an administrator installed, under /etc or /run, from one a package
+// ships under /usr (novox/hq ADR 0103). Empty, with no error, for a unit that loads from nowhere.
+func (arch) ServiceUnitFile(ctx context.Context, run Runner, unit string) (string, error) {
+	out, err := run(ctx, "systemctl", "show", unit, "--property=FragmentPath")
+	if err != nil {
+		return "", fmt.Errorf("the service manager did not say where %s comes from: %w", unit, err)
+	}
+	for _, line := range strings.Split(out, "\n") {
+		if path, ok := strings.CutPrefix(strings.TrimSpace(line), "FragmentPath="); ok {
+			return strings.TrimSpace(path), nil
+		}
+	}
+	return "", nil
+}
+
 // ReloadUnits has systemd read its unit files again. A unit file that changed on disk is otherwise
 // ignored: a restart runs the unit systemd already loaded, and the new text only takes effect
 // after a reload nobody asked for.