install/bootupd: chroot to deployment

When `--src-imgref` is passed, the deployed systemd does not match the
running environnement. In this case, let's run bootupd from inside
the deployment. This makes sure we are using the binaries
shipped in the image (and relevant config files such as grub fragements).

We use bwrap to set up the chroot for a easier handling of the API
filesystems.

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 #1559
Also see #1455

Assisted-by: OpenCode (Opus 4.5)
Signed-off-by: jbtrystram <jbtrystram@redhat.com>

Author: jbtrystram

.github/workflows/ci.yml

@@ -92,6 +92,10 @@ jobs:
           # Install tests
           sudo bootc-integration-tests install-alongside localhost/bootc-install
 
+          # inspect system state after the install tests.
+          sudo lsblk
+          sudo mount
+          
           # system-reinstall-bootc tests
           cargo build --release -p system-reinstall-bootc
 

Cargo.lock

@@ -2,7 +2,7 @@ use std::fs::create_dir_all;
 use std::process::Command;
 
 use anyhow::{Context, Result, anyhow, bail};
-use bootc_utils::CommandRunExt;
+use bootc_utils::{BwrapCmd, CommandRunExt};
 use camino::Utf8Path;
 use cap_std_ext::cap_std::fs::Dir;
 use cap_std_ext::dirext::CapStdExtDirExt;
@@ -91,22 +91,71 @@ 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()]
+    // When not running inside the target container (through `--src-imgref`) we use
+    // will bwrap as a chroot to run bootupctl from the deployment.
+    // This makes sure we use binaries from the target image rather than the buildroot.
+    // In that case, the target rootfs is replaced with `/` because this is just used by
+    // bootupd to find the backing device.
+    let rootfs_mount = if deployment_path.is_none() {
+        rootfs.as_str()
     } else {
-        vec![]
+        "/"
     };
-    let devpath = device.path();
+
     println!("Installing bootloader via bootupd");
-    Command::new("bootupctl")
-        .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()])
-        .log_debug()
-        .run_inherited_with_cmd_context()
+
+    // Build the bootupctl arguments
+    let mut bootupd_args: Vec<&str> = vec!["backend", "install", "--write-uuid"];
+    if let Some(v) = verbose {
+        bootupd_args.push(v);
+    }
+
+    if let Some(ref opts) = bootupd_opts {
+        bootupd_args.extend(opts.iter().copied());
+    }
+    bootupd_args.extend(["--device", device.path().as_str(), rootfs_mount]);
+
+    // Run inside a bwrap container. It takes care of mounting and creating
+    // the necessary API filesystems in the target deployment and acts as
+    // a nicer `chroot`.
+    if let Some(deploy) = deployment_path {
+        let target_root = rootfs.join(deploy);
+        let boot_path = rootfs.join("boot");
+
+        tracing::debug!("Running bootupctl via bwrap in {}", target_root);
+
+        // Prepend "bootupctl" to the args for bwrap
+        let mut bwrap_args = vec!["bootupctl"];
+        bwrap_args.extend(bootupd_args);
+
+        let mut cmd = BwrapCmd::new(&target_root)
+            // Bind mount /boot from the physical target root so bootupctl can find
+            // the boot partition and install the bootloader there
+            .bind(&boot_path, &"/boot")
+            // Bind the target block device inside the bwrap container so bootupctl can access it
+            .bind_device(device.path().as_str());
+
+        // Also bind all partitions of the tafet block device
+        for partition in &device.partitions {
+            cmd = cmd.bind_device(&partition.node);
+        }
+
+        // The $PATH in the bwrap env is not complete enough for some images
+        // so we inject a reasonnable default.
+        // This is causing bootupctl and/or sfdisk binaries
+        // to be not found with fedora 43.
+        cmd.setenv(
+            "PATH",
+            "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin",
+        )
+        .run(bwrap_args)
+    } else {
+        // Running directly without chroot
+        Command::new("bootupctl")
+            .args(&bootupd_args)
+            .log_debug()
+            .run_inherited_with_cmd_context()
+    }
 }
 
 #[context("Installing bootloader")]

crates/lib/src/bootloader.rs

@@ -10,6 +10,7 @@ repository = "https://github.com/bootc-dev/bootc"
 # Workspace dependencies
 anstream = { workspace = true }
 anyhow = { workspace = true }
+cap-std-ext = {workspace = true, features = ["fs_utf8"] }
 chrono = { workspace = true, features = ["std"] }
 owo-colors = { workspace = true }
 rustix = { workspace = true }

crates/utils/Cargo.toml

