diff --git a/internal/apply/hold.go b/internal/apply/hold.go index 8c2497e..941fc43 100644 --- a/internal/apply/hold.go +++ b/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 - // 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 - // it kept the private network from ever coming up (found by the adoption bed). + // **Found is a unit somebody put on this machine, or one the machine uses.** + // + // Where it comes from first: a unit the service manager loads from outside /usr — + // /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 diff --git a/internal/apply/hold_test.go b/internal/apply/hold_test.go index cdf8232..6b493f6 100644 --- a/internal/apply/hold_test.go +++ b/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) + } +} diff --git a/internal/system/arch.go b/internal/system/arch.go index 20a0cf7..9582757 100644 --- a/internal/system/arch.go +++ b/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.