diff --git a/internal/apply/apply.go b/internal/apply/apply.go index b63bc7b..5cbcdd1 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -64,6 +64,8 @@ type Outcome struct { scope, user string // found is, for a service, its unit as the host first found it (novox/hq ADR 0118). found *store.FoundUnit + // declared is, for a service with a state, what this apply held its unit to (novox/hq ADR 0269). + declared *store.FoundUnit // shell is, for a user, the login shell it was found with and the one the mesh set (novox/hq // ADR 0176 §2, issue 228). shell *store.LoginShell @@ -732,6 +734,7 @@ func ApplyMindingWindows( Scope: outcome.scope, User: outcome.user, Found: outcome.found, + Declared: outcome.declared, Shell: outcome.shell, Linger: outcome.linger, Groups: outcome.groups, @@ -1512,6 +1515,10 @@ func applyServiceIn(ctx context.Context, sys system.System, r *declaration.Servi } } + // What the mesh holds the unit to, recorded with what was found, so its removal can tell the mesh's + // own stopping from anybody else's (novox/hq ADR 0269). + out.declared = &store.FoundUnit{Unit: r.Unit, State: r.State, Boot: r.Boot} + before, err := sys.ServiceState(ctx, run, r.Unit) if err != nil { return out, err @@ -2750,9 +2757,11 @@ func declaredDigest(r declaration.Resource) string { // disabled at boot, as a process's unit is, before its file goes: orphans are removed newest // first, and a unit's file is declared before the service that starts 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. The report says -// what was actually done — a unit already as it was found is forgotten, not "restored". +// **Never started on the way out — but for what the mesh's own declaration stopped.** A unit the mesh +// stopped is not started again when its declaration goes: starting something is a decision, and the +// operator makes it. The exception is a unit found running or enabled that the declaration itself held +// stopped or disabled: that is the mesh's doing, and it is undone (novox/hq ADR 0269, startUnitBack). The +// report says what was actually done — a unit already as it was found is forgotten, not "restored". func removeService(ctx context.Context, sys system.System, a store.Applied, run Runner, made bool) (string, string, error) { if a.Stateless { @@ -2811,6 +2820,20 @@ func giveUnitBack(ctx context.Context, sys system.System, a store.Applied, run R stop := made || a.Found.State == "stopped" disable := made || a.Found.Boot == "disabled" + // **What the mesh's own declaration stopped is started again** (novox/hq ADR 0269, narrowing ADR + // 0118's "never started on the way out"): a unit found running that the mesh held stopped, and one + // found enabled that it held disabled. Only the mesh's declaration did that, and leaving it so is the + // mesh's doing outliving the mesh — a pool whose distribution scrub timer a module stopped would have + // no schedule at all once the module went. A unit stopped for any other reason is not known to have + // been stopped by the mesh, and is left as before. + if !made && a.Declared != nil && (a.Declared.Unit == "" || a.Declared.Unit == a.Target) { + start := a.Found.State == "running" && a.Declared.State == "stopped" + enable := a.Found.Boot == "enabled" && a.Declared.Boot == "disabled" + if start || enable { + return startUnitBack(ctx, sys, a, run, start, enable) + } + } + if !stop && !disable { // Found running, and not disabled by anyone but the mesh: nothing to give back. What the // mesh did since is said, so a unit left stopped is not reported as the machine's doing — @@ -2886,6 +2909,54 @@ func giveUnitBack(ctx context.Context, sys system.System, a store.Applied, run R return "restored", detail, nil } +// startUnitBack gives back a unit the mesh's declaration stopped or disabled, found running or enabled: +// enabled first, then started, each read back, and the report says what was done (novox/hq ADR 0269). +func startUnitBack(ctx context.Context, sys system.System, a store.Applied, run Runner, start, enable bool) (string, string, error) { + state, err := sys.ServiceState(ctx, run, a.Target) + if err != nil { + if strings.Contains(err.Error(), "does not exist on this machine") { + return "forgotten", "the unit no longer exists", nil + } + return "", "", err + } + var did, already []string + if enable { + if boot, err := sys.ServiceBoot(ctx, run, a.Target); err == nil && boot == "enabled" { + already = append(already, "enabled at boot") + } else { + if err := sys.SetServiceBoot(ctx, run, a.Target, "enabled"); err != nil { + return "", "", fmt.Errorf("enabling %s again, which the mesh disabled and found enabled: %w", a.Target, err) + } + did = append(did, "enabled at boot") + } + } + if start { + if state == "running" { + already = append(already, "running") + } else { + if err := sys.SetServiceState(ctx, run, a.Target, "running"); err != nil { + return "", "", fmt.Errorf("starting %s again, which the mesh stopped and found running: %w", a.Target, err) + } + after, err := sys.ServiceState(ctx, run, a.Target) + if err != nil { + return "", "", err + } + if after != "running" { + return "", "", fmt.Errorf("%s was started again, as the host found it, and is %s", a.Target, after) + } + did = append(did, "started") + } + } + if len(did) == 0 { + return "forgotten", "already as the host found it (" + strings.Join(already, ", ") + ")", nil + } + detail := strings.Join(did, " and ") + " again, as the host found it before the mesh's declaration stopped it" + if len(already) > 0 { + detail += "; already " + strings.Join(already, ", ") + } + return "restored", detail, nil +} + // foundAs is what a service's unit was before the mesh touched it, as far as this host can know: // what an earlier apply recorded, or — the first time the mesh is about to act on the unit — what // is there now (novox/hq ADR 0118). Nil when it cannot be known: a record written before the host @@ -2916,7 +2987,7 @@ func foundAs(ctx context.Context, sys system.System, r *declaration.Service, run // Given back as if undeclared, never as the mesh's own: whether the mesh wrote the old // unit's file is known to the removal of orphans, and that file's own record goes with it. action, detail, err := removeService(ctx, sys, - store.Applied{Type: string(declaration.TypeService), Target: unit, Found: f, + store.Applied{Type: string(declaration.TypeService), Target: unit, Found: f, Declared: previous.Declared, Scope: previous.Scope, User: previous.User}, run, false) switch { case errors.Is(err, errRemovalWaits): diff --git a/internal/apply/giveback_test.go b/internal/apply/giveback_test.go new file mode 100644 index 0000000..bbe99ce --- /dev/null +++ b/internal/apply/giveback_test.go @@ -0,0 +1,103 @@ +package apply + +import ( + "strings" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// A unit the mesh found running and enabled, and held stopped and disabled by its own declaration, is +// given back running and enabled when that declaration goes (the zfs module holds the distribution's +// scrub timer stopped while its own timer scrubs the pool; unassigning it must not leave the pool with no +// schedule at all). The report says so. +func TestAUnitTheMeshStoppedAndDisabledIsGivenBackRunningAndEnabled(t *testing.T) { + m := unitsMachine(map[string]*fakeUnit{ + "zfs-scrub-weekly@storage.timer": {active: "active", enabled: "enabled"}, + }) + _, state, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"zfs.distribution-weekly","type":"service","unit":"zfs-scrub-weekly@storage.timer","state":"stopped","boot":"disabled"}]}`, + store.State{}, m) + if err != nil { + t.Fatal(err) + } + if u := m.units["zfs-scrub-weekly@storage.timer"]; u.active != "inactive" || u.enabled != "disabled" { + t.Fatalf("the fixture did not stop and disable it: %s and %s", u.active, u.enabled) + } + report, _, err := applyOn(t, nothingButA(t), copyOf(t, state), m) + if err != nil { + t.Fatal(err) + } + if u := m.units["zfs-scrub-weekly@storage.timer"]; u.active != "active" || u.enabled != "enabled" { + t.Errorf("given back as %s and %s; it was found running and enabled", u.active, u.enabled) + } + said := "" + for _, r := range report.Outcomes { + if r.ID == "zfs.distribution-weekly" { + said = r.Action + ": " + r.Detail + } + } + if !strings.HasPrefix(said, "restored: ") || !strings.Contains(said, "started") || !strings.Contains(said, "enabled at boot") { + t.Errorf("the report says %q", said) + } +} + +// Only what the mesh's own declaration did is undone: a unit the mesh held stopped that was found stopped +// stays stopped, and one found running that the mesh never stopped is left running as before. +func TestOnlyWhatTheMeshsDeclarationStoppedIsStartedAgain(t *testing.T) { + m := unitsMachine(map[string]*fakeUnit{ + "found-stopped.timer": {active: "inactive", enabled: "disabled"}, + "left-running.timer": {active: "active", enabled: "enabled"}, + }) + _, state, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"a","type":"service","unit":"found-stopped.timer","state":"stopped","boot":"disabled"}, + {"id":"b","type":"service","unit":"left-running.timer","state":"running","boot":"enabled"}]}`, + store.State{}, m) + if err != nil { + t.Fatal(err) + } + if _, _, err := applyOn(t, nothingButA(t), state, m); err != nil { + t.Fatal(err) + } + if u := m.units["found-stopped.timer"]; u.active != "inactive" || u.enabled != "disabled" { + t.Errorf("found stopped and disabled, given back as %s and %s", u.active, u.enabled) + } + if u := m.units["left-running.timer"]; u.active != "active" || u.enabled != "enabled" { + t.Errorf("found running, given back as %s and %s", u.active, u.enabled) + } +} + +// A record from before the host kept what it declared says nothing about who stopped the unit: it is +// left as it is, never started. +func TestARecordWithoutWhatWasDeclaredStartsNothing(t *testing.T) { + m := unitsMachine(map[string]*fakeUnit{"old.timer": {active: "inactive", enabled: "disabled"}}) + state := store.State{} + state.Record(store.Applied{ID: "old", Type: "service", Target: "old.timer", Origin: store.OriginDeclared, + Found: &store.FoundUnit{Unit: "old.timer", State: "running", Boot: "enabled"}}) + if _, _, err := applyOn(t, nothingButA(t), state, m); err != nil { + t.Fatal(err) + } + if u := m.units["old.timer"]; u.active != "inactive" || u.enabled != "disabled" { + t.Errorf("an old record started %s and %s", u.active, u.enabled) + } +} + +// The plan says it before the apply does it. +func TestThePlanSaysAStoppedUnitIsStartedAgain(t *testing.T) { + m := unitsMachine(map[string]*fakeUnit{"zfs-scrub-weekly@storage.timer": {active: "active", enabled: "enabled"}}) + _, state, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"w","type":"service","unit":"zfs-scrub-weekly@storage.timer","state":"stopped","boot":"disabled"}]}`, + store.State{}, m) + if err != nil { + t.Fatal(err) + } + said := "" + for _, s := range Plan(parse(t, nothingButA(t)), state, store.OriginDeclared) { + if s.ID == "w" { + said = s.Verb + ": " + s.Why + } + } + if !strings.HasPrefix(said, "restore: ") || !strings.Contains(said, "started and enabled at boot again") { + t.Errorf("the plan says %q", said) + } +} diff --git a/internal/apply/plan.go b/internal/apply/plan.go index 68eb976..fba1bfa 100644 --- a/internal/apply/plan.go +++ b/internal/apply/plan.go @@ -125,6 +125,18 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { case f == nil: step.Verb, step.Why = "forget", "no longer declared; recorded before the host kept what it "+ "found, so it is left as it is" + case orphan.Declared != nil && ((f.State == "running" && orphan.Declared.State == "stopped") || + (f.Boot == "enabled" && orphan.Declared.Boot == "disabled")): + // startUnitBack's words (novox/hq ADR 0269): what the mesh's declaration stopped goes back. + var back []string + if f.State == "running" && orphan.Declared.State == "stopped" { + back = append(back, "started") + } + if f.Boot == "enabled" && orphan.Declared.Boot == "disabled" { + back = append(back, "enabled at boot") + } + step.Verb, step.Why = "restore", "no longer declared; the host found it running and enabled and "+ + "the mesh held it stopped, so it is "+strings.Join(back, " and ")+" again" case f.State == "stopped" || f.Boot == "disabled": var back []string if f.State == "stopped" { diff --git a/internal/store/store.go b/internal/store/store.go index a7ca2a6..e7a752f 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -99,6 +99,12 @@ type Applied struct { // 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"` + // Declared is, for a service with a state, what the mesh last held its unit to: running or stopped, + // and enabled or disabled where the declaration said. With Found, it tells a unit the mesh's own + // declaration stopped and disabled from one somebody stopped since, so removal gives back running + // and enabled only the first (novox/hq ADR 0269). Absent on a record from before the host kept it, + // and then nothing is started on the way out. + Declared *FoundUnit `json:"declared,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