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) + } + } } }