Skip to content

Commit

Permalink
packagekit: Use dnf-automatic reboot option if available
Browse files Browse the repository at this point in the history
dnf 4.15 introduced a `reboot` option for automatic updates at last
[1][2]. This is available in all current Fedoras and RHEL 9.3+, but not
yet in RHEL 8. On systems which support it (i.e. which have a `reboot=`
option in automatic.conf), set that to "when-needed" instead of the
unit drop-in hack. Remove the latter to clean up on upgrades; that will
only have an effect if the admin disables and re-enables automatic
updates, but that's better than nothing.

In testWithAvailableUpdates(), adjust the mocked package name to
"kernel-rt", to match dnf's hardcoded list in base.py `reboot_needed()`.
Also increase the number of journal lines to make sure that the message
doesn't "scroll off".

Fixes https://issues.redhat.com/browse/RHEL-16392

[1] https://github.com/rpm-software-management/dnf/blob/master/doc/automatic.rst#commands-section
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1491190
  • Loading branch information
martinpitt authored and jelly committed Nov 23, 2023
1 parent d83a0a8 commit ebab018
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 16 deletions.
20 changes: 14 additions & 6 deletions pkg/packagekit/autoupdates.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,22 @@ class DnfImpl extends ImplBase {
script += "systemctl " + (enabled ? "enable" : "disable") + " --now dnf-automatic-install.timer; ";

if (enabled) {
// HACK: enable automatic reboots after updating; dnf-automatic does not leave a log file behind for
// deciding whether it actually installed anything, so resort to grepping the journal for the last run
// (https://bugzilla.redhat.com/show_bug.cgi?id=1491190)
script += "mkdir -p /etc/systemd/system/dnf-automatic-install.service.d; ";
script += "printf '[Service]\\nExecStartPost=/bin/sh -ec \"" +
/* dnf 4.15+ supports automatic reboots; check if the config option exists, and if so, change the
default to "when-needed"; but be strict about the format, to avoid changing a customized setting */
script += "if grep '^[[:space:]]*reboot\\b' /etc/dnf/automatic.conf; then ";
script += " sed -i 's/^reboot = never$/reboot = when-needed/' /etc/dnf/automatic.conf; ";
// and drop the previous hack on upgrades */
script += " rm -f " + rebootConf + "; ";
/* HACK for older dnf: enable automatic reboots after updating; dnf-automatic does not leave a log
file behind for deciding whether it actually installed anything, so resort to grepping the journal
for the last run (https://bugzilla.redhat.com/show_bug.cgi?id=1491190) */
script += "else ";
script += " mkdir -p /etc/systemd/system/dnf-automatic-install.service.d; ";
script += " printf '[Service]\\nExecStartPost=/bin/sh -ec \"" +
"if systemctl status --no-pager --lines=100 dnf-automatic-install.service| grep -q ===========$$; then " +
"shutdown -r +5 rebooting after applying package updates; fi\"\\n' > " + rebootConf + "; ";
script += "systemctl daemon-reload; ";
script += " systemctl daemon-reload; ";
script += "fi";
} else {
// also make sure that the legacy unit name is disabled; this can fail if the unit does not exist
script += "systemctl disable --now dnf-automatic.timer 2>/dev/null || true; ";
Expand Down
30 changes: 20 additions & 10 deletions test/verify/check-packagekit
Original file line number Diff line number Diff line change
Expand Up @@ -1421,6 +1421,7 @@ class TestAutoUpdates(NoSubManCase):
# not implemented for yum and apt yet, only dnf
self.supported_backend = self.backend in ["dnf"]
if self.backend == 'dnf':
self.restore_file("/etc/dnf/automatic.conf")
self.addCleanup(self.machine.execute, "systemctl disable --now dnf-automatic-install.timer 2>/dev/null; rm -rf /etc/systemd/system/dnf-automatic-*")

def closeSettings(self, browser):
Expand Down Expand Up @@ -1461,7 +1462,15 @@ class TestAutoUpdates(NoSubManCase):
# automatic reboots should be enabled whenever timer is enabled
out = m.execute("systemctl cat dnf-automatic-install.service")
if hour:
self.assertRegex(out, "ExecStartPost=/.*shutdown")
if m.image.startswith("rhel-8") or m.image == "centos-8-stream":
# for RHEL 8, dnf-automatic does not support reboot; we have a unit drop-in hack
self.assertRegex(out, "ExecStartPost=/.*shutdown")
# validate our assumption
self.assertNotIn("reboot", m.execute("cat /etc/dnf/automatic.conf"))
else:
# newer dnf supports that natively
self.assertNotIn("ExecStartPost", out)
self.assertIn("reboot = when-needed", m.execute("cat /etc/dnf/automatic.conf"))
else:
self.assertNotIn("ExecStartPost", out)

Expand Down Expand Up @@ -1605,8 +1614,9 @@ class TestAutoUpdates(NoSubManCase):
b = self.browser
m = self.machine

self.createPackage("vanilla", "1.0", "1", install=True)
self.createPackage("vanilla", "1.0", "2")
# use a package which dnf recognizes as "needs reboot"
self.createPackage("kernel-rt", "1.0", "1", install=True)
self.createPackage("kernel-rt", "1.0", "2")
self.enableRepo()

m.start_cockpit()
Expand All @@ -1631,24 +1641,24 @@ class TestAutoUpdates(NoSubManCase):
self.sed_file("/random_sleep/ s/=.*$/= 3/", "/etc/dnf/automatic.conf")
# then manually start the upgrade job like the timer would
m.execute("systemctl start dnf-automatic-install.service")
# new vanilla package got installed, and triggered reboot; cancel that
m.execute("test -f /stamp-vanilla-1.0-2")
# new kernel-rt package got installed, and triggered reboot; cancel that
m.execute("test -f /stamp-kernel-rt-1.0-2")
m.execute("until test -f /run/nologin; do sleep 1; done")
m.execute("shutdown -c; test ! -f /run/nologin")
# service should show vanilla upgrade and scheduling shutdown
# service should show kernel-rt upgrade and scheduling shutdown
out = m.execute(
"if systemctl status dnf-automatic-install.service; then echo 'expected service to be stopped'; exit 1; fi")
self.assertIn("vanilla", out)
"if systemctl status --lines=50 dnf-automatic-install.service; then echo 'expected service to be stopped'; exit 1; fi")
self.assertIn("kernel-rt", out)
# systemd 245.7 correctly says "reboot", older version say "shutdown"
self.assertRegex(out, "(Shutdown|Reboot) scheduled")

# run it again, now there are no available updates → no reboot
m.execute("systemctl start dnf-automatic-install.service")
m.execute("test -f /stamp-vanilla-1.0-2; test ! -f /run/nologin")
m.execute("test -f /stamp-kernel-rt-1.0-2; test ! -f /run/nologin")
# service should not do much
out = m.execute(
"if systemctl status dnf-automatic-install.service; then echo 'expected service to be stopped'; exit 1; fi")
self.assertNotIn("vanilla", out)
self.assertNotIn("kernel-rt", out)
self.assertNotIn("Shutdown", out)
else:
raise NotImplementedError(self.backend)
Expand Down

0 comments on commit ebab018

Please sign in to comment.