Skip to content

Commit

Permalink
daemon/update: disable systemd unit before overwriting
Browse files Browse the repository at this point in the history
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 openshift#3967 which does
exactly that (changing `[Install]` sections) and openshift#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.

Fixes: https://issues.redhat.com/browse/OCPBUGS-33694
  • Loading branch information
jlebon authored and openshift-cherrypick-robot committed Jun 30, 2024
1 parent 5f5ce30 commit 793f3c8
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions pkg/daemon/file_writers.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,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.Infof("Disabling systemd unit %s before re-writing it", u.Name)
disableOut, err := exec.Command("systemctl", "disable", u.Name).CombinedOutput()
if err != nil {
return fmt.Errorf("disabling %s failed: %w (output: %s)", u.Name, err, string(disableOut))
}
}
if err := writeFileAtomicallyWithDefaults(fpath, []byte(*u.Contents)); err != nil {
return fmt.Errorf("failed to write systemd unit %q: %w", u.Name, err)
}
Expand Down

0 comments on commit 793f3c8

Please sign in to comment.