diff --git a/crates/lib/src/bootc_composefs/repo.rs b/crates/lib/src/bootc_composefs/repo.rs index a38e36c9c..9cee4ff1e 100644 --- a/crates/lib/src/bootc_composefs/repo.rs +++ b/crates/lib/src/bootc_composefs/repo.rs @@ -74,6 +74,7 @@ pub(crate) fn open_composefs_repo(rootfs_dir: &Dir) -> Result Result<()> { + // systemd-gpt-auto-generator automounts ESP at /boot + let is_boot_mountpoint = root.is_mountpoint("boot")?; + + if matches!(is_boot_mountpoint, Some(true)) { + let statfs = statfs(root_path.join("boot").as_std_path())?; + + // We probably don't need to be this thorough, but no harm done + if statfs.f_type == libc::MSDOS_SUPER_MAGIC { + return Ok(()); + } + } + let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR); let Some(esp_fd) = root .open_dir_optional(&efi_path) diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 78974fef5..c2ef18fab 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -187,7 +187,7 @@ use serde::{Deserialize, Serialize}; #[cfg(feature = "install-to-disk")] use self::baseline::InstallBlockDeviceOpts; -use crate::bootc_composefs::status::ComposefsCmdline; +use crate::bootc_composefs::status::{ComposefsCmdline, get_bootloader}; use crate::bootc_composefs::{ boot::setup_composefs_boot, repo::initialize_composefs_repository, status::get_container_manifest_and_config, @@ -642,8 +642,12 @@ pub(crate) struct State { #[allow(dead_code)] pub(crate) composefs_required: bool, - // If Some, then --composefs_native is passed + /// If Some, then --composefs-backend is passed pub(crate) composefs_options: InstallComposefsOpts, + + /// The bootloader on the host (determined by reading LoaderInfo from efivars) + /// Only Some when bootc is invoked with `install to-existing-root` + pub(crate) host_bootloader: Option, } // Shared read-only global state @@ -1542,12 +1546,21 @@ async fn verify_target_fetch( } /// Preparation for an install; validates and prepares some (thereafter immutable) global state. +/// +/// # Parameters +/// - `config_opts`: Installation configuration options (root user setup, generic image, etc.) +/// - `source_opts`: Source image reference; if `None`, assumes running inside a container +/// - `target_opts`: Target image reference and root path options +/// - `composefs_options`: composefs-related settings for the installation +/// - `target_fs`: Target filesystem type; used for `install to-filesystem` +/// - `replace_mode`: If `Some`, indicates an `install to-filesystem` or `install to-existing-root` with the given replacement mode async fn prepare_install( mut config_opts: InstallConfigOpts, source_opts: InstallSourceOpts, mut target_opts: InstallTargetOpts, mut composefs_options: InstallComposefsOpts, target_fs: Option, + replace_mode: Option, ) -> Result> { tracing::trace!("Preparing install"); let rootfs = cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority()) @@ -1698,6 +1711,17 @@ async fn prepare_install( setup_sys_mount("efivarfs", EFIVARFS)?; + // Read efivars to get the bootloader + // Only if the operation is to replace the existing installation + let host_bootloader = match replace_mode { + Some(..) => { + let host_bootloader = get_bootloader().context("Determining existing bootloader")?; + println!("Detected bootloader on host: {host_bootloader}"); + Some(host_bootloader) + } + None => None, + }; + // Now, deal with SELinux state. let selinux_state = reexecute_self_for_selinux_if_needed(&source, config_opts.disable_selinux)?; tracing::debug!("SELinux state: {selinux_state:?}"); @@ -1804,6 +1828,7 @@ async fn prepare_install( host_is_container, composefs_required, composefs_options, + host_bootloader, }); Ok(state) @@ -1811,19 +1836,52 @@ async fn prepare_install( impl PostFetchState { pub(crate) fn new(state: &State, d: &Dir) -> Result { + let supports_bootupd = crate::bootloader::supports_bootupd(d)?; + // Determine bootloader type for the target system // Priority: user-specified > bootupd availability > systemd-boot fallback let detected_bootloader = { if let Some(bootloader) = state.config_opts.bootloader.clone() { bootloader } else { - if crate::bootloader::supports_bootupd(d)? { + // TODO(Johan-Liebert1): The new release of bootupd would support all + if supports_bootupd { crate::spec::Bootloader::Grub } else { crate::spec::Bootloader::Systemd } } }; + + // If this exists it means we're replacing the current root + // or installing alongside it. The new image may or may not have + // the same bootloader as the host + // + // We could simply throw an error here, but at this point we'd have + // already nuked the boot or ESP so we should try our best to figure + // out what to install + let detected_bootloader = match state.host_bootloader { + Some(b) => match b { + Bootloader::Grub | Bootloader::GrubCC => { + if supports_bootupd { + b + } else { + Bootloader::Systemd + } + } + Bootloader::Systemd => match crate::bootloader::bootctl_systemd_version() { + Ok(_) => Bootloader::Systemd, + Err(_) => { + println!("Could not find bootctl, defaulting to Grub"); + Bootloader::Grub + } + }, + Bootloader::None => Bootloader::None, + }, + + None => detected_bootloader, + }; + println!("Bootloader: {detected_bootloader}"); let r = Self { detected_bootloader, @@ -2171,6 +2229,7 @@ pub(crate) async fn install_to_disk(mut opts: InstallToDiskOpts) -> Result<()> { opts.target_opts, opts.composefs_opts, block_opts.filesystem, + None, ) .await?; @@ -2297,12 +2356,14 @@ fn remove_all_in_dir_no_xdev(d: &Dir, mount_err: bool) -> Result<()> { if etype == FileType::dir() { if let Some(subdir) = d.open_dir_noxdev(&name)? { remove_all_in_dir_no_xdev(&subdir, mount_err)?; - d.remove_dir(&name)?; + d.remove_dir(&name) + .with_context(|| format!("Removing dir {name:?}"))?; } else if mount_err { anyhow::bail!("Found unexpected mount point {name:?}"); } } else { - d.remove_file_optional(&name)?; + d.remove_file_optional(&name) + .with_context(|| format!("Removing {name:?}"))?; } } anyhow::Ok(()) @@ -2558,6 +2619,7 @@ pub(crate) async fn install_to_filesystem( opts.target_opts, opts.composefs_opts, Some(inspect.fstype.as_str().try_into()?), + fsopts.replace, ) .await?; @@ -2631,18 +2693,22 @@ pub(crate) async fn install_to_filesystem( false } }; + // Find the UUID of /boot because we need it for GRUB. - let boot_uuid = if boot_is_mount { - let boot_path = target_root_path.join(BOOT); - tracing::debug!("boot_path={boot_path}"); - let u = bootc_mount::inspect_filesystem(&boot_path) - .with_context(|| format!("Inspecting /{BOOT}"))? - .uuid - .ok_or_else(|| anyhow!("No UUID found for /{BOOT}"))?; - Some(u) - } else { - None + let boot_uuid = match state.config_opts.bootloader { + Some(Bootloader::Grub) | None if boot_is_mount => { + let boot_path = target_root_path.join(BOOT); + tracing::debug!("boot_path={boot_path}"); + let u = bootc_mount::inspect_filesystem(&boot_path) + .with_context(|| format!("Inspecting /{BOOT}"))? + .uuid + .ok_or_else(|| anyhow!("No UUID found for /{BOOT}"))?; + Some(u) + } + + _ => None, }; + tracing::debug!("boot UUID: {boot_uuid:?}"); // Find the real underlying backing device for the root. This is currently just required diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index d7274d868..028fa6a1a 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -137,7 +137,13 @@ pub(crate) const COMPOSEFS_MODE: Mode = Mode::from_raw_mode(0o700); /// Ensure the composefs directory exists in the given physical root /// with the correct permissions (mode 0700). +#[context("Ensuring composefs directory")] pub(crate) fn ensure_composefs_dir(physical_root: &Dir) -> Result<()> { + // Usually the case with bootc install to-existing-root + if matches!(physical_root.is_mountpoint("."), Ok(Some(true))) { + crate::utils::open_dir_remount_rw(physical_root, ".".into())?; + } + let mut db = DirBuilder::new(); db.mode(COMPOSEFS_MODE.as_raw_mode()); physical_root @@ -608,6 +614,7 @@ impl Storage { /// /// This lazily opens the composefs repository, creating the directory if needed /// and bootstrapping verity settings from the ostree configuration. + #[context("Ensuring composefs")] pub(crate) fn get_ensure_composefs(&self) -> Result> { if let Some(composefs) = self.composefs.get() { return Ok(Arc::clone(composefs)); diff --git a/crates/lib/src/utils.rs b/crates/lib/src/utils.rs index 876b2e9f0..5086e6aac 100644 --- a/crates/lib/src/utils.rs +++ b/crates/lib/src/utils.rs @@ -1,6 +1,6 @@ use std::future::Future; use std::io::Write; -use std::os::fd::BorrowedFd; +use std::os::fd::{AsFd, BorrowedFd}; use std::path::{Component, Path, PathBuf}; use std::process::Command; use std::time::Duration; @@ -17,6 +17,7 @@ use libsystemd::logging::journal_print; use ostree::glib; use ostree_ext::container::SignatureSource; use ostree_ext::ostree; +use rustix::fs::StatVfsMountFlags; /// Try to look for keys injected by e.g. rpm-ostree requesting machine-local /// changes; if any are present, return `true`. @@ -83,7 +84,18 @@ pub fn have_executable(name: &str) -> Result { /// Given a target directory, if it's a read-only mount, then remount it writable #[context("Opening {target} with writable mount")] pub(crate) fn open_dir_remount_rw(root: &Dir, target: &Utf8Path) -> Result { - if matches!(root.is_mountpoint(target), Ok(Some(true))) { + let target_dir = root + .open_dir(target) + .with_context(|| format!("Opening {target}"))?; + + if matches!(target_dir.is_mountpoint("."), Ok(Some(true))) { + let st = rustix::fs::fstatvfs(target_dir.as_fd()) + .with_context(|| format!("Getting filesystem info for {target}"))?; + + if !st.f_flag.contains(StatVfsMountFlags::RDONLY) { + return Ok(target_dir); + }; + tracing::debug!("Target {target} is a mountpoint, remounting rw"); let st = Command::new("mount") .args(["-o", "remount,rw", target.as_str()]) @@ -92,7 +104,8 @@ pub(crate) fn open_dir_remount_rw(root: &Dir, target: &Utf8Path) -> Result anyhow::ensure!(st.success(), "Failed to remount: {st:?}"); } - root.open_dir(target).map_err(anyhow::Error::new) + + Ok(target_dir) } /// Given a target path, remove its immutability if present diff --git a/crates/system-reinstall-bootc/src/podman.rs b/crates/system-reinstall-bootc/src/podman.rs index 7a9e104f8..85a083787 100644 --- a/crates/system-reinstall-bootc/src/podman.rs +++ b/crates/system-reinstall-bootc/src/podman.rs @@ -30,6 +30,8 @@ pub(crate) fn reinstall_command( ssh_key_file: &str, has_clean: bool, ) -> Result { + // NOTE: We mount /sys/firmware/efi/efivars during installation setup in + // [`bootc_lib::prepare_install`] so we don't explicitly need it here let mut podman_command_and_args = [ // We use podman to run the bootc container. This might change in the future to remove the // podman dependency. @@ -50,7 +52,9 @@ pub(crate) fn reinstall_command( "--security-opt", "label=type:unconfined_t", "-v", - "/:/target", + // systemd gpt-auto-generator has boot.mount autofs mounted on /boot + // We want to propage that inside the container + "/:/target:rslave", ] .map(String::from) .to_vec(); diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index e032999f6..0b090c889 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -292,4 +292,11 @@ execute: how: fmf test: - /tmt/tests/tests/test-46-etc-merge-conflict + +/plan-47-system-reinstall-composefs: + summary: Test system-reinstall with composefs backend and bootloader compatibility + discover: + how: fmf + test: + - /tmt/tests/tests/test-47-system-reinstall-composefs # END GENERATED PLANS diff --git a/tmt/tests/booted/test-system-reinstall-composefs.nu b/tmt/tests/booted/test-system-reinstall-composefs.nu new file mode 100644 index 000000000..d2580b471 --- /dev/null +++ b/tmt/tests/booted/test-system-reinstall-composefs.nu @@ -0,0 +1,164 @@ +# number: 47 +# tmt: +# summary: Test system-reinstall with composefs backend and bootloader compatibility +# duration: 60m +# +# Tests that system-reinstall-bootc works with composefs backend by running +# `bootc install to-existing-root --composefs-backend` on the live root, +# matching the invocation pattern of system-reinstall-bootc. +# +# Three scenarios across three reboot cycles: +# +# Reboot 0: +# Same bootloader: image supports the host's bootloader +# → that bootloader should be installed +# +# Reboot 1: +# Verify same-bootloader result, then: +# Bootloader mismatch: image lacks support for the host's bootloader +# → fallback bootloader should be installed +# (skipped for UKI boot type where switching bootloaders is unsafe) +# +# Reboot 2: +# Verify mismatch fallback result +# +use std assert +use tap.nu + +if not (tap is_composefs) { + exit 0 +} + +let st = bootc status --json | from json + +# Run bootc install to-existing-root --composefs-backend via podman, +# matching the system-reinstall-bootc invocation from podman.rs. +# Key details tested by these flags: +# /:/target:rslave — propagates boot automount (commit 3bf1acba) +# --composefs-backend — exercises composefs repo init fix (commit fa18de18) +# efivars read — bootloader detection (commits fa18de18, 4cb1e63b) +def run_reinstall [image: string] { + (podman run + --rm + --privileged + --pid=host + --user=root:root + -v /var/lib/containers:/var/lib/containers + -v /dev:/dev + --security-opt label=type:unconfined_t + -v /:/target:rslave + $image + bootc install to-existing-root + --acknowledge-destructive + --skip-fetch-check + --composefs-backend + --disable-selinux) +} + +def first_boot [] { + tap begin "system-reinstall composefs + bootloader compatibility" + + let bootloader = ($st.status.booted.composefs.bootloader | str downcase) + let boot_type = ($st.status.booted.composefs.bootType | str downcase) + + # Persist host state for verification across reboots + $bootloader | save /var/host-bootloader + $boot_type | save /var/host-boot-type + + print $"Host bootloader: ($bootloader), boot type: ($boot_type)" + + bootc image copy-to-storage + + # Build derived image preserving the same bootloader support as the host. + # make_uki_containerfile appends UKI sealing stages when running on UKI. + let containerfile = (tap make_uki_containerfile " + FROM localhost/bootc as base + RUN rm -rf /usr/lib/bootc/bound-images.d + RUN touch /usr/share/testing-reinstall-same-bl + ") + + let td = mktemp -d + cd $td + $containerfile | save Dockerfile + podman build -t localhost/bootc-reinstall-same . + + print "Running to-existing-root --composefs-backend (same bootloader)" + run_reinstall localhost/bootc-reinstall-same + + tmt-reboot +} + +def second_boot [] { + print "Verifying reinstall with same bootloader" + + assert (tap is_composefs) "composefs should be active after reinstall" + assert ("/usr/share/testing-reinstall-same-bl" | path exists) "same-bootloader marker should exist" + + let orig_bootloader = (open /var/host-bootloader | str trim) + let current_bootloader = ($st.status.booted.composefs.bootloader | str downcase) + + print $"Original bootloader: ($orig_bootloader), Current: ($current_bootloader)" + assert equal $current_bootloader $orig_bootloader "bootloader should match host after same-bootloader reinstall" + + # Build image that lacks the host's bootloader, forcing a fallback: + # host=systemd-boot -> remove bootctl -> falls back to grub + # host=grub/grub-cc -> remove bootupd -> falls back to systemd-boot + bootc image copy-to-storage + + let containerfile = if $orig_bootloader == "systemd" { + print "Building image without systemd-boot support (removing bootctl)" + " + FROM localhost/bootc as base + RUN rm -f /usr/bin/bootctl + RUN rm -rf /usr/lib/bootc/bound-images.d + RUN touch /usr/share/testing-reinstall-mismatch + " + } else { + print "Building image without grub/bootupd support" + " + FROM localhost/bootc as base + RUN rm -rf /usr/lib/bootupd/updates /usr/bin/bootupctl + RUN rm -rf /usr/lib/bootc/bound-images.d + RUN touch /usr/share/testing-reinstall-mismatch + " + } + + let td = mktemp -d + cd $td + $containerfile | save Dockerfile + podman build -t localhost/bootc-reinstall-mismatch . + + print "Running to-existing-root --composefs-backend (mismatched bootloader)" + run_reinstall localhost/bootc-reinstall-mismatch + + tmt-reboot +} + +def third_boot [] { + print "Verifying reinstall with mismatched bootloader" + + assert (tap is_composefs) "composefs should be active after mismatch reinstall" + assert ("/usr/share/testing-reinstall-mismatch" | path exists) "mismatch marker should exist" + + let orig_bootloader = (open /var/host-bootloader | str trim) + let current_bootloader = ($st.status.booted.composefs.bootloader | str downcase) + + print $"Original host bootloader: ($orig_bootloader), Installed: ($current_bootloader)" + + if $orig_bootloader == "systemd" { + assert equal $current_bootloader "grub" "should fall back to grub when image lacks systemd-boot" + } else { + assert equal $current_bootloader "systemd" "should fall back to systemd-boot when image lacks grub" + } + + tap ok +} + +def main [] { + match $env.TMT_REBOOT_COUNT? { + null | "0" => first_boot, + "1" => second_boot, + "2" => third_boot, + $o => { error make { msg: $"Invalid TMT_REBOOT_COUNT ($o)" } }, + } +} diff --git a/tmt/tests/tests.fmf b/tmt/tests/tests.fmf index 0c12b5d4a..a6fb0abe2 100644 --- a/tmt/tests/tests.fmf +++ b/tmt/tests/tests.fmf @@ -183,3 +183,8 @@ check: summary: Verify etc merge conflicts are caught during upgrade, not finalization duration: 15m test: nu booted/test-etc-merge-conflict.nu + +/test-47-system-reinstall-composefs: + summary: Test system-reinstall with composefs backend and bootloader compatibility + duration: 60m + test: nu booted/test-system-reinstall-composefs.nu