Merge pull request 'Leave a unit alone when another declared service still holds it (hq issue 190)' (#26) from test/190-into-json-member-handover into main
This commit was merged in pull request #26.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user