-
Notifications
You must be signed in to change notification settings - Fork 591
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
Option to upgrade packages and reboot if necessary #2119
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you want to set package_update
as well?
I'm tempted to argue that we should expose just a single setting that will turn all 3 options to true
:
package_update: true
package_upgrade: true
package_reboot_if_required: true
Having the individual setting seems like useless flexibility in the context of Lima.
However, I think being able to specify packages
might be useful. Unfortunately it has a bit of a complex schema, so validating it will be a bit of busy-work. I guess it could also be left for a future PR...
When you select package_upgrade (or packages), the package_update is automatically enabled as well. So I only think you need to set that, if you want to With Fedora it is not needed anyway, because
Mostly I just mirrored the cloud-init configuration. But without it (reboot), you would still run the old kernel... The documentation is a bit spread out: #cloud-config
# Update apt database on first boot
# (ie run apt-get update)
#
# Default: true
package_update: false
# Upgrade the instance on first boot
#
# Default: false
package_upgrade: true
# Reboot after package install/update if necessary
# Default: false
package_reboot_if_required: true https://cloudinit.readthedocs.io/en/latest/reference/examples.html#run-apt-or-yum-upgrade
|
Spoke too soon, since package_reboot_if_required unfortunately only works with apt (bug in cloud-init) It knows that it should restart, but it doesn't do it.
# TODO(smoser): handle this less violently
# kernel and openssl (possibly some other packages)
# write a file /var/run/reboot-required after upgrading.
# if that file exists and configured, then just stop right now and reboot
reboot_fn_exists = os.path.isfile(REBOOT_FILE)
if (upgrade or pkglist) and reboot_if_required and reboot_fn_exists: In DNF it is done by a separate command, called $ sudo dnf needs-restarting -r
Core libraries or services have been updated since boot-up:
* dbus-broker
* glibc
* kernel-core
* systemd
Reboot is required to fully utilize these updates.
More information: https://access.redhat.com/solutions/27943 So it needs to be patched in cloud-init, i.e. implemented properly in API. |
35724a7
to
73d5b7d
Compare
I didn't know that, and didn't find it anywhere in the docs. But even if it is, it doesn't hurt to explicitly set it to
This example config contradicts the schema definition:
@afbjorklund writes:
I know, but I don't see the point of importing the complexity into Lima. In # Update packages from repo after boot
# Default: false
update_packages: null and if the user sets it to package_update: true
package_upgrade: true
package_reboot_if_required: true Even if you think
Can't we do this in a |
Sorry, I didn't realize you updated the code as well as added the comments. I've only replied to your comments; haven't looked at the updated code yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good to me.
I do wonder though which other keys you expect under package
? If upgrade
remains the only subkey, then a single-level package_upgrade
seems more appropriate. Personally I would prefer upgrade_packages
?
The only thing I can see adding would be the packages
list. But that could also be a plain top-level key packages
instead of adding e.g. package.list
.
# Reboot after upgrade if required | ||
# 🟢 Builtin default: false | ||
upgrade: null | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid inflating the numbers of the limayaml fields, can we consider to support templating cloud-init yaml in limayaml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should have opened an issue first, before starting on the solution. The user just wants up-to-date packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want the user to have to learn cloud-init, since they sort of use lima.yaml to not have to do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cloud-init
support is a good feature for advanced users, just not required for this particular feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I'm in favour of approving this PR; @AkihiroSuda are you going to veto it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user just wants up-to-date packages.
You can already do this with a dependency provisioning script (untested):
provision:
- mode: dependency
script: |
apt-get update
apt-get upgrade
if [ -f /var/run/reboot-required ]; then
reboot
fi
This script is executed as part of 30-install-packages.sh
, just before additional packages are installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was naively hoping this to be a oneliner, both in the lima.yaml and in the cloud-init.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I'm in favour of approving this PR; @AkihiroSuda are you going to veto it?
@AkihiroSuda can you please let us know if you are opposed to making this change, and want to wait until we have some generic cloudInit
mechanism? Because then we can stop arguing about this PR if it has no chance of getting merged. 😃
I think this PR provides reasonable functionality to expose via Lima directly without requiring the user to learn about cloud-init
config files. We also simplify this to a single setting.
Also, contrary to my earlier comments, I don't think we should provide the packages
field. So the upgrade_packages
would be the only key I would expect to be added to lima.yaml
.
Sounds like a reasonable workaround, should be safe once it is fixed too. |
73d5b7d
to
3b30cbf
Compare
@jandubois renamed to [anders@lima-fedora lima]$ uname -r
6.6.9-200.fc39.x86_64
[anders@lima-fedora lima]$ rpm -q kernel-core
kernel-core-6.5.6-300.fc39.x86_64
kernel-core-6.6.9-200.fc39.x86_64 The only downside is that it takes the system "slightly longer" to get ready for ssh, since it does the upgrade.
|
examples/default.yaml
Outdated
@@ -166,6 +166,11 @@ caCerts: | |||
# YOUR-ORGS-TRUSTED-CA-CERT-HERE | |||
# -----END CERTIFICATE----- | |||
|
|||
# Upgrade the instance on first boot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure it only happens on first boot? We intentionally change the instance id on each boot to make sure cloud-init executes the first-boot logic on each reboot (e.g. to update network configurations). See #273.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there would be too many packages between the reboots, anyway ? But that's what the docs said:
https://cloudinit.readthedocs.io/en/stable/reference/examples.html#update-apt-database-on-first-boot
|
||
if command -v dnf >/dev/null 2>&1; then | ||
# dnf-utils needs to be installed, for needs-restarting | ||
if dnf needs-restarting -h >/dev/null 2>&1; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is -h
always supported? I only see these options:
dnf needs-restarting [-u] [-r] [-s]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is --help, I am just checking to see if it is there - because of the false-is-true return code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should have been spelled as dnf -h needs-restarting
, then it would be less cryptic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[root@b144ba347084 /]# dnf -h needs-restarting
No such command: needs-restarting. Please use /usr/bin/dnf --help
It could be a DNF plugin command, try: "dnf install 'dnf-command(needs-restarting)'"
[root@b144ba347084 /]# dnf install 'dnf-command(needs-restarting)'
Fedora 39 - x86_64 9.9 MB/s | 89 MB 00:09
Fedora 39 openh264 (From Cisco) - x86_64 1.6 kB/s | 2.5 kB 00:01
Fedora 39 - x86_64 - Updates 6.5 MB/s | 28 MB 00:04
No match for argument: dnf-command(needs-restarting)
Error: Unable to find a match: dnf-command(needs-restarting)
[root@b144ba347084 /]# dnf install dnf-utils
Last metadata expiration check: 0:00:17 ago on Sun Jan 14 09:08:48 2024.
Dependencies resolved.
=======================================================================================================================================================================
Package Architecture Version Repository Size
=======================================================================================================================================================================
Installing:
dnf-utils noarch 4.4.4-1.fc39 updates 37 k
Installing dependencies:
dbus-libs x86_64 1:1.14.10-1.fc39 fedora 156 k
dnf-plugins-core noarch 4.4.4-1.fc39 updates 38 k
fonts-filesystem noarch 1:2.0.5-12.fc39 fedora 8.2 k
js-jquery noarch 3.6.4-2.fc39 fedora 172 k
python3-dateutil noarch 1:2.8.2-10.fc39 fedora 355 k
python3-dbus x86_64 1.3.2-4.fc39 fedora 157 k
python3-distro noarch 1.8.0-6.fc39 fedora 49 k
python3-dnf-plugins-core noarch 4.4.4-1.fc39 updates 317 k
python3-six noarch 1.16.0-12.fc39 fedora 41 k
python3-systemd x86_64 235-5.fc39 fedora 107 k
web-assets-filesystem noarch 5-20.fc39 fedora 7.9 k
Installing weak dependencies:
python-systemd-doc x86_64 235-5.fc39 fedora 75 k
Transaction Summary
=======================================================================================================================================================================
Install 13 Packages
Total download size: 1.5 M
Installed size: 4.7 M
Is this ok [y/N]: y
Downloading Packages:
(1/13): fonts-filesystem-2.0.5-12.fc39.noarch.rpm 112 kB/s | 8.2 kB 00:00
(2/13): python-systemd-doc-235-5.fc39.x86_64.rpm 942 kB/s | 75 kB 00:00
(3/13): dbus-libs-1.14.10-1.fc39.x86_64.rpm 806 kB/s | 156 kB 00:00
(4/13): js-jquery-3.6.4-2.fc39.noarch.rpm 873 kB/s | 172 kB 00:00
(5/13): python3-distro-1.8.0-6.fc39.noarch.rpm 1.0 MB/s | 49 kB 00:00
(6/13): python3-dbus-1.3.2-4.fc39.x86_64.rpm 2.2 MB/s | 157 kB 00:00
(7/13): python3-dateutil-2.8.2-10.fc39.noarch.rpm 2.4 MB/s | 355 kB 00:00
(8/13): python3-six-1.16.0-12.fc39.noarch.rpm 704 kB/s | 41 kB 00:00
(9/13): python3-systemd-235-5.fc39.x86_64.rpm 2.2 MB/s | 107 kB 00:00
(10/13): web-assets-filesystem-5-20.fc39.noarch.rpm 188 kB/s | 7.9 kB 00:00
(11/13): dnf-plugins-core-4.4.4-1.fc39.noarch.rpm 288 kB/s | 38 kB 00:00
(12/13): dnf-utils-4.4.4-1.fc39.noarch.rpm 256 kB/s | 37 kB 00:00
(13/13): python3-dnf-plugins-core-4.4.4-1.fc39.noarch.rpm 1.1 MB/s | 317 kB 00:00
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------
Total 1.5 MB/s | 1.5 MB 00:01
Running transaction check
Transaction check succeeded.
Running transaction test
Transaction test succeeded.
Running transaction
Preparing : 1/1
Installing : python3-six-1.16.0-12.fc39.noarch 1/13
Installing : python3-dateutil-1:2.8.2-10.fc39.noarch 2/13
Installing : python3-distro-1.8.0-6.fc39.noarch 3/13
Installing : fonts-filesystem-1:2.0.5-12.fc39.noarch 4/13
Installing : web-assets-filesystem-5-20.fc39.noarch 5/13
Installing : js-jquery-3.6.4-2.fc39.noarch 6/13
Installing : python-systemd-doc-235-5.fc39.x86_64 7/13
Installing : python3-systemd-235-5.fc39.x86_64 8/13
Installing : dbus-libs-1:1.14.10-1.fc39.x86_64 9/13
Installing : python3-dbus-1.3.2-4.fc39.x86_64 10/13
Installing : python3-dnf-plugins-core-4.4.4-1.fc39.noarch 11/13
Installing : dnf-plugins-core-4.4.4-1.fc39.noarch 12/13
Installing : dnf-utils-4.4.4-1.fc39.noarch 13/13
Running scriptlet: dnf-utils-4.4.4-1.fc39.noarch 13/13
Verifying : dbus-libs-1:1.14.10-1.fc39.x86_64 1/13
Verifying : fonts-filesystem-1:2.0.5-12.fc39.noarch 2/13
Verifying : js-jquery-3.6.4-2.fc39.noarch 3/13
Verifying : python-systemd-doc-235-5.fc39.x86_64 4/13
Verifying : python3-dateutil-1:2.8.2-10.fc39.noarch 5/13
Verifying : python3-dbus-1.3.2-4.fc39.x86_64 6/13
Verifying : python3-distro-1.8.0-6.fc39.noarch 7/13
Verifying : python3-six-1.16.0-12.fc39.noarch 8/13
Verifying : python3-systemd-235-5.fc39.x86_64 9/13
Verifying : web-assets-filesystem-5-20.fc39.noarch 10/13
Verifying : dnf-plugins-core-4.4.4-1.fc39.noarch 11/13
Verifying : dnf-utils-4.4.4-1.fc39.noarch 12/13
Verifying : python3-dnf-plugins-core-4.4.4-1.fc39.noarch 13/13
Installed:
dbus-libs-1:1.14.10-1.fc39.x86_64 dnf-plugins-core-4.4.4-1.fc39.noarch dnf-utils-4.4.4-1.fc39.noarch fonts-filesystem-1:2.0.5-12.fc39.noarch
js-jquery-3.6.4-2.fc39.noarch python-systemd-doc-235-5.fc39.x86_64 python3-dateutil-1:2.8.2-10.fc39.noarch python3-dbus-1.3.2-4.fc39.x86_64
python3-distro-1.8.0-6.fc39.noarch python3-dnf-plugins-core-4.4.4-1.fc39.noarch python3-six-1.16.0-12.fc39.noarch python3-systemd-235-5.fc39.x86_64
web-assets-filesystem-5-20.fc39.noarch
Complete!
[root@b144ba347084 /]# dnf -h needs-restarting
usage: dnf needs-restarting [-c [config file]] [-q] [-v] [--version] [--installroot [path]] [--nodocs] [--noplugins] [--enableplugin [plugin]]
[--disableplugin [plugin]] [--releasever RELEASEVER] [--setopt SETOPTS] [--skip-broken] [-h] [--allowerasing] [-b | --nobest] [-C]
[-R [minutes]] [-d [debug level]] [--debugsolver] [--showduplicates] [-e ERRORLEVEL] [--obsoletes] [--rpmverbosity [debug level name]]
[-y] [--assumeno] [--enablerepo [repo]] [--disablerepo [repo] | --repo [repo]] [--enable | --disable] [-x [package]]
[--disableexcludes [repo]] [--repofrompath [repo,path]] [--noautoremove] [--nogpgcheck] [--color COLOR] [--refresh] [-4] [-6]
[--destdir DESTDIR] [--downloadonly] [--comment COMMENT] [--bugfix] [--enhancement] [--newpackage] [--security] [--advisory ADVISORY]
[--bz BUGZILLA] [--cve CVES] [--sec-severity {Critical,Important,Moderate,Low}] [--forcearch ARCH] [-u] [-r] [-s]
determine updated binaries that need restarting
General DNF options:
-c [config file], --config [config file]
config file location
-q, --quiet quiet operation
-v, --verbose verbose operation
--version show DNF version and exit
--installroot [path] set install root
--nodocs do not install documentations
--noplugins disable all plugins
--enableplugin [plugin]
enable plugins by name
--disableplugin [plugin]
disable plugins by name
--releasever RELEASEVER
override the value of $releasever in config and repo files
--setopt SETOPTS set arbitrary config and repo options
--skip-broken resolve depsolve problems by skipping packages
-h, --help, --help-cmd
show command help
--allowerasing allow erasing of installed packages to resolve dependencies
-b, --best try the best available package versions in transactions.
--nobest do not limit the transaction to the best candidate
-C, --cacheonly run entirely from system cache, don't update cache
-R [minutes], --randomwait [minutes]
maximum command wait time
-d [debug level], --debuglevel [debug level]
debugging output level
--debugsolver dumps detailed solving results into files
--showduplicates show duplicates, in repos, in list/search commands
-e ERRORLEVEL, --errorlevel ERRORLEVEL
error output level
--obsoletes enables dnf's obsoletes processing logic for upgrade or display capabilities that the package obsoletes for info, list and repoquery
--rpmverbosity [debug level name]
debugging output level for rpm
-y, --assumeyes automatically answer yes for all questions
--assumeno automatically answer no for all questions
--enablerepo [repo] Temporarily enable repositories for the purpose of the current dnf command. Accepts an id, a comma-separated list of ids, or a glob of ids.
This option can be specified multiple times.
--disablerepo [repo] Temporarily disable active repositories for the purpose of the current dnf command. Accepts an id, a comma-separated list of ids, or a glob
of ids. This option can be specified multiple times, but is mutually exclusive with `--repo`.
--repo [repo], --repoid [repo]
enable just specific repositories by an id or a glob, can be specified multiple times
--enable enable repos with config-manager command (automatically saves)
--disable disable repos with config-manager command (automatically saves)
-x [package], --exclude [package], --excludepkgs [package]
exclude packages by name or glob
--disableexcludes [repo], --disableexcludepkgs [repo]
disable excludepkgs
--repofrompath [repo,path]
label and path to an additional repository to use (same path as in a baseurl), can be specified multiple times.
--noautoremove disable removal of dependencies that are no longer used
--nogpgcheck disable gpg signature checking (if RPM policy allows)
--color COLOR control whether color is used
--refresh set metadata as expired before running the command
-4 resolve to IPv4 addresses only
-6 resolve to IPv6 addresses only
--destdir DESTDIR, --downloaddir DESTDIR
set directory to copy packages to
--downloadonly only download packages
--comment COMMENT add a comment to transaction
--bugfix Include bugfix relevant packages, in updates
--enhancement Include enhancement relevant packages, in updates
--newpackage Include newpackage relevant packages, in updates
--security Include security relevant packages, in updates
--advisory ADVISORY, --advisories ADVISORY
Include packages needed to fix the given advisory, in updates
--bz BUGZILLA, --bzs BUGZILLA
Include packages needed to fix the given BZ, in updates
--cve CVES, --cves CVES
Include packages needed to fix the given CVE, in updates
--sec-severity {Critical,Important,Moderate,Low}, --secseverity {Critical,Important,Moderate,Low}
Include security relevant packages matching the severity, in updates
--forcearch ARCH Force the use of an architecture
Needs-restarting command-specific options:
-u, --useronly only consider this user's processes
-r, --reboothint only report whether a reboot is required (exit code 1) or not (exit code 0)
-s, --services only report affected systemd services
It is installed by default in Fedora, but there could be custom templates using DNF that doesn't have it.
And of course the help text is broken / doesn't work, but that is to be expected (nobody tests these...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we shouldn't attempt to install the plugin (but only when package_reboot_if_required
is true)? Something like:
if command -v dnf >/dev/null 2>&1; then
if grep -q "package_reboot_if_required.*true" "${LIMA_CIDATA_MNT}/user-data"; then
# dnf-utils needs to be installed, for needs-restarting
if ! dnf -h needs-restarting >/dev/null 2>&1; then
dnf install dnf-utils
fi
fi
if dnf -h needs-restarting >/dev/null 2>&1; then
# needs-restarting returns "false" if needed (!)
if ! dnf needs-restarting -r >/dev/null 2>&1; then
systemctl reboot
fi
fi
fi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The best would to propose a PR to cloud-init (to fix the issue), but that's going to take longer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've confirmed now that the package upgrade runs during every boot, so please update the comment accordingly.
This will run "apt upgrade" or "dnf upgrade" on the first boot, to make sure that all packages are updated to the latest version. It adds to the initial startup time and to the size of the diffdisk, so it is better to update the basedisk with a newer cloud release. Signed-off-by: Anders F Björklund <[email protected]>
3b30cbf
to
f1a8dc1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think that adding the code from #2119 (comment) would be the right thing to do, to cover dnf-based images that don't come with dnf-utils
pre-installed.
However, this is just a theoretical concern. @afbjorklund confirmed that it is included in the Fedora images, and we are not aware of any distro that is missing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
This will run "apt upgrade" or "dnf upgrade" on the first boot, to make sure that all packages are updated to the latest version.
It adds to the initial startup time and to the size of the diffdisk, so it is better to update the basedisk with a newer cloud release.
But it is a good way to make sure that you start your Fedora with the latest and greatest kernel...
It would be possible to extend the feature to be able to install some missing system packages too.
Closes #2129