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.
This commit is contained in:
2026-10-28 15:43:35 +01:00
parent 690aa662a8
commit 7f0eb9c51d
2 changed files with 77 additions and 0 deletions
+38
View File
@@ -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
+39
View File
@@ -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)
}
}
}
}