From dae620e5afe0a4a46130231c2437f650940307f0 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 21 Jun 2024 12:40:13 -0400 Subject: [PATCH] daemon/update: disable systemd unit before overwriting When overwriting a systemd unit with new content, we need to account for the case where the new unit content has a different `[Install]` section. If it does, then simply overwriting will leak the previous enablement symlinks and become node state. That's OK most of the time, but this can cause real issues as we've seen with the combination of #3967 which does exactly that (changing `[Install]` sections) and #4213 which assumed that those symlinks were cleaned up. More details on that cocktail in: https://issues.redhat.com/browse/OCPBUGS-33694?focusedId=24917003#comment-24917003 Fix this by always checking if the unit is currently enabled, and if so, running `systemctl disable` *before* overwriting its contents. The unit will then be re-enabled (or not) based on the MachineConfig. --- pkg/daemon/file_writers.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/daemon/file_writers.go b/pkg/daemon/file_writers.go index d7954cd439..c705b16184 100644 --- a/pkg/daemon/file_writers.go +++ b/pkg/daemon/file_writers.go @@ -297,6 +297,18 @@ func writeUnit(u ign3types.Unit, systemdRoot string, isCoreOSVariant bool) error return err } } + // If the unit is currently enabled, disable it before overwriting since we might be + // changing its WantedBy= or RequiredBy= directive (see OCPBUGS-33694). Later code will + // re-enable the new unit as directed by the MachineConfig. + cmd := exec.Command("systemctl", "is-enabled", u.Name) + out, _ := cmd.CombinedOutput() + if cmd.ProcessState.ExitCode() == 0 && strings.TrimSpace(string(out)) == "enabled" { + klog.V(2).Infof("Disabling systemd unit %s before re-writing it", u.Name) + disable_out, err := exec.Command("systemctl", "disable", u.Name).CombinedOutput() + if err != nil { + return fmt.Errorf("disabling %s failed: %w (output: %s)", u.Name, err, string(disable_out)) + } + } if err := writeFileAtomicallyWithDefaults(fpath, []byte(*u.Contents)); err != nil { return fmt.Errorf("failed to write systemd unit %q: %w", u.Name, err) }