internal/apply/apply.go

@@ -56,6 +56,8 @@ type Outcome struct {
 	kept string
 	// stateless is a service whose unit's lifecycle is the machine's (novox/hq ADR 0117).
 	stateless bool
+	// found is, for a service, its unit as the host first found it (novox/hq ADR 0118).
+	found *store.FoundUnit
 	// reads is, for a container, the digest of each file it was created reading, by path — so
 	// the next apply can say which one changed (novox/hq 04-ISSUES/103).
 	reads map[string]string
@@ -465,6 +467,7 @@ func ApplyKeeping(
 			Kept:      kept,
 			Reads:     outcome.reads,
 			Stateless: outcome.stateless,
+			Found:     outcome.found,
 			Holds:     holds(resource),
 		})
 		// Its module has been taken, and what was held for it is now the mesh's. A file written
@@ -545,7 +548,7 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru
 	case *declaration.File:
 		return applyFile(res, previous, unseal, keepFound)
 	case *declaration.Service:
-		return applyService(ctx, sys, res, run, changed)
+		return applyService(ctx, sys, res, run, changed, previous)
 	case *declaration.Package:
 		return applyPackage(ctx, sys, res, run)
 	case *declaration.Container:
@@ -922,11 +925,34 @@ type unitReloader interface {
 }
 
 func applyService(ctx context.Context, sys system.System, r *declaration.Service, run Runner,
-	changed map[string]bool) (Outcome, error) {
+	changed map[string]bool, previous store.Applied) (Outcome, error) {
 	if r.Stateless() {
 		return reflectOnly(ctx, sys, r, run, changed)
 	}
 	out := begin(r)
+
+	// **What the unit was before the mesh touched it**, read once — the first time this host
+	// applies it — and carried in the record from then on (novox/hq ADR 0118). Undeclared, the
+	// unit is given back to exactly this: it is the one fact that separates the container runtime,
+	// running before the mesh arrived and to be left running, from the mesh's packet filter,
+	// stopped until the mesh started it and to be stopped again. A record that predates this
+	// field is not read now: the unit's state by then is the mesh's doing, not what was found.
+	switch {
+	case previous.Found != nil:
+		out.found = previous.Found
+	case previous.ID == "":
+		state, err := sys.ServiceState(ctx, run, r.Unit)
+		if err != nil {
+			return out, err
+		}
+		found := &store.FoundUnit{State: state}
+		if r.Boot != "" {
+			if found.Boot, err = sys.ServiceBoot(ctx, run, r.Unit); err != nil {
+				return out, err
+			}
+		}
+		out.found = found
+	}
 	var changes []string
 
 	// A file the service reflects changed, and it may be the unit's own file or a drop-in: the
@@ -1160,30 +1186,12 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner)
 		return "removed", "no longer declared", nil
 
 	case declaration.TypeService:
-		// A unit whose lifecycle was the machine's is left exactly as it is (novox/hq ADR 0117):
+		return removeService(ctx, sys, a, run)
-		// stopping it here is how unassigning an uplink module would take down the machine's
+
-		// network manager, and with it the channel the mesh reaches the machine on.
+	case declaration.TypeProcess:
-		if a.Stateless {
+		// The other side of the same line: a process's unit is the host's own — it wrote the unit
-			return "forgotten", "its state was never the mesh's", nil
+		// file and unpacked the bundle — so it goes with its declaration (novox/hq ADR 0118).
-		}
+		return removeProcess(ctx, a, run)
-		// A unit that is no longer declared is stopped, not deleted. The host did not install
-		// it and does not own the unit file — only the state it put the unit into.
-		//
-		// A unit that no longer EXISTS is already in the state removal is trying to reach, and
-		// saying so matters: stopping it fails, and a failure here fails the whole apply. A
-		// host holding a record of an uninstalled unit would then be unable to apply anything,
-		// ever, with no way out but editing its state by hand. Removal is idempotent for the
-		// same reason `os.RemoveAll` is.
-		if _, err := sys.ServiceState(ctx, run, a.Target); err != nil {
-			if strings.Contains(err.Error(), "does not exist on this machine") {
-				return "forgotten", "the unit no longer exists", nil
-			}
-			return "", "", err
-		}
-		if err := sys.SetServiceState(ctx, run, a.Target, "stopped"); err != nil {
-			return "", "", fmt.Errorf("stopping %s: %w", a.Target, err)
-		}
-		return "removed", "stopped; the unit file is not the host's to delete", nil
 
 	case declaration.TypeContainer:
 		// The host CREATED this one, so the host removes it. That is the line: it removes what
@@ -2019,3 +2027,54 @@ func declaredDigest(r declaration.Resource) string {
 	}
 	return fmt.Sprintf("%x", sha256.Sum256([]byte(material)))
 }
+
+// removeService gives a unit back the state the host first found it in, and nothing more.
+//
+// **Removes what it made, gives back what it changed, leaves what was the machine's**
+// (novox/hq ADR 0118). A service resource never installs a unit; it puts one that already existed
+// into a state. So undeclaring it cannot mean stopping it — that is how unassigning the private
+// network stopped the container runtime and every container with it, how unassigning sshd would
+// have stopped ssh, and how an uplink module would have taken a machine off its only link
+// (novox/hq issue 130). It means undoing what the mesh did to it: a unit found running is left
+// running; a unit the mesh started — the packet filter a converge loaded, which returning to
+// adopted must unload — is stopped again, and disabled again if the mesh enabled it.
+//
+// **Never started on the way out.** A unit the mesh stopped is not started again when its
+// declaration goes: starting something is a decision, and the operator makes it.
+func removeService(ctx context.Context, sys system.System, a store.Applied, run Runner) (string, string, error) {
+	if a.Stateless {
+		return "forgotten", "its state was never the mesh's", nil
+	}
+	if a.Found == nil {
+		// Recorded before the host kept what it found. Not knowing, it leaves the unit as it is:
+		// a unit left running can be stopped by the operator, and one stopped by mistake may be
+		// the link the operator would use to do it.
+		return "forgotten", "the unit is the machine's; left as it is", nil
+	}
+	if a.Found.State != "stopped" && (a.Found.Boot == "" || a.Found.Boot == "enabled") {
+		return "forgotten", "it was running before the mesh; left as it is", nil
+	}
+	// Something is to be given back, so the unit must still be there to give it to. One since
+	// uninstalled is already as far from the mesh as it can be, and saying so keeps a record of it
+	// from failing every apply.
+	if _, err := sys.ServiceState(ctx, run, a.Target); err != nil {
+		if strings.Contains(err.Error(), "does not exist on this machine") {
+			return "forgotten", "the unit no longer exists", nil
+		}
+		return "", "", err
+	}
+	var gave []string
+	if a.Found.State == "stopped" {
+		if err := sys.SetServiceState(ctx, run, a.Target, "stopped"); err != nil {
+			return "", "", fmt.Errorf("stopping %s, which the mesh started: %w", a.Target, err)
+		}
+		gave = append(gave, "stopped again")
+	}
+	if a.Found.Boot == "disabled" {
+		if err := sys.SetServiceBoot(ctx, run, a.Target, "disabled"); err != nil {
+			return "", "", fmt.Errorf("disabling %s, which the mesh enabled: %w", a.Target, err)
+		}
+		gave = append(gave, "disabled at boot again")
+	}
+	return "restored", strings.Join(gave, ", ") + ", as the host found it", nil
+}

internal/apply/apply_test.go

@@ -348,9 +348,27 @@ func TestAnUnknownServiceStateIsRefusedNotGuessed(t *testing.T) {
 	}
 }
 
