diff --git a/internal/apply/apply.go b/internal/apply/apply.go index fff75e1..e5e23f0 100644 --- a/internal/apply/apply.go +++ b/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): - // 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. - if a.Stateless { - return "forgotten", "its state was never the mesh's", nil - } - // 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 + return removeService(ctx, sys, a, run) + + case declaration.TypeProcess: + // The other side of the same line: a process's unit is the host's own — it wrote the unit + // file and unpacked the bundle — so it goes with its declaration (novox/hq ADR 0118). + return removeProcess(ctx, a, run) 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 +} diff --git a/internal/apply/apply_test.go b/internal/apply/apply_test.go index 31c04e5..fdde6c0 100644 --- a/internal/apply/apply_test.go +++ b/internal/apply/apply_test.go @@ -348,33 +348,111 @@ func TestAnUnknownServiceStateIsRefusedNotGuessed(t *testing.T) { } } -func TestADroppedServiceIsStoppedNotDeleted(t *testing.T) { - // The host did not install the unit and does not own the unit file — only the state it put - // the unit into. - var commands []string +func TestADroppedServiceIsGivenBackTheStateItWasFoundIn(t *testing.T) { + // novox/hq ADR 0118: undeclaring removes what the mesh made, gives back what it changed, and + // 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, " ")) + if args[0] == "show" { + return "LoadState=loaded\nActiveState=active\n", nil + } + return "", nil + } + state := store.State{Resources: []store.Applied{ + {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 != nil { + t.Fatal(err) + } + joined := strings.Join(commands, "; ") + if stopped := strings.Contains(joined, "stop unit.service"); stopped != c.stop { + t.Errorf("stopped %v, want %v: %s", stopped, c.stop, joined) + } + if disabled := strings.Contains(joined, "disable unit.service"); disabled != c.disable { + 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) { - commands = append(commands, strings.Join(args, " ")) - if args[0] == "show" { - return "LoadState=loaded\nActiveState=active\n", nil + 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 } - state := store.State{Resources: []store.Applied{ - {ID: "s", Type: "service", Target: "gone.service"}, - }} d := parse(t, `{"declaration":1,"resources":[ - {"id":"other","type":"file","path":"`+filepath.Join(t.TempDir(), "a")+`","content":"a\n"} + {"id":"s","type":"service","unit":"filter.service","state":"running","boot":"enabled"} ]}`) - - if _, _, err := Apply(context.Background(), archHost(t), d, state, store.OriginCarried, run, nil, nil); err != nil { + _, first, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil) + if err != nil { t.Fatal(err) } - joined := strings.Join(commands, "; ") - if !strings.Contains(joined, "stop gone.service") { - t.Errorf("the dropped service was not stopped: %s", joined) + 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) } - if strings.Contains(joined, "disable") || strings.Contains(joined, "mask") { - t.Errorf("the host did more than stop a unit it does not own: %s", joined) + _, 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) } } diff --git a/internal/apply/opening_test.go b/internal/apply/opening_test.go index 5f976ee..1be33f5 100644 --- a/internal/apply/opening_test.go +++ b/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) diff --git a/internal/apply/plan.go b/internal/apply/plan.go index c79212b..a142d6e 100644 --- a/internal/apply/plan.go +++ b/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" diff --git a/internal/apply/process.go b/internal/apply/process.go index d9956e7..e2f37a6 100644 --- a/internal/apply/process.go +++ b/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 +} diff --git a/internal/apply/process_test.go b/internal/apply/process_test.go index 3d38e22..93dca73 100644 --- a/internal/apply/process_test.go +++ b/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) + } +} diff --git a/internal/store/store.go b/internal/store/store.go index fc1d26b..0878b62 100644 --- a/internal/store/store.go +++ b/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"` +}