internal/apply/apply.go

@@ -316,40 +316,101 @@ func writeAtomically(path string, content []byte, mode os.FileMode) error {
 
 func applyService(ctx context.Context, r *declaration.Service, run Runner) (Outcome, error) {
 	out := begin(r)
+	var changes []string
+
+	// Boot first. A unit asked to be running and enabled should survive this apply failing
+	// half way in the more useful direction: enabled-and-stopped comes back at the next boot,
+	// where running-and-disabled does not.
+	if r.Boot != "" {
+		bootBefore, err := serviceBoot(ctx, r.Unit, run)
+		if err != nil {
+			return out, err
+		}
+		if bootBefore != r.Boot {
+			verb := "enable"
+			if r.Boot == "disabled" {
+				verb = "disable"
+			}
+			if _, err := run(ctx, "systemctl", verb, r.Unit); err != nil {
+				return out, fmt.Errorf("%s %s: %w", verb, r.Unit, err)
+			}
+			bootAfter, err := serviceBoot(ctx, r.Unit, run)
+			if err != nil {
+				return out, err
+			}
+			if bootAfter != r.Boot {
+				return out, fmt.Errorf(
+					"%s was asked to be %s at boot and is %s", r.Unit, r.Boot, bootAfter)
+			}
+			changes = append(changes, "boot "+bootBefore+" to "+bootAfter)
+		}
+	}
 
 	before, err := serviceState(ctx, r.Unit, run)
 	if err != nil {
 		return out, err
 	}
-	if before == r.State {
+	if before != r.State {
+		verb := "start"
+		if r.State == "stopped" {
+			verb = "stop"
+		}
+		if _, err := run(ctx, "systemctl", verb, r.Unit); err != nil {
+			return out, fmt.Errorf("%s %s: %w", verb, r.Unit, err)
+		}
+
+		// Read back. `systemctl start` returning zero says the transaction was accepted, not
+		// that the unit is running — a unit that starts and immediately dies satisfies the
+		// command.
+		after, err := serviceState(ctx, r.Unit, run)
+		if err != nil {
+			return out, err
+		}
+		if after != r.State {
+			return out, fmt.Errorf("%s was asked to be %s and is %s", r.Unit, r.State, after)
+		}
+		changes = append(changes, before+" to "+after)
+	}
+
+	if len(changes) == 0 {
 		out.Action = "unchanged"
 		out.Detail = before
 		return out, nil
 	}
-
-	verb := "start"
-	if r.State == "stopped" {
-		verb = "stop"
-	}
-	if _, err := run(ctx, "systemctl", verb, r.Unit); err != nil {
-		return out, fmt.Errorf("%s %s: %w", verb, r.Unit, err)
-	}
-
-	// Read back. `systemctl start` returning zero says the transaction was accepted, not that
-	// the unit is running — a unit that starts and immediately dies satisfies the command.
-	after, err := serviceState(ctx, r.Unit, run)
-	if err != nil {
-		return out, err
-	}
-	if after != r.State {
-		return out, fmt.Errorf("%s was asked to be %s and is %s", r.Unit, r.State, after)
-	}
-
 	out.Action = "updated"
-	out.Detail = before + " to " + after
+	out.Detail = strings.Join(changes, ", ")
 	return out, nil
 }
 
+// serviceBoot reads whether a unit starts at boot.
+//
+// The same trap as serviceState, in a new place. `systemctl is-enabled` exits non-zero for
+// nearly everything that is not "enabled", so the exit code says nothing useful — and it has
+// more than two answers. `static` in particular is neither enabled nor disabled: the unit has
+// no install section and CANNOT be enabled, so reporting it as "disabled" would let the host
+// try, fail, and blame the wrong thing.
+func serviceBoot(ctx context.Context, unit string, run Runner) (string, error) {
+	out, _ := run(ctx, "systemctl", "is-enabled", unit)
+	switch state := strings.TrimSpace(out); state {
+	case "enabled", "enabled-runtime", "alias":
+		return "enabled", nil
+	case "disabled":
+		return "disabled", nil
+	case "":
+		return "", fmt.Errorf("the service manager said nothing about whether %s starts at boot", unit)
+	case "static":
+		return "", fmt.Errorf(
+			"%s is static — it has no install section, so it cannot be enabled or disabled. "+
+				"Something else pulls it in, and that is what a declaration should name", unit)
+	case "masked", "masked-runtime":
+		return "", fmt.Errorf("%s is masked, so its boot state cannot be declared", unit)
+	default:
+		return "", fmt.Errorf(
+			"the service manager reports %s as %q at boot, which is neither enabled nor disabled",
+			unit, state)
+	}
+}
+
 // serviceState reads what the service manager says about a unit.
 //
 // Two traps here, and both were hit before this read what it now reads.
@@ -614,10 +675,9 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner) (
 	out := begin(r)
 	want := containerSpec(r)
 
-	if _, err := run(ctx, "docker", "version", "--format", "{{.Server.Version}}"); err != nil {
-		return out, fmt.Errorf(
-			"the container runtime does not answer on this machine, so nothing can be said "+
-				"about %q: %w", r.Name, err)
+	cri, err := containerRuntime(ctx, run)
+	if err != nil {
+		return out, fmt.Errorf("%w, so nothing can be said about %q", err, r.Name)
 	}
 
 	before, err := containerState(ctx, r.Name, run)
@@ -628,7 +688,7 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner) (
 		out.Action = "unchanged"
 		return out, nil
 	case existed:
-		if _, err := run(ctx, "docker", "rm", "-f", r.Name); err != nil {
+		if _, err := run(ctx, cri, "rm", "-f", r.Name); err != nil {
 			return out, fmt.Errorf("replacing container %s: %w", r.Name, err)
 		}
 	}
@@ -647,7 +707,7 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner) (
 	args = append(args, r.Image)
 	args = append(args, r.Args...)
 
-	if _, err := run(ctx, "docker", args...); err != nil {
+	if _, err := run(ctx, cri, args...); err != nil {
 		return out, fmt.Errorf("starting container %s: %w", r.Name, err)
 	}
 
@@ -726,3 +786,42 @@ func runAction(ctx context.Context, r *declaration.Action, argv []string, run Ru
 	}
 	return run(ctx, argv[0], argv[1:]...)
 }
+
+// Container runtimes the host knows how to ask.
+//
+// Two, because two exist on machines the mesh runs on. The list is short on purpose: each entry
+// is a claim that its probe and its CLI have been checked, not that a binary of that name might
+// work (novox/hq ADR 0060).
+//
+// The probe differs and the rest does not, which is what makes this a lookup rather than an
+// interface. `docker info --format {{.ServerVersion}}` fails on podman — the field does not
+// exist in its report — while `run`, `inspect --format` and `rm -f` are identical, including
+// docker's own Go template syntax for reading state and labels.
+var containerRuntimes = []struct {
+	command string
+	// probe asks the runtime for its version in the form THAT runtime understands. It must
+	// prove the runtime is FUNCTIONING, never that a binary is on disk
+	// (novox/hq 04-ISSUES/007).
+	probe []string
+}{
+	{command: "docker", probe: []string{"info", "--format", "{{.ServerVersion}}"}},
+	{command: "podman", probe: []string{"info", "--format", "{{.Version.Version}}"}},
+}
+
+// containerRuntime returns the runtime this machine actually has, or says there is none.
+//
+// Detected rather than declared, because a machine already carrying one keeps it: adoption
+// takes over what is there rather than replacing it (novox/hq research 012). Which runtime a
+// machine has is reported upward in the profile; what to install on a machine with none is the
+// control plane's decision, not this one's.
+func containerRuntime(ctx context.Context, run Runner) (string, error) {
+	var tried []string
+	for _, rt := range containerRuntimes {
+		if _, err := run(ctx, rt.command, rt.probe...); err == nil {
+			return rt.command, nil
+		}
+		tried = append(tried, rt.command)
+	}
+	return "", fmt.Errorf(
+		"no container runtime answers on this machine (tried %s)", strings.Join(tried, ", "))
+}

internal/apply/apply_test.go

@@ -589,7 +589,7 @@ func TestAContainerThatExitsImmediatelyFailsTheApply(t *testing.T) {
 	// that came up does — which is the read-back rule, in the place it matters most.
 	run := func(ctx context.Context, name string, args ...string) (string, error) {
 		switch {
-		case args[0] == "version":
+		case args[0] == "info":
 			return "27.0\n", nil
 		case args[0] == "inspect":
 			return "false\t" + "", nil // exists, not running
@@ -627,7 +627,7 @@ func TestAContainerWhoseDeclarationChangedIsReplaced(t *testing.T) {
 	var removed, created bool
 	run := func(ctx context.Context, name string, args ...string) (string, error) {
 		switch args[0] {
-		case "version":
+		case "info":
 			return "27.0\n", nil
 		case "inspect":
 			if created {
@@ -665,7 +665,7 @@ func TestAContainerThatMatchesIsLeftAlone(t *testing.T) {
 	var touched bool
 	run := func(ctx context.Context, name string, args ...string) (string, error) {
 		switch args[0] {
-		case "version":
+		case "info":
 			return "27.0\n", nil
 		case "inspect":
 			return "true\t" + spec, nil
@@ -685,3 +685,249 @@ func TestAContainerThatMatchesIsLeftAlone(t *testing.T) {
 		t.Errorf("a matching container reported a change: %+v", report.Outcomes)
 	}
 }
+
+// --- boot state (novox/hq: a unit started but not enabled stops being true at the next reboot) ---
+
+// systemctlStub answers `show` and `is-enabled` the way systemd does, and records the verbs it
+// was asked to perform. Real command shapes, because the trap being tested is what systemd
+// actually says rather than what a fake would.
+func systemctlStub(t *testing.T, load, active, enabled string, verbs *[]string) Runner {
+	t.Helper()
+	return func(ctx context.Context, name string, args ...string) (string, error) {
+		switch args[0] {
+		case "show":
+			return "LoadState=" + load + "\nActiveState=" + active + "\n", nil
+		case "is-enabled":
+			// Non-zero for everything but "enabled" — the exit code says nothing useful, which
+			// is the whole reason this reads the output.
+			if enabled == "enabled" {
+				return enabled + "\n", nil
+			}
+			return enabled + "\n", errors.New("exit status 1")
+		case "enable":
+			*verbs = append(*verbs, "enable")
+			enabled = "enabled"
+			return "", nil
+		case "disable":
+			*verbs = append(*verbs, "disable")
+			enabled = "disabled"
+			return "", nil
+		case "start":
+			*verbs = append(*verbs, "start")
+			active = "active"
+			return "", nil
+		case "stop":
+			*verbs = append(*verbs, "stop")
+			active = "inactive"
+			return "", nil
+		}
+		return "", nil
+	}
+}
+
+func TestAServiceIsEnabledAtBootWhenAsked(t *testing.T) {
+	// The gap this closes: the host could start a unit and never make it survive a reboot, so
+	// the declaration reported success and stopped being true at the next power cut.
+	var verbs []string
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"rt","type":"service","unit":"docker.service","state":"running","boot":"enabled"}
+	]}`)
+
+	report, _, err := Apply(context.Background(), d, store.State{},
+		systemctlStub(t, "loaded", "inactive", "disabled", &verbs), nil)
+	if err != nil {
+		t.Fatalf("apply failed: %v", err)
+	}
+	if len(verbs) != 2 || verbs[0] != "enable" || verbs[1] != "start" {
+		t.Errorf("expected enable then start, got %v", verbs)
+	}
+	if report.Outcomes[0].Action != "updated" {
+		t.Errorf("enabling and starting was not reported as an update: %+v", report.Outcomes[0])
+	}
+}
+
+func TestBootIsEnabledBeforeTheUnitIsStarted(t *testing.T) {
+	// Order matters when an apply fails part way. Enabled-and-stopped comes back at the next
+	// boot; running-and-disabled does not. So the more durable half is made true first.
+	var verbs []string
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"rt","type":"service","unit":"docker.service","state":"running","boot":"enabled"}
+	]}`)
+	if _, _, err := Apply(context.Background(), d, store.State{},
+		systemctlStub(t, "loaded", "inactive", "disabled", &verbs), nil); err != nil {
+		t.Fatal(err)
+	}
+	if len(verbs) < 2 || verbs[0] != "enable" {
+		t.Errorf("boot state was not made true first: %v", verbs)
+	}
+}
+
+func TestAlreadyEnabledAndRunningIsUnchanged(t *testing.T) {
+	var verbs []string
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"rt","type":"service","unit":"docker.service","state":"running","boot":"enabled"}
+	]}`)
+
+	report, _, err := Apply(context.Background(), d, store.State{},
+		systemctlStub(t, "loaded", "active", "enabled", &verbs), nil)
+	if err != nil {
+		t.Fatalf("apply failed: %v", err)
+	}
+	if len(verbs) != 0 {
+		t.Errorf("a unit already in the declared state was touched: %v", verbs)
+	}
+	if report.Changed() {
+		t.Errorf("an unchanged service reported a change: %+v", report.Outcomes)
+	}
+}
+
+func TestOmittingBootLeavesItAlone(t *testing.T) {
+	// Absent means the host asserts nothing. A machine whose operator enabled something must
+	// not have it silently disabled because a declaration did not mention it.
+	var verbs []string
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"rt","type":"service","unit":"docker.service","state":"running"}
+	]}`)
+	if _, _, err := Apply(context.Background(), d, store.State{},
+		systemctlStub(t, "loaded", "inactive", "enabled", &verbs), nil); err != nil {
+		t.Fatal(err)
+	}
+	for _, v := range verbs {
+		if v == "enable" || v == "disable" {
+			t.Errorf("boot state was changed by a declaration that did not mention it: %v", verbs)
+		}
+	}
+}
+
+func TestAStaticUnitCannotBeEnabled(t *testing.T) {
+	// `static` is neither enabled nor disabled: the unit has no install section and CANNOT be
+	// enabled. Reading it as "disabled" would have the host try, fail, and blame the wrong
+	// thing — the same shape as reading a missing unit as "stopped".
+	var verbs []string
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"rt","type":"service","unit":"dbus.socket","state":"running","boot":"enabled"}
+	]}`)
+
+	_, _, err := Apply(context.Background(), d, store.State{},
+		systemctlStub(t, "loaded", "active", "static", &verbs), nil)
+	if err == nil {
+		t.Fatal("a static unit was accepted as enable-able")
+	}
+	if !strings.Contains(err.Error(), "no install section") {
+		t.Errorf("failed for the wrong reason: %v", err)
+	}
+}
+
+func TestAnUnknownBootStateIsRefusedNotGuessed(t *testing.T) {
+	var verbs []string
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"rt","type":"service","unit":"x.service","state":"running","boot":"enabled"}
+	]}`)
+	_, _, err := Apply(context.Background(), d, store.State{},
+		systemctlStub(t, "loaded", "active", "indirect", &verbs), nil)
+	if err == nil {
+		t.Fatal("an unrecognised boot state was guessed at instead of refused")
+	}
+}
+
+// --- more than one container runtime (novox/hq ADR 0060) ---
+
+func TestTheRuntimeProbeIsPerRuntime(t *testing.T) {
+	// Verified against a real podman 6.1.0 before this was written:
+	//
+	//   docker info --format '{{.ServerVersion}}'   ->  29.7.2
+	//   podman info --format '{{.ServerVersion}}'   ->  Error: can't evaluate field
+	//                                                   ServerVersion in type system.infoReport
+	//   podman info --format '{{.Version.Version}}' ->  6.1.0
+	//
+	// So a single probe cannot find both, and a host that used docker's would report a machine
+	// with podman as having no container runtime at all.
+	for _, tc := range []struct {
+		name, present, wantProbe string
+	}{
+		{"docker", "docker", "{{.ServerVersion}}"},
+		{"podman", "podman", "{{.Version.Version}}"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			var probedWith string
+			run := func(ctx context.Context, name string, args ...string) (string, error) {
+				if name != tc.present {
+					return "", errors.New("not installed")
+				}
+				if args[0] == "info" {
+					probedWith = args[2]
+				}
+				return "ok\n", nil
+			}
+			got, err := containerRuntime(context.Background(), run)
+			if err != nil {
+				t.Fatalf("%s was present and was not found: %v", tc.present, err)
+			}
+			if got != tc.present {
+				t.Errorf("found %q, expected %q", got, tc.present)
+			}
+			if probedWith != tc.wantProbe {
+				t.Errorf("probed %s with %q; that template does not work on it",
+					tc.present, probedWith)
+			}
+		})
+	}
+}
+
+func TestAContainerUsesTheRuntimeTheMachineHas(t *testing.T) {
+	// The applier must not call `docker` on a machine that has podman. Adoption keeps what the
+	// machine already has (novox/hq research 012), so hardcoding one contradicts it.
+	var calledWith []string
+	run := func(ctx context.Context, name string, args ...string) (string, error) {
+		if name == "docker" {
+			return "", errors.New("not installed")
+		}
+		calledWith = append(calledWith, name)
+		switch args[0] {
+		case "info":
+			return "6.1.0\n", nil
+		case "inspect":
+			return "false\t\n", errors.New("no such container")
+		case "run":
+			return "deadbeef\n", nil
+		}
+		return "", nil
+	}
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"store","type":"container","name":"store","image":"`+pinned+`"}
+	]}`)
+
+	// It will fail at read-back — the stub never reports it running — and what matters is
+	// WHICH binary it used getting there.
+	_, _, _ = Apply(context.Background(), d, store.State{}, run, nil)
+
+	for _, c := range calledWith {
+		if c != "podman" {
+			t.Errorf("called %q on a machine that only has podman", c)
+		}
+	}
+	if len(calledWith) == 0 {
+		t.Error("nothing was called; the runtime was not found")
+	}
+}
+
+func TestNoRuntimeIsSaidPlainly(t *testing.T) {
+	// Naming what was tried, because "docker: command not found" on a machine that deliberately
+	// runs podman sends the reader looking for the wrong thing.
+	run := func(ctx context.Context, name string, args ...string) (string, error) {
+		return "", errors.New("not installed")
+	}
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"store","type":"container","name":"store","image":"`+pinned+`"}
+	]}`)
+
+	_, _, err := Apply(context.Background(), d, store.State{}, run, nil)
+	if err == nil {
+		t.Fatal("a machine with no container runtime applied a container")
+	}
+	for _, want := range []string{"docker", "podman", "no container runtime"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("the failure does not mention %q: %v", want, err)
+		}
+	}
+}

