From a8288be21a3984fbedd2eacc37aaa65d72ac400b Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Fri, 28 Nov 2025 18:08:34 +0100 Subject: [PATCH 1/5] install/bootupd: chroot to deployment When `--src-imgref` is passed, the deployed systemd does not match the running environnement. In this case, let's chroot into the deployment before calling bootupd. This makes sure we are using the binaries shipped in the image (and relevant config files such as grub fragements). We could do that in all cases but i kept it behind the `--src-imgref` option since when using the target container as the buildroot it will have no impact, and we expect this scenario to be the most common. In CoreOS we have a specific test that checks if the bootloader was installed with the `grub2-install` of the image. Fixes https://github.com/bootc-dev/bootc/issues/1559 Also see https://github.com/bootc-dev/bootc/issues/1455 Signed-off-by: jbtrystram --- crates/lib/src/bootloader.rs | 76 ++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 7 deletions(-) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 533ad4e43..53156d1dc 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -7,9 +7,12 @@ use camino::Utf8Path; use cap_std_ext::cap_std::fs::Dir; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; +use tempfile::TempDir; use bootc_blockdev::{Partition, PartitionTable}; use bootc_mount as mount; +use rustix::mount::UnmountFlags; +use rustix::path::Arg; use crate::bootc_composefs::boot::{get_sysroot_parent_dev, mount_esp, SecurebootKeys}; use crate::{discoverable_partition_specification, utils}; @@ -91,22 +94,81 @@ pub(crate) fn install_via_bootupd( // bootc defaults to only targeting the platform boot method. let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]); - let abs_deployment_path = deployment_path.map(|v| rootfs.join(v)); - let src_root_arg = if let Some(p) = abs_deployment_path.as_deref() { - vec!["--src-root", p.as_str()] + let abs_deployment_path = deployment_path.map(|deploy| rootfs.join(deploy)); + // When not running inside the target container (through `--src-imgref`) we chroot + // into the deployment before running bootupd. This makes sure we use binaries + // from the target image rather than the buildroot + // In some cases (e.g. --write-uuid), bootupd needs to find the underlying device + // for /boot. But since we don't control where the destination rootfs is mounted + // let's bind mount it to a temp mountpoint under /run + // so it gets carried over in the chroot. + + let rootfs_mountpoint: TempDir; + let rootfs_mount = if rootfs.starts_with("/run") { + rootfs.as_str() + } else { + rootfs_mountpoint = tempfile::tempdir_in("/run")?; + rustix::mount::mount_bind_recursive( + rootfs.as_std_path(), + &rootfs_mountpoint.path().to_owned(), + )?; + rootfs_mountpoint + .path() + .to_str() + .expect("Invalid tempdir path") + }; + + // We mount the linux API file systems into the target deployment before chrooting + // so bootupd can find the proper backing device. + // xref https://systemd.io/API_FILE_SYSTEMS/ + let bind_mount_dirs = ["/dev", "/run", "/proc", "/sys"]; + let chroot_args = if let Some(target_root) = abs_deployment_path.as_deref() { + tracing::debug!("Setting up bind-mounts before chrooting to the target deployment"); + for src in bind_mount_dirs { + let dest = target_root + // joining an absolute path + // makes it replace self, so we strip the prefix + .join_os(src.strip_prefix("/").unwrap()); + tracing::debug!("bind mounting {}", dest.display()); + rustix::mount::mount_bind_recursive(src, dest)?; + } + // Append the `bootupctl` command, it will be passed as + // an argument to chroot + vec![target_root.as_str(), "bootupctl"] } else { vec![] }; + let devpath = device.path(); println!("Installing bootloader via bootupd"); - Command::new("bootupctl") + let mut bootupctl = if abs_deployment_path.is_some() { + Command::new("chroot") + } else { + Command::new("bootupctl") + }; + let install_result = bootupctl + .args(chroot_args) .args(["backend", "install", "--write-uuid"]) .args(verbose) .args(bootupd_opts.iter().copied().flatten()) - .args(src_root_arg) - .args(["--device", devpath.as_str(), rootfs.as_str()]) + .args(["--device", devpath.as_str(), rootfs_mount]) .log_debug() - .run_inherited_with_cmd_context() + .run_inherited_with_cmd_context(); + + // Clean up the mounts after ourselves + if let Some(target_root) = abs_deployment_path { + for dir in bind_mount_dirs { + let mount = target_root + .join(dir.strip_prefix("/").unwrap()) + .into_std_path_buf(); + if let Err(e) = rustix::mount::unmount(&mount, UnmountFlags::DETACH) { + // let's not propagate the error up because in some cases we can't unmount + // e.g. when running `to-existing-root` + tracing::warn!("Error unmounting {}: {e}", mount.display()); + } + } + } + install_result } #[context("Installing bootloader")] From 09299f87381d67e735293de21a90f260e843f232 Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Wed, 7 Jan 2026 12:21:31 +0100 Subject: [PATCH 2/5] DNM: add more debug output to tests Signed-off-by: jbtrystram --- crates/tests-integration/src/install.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/tests-integration/src/install.rs b/crates/tests-integration/src/install.rs index 66db08bc6..b1674ff23 100644 --- a/crates/tests-integration/src/install.rs +++ b/crates/tests-integration/src/install.rs @@ -9,7 +9,17 @@ use fn_error_context::context; use libtest_mimic::Trial; use xshell::{cmd, Shell}; -pub(crate) const BASE_ARGS: &[&str] = &["podman", "run", "--rm", "--privileged", "--pid=host"]; +pub(crate) const BASE_ARGS: &[&str] = &[ + "podman", + "run", + "--rm", + "--privileged", + "--pid=host", + "--env", + "BOOTC_BOOTLOADER_DEBUG=true", + "--env", + "RUST_LOG=debug", +]; // Arbitrary const NON_DEFAULT_STATEROOT: &str = "foo"; From 3a1f2d937c1b625c49b28db148bee4c54640fde7 Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Wed, 17 Dec 2025 21:32:21 +0100 Subject: [PATCH 3/5] insert a reasonnable default PATH into the chroot Signed-off-by: jbtrystram --- crates/lib/src/bootloader.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 53156d1dc..fe08471d5 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -148,6 +148,13 @@ pub(crate) fn install_via_bootupd( }; let install_result = bootupctl .args(chroot_args) + // Inject a reasonnable PATH here so we find the required tools + // when running chrooted in the deployment. Testing show that + // the default $PATH value in the chroot is insufficient. + .env( + "PATH", + "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", + ) .args(["backend", "install", "--write-uuid"]) .args(verbose) .args(bootupd_opts.iter().copied().flatten()) From 0bc7df564a4d0b6c2dceb01c688cf563f745fccf Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Wed, 7 Jan 2026 13:45:08 +0100 Subject: [PATCH 4/5] test: just bind-mount target/boot in deploymnt/boot --- crates/lib/src/bootloader.rs | 41 +++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index fe08471d5..2ff2d52a7 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -7,12 +7,10 @@ use camino::Utf8Path; use cap_std_ext::cap_std::fs::Dir; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; -use tempfile::TempDir; use bootc_blockdev::{Partition, PartitionTable}; use bootc_mount as mount; use rustix::mount::UnmountFlags; -use rustix::path::Arg; use crate::bootc_composefs::boot::{get_sysroot_parent_dev, mount_esp, SecurebootKeys}; use crate::{discoverable_partition_specification, utils}; @@ -103,21 +101,19 @@ pub(crate) fn install_via_bootupd( // let's bind mount it to a temp mountpoint under /run // so it gets carried over in the chroot. - let rootfs_mountpoint: TempDir; + // let rootfs_mountpoint: TempDir; let rootfs_mount = if rootfs.starts_with("/run") { rootfs.as_str() } else { - rootfs_mountpoint = tempfile::tempdir_in("/run")?; - rustix::mount::mount_bind_recursive( - rootfs.as_std_path(), - &rootfs_mountpoint.path().to_owned(), - )?; - rootfs_mountpoint - .path() - .to_str() - .expect("Invalid tempdir path") + "/" }; + // rootfs_mountpoint + // .path() + // .to_str() + // .expect("Invalid tempdir path") + //}; + // We mount the linux API file systems into the target deployment before chrooting // so bootupd can find the proper backing device. // xref https://systemd.io/API_FILE_SYSTEMS/ @@ -132,6 +128,21 @@ pub(crate) fn install_via_bootupd( tracing::debug!("bind mounting {}", dest.display()); rustix::mount::mount_bind_recursive(src, dest)?; } + // WIP : let's try to bind-mount /target/boot into the deployment as well rather than bind-mounting the whole thing?? + if !rootfs.starts_with("/run") { + tracing::debug!( + "We need to access the target /boot filesystem so let's also bind-mount it" + ); + let trgt_boot = rootfs.as_std_path().join("boot"); + let chrooted_boot = target_root.join_os("boot"); + tracing::debug!( + "bind-mounting {} in {}", + &trgt_boot.display(), + &chrooted_boot.display() + ); + rustix::mount::mount_bind_recursive(trgt_boot, chrooted_boot)?; + } + // Append the `bootupctl` command, it will be passed as // an argument to chroot vec![target_root.as_str(), "bootupctl"] @@ -164,6 +175,12 @@ pub(crate) fn install_via_bootupd( // Clean up the mounts after ourselves if let Some(target_root) = abs_deployment_path { + if let Err(e) = rustix::mount::unmount( + target_root.join("boot").into_std_path_buf(), + UnmountFlags::DETACH, + ) { + tracing::warn!("Error unmounting target/boot: {e}"); + } for dir in bind_mount_dirs { let mount = target_root .join(dir.strip_prefix("/").unwrap()) From 93aaa72bbfa07d3461d1c6d73e47d860529e94fc Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Fri, 9 Jan 2026 09:44:43 +0100 Subject: [PATCH 5/5] wip --- crates/lib/src/bootloader.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 2ff2d52a7..f34cd7ed2 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -108,12 +108,6 @@ pub(crate) fn install_via_bootupd( "/" }; - // rootfs_mountpoint - // .path() - // .to_str() - // .expect("Invalid tempdir path") - //}; - // We mount the linux API file systems into the target deployment before chrooting // so bootupd can find the proper backing device. // xref https://systemd.io/API_FILE_SYSTEMS/