From 7f0eb9c51d20bd06160f5a29b6a654605840bd03 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 28 Oct 2026 15:43:35 +0100 Subject: [PATCH 1/2] apply: reload the service manager once when a file in a unit directory moved (issue 1126) A drop-in that no service reflects, like the power module's bound on reboot.target, was written but never loaded: the node-engine reloaded units only for a file named in a service's restart-on, so the next reboot still ran the target as loaded. Any file written or removed under a system unit directory, or a drop-in directory below one, now gets one daemon-reload per apply. --- internal/apply/apply.go | 38 +++++++++++++++++++++++++++++++++++ internal/apply/apply_test.go | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 0864f60..b8f21f5 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -1223,6 +1223,19 @@ func applyScoped( } } + // A file written or removed in a unit directory is read by the service manager only after a reload, whether or not + // a service reflects it: a drop-in on reboot.target is reflected by no service, and without the reload the next + // reboot still runs the target as it was loaded (issue 1126). Once per apply. + if at := unitFileMoved(report.Outcomes); at != "" { + if u, ok := sys.(unitReloader); ok { + if err := u.ReloadUnits(ctx, run); err != nil { + failures = append(failures, &Error{Resource: at, Err: fmt.Errorf("reloading the service manager's units: %w", err)}) + } else { + log(fmt.Sprintf(" reloaded the service manager's units: %s changed", at)) + } + } + } + // Only the mesh says what is assigned: a carried bundle's silence undeclares no identity and no folder. if origin == store.OriginDeclared && scope.isNode() { // And an identity, then a library group, no longer declared goes, once what ran as it went above (ADR 0328 @@ -1997,6 +2010,31 @@ type serviceReloader interface { ReloadService(ctx context.Context, run system.Runner, unit string) error } +// UnitDirs are the system service manager's unit directories: a file the apply writes or removes in one of them, or in a +// drop-in directory below one, is read by the manager only after a reload (issue 1126). Replaceable in tests. +var UnitDirs = []string{"/etc/systemd/system", "/run/systemd/system", "/usr/local/lib/systemd/system"} + +// unitFileMoved is the first file this apply wrote or removed in a unit directory or a drop-in directory below one, +// or "" when it moved none. +func unitFileMoved(outcomes []Outcome) string { + for _, o := range outcomes { + if declaration.Type(o.Type) != declaration.TypeFile || !moved(o) { + continue + } + switch o.Action { + case "kept", "forgotten", "waiting": + continue + } + at := filepath.Clean(o.Target) + for _, dir := range UnitDirs { + if rel, err := filepath.Rel(dir, at); err == nil && rel != "." && !strings.HasPrefix(rel, "..") { + return at + } + } + } + return "" +} + // unitReloader is a service manager that caches unit files and must be told to read them again. type unitReloader interface { ReloadUnits(ctx context.Context, run system.Runner) error diff --git a/internal/apply/apply_test.go b/internal/apply/apply_test.go index a099fd6..602b3a8 100644 --- a/internal/apply/apply_test.go +++ b/internal/apply/apply_test.go @@ -1261,6 +1261,45 @@ func TestAServiceIsRestartedWhenWhatItReflectsChanges(t *testing.T) { if reloaded < 0 || reloaded > stop { t.Errorf("the service was restarted without the unit files being read again first; commands were %v", commands) } + + // A drop-in in a unit directory that no service reflects — the power module's bound on reboot.target + // (issue 1126) — is read by the service manager only after a reload: one per apply, and no restart. + // Unchanged, it reloads nothing. + units := filepath.Join(dir, "system") + saved := UnitDirs + UnitDirs = []string{units} + t.Cleanup(func() { UnitDirs = saved }) + dropIn := filepath.Join(units, "reboot.target.d", "mesh-power.conf") + withDropIn := parse(t, fmt.Sprintf(`{"declaration":1,"resources":[ + {"id":"conf","type":"file","path":%q,"content":"second\n","mode":"0644"}, + {"id":"svc","type":"service","unit":"thing.service","state":"running","restart-on":["conf"]}, + {"id":"units","type":"directory","path":%q,"mode":"0755"}, + {"id":"drop-ins","type":"directory","path":%q,"mode":"0755"}, + {"id":"bound","type":"file","path":%q,"content":"[Unit]\nJobTimeoutSec=10min\n","mode":"0644"}, + {"id":"bound-too","type":"file","path":%q,"content":"[Unit]\nJobTimeoutSec=10min\n","mode":"0644"} + ]}`, path, units, filepath.Dir(dropIn), dropIn, filepath.Join(filepath.Dir(dropIn), "other.conf"))) + for _, row := range []struct { + name string + reloads int + }{{"a drop-in written", 1}, {"the same drop-in again", 0}} { + commands = nil + if _, state, err = Apply(context.Background(), archHost(t), withDropIn, state, + store.OriginCarried, run, nil, nil); err != nil { + t.Fatal(err) + } + reloads := 0 + for _, c := range commands { + if strings.Contains(c, "daemon-reload") { + reloads++ + } + if strings.Contains(c, "stop thing.service") { + t.Errorf("%s: a drop-in no service reflects restarted one: %v", row.name, commands) + } + } + if reloads != row.reloads { + t.Errorf("%s: %d reloads, want %d; commands were %v", row.name, reloads, row.reloads, commands) + } + } } } From bc28606d456670498a1cc32dee936af04d18a55e Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 28 Oct 2026 15:49:34 +0100 Subject: [PATCH 2/2] apply: reload units only for a unit file actually written or removed (issue 1126) A skipped or held file comes back on every apply; counting it would reload the service manager on every apply of a node with a gated or held module. The same allowlist as MovesRoot: created, updated, corrected, removed. --- internal/apply/apply.go | 7 +++++-- internal/apply/apply_test.go | 7 +++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index b8f21f5..1c1023c 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -2018,11 +2018,14 @@ var UnitDirs = []string{"/etc/systemd/system", "/run/systemd/system", "/usr/loca // or "" when it moved none. func unitFileMoved(outcomes []Outcome) string { for _, o := range outcomes { - if declaration.Type(o.Type) != declaration.TypeFile || !moved(o) { + // Only what changed the file on disk, as MovesRoot reads it: skipped, held, kept, forgotten and waiting come + // back on every apply and change nothing. + if declaration.Type(o.Type) != declaration.TypeFile { continue } switch o.Action { - case "kept", "forgotten", "waiting": + case "created", "updated", "corrected", "removed": + default: continue } at := filepath.Clean(o.Target) diff --git a/internal/apply/apply_test.go b/internal/apply/apply_test.go index 602b3a8..b5b6229 100644 --- a/internal/apply/apply_test.go +++ b/internal/apply/apply_test.go @@ -1300,6 +1300,13 @@ func TestAServiceIsRestartedWhenWhatItReflectsChanges(t *testing.T) { t.Errorf("%s: %d reloads, want %d; commands were %v", row.name, reloads, row.reloads, commands) } } + // A drop-in whose module is skipped behind its step, or held as found, comes back so on every apply: it moved + // nothing, and reloads nothing. + for _, action := range []string{"skipped", "held"} { + if at := unitFileMoved([]Outcome{{ID: "bound", Type: "file", Target: dropIn, Action: action}}); at != "" { + t.Errorf("a drop-in %s reloads the service manager (%s)", action, at) + } + } } }