internal/declaration/declaration.go

@@ -98,11 +98,21 @@ func (f *File) validate(where string, _ bool) []string {
 }
 
 // Service is a unit the host puts into a state. It does not install the unit.
+//
+// Two states, and they are orthogonal rather than one scale. A unit can be enabled and stopped
+// (it will come back at boot), or disabled and running (started by hand, gone after a reboot).
+// Folding them into one field would make the second expressible only by accident.
 type Service struct {
 	ID    string `json:"id"`
 	Type  Type   `json:"type"`
 	Unit  string `json:"unit"`
 	State string `json:"state"`
+	// Boot is "enabled" or "disabled" — whether the unit starts at boot. Optional: absent means
+	// the host asserts nothing about it and leaves whatever is there.
+	//
+	// Without this the host could start a unit and not make it survive a reboot, which is a
+	// declaration that reports success and stops being true at the next power cut.
+	Boot string `json:"boot,omitempty"`
 }
 
 func (s *Service) Identity() string { return s.ID }
@@ -118,6 +128,11 @@ func (s *Service) validate(where string, _ bool) []string {
 		problems = append(problems, fmt.Sprintf(
 			"%s: state %q; a service is \"running\" or \"stopped\"", where, s.State))
 	}
+	if s.Boot != "" && s.Boot != "enabled" && s.Boot != "disabled" {
+		problems = append(problems, fmt.Sprintf(
+			"%s: boot %q; a service is \"enabled\" or \"disabled\" at boot, or omits it to "+
+				"leave the machine's own setting alone", where, s.Boot))
+	}
 	return problems
 }