-func TestADroppedServiceIsStoppedNotDeleted(t *testing.T) {
+func TestADroppedServiceIsGivenBackTheStateItWasFoundIn(t *testing.T) {
-	// The host did not install the unit and does not own the unit file — only the state it put
+	// novox/hq ADR 0118: undeclaring removes what the mesh made, gives back what it changed, and
-	// the unit into.
+	// leaves what was the machine's. A service resource never installs a unit — so undeclaring it
+	// undoes what the mesh did to the unit, and nothing more. Stopping every undeclared unit is
+	// how unassigning the private network stopped the container runtime (novox/hq issue 130).
+	cases := []struct {
+		name    string
+		found   *store.FoundUnit
+		action  string
+		stop    bool
+		disable bool
+	}{
+		{"recorded before the host kept what it found", nil, "forgotten", false, false},
+		{"running before the mesh", &store.FoundUnit{State: "running"}, "forgotten", false, false},
+		{"running and enabled before the mesh", &store.FoundUnit{State: "running", Boot: "enabled"}, "forgotten", false, false},
+		{"started by the mesh", &store.FoundUnit{State: "stopped"}, "restored", true, false},
+		{"started and enabled by the mesh", &store.FoundUnit{State: "stopped", Boot: "disabled"}, "restored", true, true},
+		{"enabled by the mesh, running before it", &store.FoundUnit{State: "running", Boot: "disabled"}, "restored", false, true},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
 			var commands []string
 			run := func(ctx context.Context, name string, args ...string) (string, error) {
 				commands = append(commands, strings.Join(args, " "))
@@ -360,21 +378,81 @@ func TestADroppedServiceIsStoppedNotDeleted(t *testing.T) {
 				return "", nil
 			}
 			state := store.State{Resources: []store.Applied{
-		{ID: "s", Type: "service", Target: "gone.service"},
+				{ID: "s", Type: "service", Target: "unit.service", Found: c.found},
 			}}
 			d := parse(t, `{"declaration":1,"resources":[
 			  {"id":"other","type":"file","path":"`+filepath.Join(t.TempDir(), "a")+`","content":"a\n"}
 			]}`)
-
+			report, after, err := Apply(context.Background(), archHost(t), d, state, store.OriginCarried, run, nil, nil)
-	if _, _, err := Apply(context.Background(), archHost(t), d, state, store.OriginCarried, run, nil, nil); err != nil {
+			if err != nil {
 				t.Fatal(err)
 			}
 			joined := strings.Join(commands, "; ")
-	if !strings.Contains(joined, "stop gone.service") {
+			if stopped := strings.Contains(joined, "stop unit.service"); stopped != c.stop {
-		t.Errorf("the dropped service was not stopped: %s", joined)
+				t.Errorf("stopped %v, want %v: %s", stopped, c.stop, joined)
 			}
-	if strings.Contains(joined, "disable") || strings.Contains(joined, "mask") {
+			if disabled := strings.Contains(joined, "disable unit.service"); disabled != c.disable {
-		t.Errorf("the host did more than stop a unit it does not own: %s", joined)
+				t.Errorf("disabled %v, want %v: %s", disabled, c.disable, joined)
+			}
+			if strings.Contains(joined, "start unit.service") || strings.Contains(joined, "mask") {
+				t.Errorf("the host started or masked a unit on its way out: %s", joined)
+			}
+			if o := outcomeOf(report, "s"); o.Action != c.action {
+				t.Errorf("outcome %+v, want %s", o, c.action)
+			}
+			if _, still := after.Find("s"); still {
+				t.Error("the host still believes it owns the undeclared service")
+			}
+		})
+	}
+}
+
+func TestAServiceRecordsTheStateItWasFoundInOnceAndCarriesIt(t *testing.T) {
+	// Read the first time the host applies the unit — before it starts or enables anything —
+	// and never again: by the next apply, the unit's state is the mesh's doing.
+	active := "inactive"
+	enabled := "disabled"
+	run := func(ctx context.Context, name string, args ...string) (string, error) {
+		switch args[0] {
+		case "show":
+			return "LoadState=loaded\nActiveState=" + active + "\n", nil
+		case "is-enabled":
+			return enabled, nil
+		case "start":
+			active = "active"
+		case "enable":
+			enabled = "enabled"
+		}
+		return "", nil
+	}
+	d := parse(t, `{"declaration":1,"resources":[
+	  {"id":"s","type":"service","unit":"filter.service","state":"running","boot":"enabled"}
+	]}`)
+	_, first, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rec, _ := first.Find("s")
+	if rec.Found == nil || rec.Found.State != "stopped" || rec.Found.Boot != "disabled" {
+		t.Fatalf("first apply recorded %+v, want stopped and disabled", rec.Found)
+	}
+	_, second, err := Apply(context.Background(), archHost(t), d, first, store.OriginCarried, run, nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rec, _ = second.Find("s")
+	if rec.Found == nil || rec.Found.State != "stopped" {
+		t.Errorf("a later apply replaced what was found with what the mesh made: %+v", rec.Found)
+	}
+
+	// A record from before the host kept what it found is not given one later.
+	old := store.State{Resources: []store.Applied{{ID: "s", Type: "service", Target: "filter.service"}}}
+	_, third, err := Apply(context.Background(), archHost(t), d, old, store.OriginCarried, run, nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if rec, _ = third.Find("s"); rec.Found != nil {
+		t.Errorf("a record that predates the field was given one after the mesh had acted: %+v", rec.Found)
 	}
 }
 

internal/apply/opening_test.go

@@ -359,7 +359,9 @@ func TestReturningToAdoptedLoadsTheGuardBeforeRemovingTheFilter(t *testing.T) {
 			return "", nil
 		}
 		converged := store.State{Resources: []store.Applied{
-			{ID: "nftables.load", Type: "service", Target: "mesh-filter.service", Origin: store.OriginDeclared}}}
+			{ID: "nftables.load", Type: "service", Target: "mesh-filter.service", Origin: store.OriginDeclared,
+				// The mesh loaded this filter at converge: it was not running before (novox/hq ADR 0118).
+				Found: &store.FoundUnit{State: "stopped"}}}}
 		_, state, err := applyWith(t, adopted(t, `{"taken":[]}`, withConf(dir)+","+guardFile), converged, run)
 		if !guardUpAtStop {
 			t.Errorf("stop fails %v: the derived filter was stopped before the guard was written", stopFails)

internal/apply/plan.go

@@ -80,8 +80,17 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step {
 	for _, orphan := range known.Orphans(declared, origin) {
 		step := Step{Verb: "remove", Type: orphan.Type, ID: orphan.ID, Target: orphan.Target,
 			Why: "recorded here and no longer declared"}
-		if orphan.Stateless {
+		switch {
+		case orphan.Stateless:
 			step.Verb, step.Why = "forget", "no longer declared; its unit's state was never the mesh's and is left as it is"
+		case orphan.Type == string(declaration.TypeService):
+			// What removal will do, said before it does it (novox/hq ADR 0118): a unit is given
+			// back what the host found, so the preview names which units that stops.
+			if f := orphan.Found; f != nil && (f.State == "stopped" || f.Boot == "disabled") {
+				step.Verb, step.Why = "restore", "no longer declared; the mesh started or enabled it, and it goes back as it was found"
+			} else {
+				step.Verb, step.Why = "forget", "no longer declared; the unit is the machine's and is left as it is"
+			}
 		}
 		if d.Adoption == nil && strings.HasPrefix(orphan.ID, declaration.AdoptionPrefix) {
 			step.Why = "what protected this node while adopted; removed last, once everything else applied"

internal/apply/process.go

@@ -30,7 +30,7 @@ import (
 // Under the mesh's own directory rather than somewhere a distribution owns: these are files the
 // mesh puts there and replaces, and putting them where a package manager also writes is how two
 // owners end up disagreeing about one path.
-const daemonRoot = "/var/lib/mesh/daemons"
+var daemonRoot = "/var/lib/mesh/daemons"
 
 // unitDir is where the mesh writes the units it owns. A variable only so a test can point it at a
 // directory of its own.
@@ -290,3 +290,53 @@ func unitValue(key, value string) string {
 	).Replace(value)
 	return `"` + key + "=" + escaped + `"`
 }
+
+// removeProcess takes away a process the mesh no longer declares: its timer and unit stopped and
+// disabled, their files removed, the supervisor told, and the unpacked bundle deleted.
+//
+// **All of it is the host's**, which is why all of it goes (novox/hq ADR 0118). Before this there
+// was no way to remove a process at all, and one left undeclared failed every apply on its node
+// until someone edited the host's state by hand — unassigning any module that ran its own code
+// stranded the machine.
+//
+// Idempotent, like every removal: a unit already gone is not an error, and a record whose files
+// have all vanished is forgotten rather than reported as removed.
+func removeProcess(ctx context.Context, a store.Applied, run Runner) (string, string, error) {
+	name := a.Target
+	if name == "" || strings.ContainsAny(name, "/ \t") {
+		// The name is a unit name and a directory under the mesh's own. One that could climb out
+		// of either is refused rather than acted on, whatever wrote it into the record.
+		return "", "", fmt.Errorf("a process recorded under %q cannot be removed by name", name)
+	}
+	found := false
+	for _, unit := range []string{name + ".timer", name + ".service"} {
+		path := filepath.Join(unitDir, unit)
+		if _, err := os.Stat(path); err != nil {
+			continue
+		}
+		found = true
+		// The timer first, so a scheduled run cannot start the service between the two.
+		if _, err := run(ctx, "systemctl", "disable", "--now", unit); err != nil {
+			return "", "", fmt.Errorf("stopping %s: %w", unit, err)
+		}
+		if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
+			return "", "", err
+		}
+	}
+	if found {
+		if _, err := run(ctx, "systemctl", "daemon-reload"); err != nil {
+			return "", "", err
+		}
+	}
+	bundle := filepath.Join(daemonRoot, name)
+	if _, err := os.Stat(bundle); err == nil {
+		found = true
+		if err := os.RemoveAll(bundle); err != nil {
+			return "", "", err
+		}
+	}
+	if !found {
+		return "forgotten", "no longer there", nil
+	}
+	return "removed", "stopped; its unit and its bundle removed — the mesh's own code", nil
+}

internal/apply/process_test.go

@@ -1,10 +1,14 @@
 package apply
 
 import (
+	"context"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
 	"github.com/novox/mesh-host/internal/declaration"
+	"github.com/novox/mesh-host/internal/store"
 )
 
 func aProcess() *declaration.Process {
@@ -163,3 +167,60 @@ func TestAnEnvironmentValueMeansWhatTheDeclarationSaid(t *testing.T) {
 		t.Fatalf("a quote was not escaped, so the value ends early: %s", line)
 	}
 }
+
+func TestAnUndeclaredProcessIsRemovedWithItsUnitAndBundle(t *testing.T) {
+	// novox/hq ADR 0118: a process's unit and bundle are the host's own, so they go with the
+	// declaration. Before, there was no way to remove a process at all, and one left undeclared
+	// failed every apply on its node.
+	units, bundles := t.TempDir(), t.TempDir()
+	wasUnits, wasBundles := unitDir, daemonRoot
+	unitDir, daemonRoot = units, bundles
+	t.Cleanup(func() { unitDir, daemonRoot = wasUnits, wasBundles })
+
+	for _, f := range []string{"mesh-job.service", "mesh-job.timer"} {
+		if err := os.WriteFile(filepath.Join(units, f), []byte("[Unit]\n"), 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := os.MkdirAll(filepath.Join(bundles, "mesh-job", "bin"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	var commands []string
+	run := func(ctx context.Context, name string, args ...string) (string, error) {
+		commands = append(commands, strings.Join(args, " "))
+		return "", nil
+	}
+	known := store.State{Resources: []store.Applied{{ID: "p", Type: "process", Target: "mesh-job"}}}
+	d := parse(t, `{"declaration":1,"resources":[
+	  {"id":"f","type":"file","path":"`+filepath.Join(t.TempDir(), "a")+`","content":"a\n"}
+	]}`)
+	report, after, err := Apply(context.Background(), archHost(t), d, known, store.OriginCarried, run, nil, nil)
+	if err != nil {
+		t.Fatalf("an undeclared process failed the apply: %v", err)
+	}
+	joined := strings.Join(commands, "; ")
+	timer, service := strings.Index(joined, "disable --now mesh-job.timer"), strings.Index(joined, "disable --now mesh-job.service")
+	if timer < 0 || service < 0 || timer > service {
+		t.Errorf("the timer and the unit were not stopped, timer first: %s", joined)
+	}
+	if !strings.Contains(joined, "daemon-reload") {
+		t.Errorf("the service manager was not told its units changed: %s", joined)
+	}
+	for _, gone := range []string{filepath.Join(units, "mesh-job.service"), filepath.Join(units, "mesh-job.timer"), filepath.Join(bundles, "mesh-job")} {
+		if _, err := os.Stat(gone); !os.IsNotExist(err) {
+			t.Errorf("%s is still there", gone)
+		}
+	}
+	if o := outcomeOf(report, "p"); o.Action != "removed" {
+		t.Errorf("outcome %+v, want removed", o)
+	}
+	if _, still := after.Find("p"); still {
+		t.Error("the host still believes it owns the process")
+	}
+
+	// Again, with everything already gone: forgotten, not an error.
+	_, _, err = Apply(context.Background(), archHost(t), d, known, store.OriginCarried, run, nil, nil)
+	if err != nil {
+		t.Errorf("removing a process that is already gone failed: %v", err)
+	}
+}

internal/store/store.go

@@ -82,6 +82,13 @@ type Applied struct {
 	// service removed as if it had a state is stopped: the machine's network manager, for one.
 	Stateless bool `json:"stateless,omitempty"`
 
+	// Found is, for a service, the state its unit was in when this host first applied it — before
+	// the mesh started, stopped, enabled or disabled anything. Removal gives that back and nothing
+	// more (novox/hq ADR 0118): a unit that was running before the mesh arrived keeps running
+	// when its declaration goes; one the mesh started is stopped again. Absent on a record written
+	// before the host kept it, and then the unit is left exactly as it is.
+	Found *FoundUnit `json:"found,omitempty"`
+
 	// Into is set for a file written into rather than over (novox/hq ADR 0102): the format, what
 	// each of the mesh's keys held before it set them, which of them were absent, and whether the
 	// file itself was — so undeclaring it gives the machine back exactly what it had.
@@ -447,3 +454,12 @@ func originOf(r Applied) string {
 	}
 	return r.Origin
 }
+
+// FoundUnit is a service's unit as the host first found it.
+type FoundUnit struct {
+	// State is "running" or "stopped".
+	State string `json:"state"`
+	// Boot is "enabled" or "disabled" — or empty when the declaration never set it, and the host
+	// never touched it.
+	Boot string `json:"boot,omitempty"`
+}