diff --git a/internal/apply/apply.go b/internal/apply/apply.go index e5e23f0..6bdb549 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -170,6 +170,14 @@ func ApplyKeeping( // as the service that took it over is (novox/hq ADR 0105). declared[takeOverID(svc)] = true } + // What an unfinished apply found a unit as is kept only while its service is declared: one + // declared again later is read afresh, as any resource with no record is (novox/hq ADR 0118). + known.DropFoundUndeclared(declared, origin) + + // Which units the mesh made, read before any removal can take a unit file's record away — so + // whichever order a module declared a unit and its file in, the unit is known for the mesh's + // when its service goes (novox/hq ADR 0118). + made := meshMadeUnits(known) // Which firewall is found here, before anything else, since an unsupported one refuses the // whole declaration (novox/hq ADR 0100). Nothing for a converged node. @@ -188,7 +196,7 @@ func ApplyKeeping( if declaration.Type(orphan.Type) == declaration.TypeOpening { action, detail, err = removeOpening(ctx, orphan, run, known.Firewall) } else { - action, detail, err = remove(ctx, sys, orphan, run) + action, detail, err = remove(ctx, sys, orphan, run, made) } if err != nil { return &Error{Resource: orphan.ID, Err: err, Done: report} @@ -385,6 +393,14 @@ func ApplyKeeping( } was, _ := known.Find(resource.Identity()) + // What an earlier apply of this service found its unit as and could not yet record is + // newer than anything the record says — the record's own finding with what was read + // since — so it is what this apply goes on from (novox/hq ADR 0118). + previous := was + if p, ok := known.FoundFirst[resource.Identity()]; ok { + f := p.FoundUnit + previous.Found = &f + } var outcome Outcome var err error if o, isOpening := resource.(*declaration.Opening); isOpening { @@ -397,9 +413,15 @@ func ApplyKeeping( !known.Recorded(string(declaration.TypeFile), f.Path) { keepFound = keep } - outcome, err = applyOne(ctx, sys, resource, run, changed, in, was, unseal, keepFound) + outcome, err = applyOne(ctx, sys, resource, run, changed, in, previous, unseal, keepFound) } if err != nil { + // **What was found is kept whatever the apply then did** (novox/hq ADR 0118). The + // failure may have come after the mesh enabled or started the unit, and the next apply + // would otherwise read that as the machine's own. + if outcome.found != nil { + known.KeepFound(resource.Identity(), origin, *outcome.found) + } failed := &Error{Resource: resource.Identity(), Err: err, Done: report} failures = append(failures, failed) log(fmt.Sprintf(" failed %s (%s): %v", resource.Identity(), outcome.Target, err)) @@ -470,6 +492,9 @@ func ApplyKeeping( Found: outcome.found, Holds: holds(resource), }) + if outcome.found != nil { + known.DropFound(resource.Identity()) + } // Its module has been taken, and what was held for it is now the mesh's. A file written // into, or a service whose lifecycle is the machine's, replaced nothing that was found, so // its outcome says what the apply did, not that a cutover happened; a hold from when it was @@ -930,29 +955,6 @@ func applyService(ctx context.Context, sys system.System, r *declaration.Service 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 @@ -966,6 +968,21 @@ func applyService(ctx context.Context, sys system.System, r *declaration.Service } } + // **What the unit was before the mesh touched it**, read once 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. Read after the reload above, never before it: a unit whose file this same apply wrote + // is not a unit the service manager knows until then, and reading it first would find nothing. + found, gaveBack, err := foundAs(ctx, sys, r, run, previous) + if err != nil { + return out, err + } + out.found = found + if gaveBack != "" { + changes = append(changes, gaveBack) + } + // 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. @@ -974,6 +991,15 @@ func applyService(ctx context.Context, sys system.System, r *declaration.Service if err != nil { return out, err } + // Whether it started at boot is found the first time the mesh is about to change that — + // which is not always the first apply: a declaration that said nothing about boot never + // touched it, so what is there when one first does is still the machine's (novox/hq ADR + // 0118). Kept before the change, so a failure after it still has it. + if out.found != nil && out.found.Boot == "" { + f := *out.found + f.Boot = bootBefore + out.found = &f + } if bootBefore != r.Boot { if err := sys.SetServiceBoot(ctx, run, r.Unit, r.Boot); err != nil { return out, fmt.Errorf("setting %s to %s at boot: %w", r.Unit, r.Boot, err) @@ -1138,7 +1164,10 @@ func reflectOnly(ctx context.Context, sys system.System, r *declaration.Service, // It returns the action rather than assuming "removed", because for half the vocabulary the // honest word is "forgotten". A host that reported a package removed when it left the package // installed would be describing an effect it declined to have. -func remove(ctx context.Context, sys system.System, a store.Applied, run Runner) (string, string, error) { +// +// made is the units whose unit file this host wrote where there was none (meshMadeUnits). +func remove(ctx context.Context, sys system.System, a store.Applied, run Runner, + made map[string]bool) (string, string, error) { switch declaration.Type(a.Type) { case declaration.TypeDirectory: // **A directory with anything left in it is kept, and that is the rule that protects @@ -1186,7 +1215,7 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner) return "removed", "no longer declared", nil case declaration.TypeService: - return removeService(ctx, sys, a, run) + return removeService(ctx, sys, a, run, made[a.Target]) case declaration.TypeProcess: // The other side of the same line: a process's unit is the host's own — it wrote the unit @@ -2036,45 +2065,171 @@ func declaredDigest(r declaration.Resource) string { // 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. +// running; a unit the mesh started is stopped again, and disabled again if the mesh enabled it. +// +// **A unit whose file the mesh wrote is the mesh's, whatever was found** — made is that. The +// adoption guard and the converge filter are units of exactly this kind: their unit files are the +// mesh's own `file` resources, written where there was none, and records written before the host +// kept what it found say nothing about them. Forgetting one would leave its table loaded — the +// guard beside a converged node's own filter, or the filter beside the predecessor's firewall +// re-enabled — and its unit file then deleted from under a running unit. So it is stopped and +// 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. -func removeService(ctx context.Context, sys system.System, a store.Applied, run Runner) (string, string, error) { +// 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". +func removeService(ctx context.Context, sys system.System, a store.Applied, run Runner, + made bool) (string, string, error) { if a.Stateless { + // Declared with no state (novox/hq ADR 0117): its lifecycle was never the mesh's, and the + // declaration that said so is the operator's word to hold to, a file of the mesh's or not. return "forgotten", "its state was never the mesh's", nil } - if a.Found == nil { + if !made && 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 + return "forgotten", "recorded before the host kept what it found; left as it is", nil } - if a.Found.State != "stopped" && (a.Found.Boot == "" || a.Found.Boot == "enabled") { + stop := made || a.Found.State == "stopped" + disable := made || a.Found.Boot == "disabled" + + 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 — + // read as well as the machine answers, since nothing here acts on the answer. + state, err := sys.ServiceState(ctx, run, a.Target) + if err != nil && strings.Contains(err.Error(), "does not exist on this machine") { + return "forgotten", "the unit no longer exists", nil + } + var did []string + if err == nil && state == "stopped" { + did = append(did, "stopped it") + } + if a.Found.Boot == "enabled" { + if boot, err := sys.ServiceBoot(ctx, run, a.Target); err == nil && boot == "disabled" { + did = append(did, "disabled it at boot") + } + } + if len(did) > 0 { + return "forgotten", "left as it is; the mesh " + strings.Join(did, " and ") + + " and does not start anything on the way out", nil + } 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 + + // Something may 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 { + 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 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) + var did, already []string + if stop { + if 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) + } + did = append(did, "stopped") + } else { + already = append(already, "stopped") } - 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) + if disable { + // Disabled unless it reads as disabled: a unit the service manager cannot say a boot state + // for — one with no install section — takes a disable as a no-op, and asking is how the + // host stays sure the mesh's enable did not outlive it. + if boot, err := sys.ServiceBoot(ctx, run, a.Target); err == nil && boot == "disabled" { + already = append(already, "disabled at boot") + } else { + if err := sys.SetServiceBoot(ctx, run, a.Target, "disabled"); err != nil { + return "", "", fmt.Errorf("disabling %s, which the mesh enabled: %w", a.Target, err) + } + did = append(did, "disabled at boot") } - gave = append(gave, "disabled at boot again") } - return "restored", strings.Join(gave, ", ") + ", as the host found it", nil + + if made { + if len(did) == 0 { + return "forgotten", "already stopped and disabled at boot; the mesh wrote its unit file", nil + } + return "removed", strings.Join(did, ", ") + "; the mesh wrote its unit file, so the unit is the mesh's own", nil + } + if len(did) == 0 { + return "forgotten", "already as the host found it (" + strings.Join(already, ", ") + ")", nil + } + detail := strings.Join(did, ", ") + ", as the host found 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 +// kept this has had the mesh acting on the unit since, and a reading now would be the mesh's doing. +// +// Read now as well, besides on a first apply, where the mesh has never acted on this unit's state +// although a record exists: the record is of a service declared with no state (novox/hq ADR 0117), +// which the mesh never starts or stops, or of another unit altogether. What was found about one +// unit says nothing about another, so a service moved to a new unit gives the old one back, just as +// if it had been undeclared, and is read afresh for the new one; gave says what that gave back. +func foundAs(ctx context.Context, sys system.System, r *declaration.Service, run Runner, + previous store.Applied) (found *store.FoundUnit, gave string, err error) { + if f := previous.Found; f != nil { + unit := f.Unit + if unit == "" { + unit = previous.Target + } + if unit == "" || unit == r.Unit { + return f, "", nil + } + // 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}, run, false) + if err != nil { + return nil, "", fmt.Errorf("giving %s back as the host found it, now that %s is declared instead: %w", + unit, r.Unit, err) + } + if action == "restored" { + gave = unit + " " + detail + } + } else if previous.ID != "" && !previous.Stateless && previous.Target == r.Unit { + return nil, "", nil + } + state, err := sys.ServiceState(ctx, run, r.Unit) + if err != nil { + return nil, gave, err + } + return &store.FoundUnit{Unit: r.Unit, State: state}, gave, nil +} + +// meshMadeUnits is every unit whose unit file this host wrote whole where there was none: a `file` +// record with no kept original and not written into, at a unit's own path under a directory the +// service manager loads administrators' units from — or the one the host writes a process's unit +// in. Such a unit is the mesh's, whatever was found (novox/hq ADR 0118); see removeService. +// +// A drop-in is not the unit's own file, and a file the host wrote over is the machine's unit with +// the mesh's text in it: its original is kept, and put back when the file's record goes. +func meshMadeUnits(known store.State) map[string]bool { + made := map[string]bool{} + dirs := map[string]bool{"/etc/systemd/system": true, "/run/systemd/system": true, + filepath.Clean(unitDir): true} + for _, r := range known.Resources { + if r.Type != string(declaration.TypeFile) || r.Kept != "" || r.Into != nil { + continue + } + path := filepath.Clean(r.Target) + if dirs[filepath.Dir(path)] { + made[filepath.Base(path)] = true + } + } + return made } diff --git a/internal/apply/found_test.go b/internal/apply/found_test.go new file mode 100644 index 0000000..01a5248 --- /dev/null +++ b/internal/apply/found_test.go @@ -0,0 +1,387 @@ +package apply + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// Defends novox/hq ADR 0118: undeclaring removes what the mesh made, gives back what it changed, +// and leaves what was the machine's — which needs what was found kept exactly, and a unit the mesh +// made known for the mesh's whatever its record says. + +func unitsMachine(units map[string]*fakeUnit) *machine { + return &machine{containers: map[string]*fakeContainer{}, units: units} +} + +// nothingButA is a declaration of one unrelated file, so everything recorded is undeclared. +func nothingButA(t *testing.T) string { + return `{"declaration":1,"resources":[ + {"id":"other","type":"file","path":"` + filepath.Join(t.TempDir(), "a") + `","content":"a\n"}]}` +} + +// copyOf is a state as a later load of it would be: sharing nothing with the one it came from. +func copyOf(t *testing.T, s store.State) store.State { + t.Helper() + raw, err := json.Marshal(s) + if err != nil { + t.Fatal(err) + } + var out store.State + if err := json.Unmarshal(raw, &out); err != nil { + t.Fatal(err) + } + return out +} + +func applyOn(t *testing.T, raw string, known store.State, m *machine) (Report, store.State, error) { + t.Helper() + return Apply(context.Background(), archHost(t), parse(t, raw), known, store.OriginDeclared, m.run, nil, nil) +} + +func TestAUnitWhoseFileTheMeshWroteIsStoppedWhateverItsRecordSays(t *testing.T) { + // The adoption guard's unit file is the mesh's own file resource, written where there was none. + // Its service recorded before the host kept what it found has no Found, and forgetting it left + // the guard's table loaded on a converged node and its unit file deleted from under it. + units := t.TempDir() + was := unitDir + unitDir = units + t.Cleanup(func() { unitDir = was }) + unitFile := filepath.Join(units, "mesh-guard.service") + if err := os.WriteFile(unitFile, []byte("[Unit]\n"), 0o644); err != nil { + t.Fatal(err) + } + m := unitsMachine(map[string]*fakeUnit{"mesh-guard.service": {active: "active", enabled: "enabled"}}) + fileThereAtStop := false + run := func(ctx context.Context, name string, args ...string) (string, error) { + if name == "systemctl" && args[0] == "stop" { + _, err := os.Stat(unitFile) + fileThereAtStop = err == nil + } + return m.run(ctx, name, args...) + } + // As a host before this change recorded them: the unit file first, then the service it + // starts, and no Found on the service. + known := store.State{Resources: []store.Applied{ + {ID: "adoption.guard-unit", Type: "file", Target: unitFile, Origin: store.OriginDeclared}, + {ID: "adoption.guard-running", Type: "service", Target: "mesh-guard.service", Origin: store.OriginDeclared}, + }} + report, after, err := applyWith(t, parse(t, nothingButA(t)), known, run) + if err != nil { + t.Fatal(err) + } + if u := m.units["mesh-guard.service"]; u.active != "inactive" || u.enabled != "disabled" { + t.Errorf("the mesh's own unit was left %s and %s: %v", u.active, u.enabled, m.asked) + } + if !fileThereAtStop { + t.Error("the unit was stopped after its file was deleted, or not at all") + } + if o := outcomeOf(report, "adoption.guard-running"); o.Action != "removed" || !strings.Contains(o.Detail, "unit file") { + t.Errorf("outcome %+v", o) + } + if _, err := os.Stat(unitFile); !os.IsNotExist(err) { + t.Error("the unit file outlived its record") + } + if len(after.Resources) != 1 { + t.Errorf("still recorded: %v", after.IDs()) + } + + // A unit file the host wrote OVER — its original kept — is the machine's unit, and a record + // with no Found leaves it as it is. + if err := os.WriteFile(unitFile, []byte("[Unit]\n"), 0o644); err != nil { + t.Fatal(err) + } + m = unitsMachine(map[string]*fakeUnit{"mesh-guard.service": {active: "active", enabled: "enabled"}}) + known.Resources[0].Kept = filepath.Join(t.TempDir(), "original") + if err := os.WriteFile(known.Resources[0].Kept, []byte("[Unit]\n"), 0o600); err != nil { + t.Fatal(err) + } + if _, _, err := applyWith(t, parse(t, nothingButA(t)), known, m.run); err != nil { + t.Fatal(err) + } + if m.did("systemctl stop") || m.did("systemctl disable") { + t.Errorf("a unit whose file the mesh only wrote over was stopped: %v", m.asked) + } +} + +func TestOnlyAUnitsOwnFileTheMeshCreatedMakesItTheMeshs(t *testing.T) { + known := store.State{Resources: []store.Applied{ + {ID: "a", Type: "file", Target: "/etc/systemd/system/made.service"}, + {ID: "b", Type: "file", Target: "/run/systemd/system/runtime.service"}, + {ID: "c", Type: "file", Target: "/etc/systemd/system/kept.service", Kept: "/var/lib/mesh-host/kept/x"}, + {ID: "d", Type: "file", Target: "/etc/systemd/system/into.service", Into: &store.Into{Format: "block"}}, + {ID: "e", Type: "file", Target: "/etc/systemd/system/docker.service.d/mesh.conf"}, + {ID: "f", Type: "file", Target: "/etc/mesh/elsewhere.service"}, + {ID: "g", Type: "directory", Target: "/etc/systemd/system/dir.service"}, + }} + made := meshMadeUnits(known) + for unit, want := range map[string]bool{"made.service": true, "runtime.service": true, "kept.service": false, + "into.service": false, "mesh.conf": false, "docker.service.d": false, "elsewhere.service": false, "dir.service": false} { + if made[unit] != want { + t.Errorf("%s: made %v, want %v", unit, made[unit], want) + } + } +} + +func TestWhatWasFoundOutlivesAFirstApplyThatFailed(t *testing.T) { + // Enabled, then it would not start: no record. The next apply must not read the enable as + // the machine's — or undeclaring leaves enabled a unit the mesh enabled. + m := unitsMachine(map[string]*fakeUnit{"filter.service": {active: "inactive", enabled: "disabled", wontStart: true}}) + declared := `{"declaration":1,"resources":[ + {"id":"s","type":"service","unit":"filter.service","state":"running","boot":"enabled"}]}` + _, first, err := applyOn(t, declared, store.State{}, m) + if err == nil || !m.did("systemctl enable filter.service") { + t.Fatalf("the fixture did not enable and then fail: %v, %v", err, m.asked) + } + if _, ok := first.Find("s"); ok { + t.Fatal("a failed apply was recorded") + } + if p := first.FoundFirst["s"]; p.State != "stopped" || p.Boot != "disabled" || p.Unit != "filter.service" { + t.Fatalf("what was found was not kept through the failure: %+v", first.FoundFirst) + } + + failed := copyOf(t, first) + + m.units["filter.service"].wontStart = false + _, second, err := applyOn(t, declared, first, m) + if err != nil { + t.Fatal(err) + } + if rec, _ := second.Find("s"); rec.Found == nil || rec.Found.State != "stopped" || rec.Found.Boot != "disabled" { + t.Errorf("the record carries the mesh's own effect as found: %+v", rec.Found) + } + if second.FoundFirst != nil { + t.Errorf("kept apart after the record carried it: %+v", second.FoundFirst) + } + + if _, _, err := applyOn(t, nothingButA(t), second, m); err != nil { + t.Fatal(err) + } + if u := m.units["filter.service"]; u.active != "inactive" || u.enabled != "disabled" { + t.Errorf("undeclared, the unit was left %s and %s", u.active, u.enabled) + } + + // Undeclared before it was ever recorded, what was found goes with it — only for its origin. + if _, kept, _ := Apply(context.Background(), archHost(t), parse(t, nothingButA(t)), copyOf(t, failed), + store.OriginCarried, m.run, nil, nil); kept.FoundFirst["s"].State == "" { + t.Error("a carried apply dropped what the mesh's declaration found") + } + if _, dropped, err := applyOn(t, nothingButA(t), copyOf(t, failed), m); err != nil || dropped.FoundFirst != nil { + t.Errorf("kept for a service no longer declared: %+v, %v", dropped.FoundFirst, err) + } +} + +func TestBootIsFoundTheFirstTimeTheMeshSetsIt(t *testing.T) { + // The declaration said nothing about boot at first, so the mesh never touched it: what is + // there when a declaration first does is still the machine's. + m := unitsMachine(map[string]*fakeUnit{"web.service": {active: "active", enabled: "disabled"}}) + known := store.State{Resources: []store.Applied{{ID: "s", Type: "service", Target: "web.service", + Origin: store.OriginDeclared, Found: &store.FoundUnit{Unit: "web.service", State: "running"}}}} + _, after, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"s","type":"service","unit":"web.service","state":"running","boot":"enabled"}]}`, known, m) + if err != nil { + t.Fatal(err) + } + rec, _ := after.Find("s") + if rec.Found == nil || rec.Found.State != "running" || rec.Found.Boot != "disabled" { + t.Fatalf("found %+v, want running and disabled", rec.Found) + } + report, _, err := applyOn(t, nothingButA(t), after, m) + if err != nil { + t.Fatal(err) + } + if u := m.units["web.service"]; u.active != "active" || u.enabled != "disabled" { + t.Errorf("undeclared, the unit is %s and %s; want running, and disabled again", u.active, u.enabled) + } + if o := outcomeOf(report, "s"); o.Action != "restored" || o.Detail != "disabled at boot, as the host found it" { + t.Errorf("outcome %+v", o) + } +} + +func TestAServiceOnceDeclaredWithNoStateIsFoundWhenFirstGivenOne(t *testing.T) { + // Declared with no state (novox/hq ADR 0117), the mesh never started or stopped it — so what + // is there when a declaration first gives it one is what the machine had. + m := unitsMachine(map[string]*fakeUnit{"net.service": {active: "inactive", enabled: "disabled"}}) + known := store.State{Resources: []store.Applied{{ID: "s", Type: "service", Target: "net.service", + Origin: store.OriginDeclared, Stateless: true}}} + _, after, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"s","type":"service","unit":"net.service","state":"running"}]}`, known, m) + if err != nil { + t.Fatal(err) + } + if rec, _ := after.Find("s"); rec.Found == nil || rec.Found.State != "stopped" { + t.Fatalf("found %+v, want stopped", rec.Found) + } + if _, _, err := applyOn(t, nothingButA(t), after, m); err != nil { + t.Fatal(err) + } + if m.units["net.service"].active != "inactive" { + t.Error("the unit the mesh started outlived its declaration") + } +} + +func TestAUnitWrittenInTheSameApplyIsLoadedBeforeItIsRead(t *testing.T) { + // Read before the service manager is told about its new file, the unit is not there to find. + dir := t.TempDir() + m := unitsMachine(map[string]*fakeUnit{"fresh.service": {active: "inactive", enabled: "disabled"}}) + _, _, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"unit","type":"file","path":"`+filepath.Join(dir, "fresh.service")+`","content":"[Unit]\n"}, + {"id":"s","type":"service","unit":"fresh.service","state":"running","restart-on":["unit"]}]}`, + store.State{}, m) + if err != nil { + t.Fatal(err) + } + reload, show := -1, -1 + for i, a := range m.asked { + if a == "systemctl daemon-reload" && reload < 0 { + reload = i + } + if strings.HasPrefix(a, "systemctl show fresh.service") && show < 0 { + show = i + } + } + if reload < 0 || show < 0 || reload > show { + t.Errorf("the unit was read before its file was loaded: %v", m.asked) + } +} + +func TestAServiceMovedToAnotherUnitGivesTheOldOneBackAndFindsTheNewOne(t *testing.T) { + m := unitsMachine(map[string]*fakeUnit{ + "old.service": {active: "active", enabled: "enabled"}, + "new.service": {active: "inactive", enabled: "disabled"}, + }) + known := store.State{Resources: []store.Applied{{ID: "s", Type: "service", Target: "old.service", + Origin: store.OriginDeclared, Found: &store.FoundUnit{Unit: "old.service", State: "stopped"}}}} + report, after, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"s","type":"service","unit":"new.service","state":"running"}]}`, known, m) + if err != nil { + t.Fatal(err) + } + if m.units["old.service"].active != "inactive" { + t.Error("the unit the mesh started is still running though nothing declares it") + } + if m.units["new.service"].active != "active" { + t.Error("the unit now declared was not started") + } + rec, _ := after.Find("s") + if rec.Found == nil || rec.Found.Unit != "new.service" || rec.Found.State != "stopped" { + t.Errorf("what was found about the old unit was carried to the new one: %+v", rec.Found) + } + if o := outcomeOf(report, "s"); !strings.Contains(o.Detail, "old.service stopped, as the host found it") { + t.Errorf("giving the old unit back went unsaid: %+v", o) + } +} + +func TestARemovalSaysWhatItDid(t *testing.T) { + cases := []struct { + name string + found *store.FoundUnit + unit *fakeUnit + action, detail string + }{ + {"found stopped and still stopped", &store.FoundUnit{State: "stopped"}, + &fakeUnit{active: "inactive", enabled: "disabled"}, "forgotten", "already as the host found it (stopped)"}, + {"started by the mesh", &store.FoundUnit{State: "stopped"}, + &fakeUnit{active: "active", enabled: "disabled"}, "restored", "stopped, as the host found it"}, + {"started and enabled by the mesh", &store.FoundUnit{State: "stopped", Boot: "disabled"}, + &fakeUnit{active: "active", enabled: "enabled"}, "restored", "stopped, disabled at boot, as the host found it"}, + {"found running, stopped by the mesh", &store.FoundUnit{State: "running"}, + &fakeUnit{active: "inactive", enabled: "disabled"}, "forgotten", + "left as it is; the mesh stopped it and does not start anything on the way out"}, + {"found running and enabled, disabled by the mesh", &store.FoundUnit{State: "running", Boot: "enabled"}, + &fakeUnit{active: "active", enabled: "disabled"}, "forgotten", + "left as it is; the mesh disabled it at boot and does not start anything on the way out"}, + {"found running, still running", &store.FoundUnit{State: "running"}, + &fakeUnit{active: "active", enabled: "enabled"}, "forgotten", "it was running before the mesh; left as it is"}, + // Found says to stop it, so the machine is asked about it — and it is gone. + {"started by the mesh, since uninstalled", &store.FoundUnit{State: "stopped"}, + nil, "forgotten", "the unit no longer exists"}, + {"recorded before the host kept what it found", nil, + &fakeUnit{active: "active", enabled: "enabled"}, "forgotten", "recorded before the host kept what it found; left as it is"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + units := map[string]*fakeUnit{} + if c.unit != nil { + units["unit.service"] = c.unit + } + m := unitsMachine(units) + known := store.State{Resources: []store.Applied{{ID: "s", Type: "service", Target: "unit.service", + Origin: store.OriginDeclared, Found: c.found}}} + report, after, err := applyOn(t, nothingButA(t), known, m) + if err != nil { + t.Fatal(err) + } + if o := outcomeOf(report, "s"); o.Action != c.action || o.Detail != c.detail { + t.Errorf("said %s: %s; want %s: %s", o.Action, o.Detail, c.action, c.detail) + } + if c.unit == nil && !m.did("systemctl show unit.service") { + t.Errorf("the machine was never asked whether the unit is there: %v", m.asked) + } + if m.did("systemctl start") || m.did("systemctl enable") { + t.Errorf("something was started on the way out: %v", m.asked) + } + if _, still := after.Find("s"); still { + t.Error("still recorded") + } + }) + } +} + +func TestAUnitTheMeshStartedIsStoppedWhenUndeclaredAndOneFoundRunningIsNot(t *testing.T) { + // End to end: found by the first apply, carried, given back. + m := unitsMachine(map[string]*fakeUnit{ + "filter.service": {active: "inactive", enabled: "disabled"}, + "runtime.service": {active: "active", enabled: "enabled"}, + }) + _, state, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"filter","type":"service","unit":"filter.service","state":"running","boot":"enabled"}, + {"id":"runtime","type":"service","unit":"runtime.service","state":"running","boot":"enabled"}]}`, + store.State{}, m) + if err != nil { + t.Fatal(err) + } + if m.units["filter.service"].active != "active" { + t.Fatal("the fixture did not start the filter") + } + if _, _, err := applyOn(t, nothingButA(t), state, m); err != nil { + t.Fatal(err) + } + if u := m.units["filter.service"]; u.active != "inactive" || u.enabled != "disabled" { + t.Errorf("the filter the mesh started and enabled is %s and %s", u.active, u.enabled) + } + if u := m.units["runtime.service"]; u.active != "active" || u.enabled != "enabled" { + t.Errorf("the runtime that was running before the mesh is %s and %s", u.active, u.enabled) + } +} + +func TestAUnitHeldAndThenTakenIsFoundAsThePredecessorLeftIt(t *testing.T) { + // Held while its module was untaken, nothing was applied and nothing found; taken, the first + // apply finds the predecessor's unit — stopped, but started at boot — before starting it. + dir := t.TempDir() + m := unitsMachine(map[string]*fakeUnit{"hello.service": {active: "inactive", enabled: "enabled"}}) + service := `{"id":"hello-web.unit","type":"service","unit":"hello.service","state":"running","boot":"enabled"}` + _, held := applyAdopted(t, adopted(t, untaken("hello-web.unit"), service), store.State{}, m, dir) + if _, ok := held.HeldAt("hello-web.unit"); !ok { + t.Fatal("the fixture's unit was not held") + } + _, taken := applyAdopted(t, adopted(t, `{"taken":["hello-web"]}`, service), held, m, dir) + rec, _ := taken.Find("hello-web.unit") + if rec.Found == nil || rec.Found.State != "stopped" || rec.Found.Boot != "enabled" { + t.Fatalf("found %+v, want the predecessor's stopped and enabled", rec.Found) + } + if m.units["hello.service"].active != "active" { + t.Fatal("the taken unit was not started") + } + if _, _, err := applyOn(t, nothingButA(t), taken, m); err != nil { + t.Fatal(err) + } + if u := m.units["hello.service"]; u.active != "inactive" || u.enabled != "enabled" { + t.Errorf("given back as %s and %s; the predecessor left it stopped and enabled", u.active, u.enabled) + } +} diff --git a/internal/apply/opening_test.go b/internal/apply/opening_test.go index 1be33f5..15a3a90 100644 --- a/internal/apply/opening_test.go +++ b/internal/apply/opening_test.go @@ -339,8 +339,18 @@ func TestReturningToAdoptedLoadsTheGuardBeforeRemovingTheFilter(t *testing.T) { dir := t.TempDir() guard := filepath.Join(dir, "guard.nft") guardFile := `{"id":"adoption.guard","type":"file","path":"` + guard + `","content":"table inet mesh_guard {}\n"}` + // The filter's unit file is the mesh's own, written where there was none — which is what makes + // its unit the mesh's to stop, with or without a record of what was found (novox/hq ADR 0118). + units := t.TempDir() + was := unitDir + unitDir = units + t.Cleanup(func() { unitDir = was }) + filterUnit := filepath.Join(units, "mesh-filter.service") for _, stopFails := range []bool{false, true} { _ = os.Remove(guard) + if err := os.WriteFile(filterUnit, []byte("[Unit]\n"), 0o644); err != nil { + t.Fatal(err) + } guardUpAtStop := false run := func(_ context.Context, name string, args ...string) (string, error) { if name != "systemctl" { @@ -358,10 +368,10 @@ func TestReturningToAdoptedLoadsTheGuardBeforeRemovingTheFilter(t *testing.T) { } return "", nil } + // As a host recorded them before it kept what it found: no Found on the service. converged := store.State{Resources: []store.Applied{ - {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"}}}} + {ID: "nftables.unit", Type: "file", Target: filterUnit, Origin: store.OriginDeclared}, + {ID: "nftables.load", Type: "service", Target: "mesh-filter.service", Origin: store.OriginDeclared}}} _, 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 a142d6e..1937617 100644 --- a/internal/apply/plan.go +++ b/internal/apply/plan.go @@ -19,7 +19,7 @@ import ( // Step is one thing an apply would do to this machine. type Step struct { - // Verb is create · update · check · hold · run · remove · forget · disable · enable. + // Verb is create · update · check · hold · run · remove · forget · restore · disable · enable. Verb string `json:"verb"` Type string `json:"type,omitempty"` ID string `json:"id,omitempty"` @@ -77,6 +77,7 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { } var protecting, orphans []Step + made := meshMadeUnits(known) 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"} @@ -84,12 +85,32 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { 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" + // What removal will do, said before it does it (novox/hq ADR 0118), in removeService's + // words. "restore" only where it may stop or disable something — the record cannot say + // whether the unit is still as the mesh left it, so "may" is as far as a preview goes — + // and a unit whose file the mesh wrote is named as the mesh's, since that one is + // stopped whatever was found. + f := orphan.Found + switch { + case made[orphan.Target]: + step.Verb, step.Why = "remove", "no longer declared; the mesh wrote its unit file, so it is "+ + "stopped and disabled at boot before that file goes" + 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 f.State == "stopped" || f.Boot == "disabled": + var back []string + if f.State == "stopped" { + back = append(back, "stopped") + } + if f.Boot == "disabled" { + back = append(back, "disabled at boot") + } + step.Verb, step.Why = "restore", "no longer declared; the host found it "+ + strings.Join(back, " and ")+", and it goes back to that if the mesh changed it" + default: + step.Verb, step.Why = "forget", "no longer declared; it was running before the mesh and is "+ + "left as it is — nothing is started or stopped on the way out" } } if d.Adoption == nil && strings.HasPrefix(orphan.ID, declaration.AdoptionPrefix) { diff --git a/internal/apply/plan_test.go b/internal/apply/plan_test.go index 4f8f7ce..dcbeecc 100644 --- a/internal/apply/plan_test.go +++ b/internal/apply/plan_test.go @@ -261,3 +261,42 @@ func TestAPlanSaysAContainerIsRecreatedWhenAFileItReadsChanged(t *testing.T) { t.Errorf("a container with no record of what it read is planned as %q", got) } } + +func TestAPlanSaysWhichUnitsAnUndeclareGivesBackAndWhichItLeaves(t *testing.T) { + // novox/hq ADR 0118, said before it is done: "restore" only where removal may stop or disable + // something, and a unit whose file the mesh wrote named as the mesh's. + known := store.State{} + known.Record(store.Applied{ID: "guard-unit", Type: "file", Target: "/etc/systemd/system/mesh-guard.service"}) + known.Record(store.Applied{ID: "guard", Type: "service", Target: "mesh-guard.service"}) + known.Record(store.Applied{ID: "filter", Type: "service", Target: "filter.service", + Found: &store.FoundUnit{State: "stopped"}}) + known.Record(store.Applied{ID: "boot", Type: "service", Target: "boot.service", + Found: &store.FoundUnit{State: "running", Boot: "disabled"}}) + known.Record(store.Applied{ID: "runtime", Type: "service", Target: "docker.service", + Found: &store.FoundUnit{State: "running", Boot: "enabled"}}) + known.Record(store.Applied{ID: "old", Type: "service", Target: "sshd.service"}) + known.Record(store.Applied{ID: "nm", Type: "service", Target: "NetworkManager.service", Stateless: true}) + + steps := Plan(parse(t, nothingButA(t)), known, store.OriginCarried) + got := verbs(steps) + want := "forget nm, forget old, forget runtime, restore boot, restore filter, remove guard, remove guard-unit, create other" + if got != want { + t.Fatalf("planned %s\nwant %s", got, want) + } + for _, s := range steps { + switch s.ID { + case "guard": + if !strings.Contains(s.Why, "unit file") { + t.Errorf("the mesh's own unit was not named as the mesh's: %q", s.Why) + } + case "filter": + if !strings.Contains(s.Why, "found it stopped") { + t.Errorf("what the unit goes back to went unsaid: %q", s.Why) + } + case "old": + if !strings.Contains(s.Why, "left as it is") { + t.Errorf("a unit with nothing found was not said to be left: %q", s.Why) + } + } + } +} diff --git a/internal/store/store.go b/internal/store/store.go index 0878b62..71485a0 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -154,6 +154,18 @@ type State struct { // other mode can be refused before it is applied (novox/hq issue 104). Mode string `json:"mode,omitempty"` + // FoundFirst is, by resource id, what a service's unit was found as by an apply of it that did + // not finish — kept apart from Resources, because a record follows the fact and this apply's + // fact never came (novox/hq ADR 0118). + // + // **The capture is the one reading that cannot be taken again.** A first apply that enabled a + // unit and then failed to start it leaves no record; without this, the next apply would find + // the unit enabled, take that for what the machine had, and undeclaring would leave enabled a + // unit the mesh enabled. So what is found is written the moment it is read, whatever the apply + // of the resource then does, and a later apply reads it here before it reads the machine. + // Dropped once a record carrying it is written, and when its resource is no longer declared. + FoundFirst map[string]PendingFound `json:"found_first,omitempty"` + // Genesis is the bundle this host consumed raising the foundation, if it has. Once recorded, // the bundle carried in the binary is not applied again: what genesis applied was rewritten // for this machine, and the mesh has said more since (novox/hq issue 104). @@ -457,9 +469,47 @@ func originOf(r Applied) string { // FoundUnit is a service's unit as the host first found it. type FoundUnit struct { + // Unit is which unit this was read from. What was found about one unit says nothing about + // another, so a service whose declaration moves to a different unit is read again for that one + // (novox/hq ADR 0118). Empty on what was kept before this was: the record's Target then says. + Unit string `json:"unit,omitempty"` // 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"` } + +// PendingFound is a unit as found by an apply of its service that has not yet been recorded, and +// who asked for that apply — so only a declaration from the same origin can say it is gone. +type PendingFound struct { + FoundUnit + Origin string `json:"origin,omitempty"` +} + +// KeepFound writes down what an unfinished apply found a service's unit as. +func (s *State) KeepFound(id, origin string, f FoundUnit) { + if s.FoundFirst == nil { + s.FoundFirst = map[string]PendingFound{} + } + s.FoundFirst[id] = PendingFound{FoundUnit: f, Origin: origin} +} + +// DropFound forgets what was found for one resource: its record now carries it, or it is gone. +func (s *State) DropFound(id string) { + delete(s.FoundFirst, id) + if len(s.FoundFirst) == 0 { + s.FoundFirst = nil + } +} + +// DropFoundUndeclared forgets what was found for every resource of this origin the declaration no +// longer names. Only this origin's, for the reason Orphans gives: a mesh declaration's silence says +// nothing about what the bundle applies, nor the other way round. +func (s *State) DropFoundUndeclared(declared map[string]bool, origin string) { + for id, p := range s.FoundFirst { + if !declared[id] && originOf(Applied{Origin: p.Origin}) == origin { + s.DropFound(id) + } + } +}