Import GRUB static migration code #790

travier · 2024-12-03T19:04:17Z

Fixes: #789

openshift-ci · 2024-12-03T19:04:30Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

cgwalters

Thanks for this!

cgwalters · 2024-12-03T19:06:39Z

src/bootupd.rs

@@ -489,6 +496,88 @@ pub(crate) fn client_run_validate() -> Result<()> {
    Ok(())
 }

+pub(crate) fn client_run_migrate() -> Result<()> {
+    // Used to condition execution of this unit at the systemd level
+    let stamp_file="/var/lib/.fedora_atomic_desktops_static_grub";


Missing a cargo fmt.

Also... now that this is part of bootupd calling the stamp file name "fedora_atomic_desktops_" seems odd.

Bikeshedding things a bit more...bootupd already has its own little database where we could store this state.

I'm also fine just keeping it as a stamp file, but how about e.g. .bootupd-static-migration-complete? Also since this is about data in /boot I think we should probably keep the stamp file there?

If we want to have this represented in bootup state directly then we would have to add a new mode to distinguish between the bootupd managed static grub configs and the ones that we imported from a system that are still user managed because they may contain arbitrary changes, os-prober systems, etc. and we don't want bootupd to override those on static config updates (when we'll implement that).

Hmm...I more meant that we add a new key to the JSON file

So in the end, I have not done that yet as:

I would have to figure out how to do it and test it

Having a stamp file makes it easy to skip running this later during boot using systemd

src/bootupd.rs

travier · 2024-12-04T15:41:01Z

I immediately ran into fedora-selinux/selinux-policy#2444 while testing this.

HuijingHei · 2024-12-06T07:53:27Z

Overall LGTM, I can help to do some testing if you have some instructions.

travier · 2024-12-12T18:16:44Z

How I'm (manually) testing this change:

Install Fedora Silverblue 40 in two VMs, one using BIOS & another UEFI (see https://gitlab.com/fedora/ostree/scripts for scripts that automate that), do the same for Fedora Silverblue 41
Update the Fedora 40 VMs to Fedora 41
Optional: Make a snapshot of the VM
Build bootupd and copy it to the VM:

$ cargo build --release
$ scp target/release/bootupd silverblue-41-uefi-bootupd:
[silverblue]$ sudo rpm-ostree usroverlay
[silverblue]$ sudo cp bootupd /usr/bin/bootupctl && sudo cp bootupd /usr/libexec/bootupd

Run the migration:

$ sudo bootupctl migrate

Validate the state of the system:
- Things to verify, on a system installed from Fedora 41:

root@fedora:~# sudo ls -alh /boot/grub2
total 16K
drwx------. 2 root root 4.0K Oct 25 13:29 .
drwxr-xr-x. 7 root root 4.0K Nov 29 14:37 ..
-rw-r--r--. 1 root root   53 Oct 25 13:29 bootuuid.cfg
-rw-r--r--. 1 root root 2.6K Oct 25 13:29 grub.cfg

root@fedora:~# grep ostree /boot/grub2/grub.cfg
<empty>

root@fedora:~# grep blscfg /boot/grub2/grub.cfg
blscfg

root@fedora:~# sudo cat /sysroot/ostree/repo/config
[core]
repo_version=1
mode=bare

[sysroot]
readonly=true
bootloader=none

Things to verify, on a system installed from an older release:

root@fedora:~# ls -alh /boot/grub2
total 64K
drwx------. 5 root root 4.0K Dec  2 20:30 .
drwxr-xr-x. 6 root root 4.0K Dec  2 20:13 ..
-rw-r--r--. 1 root root   64 Oct 18 17:50 device.map
drwx------. 2 root root 4.0K Jan  1  1970 fonts
-rw-r--r--. 1 root root    0 Dec  2 20:30 .grub2-blscfg-supported
-rw-------. 1 root root 7.0K Dec  2 20:30 grub.cfg
-rw-------. 1 root root 8.8K Nov 29 10:15 grub.cfg.backup
-rw-------. 1 root root 1.0K Dec  2 15:22 grubenv
drwxr-xr-x. 2 root root  20K Dec  2 20:13 i386-pc
drwxr-xr-x. 2 root root 4.0K Dec  2 20:13 locale

root@fedora:~# ls -alh /boot/grub2/.grub2-blscfg-supported 
-rw-r--r--. 1 root root 0 Dec  2 20:30 /boot/grub2/.grub2-blscfg-supported

root@fedora:~# grep ostree /boot/grub2/grub.cfg
  set kernelopts="root=UUID=f59453a5-036d-4b9a-b1a2-4428bf5eaecf ro rootflags=subvol=root/ostree/deploy/fedora/deploy/a7eefc0751cf4688e6daf0d998d24a435ae4d44ab193d8dce7e01f66d366fa08.0 rhgb quiet "
### BEGIN /etc/grub.d/15_ostree ###
### END /etc/grub.d/15_ostree ###

root@fedora:~# grep blscfg /boot/grub2/grub.cfg
# The blscfg command parses the BootLoaderSpec files stored in /boot/loader/entries and
insmod blscfg
blscfg

root@fedora:~# cat /sysroot/ostree/repo/config
[core]
repo_version=1
mode=bare

[sysroot]
readonly=true
bootloader=none

travier · 2024-12-12T18:39:00Z

So this works as far as I've tested but we have no tests for it yet.

HuijingHei · 2024-12-13T14:30:10Z

Install Fedora Silverblue 40 in two VMs, one using BIOS & another UEFI (see https://gitlab.com/fedora/ostree/scripts for scripts that automate that)
Update the Fedora 40 VMs to Fedora 41

Do testing on BIOS VM and UEFI VM, then upgrade to f41, copy the new bootupd to the vm, run $ sudo bootupctl migrate, check the results are expected as above, also remove ostree-grub2 (seems it is removed in https://pagure.io/workstation-ostree-config/pull-request/591), and reboot successfully.
Let me know if I missed something, thanks!

travier · 2024-12-18T11:43:27Z

I've added the systemd unit to this PR and rebased it on top of #803 to avoid conflicts for the systemd unit setup in the Makefile & specfile.

I've split the clippy lint fixes in #804.

cgwalters · 2024-12-18T13:23:10Z

contrib/packaging/bootupd-static-grub-migration.service

+Documentation=https://github.com/coreos/bootupd
+ConditionPathExists=!/boot/.bootupd-static-migration-complete
+RequiresMountsFor=/sysroot /boot
+# Only run after a successful bootloader update


Are there any systems which would enable the static migration but not the update service?

If not, why wouldn't we just fold this functionality into the bootloader-update.service via

ExecStart=bootupctl migrate ExecStart=bootupctl update

Or I guess just have bootupctl update --migrate-if-needed or so.

We indeed want all the systems that do the migration to update their bootloader before.

We don't need to do the migration on systems which already have a static GRUB configs and we can check the state for that, so maybe this should indeed be folded into a single command and moved from a stamp file to a field in the state JSON.

If we merge both units then it also means that we have to careful when rolling this out as we won't be easily able to enable one or the other independently. But that's also kind of OK as I'm looking at doing that in F41 as well before the F42 release.

src/bootupd.rs

See: https://gitlab.com/fedora/ostree/sig/-/issues/35 See: https://pagure.io/workstation-ostree-config/pull-request/591 Fixes: coreos#789

Note that the service is intentionally not enabled by default as it should be up to the distribution to add a systemd preset if the migration to a static GRUB config is needed. This will be used on Atomic Desktops & IoT systems to migrate systems to a static GRUB config before enabling composefs as GRUB curently does not interact well with it: https://bugzilla.redhat.com/show_bug.cgi?id=2308594 See: coreos#789 See: https://fedoraproject.org/wiki/Changes/ComposefsAtomicDesktops

HuijingHei · 2024-12-19T09:06:10Z

Ask a silly question, is there any reason should do the migration only when /boot/grub2/grub.cfg is symlink?

Check on silverblue40 (using efi), it is symlink to /boot/loader/grub.cfg, but on silverblue41, it is a normal file.

travier · 2024-12-19T14:09:09Z

Ask a silly question, is there any reason should do the migration only when /boot/grub2/grub.cfg is symlink?

As far I know, this is the marker that lets us tell a system with a dynamic config from one with a static one.

Check on silverblue40 (using efi), it is symlink to /boot/loader/grub.cfg, but on silverblue41, it is a normal file.

On Silverblue 40, GRUB is setup using a dynamic config like package mode Fedora and on Silverblue 41, it is setup by bootupd with a static config.

champtar · 2025-01-06T21:01:47Z

src/cli/bootupctl.rs

@@ -58,6 +58,8 @@ pub enum CtlVerb {
    AdoptAndUpdate,
    #[clap(name = "validate", about = "Validate system state")]
    Validate,
+    #[clap(name = "migrate", about = "Migrate a system to static a GRUB config")]


Migrate is a bit generic, should this be migrate-grub ?

Suggested change

#[clap(name = "migrate", about = "Migrate a system to static a GRUB config")]

#[clap(name = "migrate-grub", about = "Migrate a system to a static GRUB config")]

Sure, I'll do that.

champtar · 2025-01-09T14:31:13Z

src/bootupd.rs

+                .context("Failed to exchange symlink with current GRUB config")?;
+
+            // Remove the now unused symlink (optional cleanup, ignore any failures)
+            _ = dirfd.remove_file("grub.cfg.current");


Should we also delete the target of the symlink (/boot/loader/grub.cfg) ?
It'll be delete on the next ostree deploy, but it's cleaner IMO.

Or we could just mv $(readlink -ne /boot/grub2/grub.cfg) /boot/grub2/grub.cfg

I would also add a sync call after the mv

Should we also delete the target of the symlink (/boot/loader/grub.cfg) ?
It'll be delete on the next ostree deploy, but it's cleaner IMO.

We could, but it's harmless and as you said it will be "removed" (not generated) for the next deployment. Keeping it makes things easier for debugging for now.

Or we could just mv $(readlink -ne /boot/grub2/grub.cfg) /boot/grub2/grub.cfg

AFAIK that would not be atomic.

I would also add a sync call after the mv

We should indeed do that. I'll add this.

AFAIK that would not be atomic.

both files are under a single fs /boot so I don't see why it would not be atomic

champtar · 2025-01-09T20:37:13Z

contrib/packaging/bootupd-static-grub-migration.service

+[Unit]
+Description=bootupd static GRUB config migration
+Documentation=https://github.com/coreos/bootupd
+ConditionPathExists=!/boot/.bootupd-static-migration-complete


If we don't want to leave another stamp file around we can use

Suggested change

ConditionPathExists=!/boot/.bootupd-static-migration-complete

ConditionPathIsSymbolicLink=/etc/grub2/grub.cfg

It's /boot/grub2/grub.cfg that is a symlink but we can not do that as this would skip all the logic after the symlink change that is still needed.

(I meant /boot/grub2/grub.cfg not /etc/grub2/grub.cfg ...)
Indeed it's not good if we get interrupted

champtar · 2025-01-10T03:52:23Z

src/bootupd.rs

+    // Migration complete, let's write the stamp file
+    File::create(stamp_file)?;
+
+    println!("Static GRUB config migration completed successfully!");


In the end this script doesn't switch to static config ("static-configs":null), it just ensure we are properly switched to a BLS compatible config (really old grub or if we had ostree-grub2)

Also grub2-mkconfig will actually still use grub2-probe (os-prober) and fail with composefs enabled,
so we can't update + migrate + enable composefs in a single update (my use case is offline appliances, possibly skipping updates).

Shouldn't we switch to static config generated by bootupd (same as during install) ?

In the end this script doesn't switch to static config ("static-configs":null), it just ensure we are properly switched to a BLS compatible config (really old grub or if we had ostree-grub2)

It does switch the system to a static GRUB config that is no longer updated on deployment changes. What it does not do is use the static config from bootupd, to preseve changes that users may have done in their existing configs. Thus we can not record those in the static-configs entry in the state JSON as those are not the same configs.

Also grub2-mkconfig will actually still use grub2-probe (os-prober) and fail with composefs enabled, so we can't update + migrate + enable composefs in a single update (my use case is offline appliances, possibly skipping updates).

Indeed, this is why I'm looking at pushing that to Fedora 41 as well, before the F42 release. I'll have to investigate that more.

Shouldn't we switch to static config generated by bootupd (same as during install) ?

We can not do that as that would remove local user config customisations.

Shouldn't we switch to static config generated by bootupd (same as during install) ?

We can not do that as that would remove local user config customisations.

With your Fedora hat it makes sense to be cautious, but for the appliance use case I would really like to be able to just remove grub-tools completely and have bootupd adopt grub.cfg as well, so there is no drift between an upgraded system and a fresh install

I'm tempted to just run

rm /boot/bootupd-state.json bootupctl backend install --auto --write-uuid /

Actually I want #578

travier · 2025-01-10T16:27:24Z

Also grub2-mkconfig will actually still use grub2-probe (os-prober) and fail with composefs enabled, so we can't update + migrate + enable composefs in a single update (my use case is offline appliances, possibly skipping updates).

Indeed, this is why I'm looking at pushing that to Fedora 41 as well, before the F42 release. I'll have to investigate that more.

Thinking about this more, this means that we can not take the approach as written in this current PR as the grub configuration generation would fail as part of the generation on the first boot on F42 with composefs enabled.

We have multiple options:

Do a plain copy/paste of the config instead and strip the ostree boot entries "manually" by removing all the content between the #### ...ostree.. GRUB marker in the config. This would let us keep composefs enabled directly on the first boot in F42 but would mean that any migration failure will likely block updates.
Use the ostree.prepare-root.composefs=0 kernel argument instead and a migration process similar to the one we did with the sysroot RO one to only enable composefs once the migration has been safely completed. On the first boot in F42, we would do the migration and update the kernel argument only when ready. This would mean that we can not remove the ostree-grub2 package in F42 and we would have to also wait for F43 to statically enable composefs.

openshift-ci bot added the do-not-merge/work-in-progress label Dec 3, 2024

cgwalters reviewed Dec 3, 2024

View reviewed changes

travier mentioned this pull request Dec 4, 2024

Make bootupd_t permissive fedora-selinux/selinux-policy#2444

Merged

travier force-pushed the main-static-migration branch 6 times, most recently from 0436363 to ed15ca7 Compare December 5, 2024 17:07

travier force-pushed the main-static-migration branch 2 times, most recently from ec3c25f to e17023d Compare December 12, 2024 18:26

travier marked this pull request as ready for review December 12, 2024 18:34

travier force-pushed the main-static-migration branch from e17023d to 4a65f3b Compare December 18, 2024 11:42

travier changed the title ~~WIP: Import GRUB static migration code~~ Import GRUB static migration code Dec 18, 2024

openshift-ci bot removed the do-not-merge/work-in-progress label Dec 18, 2024

cgwalters reviewed Dec 18, 2024

View reviewed changes

travier added 2 commits December 18, 2024 16:37

migrate: Add GRUB static migration subcommand

3926dcf

See: https://gitlab.com/fedora/ostree/sig/-/issues/35 See: https://pagure.io/workstation-ostree-config/pull-request/591 Fixes: coreos#789

travier force-pushed the main-static-migration branch from 4a65f3b to 06132d0 Compare December 18, 2024 15:38

champtar reviewed Jan 6, 2025

View reviewed changes

champtar reviewed Jan 9, 2025

View reviewed changes

champtar reviewed Jan 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import GRUB static migration code #790

Import GRUB static migration code #790

travier commented Dec 3, 2024 •

edited

Loading

openshift-ci bot commented Dec 3, 2024

cgwalters left a comment

cgwalters Dec 3, 2024

travier Dec 4, 2024 •

edited

Loading

cgwalters Dec 4, 2024

travier Dec 12, 2024

travier commented Dec 4, 2024

HuijingHei commented Dec 6, 2024

travier commented Dec 12, 2024

travier commented Dec 12, 2024

HuijingHei commented Dec 13, 2024

travier commented Dec 18, 2024

cgwalters Dec 18, 2024

travier Dec 18, 2024

HuijingHei commented Dec 19, 2024

travier commented Dec 19, 2024

champtar Jan 6, 2025

travier Jan 10, 2025

champtar Jan 9, 2025

travier Jan 10, 2025

champtar Jan 10, 2025

champtar Jan 9, 2025

travier Jan 10, 2025

champtar Jan 10, 2025

champtar Jan 10, 2025

travier Jan 10, 2025

champtar Jan 10, 2025

champtar Jan 10, 2025

travier commented Jan 10, 2025 •

edited

Loading

	#[clap(name = "migrate", about = "Migrate a system to static a GRUB config")]
	#[clap(name = "migrate-grub", about = "Migrate a system to a static GRUB config")]

	ConditionPathExists=!/boot/.bootupd-static-migration-complete
	ConditionPathIsSymbolicLink=/etc/grub2/grub.cfg

Import GRUB static migration code #790

Are you sure you want to change the base?

Import GRUB static migration code #790

Conversation

travier commented Dec 3, 2024 • edited Loading

openshift-ci bot commented Dec 3, 2024

cgwalters left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travier Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travier commented Dec 4, 2024

HuijingHei commented Dec 6, 2024

travier commented Dec 12, 2024

travier commented Dec 12, 2024

HuijingHei commented Dec 13, 2024

travier commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuijingHei commented Dec 19, 2024

travier commented Dec 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travier commented Jan 10, 2025 • edited Loading

travier commented Dec 3, 2024 •

edited

Loading

travier Dec 4, 2024 •

edited

Loading

travier commented Jan 10, 2025 •

edited

Loading