From 61e7e1632a37830d8ae7eb05af0242c07b5a150a Mon Sep 17 00:00:00 2001 From: Andrew Dunn Date: Sun, 23 Aug 2026 22:14:18 -0400 Subject: [PATCH 1/3] composefs: Fix inverted rollback queued detection The systemd UKI arm (BLSConfigType::EFI) of the rollback detection in composefs_deployment_status_from() is missing the negation that the NonEFI and GRUB UKI arms have: it reports rollbackQueued=true exactly when the first (default) boot entry DOES reference the booted deployment. On an sd-boot UKI host this inverts everything derived from it: bootc status reports a queued rollback after every successful staged upgrade boot and reports clean when a rollback really is queued; bootc rollback prints "Reverting queued rollback state" while actually queueing one (and vice versa); and the guard refusing to delete the rollback deployment while it is queued to boot fires in the wrong cases. Extract the check into rollback_queued_from_first_entry() so the BLS arms share one negation, and add regression tests covering both entry types in both directions. Closes: #2405 Signed-off-by: Andrew Dunn --- crates/lib/src/bootc_composefs/status.rs | 118 +++++++++++++++++++---- 1 file changed, 101 insertions(+), 17 deletions(-) diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index c2a984e794..126fccc8d6 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -820,6 +820,32 @@ fn set_reboot_capable_uki_deployments( Ok(()) } +/// Whether the bootloader will boot a deployment other than the booted one, +/// i.e. whether the first (default) boot entry references some other deployment. +#[context("Determining if rollback is queued")] +fn rollback_queued_from_first_entry( + bls_config: &BLSConfig, + booted_composefs_digest: &str, +) -> Result { + match &bls_config.cfg_type { + // For UKI boot + BLSConfigType::EFI { key } => { + let path = match key { + EFIKey::Efi(path) | EFIKey::Uki(path) => path, + }; + Ok(!path.as_str().contains(booted_composefs_digest)) + } + + // For boot entry Type1 + BLSConfigType::NonEFI { options, .. } => Ok(!options + .as_ref() + .ok_or_else(|| anyhow::anyhow!("options key not found in bls config"))? + .contains(booted_composefs_digest)), + + BLSConfigType::Unknown => anyhow::bail!("Unknown BLS Config Type"), + } +} + #[context("Getting composefs deployment status")] async fn composefs_deployment_status_from( storage: &Storage, @@ -985,23 +1011,8 @@ async fn composefs_deployment_status_from( .first() .ok_or(anyhow::anyhow!("First boot entry not found"))?; - let is_rollback_queued = match &bls_config.cfg_type { - // For UKI boot - BLSConfigType::EFI { key } => { - let path = match key { - EFIKey::Efi(path) | EFIKey::Uki(path) => path, - }; - path.as_str().contains(booted_composefs_digest.as_ref()) - } - - // For boot entry Type1 - BLSConfigType::NonEFI { options, .. } => !options - .as_ref() - .ok_or(anyhow::anyhow!("options key not found in bls config"))? - .contains(booted_composefs_digest.as_ref()), - - BLSConfigType::Unknown => anyhow::bail!("Unknown BLS Config Type"), - }; + let is_rollback_queued = + rollback_queued_from_first_entry(bls_config, booted_composefs_digest.as_ref())?; (is_rollback_queued, Some(bls_configs), None) } @@ -1135,6 +1146,79 @@ mod tests { Ok(()) } + #[test] + fn test_rollback_queued_from_first_entry_uki() -> Result<()> { + const BOOTED: &str = "1111111111111111111111111111111111111111111111111111111111111111"; + const ROLLBACK: &str = "2222222222222222222222222222222222222222222222222222222222222222"; + + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + tempdir.create_dir_all("loader/entries")?; + + let default_entry = format!( + "title Fedora Bootc\nversion 44\nsort-key {}\nuki /EFI/Linux/bootc/bootc_composefs-{BOOTED}.efi\n", + primary_sort_key("fedora") + ); + let other_entry = format!( + "title Fedora Bootc\nversion 44\nsort-key {}\nuki /EFI/Linux/bootc/bootc_composefs-{ROLLBACK}.efi\n", + secondary_sort_key("fedora") + ); + + tempdir.atomic_write("loader/entries/bootc_fedora-44-0.conf", default_entry)?; + tempdir.atomic_write("loader/entries/bootc_fedora-44-1.conf", other_entry)?; + + let sorted = + get_sorted_type1_boot_entries_helper(&tempdir, true, false, Bootloader::Systemd)?; + let first = sorted.first().unwrap(); + + // The entry carrying the primary sort key is the bootloader default + assert_eq!( + first.sort_key.as_ref().unwrap(), + &primary_sort_key("fedora") + ); + + // The default entry references the booted deployment: nothing is queued + assert!(!rollback_queued_from_first_entry(first, BOOTED)?); + // The default entry references another deployment: a rollback is queued + assert!(rollback_queued_from_first_entry(first, ROLLBACK)?); + + Ok(()) + } + + #[test] + fn test_rollback_queued_from_first_entry_type1() -> Result<()> { + const BOOTED: &str = "7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6"; + const ROLLBACK: &str = "febdf62805de2ae7b6b597f2a9775d9c8a753ba1e5f09298fc8fbe0b0d13bf01"; + + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + tempdir.create_dir_all("loader/entries")?; + + let default_entry = format!( + "title Fedora Bootc\nversion 44\nsort-key {}\nlinux /boot/{BOOTED}/vmlinuz\ninitrd /boot/{BOOTED}/initramfs.img\noptions root=UUID=abc123 rw composefs={BOOTED}\n", + primary_sort_key("fedora") + ); + let other_entry = format!( + "title Fedora Bootc\nversion 44\nsort-key {}\nlinux /boot/{ROLLBACK}/vmlinuz\ninitrd /boot/{ROLLBACK}/initramfs.img\noptions root=UUID=abc123 rw composefs={ROLLBACK}\n", + secondary_sort_key("fedora") + ); + + tempdir.atomic_write("loader/entries/bootc_fedora-44-0.conf", default_entry)?; + tempdir.atomic_write("loader/entries/bootc_fedora-44-1.conf", other_entry)?; + + let sorted = + get_sorted_type1_boot_entries_helper(&tempdir, true, false, Bootloader::Systemd)?; + let first = sorted.first().unwrap(); + + assert_eq!( + first.sort_key.as_ref().unwrap(), + &primary_sort_key("fedora") + ); + + assert!(!rollback_queued_from_first_entry(first, BOOTED)?); + assert!(rollback_queued_from_first_entry(first, ROLLBACK)?); + + Ok(()) + } + #[test] fn test_sorted_uki_boot_entries() -> Result<()> { let user_cfg = r#" From e49fa8e220ee1c5e5d8469dea224ff2456c56894 Mon Sep 17 00:00:00 2001 From: Andrew Dunn Date: Mon, 24 Aug 2026 07:10:31 -0400 Subject: [PATCH 2/3] composefs: Fix grub BLS rollback detection sort direction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GRUBClassic/Bls arm sorted entries descending (ascending=false), putting the non-default entry first, then applied the first-entry check to it — the same inversion the previous commit fixes for sd-boot UKI, entered from the other side. The reversed list also fed the rollback-candidate selection and soft-reboot capability code, both of which expect menu order (default first), as the BLSCompatible arm already provides. Sort ascending and route through rollback_queued_from_first_entry() so every Type1-reading arm shares one negation. Add a Grub-parameterized regression test pinning the premise the check stands on: under the descending filename-release comparator the primary entry is grub's default. Also correct the existing tests' fixture pairing to match production (sort-key "-0" rides filename release "1", per boot.rs) — the Systemd comparator ignored the filenames, the Grub comparator would not. Signed-off-by: Andrew Dunn --- crates/lib/src/bootc_composefs/status.rs | 185 ++++++++++++++++------- 1 file changed, 132 insertions(+), 53 deletions(-) diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index 126fccc8d6..56cf3a72ec 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -960,63 +960,61 @@ async fn composefs_deployment_status_from( let booted_cfs = host.require_composefs_booted()?; let mut grub_menu_string = String::new(); - let (is_rollback_queued, sorted_bls_config, grub_menu_entries) = match booted_cfs - .bootloader - .kind()? - { - BootloaderKind::GRUBClassic => match boot_type { - BootType::Bls => { - let bls_configs = get_sorted_type1_boot_entries(boot_dir, false)?; - let bls_config = bls_configs - .first() - .ok_or_else(|| anyhow::anyhow!("First boot entry not found"))?; - - match &bls_config.cfg_type { - BLSConfigType::NonEFI { options, .. } => { - let is_rollback_queued = !options - .as_ref() - .ok_or_else(|| anyhow::anyhow!("options key not found in bls config"))? - .contains(booted_composefs_digest.as_ref()); - - (is_rollback_queued, Some(bls_configs), None) + let (is_rollback_queued, sorted_bls_config, grub_menu_entries) = + match booted_cfs.bootloader.kind()? { + BootloaderKind::GRUBClassic => match boot_type { + BootType::Bls => { + let bls_configs = get_sorted_type1_boot_entries(boot_dir, true)?; + let bls_config = bls_configs + .first() + .ok_or_else(|| anyhow::anyhow!("First boot entry not found"))?; + + match &bls_config.cfg_type { + BLSConfigType::NonEFI { .. } => { + let is_rollback_queued = rollback_queued_from_first_entry( + bls_config, + booted_composefs_digest.as_ref(), + )?; + + (is_rollback_queued, Some(bls_configs), None) + } + + BLSConfigType::EFI { .. } => { + anyhow::bail!("Found 'efi' field in Type1 boot entry") + } + + BLSConfigType::Unknown => anyhow::bail!("Unknown BLS Config Type"), } - - BLSConfigType::EFI { .. } => { - anyhow::bail!("Found 'efi' field in Type1 boot entry") - } - - BLSConfigType::Unknown => anyhow::bail!("Unknown BLS Config Type"), } - } - BootType::Uki => { - let menuentries = - get_sorted_grub_uki_boot_entries(boot_dir, &mut grub_menu_string)?; + BootType::Uki => { + let menuentries = + get_sorted_grub_uki_boot_entries(boot_dir, &mut grub_menu_string)?; - let is_rollback_queued = !menuentries - .first() - .ok_or(anyhow::anyhow!("First boot entry not found"))? - .body - .chainloader - .contains(booted_composefs_digest.as_ref()); + let is_rollback_queued = !menuentries + .first() + .ok_or(anyhow::anyhow!("First boot entry not found"))? + .body + .chainloader + .contains(booted_composefs_digest.as_ref()); - (is_rollback_queued, None, Some(menuentries)) - } - }, + (is_rollback_queued, None, Some(menuentries)) + } + }, - // We will have BLS stuff and the UKI stuff in the same DIR - BootloaderKind::BLSCompatible => { - let bls_configs = get_sorted_type1_boot_entries(boot_dir, true)?; - let bls_config = bls_configs - .first() - .ok_or(anyhow::anyhow!("First boot entry not found"))?; + // We will have BLS stuff and the UKI stuff in the same DIR + BootloaderKind::BLSCompatible => { + let bls_configs = get_sorted_type1_boot_entries(boot_dir, true)?; + let bls_config = bls_configs + .first() + .ok_or(anyhow::anyhow!("First boot entry not found"))?; - let is_rollback_queued = - rollback_queued_from_first_entry(bls_config, booted_composefs_digest.as_ref())?; + let is_rollback_queued = + rollback_queued_from_first_entry(bls_config, booted_composefs_digest.as_ref())?; - (is_rollback_queued, Some(bls_configs), None) - } - }; + (is_rollback_queued, Some(bls_configs), None) + } + }; // Determine rollback deployment by matching extra deployment boot entries against entires read from /boot // This collects verity digest across bls and grub enties, we should just have one of them, but still works @@ -1163,8 +1161,23 @@ mod tests { secondary_sort_key("fedora") ); - tempdir.atomic_write("loader/entries/bootc_fedora-44-0.conf", default_entry)?; - tempdir.atomic_write("loader/entries/bootc_fedora-44-1.conf", other_entry)?; + // Production pairing (boot.rs): the primary entry carries sort-key + // "...-0" AND filename release "1" — systemd-boot sorts sort-key + // ascending, grub sorts the release field descending, both put it first. + tempdir.atomic_write( + format!( + "loader/entries/{}", + type1_entry_conf_file_name("fedora", 44, FILENAME_PRIORITY_PRIMARY) + ), + default_entry, + )?; + tempdir.atomic_write( + format!( + "loader/entries/{}", + type1_entry_conf_file_name("fedora", 44, FILENAME_PRIORITY_SECONDARY) + ), + other_entry, + )?; let sorted = get_sorted_type1_boot_entries_helper(&tempdir, true, false, Bootloader::Systemd)?; @@ -1201,8 +1214,21 @@ mod tests { secondary_sort_key("fedora") ); - tempdir.atomic_write("loader/entries/bootc_fedora-44-0.conf", default_entry)?; - tempdir.atomic_write("loader/entries/bootc_fedora-44-1.conf", other_entry)?; + // Production pairing: primary sort-key rides filename release "1" + tempdir.atomic_write( + format!( + "loader/entries/{}", + type1_entry_conf_file_name("fedora", 44, FILENAME_PRIORITY_PRIMARY) + ), + default_entry, + )?; + tempdir.atomic_write( + format!( + "loader/entries/{}", + type1_entry_conf_file_name("fedora", 44, FILENAME_PRIORITY_SECONDARY) + ), + other_entry, + )?; let sorted = get_sorted_type1_boot_entries_helper(&tempdir, true, false, Bootloader::Systemd)?; @@ -1219,6 +1245,59 @@ mod tests { Ok(()) } + #[test] + fn test_rollback_queued_from_first_entry_grub_type1() -> Result<()> { + const BOOTED: &str = "7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6"; + const ROLLBACK: &str = "febdf62805de2ae7b6b597f2a9775d9c8a753ba1e5f09298fc8fbe0b0d13bf01"; + + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + tempdir.create_dir_all("loader/entries")?; + + let default_entry = format!( + "title Fedora Bootc\nversion 44\nsort-key {}\nlinux /boot/{BOOTED}/vmlinuz\ninitrd /boot/{BOOTED}/initramfs.img\noptions root=UUID=abc123 rw composefs={BOOTED}\n", + primary_sort_key("fedora") + ); + let other_entry = format!( + "title Fedora Bootc\nversion 44\nsort-key {}\nlinux /boot/{ROLLBACK}/vmlinuz\ninitrd /boot/{ROLLBACK}/initramfs.img\noptions root=UUID=abc123 rw composefs={ROLLBACK}\n", + secondary_sort_key("fedora") + ); + + tempdir.atomic_write( + format!( + "loader/entries/{}", + type1_entry_conf_file_name("fedora", 44, FILENAME_PRIORITY_PRIMARY) + ), + default_entry, + )?; + tempdir.atomic_write( + format!( + "loader/entries/{}", + type1_entry_conf_file_name("fedora", 44, FILENAME_PRIORITY_SECONDARY) + ), + other_entry, + )?; + + // Grub and GrubCC ignore sort-key and sort the filename release field + // descending ("1" > "0"), so the primary entry is grub's default — + // the premise the first-entry check stands on for those bootloaders. + let sorted = get_sorted_type1_boot_entries_helper( + &tempdir, + true, + false, + crate::spec::Bootloader::Grub, + )?; + let first = sorted.first().unwrap(); + assert_eq!( + first.sort_key.as_ref().unwrap(), + &primary_sort_key("fedora") + ); + + assert!(!rollback_queued_from_first_entry(first, BOOTED)?); + assert!(rollback_queued_from_first_entry(first, ROLLBACK)?); + + Ok(()) + } + #[test] fn test_sorted_uki_boot_entries() -> Result<()> { let user_cfg = r#" From b927321fbe4d1783c2701dbd7ce2a73f47bd933c Mon Sep 17 00:00:00 2001 From: Andrew Dunn Date: Mon, 24 Aug 2026 01:01:51 -0400 Subject: [PATCH 3/3] tmt: Assert rollbackQueued through the rollback test flow The rollback test rebooted through queued and consumed rollback states without ever checking what bootc status reported for them. Add an assertion helper and check rollbackQueued at each transition: a freshly booted deployment reports false, a queued rollback reports true, the double rollback queues then unqueues, and booting the intended deployment consumes the queue. On composefs sd-boot UKI hosts the pre-fix code inverted the first and last of these, so the test fails there without the previous commit and pins the regression across every bootloader configuration the plan runs on. Signed-off-by: Andrew Dunn --- tmt/tests/booted/test-rollback.nu | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tmt/tests/booted/test-rollback.nu b/tmt/tests/booted/test-rollback.nu index fae0ab0e6d..9e62810893 100644 --- a/tmt/tests/booted/test-rollback.nu +++ b/tmt/tests/booted/test-rollback.nu @@ -24,6 +24,14 @@ def imgsrc [] { $env.BOOTC_upgrade_image? | default "localhost/bootc-derived-local" } +# Assert that bootc status reports the expected rollbackQueued value. +# Regression check for https://github.com/bootc-dev/bootc/issues/2405 where +# the composefs sd-boot UKI path reported the inverse. +def assert_rollback_queued [expected: bool] { + let queued = (bootc status --json | from json).status.rollbackQueued + assert equal $queued $expected +} + # Run on the first boot - capture initial state and switch to new image def initial_switch [] { tap begin "bootc rollback test" @@ -69,9 +77,15 @@ def second_boot_rollback [] { assert ("/usr/share/bootc-rollback-marker" | path exists) print "New image artifacts verified" + # A freshly booted deployment has no rollback queued + assert_rollback_queued false + print "Performing bootc rollback..." bootc rollback + # ...and a real queued rollback must be reported as one + assert_rollback_queued true + print "Rollback initiated, rebooting to previous deployment..." tmt-reboot } @@ -88,6 +102,9 @@ def back_to_first_depl [boot_count] { if ("/usr/share/bootc-rollback-marker" | path exists) { error make { msg: "Rollback target marker still present - rollback may have failed" } } + + # Booting the intended deployment consumes any queued rollback + assert_rollback_queued false } # Verify that rollback was successful and we're back to original deployment @@ -96,7 +113,9 @@ def third_boot_verify [] { # Finally test a double rollback, to make sure the rollback state is queued then unqueued bootc rollback + assert_rollback_queued true bootc rollback + assert_rollback_queued false tmt-reboot }