@@ -0,0 +1,106 @@
+/// Builder for running commands inside a target os tree using bubblewrap (bwrap).
+use std::borrow::Cow;
+use std::ffi::OsStr;
+use std::os::fd::AsRawFd;
+use std::process::Command;
+
+use anyhow::Result;
+use cap_std_ext::camino::{Utf8Path, Utf8PathBuf};
+use cap_std_ext::cap_std::fs::Dir;
+
+use crate::CommandRunExt;
+
+/// Builder for running commands inside a target directory using bwrap.
+#[derive(Debug)]
+pub struct BwrapCmd<'a> {
+    /// The target directory to use as root for the container
+    chroot_path: Cow<'a, Utf8Path>,
+    /// Bind mounts in format (source, target)
+    bind_mounts: Vec<(&'a str, &'a str)>,
+    /// Device nodes to bind into the container
+    devices: Vec<&'a str>,
+    /// Environment variables to set
+    env_vars: Vec<(&'a str, &'a str)>,
+}
+
+impl<'a> BwrapCmd<'a> {
+    /// Create a new BwrapCmd builder with a root directory as a File Descriptor.
+    #[allow(dead_code)]
+    pub fn new_with_dir(path: &'a Dir) -> Self {
+        let fd_path: String = format!("/proc/self/fd/{}", path.as_raw_fd());
+        Self {
+            chroot_path: Cow::Owned(Utf8PathBuf::from(&fd_path)),
+            bind_mounts: Vec::new(),
+            devices: Vec::new(),
+            env_vars: Vec::new(),
+        }
+    }
+
+    /// Create a new BwrapCmd builder with a root directory
+    pub fn new(path: &'a Utf8Path) -> Self {
+        Self {
+            chroot_path: Cow::Borrowed(path),
+            bind_mounts: Vec::new(),
+            devices: Vec::new(),
+            env_vars: Vec::new(),
+        }
+    }
+
+    /// Add a bind mount from source to target inside the container.
+    pub fn bind(
+        mut self,
+        source: &'a impl AsRef<Utf8Path>,
+        target: &'a impl AsRef<Utf8Path>,
+    ) -> Self {
+        self.bind_mounts
+            .push((source.as_ref().as_str(), target.as_ref().as_str()));
+        self
+    }
+
+    /// Bind a device node into the container.
+    pub fn bind_device(mut self, device: &'a str) -> Self {
+        self.devices.push(device);
+        self
+    }
+
+    /// Set an environment variable for the command.
+    pub fn setenv(mut self, key: &'a str, value: &'a str) -> Self {
+        self.env_vars.push((key, value));
+        self
+    }
+
+    /// Run the specified command inside the container.
+    pub fn run<S: AsRef<OsStr>>(self, args: impl IntoIterator<Item = S>) -> Result<()> {
+        let mut cmd = Command::new("bwrap");
+
+        // Bind the root filesystem
+        cmd.args(["--bind", self.chroot_path.as_str(), "/"]);
+
+        // Setup API filesystems
+        // See https://systemd.io/API_FILE_SYSTEMS/
+        cmd.args(["--proc", "/proc"]);
+        cmd.args(["--dev", "/dev"]);
+        cmd.args(["--ro-bind", "/sys", "/sys"]);
+
+        // Add bind mounts
+        for (source, target) in &self.bind_mounts {
+            cmd.args(["--bind", source, target]);
+        }
+
+        // Add device bind mounts
+        for device in self.devices {
+            cmd.args(["--dev-bind", device, device]);
+        }
+
+        // Add environment variables
+        for (key, value) in &self.env_vars {
+            cmd.args(["--setenv", key, value]);
+        }
+
+        // Command to run
+        cmd.arg("--");
+        cmd.args(args);
+
+        cmd.log_debug().run_inherited_with_cmd_context()
+    }
+}

crates/utils/src/bwrap.rs

@@ -2,20 +2,22 @@
 //! things here that only depend on the standard library and
 //! "core" crates.
 //!
+mod bwrap;
+pub use bwrap::*;
 mod command;
 pub use command::*;
-mod path;
-pub use path::*;
 mod iterators;
 pub use iterators::*;
-mod timestamp;
-pub use timestamp::*;
-mod tracing_util;
-pub use tracing_util::*;
+mod path;
+pub use path::*;
 /// Re-execute the current process
 pub mod reexec;
 mod result_ext;
 pub use result_ext::*;
+mod timestamp;
+pub use timestamp::*;
+mod tracing_util;
+pub use tracing_util::*;
 
 /// The name of our binary
 pub const NAME: &str = "bootc";