diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 0521d3c..292e4eb 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -241,9 +241,30 @@ func ApplyMindingWindows( // this apply — a removal included — could change it or its records (novox/hq ADR 0103). before := lookBefore(ctx, sys, d, known, run) + // **A unit still declared with a state is not given back when another record of it goes** + // (novox/hq issue 190, ADR 0222). Two resources may name one unit — the private network declared + // the container runtime's service to reload it, beside the runtime's own module — and what the + // one going found is not the machine's to restore while the other still holds the unit: giving + // it back would stop or disable a unit the declaration says is running, every container on it + // with it, only for the declared one to start it again in the same apply. The record is + // forgotten; the unit's found state is the remaining record's to give back. + heldUnits := unitsHeld(d.Resources) + removeOrphan := func(orphan store.Applied) error { var action, detail string var err error + if declaration.Type(orphan.Type) == declaration.TypeService { + if by, held := heldUnits[unitKey(orphan.Scope, orphan.User, orphan.Target)]; held { + known.Forget(orphan.ID) + detail := "no longer declared; " + by + " still holds the unit, so it was left as it is" + report.Outcomes = append(report.Outcomes, Outcome{ + ID: orphan.ID, Type: orphan.Type, Target: orphan.Target, + Action: "forgotten", Detail: detail, + }) + log(fmt.Sprintf(" forgotten %s (%s): %s", orphan.ID, orphan.Target, detail)) + return nil + } + } if declaration.Type(orphan.Type) == declaration.TypeOpening { action, detail, err = removeOpening(ctx, orphan, run, known.Firewall) } else { @@ -2831,6 +2852,17 @@ func userUnitFiles(account, unit string) []string { return paths } +// unitsHeld is every unit a declared service gives a state, by unitKey, naming the service. +func unitsHeld(resources []declaration.Resource) map[string]string { + held := map[string]string{} + for _, r := range resources { + if s, ok := r.(*declaration.Service); ok && s.State != "" { + held[unitKey(s.Scope, s.User, s.Unit)] = s.ID + } + } + return held +} + // unitKey names a unit by the manager it is in and its name (novox/hq ADR 0177): the machine's // manager, or one account's. A system unit is the same key whether its record says "system" or // nothing, as every record before the scope existed does. diff --git a/internal/apply/into_handover_test.go b/internal/apply/into_handover_test.go new file mode 100644 index 0000000..640be24 --- /dev/null +++ b/internal/apply/into_handover_test.go @@ -0,0 +1,129 @@ +package apply + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" +) + +// novox/hq issue 190, ADR 0222: the private network stops writing the mesh's registry into the +// container runtime's file, and the runtime's own module writes the same member. Both are moved in +// one apply, and the machine never loses the member: the network's record goes first (its member +// leaves the list), the runtime's module is applied after (the member is added back, now recorded +// as the runtime's), and the daemon is reloaded once, never stopped, disabled or restarted — even +// though the record going says the unit was found stopped and disabled. + +const theRegistry = "anchor.internal:5100" + +func runtimeDecl(t *testing.T, path string, network bool) *declaration.Declaration { + t.Helper() + var resources []string + if network { + resources = append(resources, + fmt.Sprintf(`{"id":"networking.registry-trust","type":"file","path":%q,"into":"json","content":%q}`, + path, `{"insecure-registries":["`+theRegistry+`"]}`), + `{"id":"networking.registry-trust-reload","type":"service","unit":"docker.service","state":"running", + "reload-on":["networking.registry-trust"]}`) + } + resources = append(resources, + fmt.Sprintf(`{"id":"docker.daemon","type":"file","path":%q,"into":"json","content":%q}`, + path, `{"live-restore":true,"insecure-registries":["`+theRegistry+`"]}`), + `{"id":"docker.runtime","type":"service","unit":"docker.service","state":"running","boot":"enabled", + "reload-on":["docker.daemon"]}`) + return parse(t, `{"declaration":1,"resources":[`+strings.Join(resources, ",")+`]}`) +} + +func TestTheRegistryMovesToTheRuntimesModuleWithoutLeavingTheList(t *testing.T) { + path := filepath.Join(t.TempDir(), "daemon.json") + if err := os.WriteFile(path, []byte(`{"insecure-registries":["192.0.2.7:5000"]}`), 0o644); err != nil { + t.Fatal(err) + } + var commands []string + run := recordingServices(&commands) + + // Both declare it: the network's record added the member, so the runtime's finds it there. + _, state, err := Apply(context.Background(), archHost(t), runtimeDecl(t, path, true), store.State{}, + store.OriginDeclared, run, nil, nil) + if err != nil { + t.Fatal(err) + } + // Say the network's record found the runtime stopped and disabled: giving that back would stop + // every container on the machine. + for i := range state.Resources { + if state.Resources[i].ID == "networking.registry-trust-reload" { + state.Resources[i].Found = &store.FoundUnit{Unit: "docker.service", State: "stopped", Boot: "disabled"} + } + } + + commands = nil + report, state, err := Apply(context.Background(), archHost(t), runtimeDecl(t, path, false), state, + store.OriginDeclared, run, nil, nil) + if err != nil { + t.Fatal(err) + } + if got := fmt.Sprint(readObject(t, path)["insecure-registries"]); got != "[192.0.2.7:5000 "+theRegistry+"]" { + t.Fatalf("the list after the move: %s", got) + } + rec, _ := state.Find("docker.daemon") + if added := rec.Into.Added["insecure-registries"]; len(added) != 1 || canonical(added[0]) != `"`+theRegistry+`"` { + t.Errorf("the member is not recorded as the runtime's module's: %s", added) + } + reloads := 0 + for _, c := range commands { + if strings.Contains(c, "docker.service") && (strings.Contains(c, " stop ") || strings.Contains(c, " restart ") || + strings.Contains(c, " disable ")) { + t.Errorf("the runtime was given back as the network's record found it: %q", c) + } + if strings.Contains(c, "reload docker.service") { + reloads++ + } + } + if reloads != 1 { + t.Errorf("the runtime was reloaded %d times; commands were %v", reloads, commands) + } + for _, o := range report.Outcomes { + if o.ID == "networking.registry-trust-reload" && o.Action != "forgotten" { + t.Errorf("the network's record of the unit was %q: %s", o.Action, o.Detail) + } + } + + // Steady from here on, and undeclaring the runtime's module takes only what it added. + report, state, err = Apply(context.Background(), archHost(t), runtimeDecl(t, path, false), state, + store.OriginDeclared, run, nil, nil) + if err != nil { + t.Fatal(err) + } + for _, o := range report.Outcomes { + if o.ID == "docker.daemon" && o.Action != "unchanged" { + t.Errorf("a second apply was %q", o.Action) + } + } + if _, _, err := Apply(context.Background(), archHost(t), somethingElse(t), state, store.OriginDeclared, run, nil, nil); err != nil { + t.Fatal(err) + } + if got := fmt.Sprint(readObject(t, path)["insecure-registries"]); got != "[192.0.2.7:5000]" { + t.Errorf("undeclaring the runtime's module left %s", got) + } +} + +// And the preview says the same before it happens. +func TestThePlanForgetsARecordOfAUnitStillHeld(t *testing.T) { + path := filepath.Join(t.TempDir(), "daemon.json") + var commands []string + _, state, err := Apply(context.Background(), archHost(t), runtimeDecl(t, path, true), store.State{}, + store.OriginDeclared, recordingServices(&commands), nil, nil) + if err != nil { + t.Fatal(err) + } + for _, step := range Plan(runtimeDecl(t, path, false), state, store.OriginDeclared) { + if step.ID == "networking.registry-trust-reload" && step.Verb != "forget" { + t.Errorf("the plan would %s the network's record of a unit docker.runtime holds: %s", step.Verb, step.Why) + } + } +} diff --git a/internal/apply/plan.go b/internal/apply/plan.go index 2f1db0b..3a225be 100644 --- a/internal/apply/plan.go +++ b/internal/apply/plan.go @@ -84,10 +84,15 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { var protecting, orphans []Step made := meshMadeUnits(known) + heldUnits := unitsHeld(d.Resources) 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"} + held := heldUnits[unitKey(orphan.Scope, orphan.User, orphan.Target)] switch { + case orphan.Type == string(declaration.TypeService) && held != "": + // In removeOrphan's words (novox/hq issue 190). + step.Verb, step.Why = "forget", "no longer declared; "+held+" still holds the unit, so it is left as it is" 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.TypeArchive) && store.IsFormer(orphan.ID):