Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

packagekit: Use dnf-automatic reboot option if available #19648

Merged
merged 4 commits into from
Nov 23, 2023

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Nov 23, 2023

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.

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


I tested this interactively on a real F39 machine with lots of pending updates (including a kernel), waited for the timer to trigger, and eventually:

shutdown[13040]: Reboot scheduled for Thu 2023-11-23 09:49:54 UTC, use 'shutdown -c' to cancel.
Nov 23 09:44:54 fedora-39-127-0-0-2-2201 dnf-automatic[1677]: Updates completed at Thu 23 Nov 2023 09:44:54 AM UTC
[...]
Broadcast message from root@fedora-39-127-0-0-2-2201 (Thu 2023-11-23 09:45:54 UTC):

Rebooting after applying package updates
The system will reboot at Thu 2023-11-23 09:49:54 UTC!

So this works nicely!

Commit 5eacb70 silently disabled TestAutoUpdates, as the skipOstree()
decorator requires an argument.
In all other components we just treat this as a boolean. This is passed
from `superuser.allowed`, which is `null` while it is being initialized.
React complains about that:

> The prop `privileged` is marked as required in `KpatchSettings`, but its value is `null`

Do what other components do and don't mark this as required, so that it
tolerates `null`.

This was hidden because TestAutoUpdates haven't run for a while (see
previous commit).
…tes()

testWithAvailableUpdates() is the only caller, and the helper makes
assumptions about the setup.
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
@martinpitt
Copy link
Member Author

Ugh, fallout from disabled TestAutoUpdates, fixed.

@martinpitt martinpitt requested review from jelly and mvollmer November 23, 2023 11:32
Copy link
Member

@jelly jelly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

"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";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Less hacks \o/ Wonder if we should move to string literals at some point for readability.

@jelly jelly merged commit ebab018 into cockpit-project:main Nov 23, 2023
32 checks passed
@martinpitt martinpitt deleted the dnf-automatic-reboot branch November 23, 2023 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants