install: add target_root_path for RootSetup

.github/workflows/ci.yml

@@ -197,12 +197,24 @@ jobs:
       - name: Unit and container integration tests
         run: just test-container
 
-      - name: Run TMT tests
+      - name: Run TMT integration tests
         run: |
           if [ "${{ matrix.variant }}" = "composefs-sealeduki-sdboot" ]; then
             just test-composefs
           else
-            just test-tmt
+            just test-tmt integration
+          fi
+          just clean-local-images
+
+      - name: Run TMT test about bootc install on coreos
+        run: |
+          # Only test fedora-43 on fedora-coreos:testing-devel
+          if [ "${{ matrix.test_os }}" = "fedora-43" ] && [ "${{ matrix.variant }}" = "ostree" ]; then
+            just build-testimage-coreos target/packages
+            just test-tmt-on-coreos plan-bootc-install-on-coreos
+            just clean-local-images
+          else
+            echo "skipped"
           fi
 
       - name: Archive TMT logs

Justfile

@@ -66,6 +66,9 @@ _git-build-vars:
     echo "SOURCE_DATE_EPOCH=${SOURCE_DATE_EPOCH}"
     echo "VERSION=${VERSION}"
 
+# Needed by bootc install on ostree
+fedora-coreos := "quay.io/fedora/fedora-coreos:testing-devel"
+
 # The default target: build the container image from current sources.
 # Note commonly you might want to override the base image via e.g.
 # `just build --build-arg=base=quay.io/fedora/fedora-bootc:42`
@@ -179,6 +182,8 @@ validate:
 #
 # To run an individual test, pass it as an argument like:
 # `just test-tmt readonly`
+#
+# To run the integration tests, execute `just test-tmt integration`
 test-tmt *ARGS: build-integration-test-image _build-upgrade-image
     @just test-tmt-nobuild {{ARGS}}
 
@@ -192,6 +197,19 @@ _build-upgrade-image:
 test-tmt-nobuild *ARGS:
     cargo xtask run-tmt --env=BOOTC_variant={{variant}} --upgrade-image={{integration_upgrade_img}} {{integration_img}} {{ARGS}}
 
+# Build test container image for testing on coreos with SKIP_CONFIGS=1,
+# without configs and no curl container image
+build-testimage-coreos PATH:
+    @just build-from-package {{PATH}}
+    cd hack && podman build {{base_buildargs}} --build-arg SKIP_CONFIGS=1 -t {{integration_img}}-coreos -f Containerfile .
+
+# Run test bootc install on FCOS
+# BOOTC_target is `bootc-integration-coreos`, it will be used for bootc install.
+# Run `just build-testimage-coreos target/packages` to build test image firstly,
+# then run `just test-tmt-on-coreos plan-bootc-install-on-coreos`
+test-tmt-on-coreos *ARGS:
+    cargo xtask run-tmt --env=BOOTC_variant={{variant}} --env=BOOTC_target={{integration_img}}-coreos:latest {{fedora-coreos}} {{ARGS}}
+
 # Cleanup all test VMs created by tmt tests
 tmt-vm-cleanup:
     bcvk libvirt rm --stop --force --label bootc.test=1
@@ -206,6 +224,8 @@ test-container: build-units build-integration-test-image
 clean-local-images:
     podman images --filter "label={{testimage_label}}"
     podman images --filter "label={{testimage_label}}" --format "{{{{.ID}}" | xargs -r podman rmi -f
+    podman image prune -f
+    podman rmi {{fedora-coreos}} -f
 
 # Print the container image reference for a given short $ID-VERSION_ID for NAME
 # and 'base' or 'buildroot-base' for TYPE (base image type)

crates/lib/src/bootloader.rs

@@ -5,12 +5,13 @@ use anyhow::{anyhow, bail, Context, Result};
 use bootc_utils::CommandRunExt;
 use camino::Utf8Path;
 use cap_std_ext::cap_std::fs::Dir;
+use cap_std_ext::dirext::CapStdExtDirExt;
 use fn_error_context::context;
 
 use bootc_blockdev::{Partition, PartitionTable};
 use bootc_mount as mount;
 
-use crate::bootc_composefs::boot::{mount_esp, SecurebootKeys};
+use crate::bootc_composefs::boot::{get_sysroot_parent_dev, mount_esp, SecurebootKeys};
 use crate::{discoverable_partition_specification, utils};
 
 /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel)
@@ -30,6 +31,42 @@ pub(crate) fn esp_in(device: &PartitionTable) -> Result<&Partition> {
         .ok_or(anyhow::anyhow!("ESP not found in partition table"))
 }
 
+/// Get esp partition node based on the root dir
+pub(crate) fn get_esp_partition_node(root: &Dir) -> Result<Option<String>> {
+    let device = get_sysroot_parent_dev(&root)?;
+    let base_partitions = bootc_blockdev::partitions_of(Utf8Path::new(&device))?;
+    let esp = base_partitions.find_partition_of_esp()?;
+    Ok(esp.map(|v| v.node.clone()))
+}
+
+/// Mount ESP part at /boot/efi
+pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) -> Result<()> {
+    let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR);
+    let Some(esp_fd) = root
+        .open_dir_optional(&efi_path)
+        .context("Opening /boot/efi")?
+    else {
+        return Ok(());
+    };
+
+    let Some(false) = esp_fd.is_mountpoint(".")? else {
+        return Ok(());
+    };
+
+    tracing::debug!("Not a mountpoint: /boot/efi");
+    // On ostree env with enabled composefs, should be /target/sysroot
+    let physical_root = if is_ostree {
+        &root.open_dir("sysroot").context("Opening /sysroot")?
+    } else {
+        root
+    };
+    if let Some(esp_part) = get_esp_partition_node(physical_root)? {
+        bootc_mount::mount(&esp_part, &root_path.join(&efi_path))?;
+        tracing::debug!("Mounted {esp_part} at /boot/efi");
+    }
+    Ok(())
+}
+
 /// Determine if the invoking environment contains bootupd, and if there are bootupd-based
 /// updates in the target root.
 #[context("Querying for bootupd")]

crates/lib/src/install.rs

@@ -779,6 +779,17 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result
     // Another implementation: https://github.com/coreos/coreos-assembler/blob/3cd3307904593b3a131b81567b13a4d0b6fe7c90/src/create_disk.sh#L295
     crate::lsm::ensure_dir_labeled(rootfs_dir, "", Some("/".into()), 0o755.into(), sepolicy)?;
 
+    // If we're installing alongside existing ostree and there's a separate boot partition,
+    // we need to mount it to the sysroot's /boot so ostree can write bootloader entries there
+    if has_ostree && root_setup.boot.is_some() {
+        if let Some(boot) = &root_setup.boot {
+            let source_boot = &boot.source;
+            let target_boot = root_setup.physical_root_path.join(BOOT);
+            tracing::debug!("Mount {source_boot} to {target_boot} on ostree");
+            bootc_mount::mount(source_boot, &target_boot)?;
+        }
+    }
+
     // And also label /boot AKA xbootldr, if it exists
     if rootfs_dir.try_exists("boot")? {
         crate::lsm::ensure_dir_labeled(rootfs_dir, "boot", None, 0o755.into(), sepolicy)?;
@@ -1122,6 +1133,8 @@ pub(crate) struct RootSetup {
     pub(crate) physical_root_path: Utf8PathBuf,
     /// Directory file descriptor for the above physical root.
     pub(crate) physical_root: Dir,
+    /// Target root path /target.
+    pub(crate) target_root_path: Option<Utf8PathBuf>,
     pub(crate) rootfs_uuid: Option<String>,
     /// True if we should skip finalizing
     skip_finalize: bool,
@@ -1575,7 +1588,10 @@ async fn install_with_sysroot(
             Bootloader::Grub => {
                 crate::bootloader::install_via_bootupd(
                     &rootfs.device_info,
-                    &rootfs.physical_root_path,
+                    &rootfs
+                        .target_root_path
+                        .clone()
+                        .unwrap_or(rootfs.physical_root_path.clone()),
                     &state.config_opts,
                     Some(&deployment_path.as_str()),
                 )?;
@@ -1922,30 +1938,66 @@ fn remove_all_in_dir_no_xdev(d: &Dir, mount_err: bool) -> Result<()> {
     anyhow::Ok(())
 }
 
+#[context("Removing boot directory content except loader dir on ostree")]
+fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> {
+    let entries = bootdir
+        .entries()
+        .context("Reading boot directory entries")?;
+
+    for entry in entries {
+        let entry = entry.context("Reading directory entry")?;
+        let file_name = entry.file_name();
+        let file_name = if let Some(n) = file_name.to_str() {
+            n
+        } else {
+            anyhow::bail!("Invalid non-UTF8 filename: {file_name:?} in /boot");
+        };
+
+        // TODO: Preserve basically everything (including the bootloader entries
+        // on non-ostree) by default until the very end of the install. And ideally
+        // make the "commit" phase an optional step after.
+        if is_ostree && file_name.starts_with("loader") {
+            continue;
+        }
+
+        let etype = entry.file_type()?;
+        if etype == FileType::dir() {
+            // Open the directory and remove its contents
+            if let Some(subdir) = bootdir.open_dir_noxdev(&file_name)? {
+                remove_all_in_dir_no_xdev(&subdir, false)
+                    .with_context(|| format!("Removing directory contents: {}", file_name))?;
+                bootdir.remove_dir(&file_name)?;
+            }
+        } else {
+            bootdir
+                .remove_file_optional(&file_name)
+                .with_context(|| format!("Removing file: {}", file_name))?;
+        }
+    }
+    Ok(())
+}
+
 #[context("Removing boot directory content")]
-fn clean_boot_directories(rootfs: &Dir, is_ostree: bool) -> Result<()> {
+fn clean_boot_directories(rootfs: &Dir, rootfs_path: &Utf8Path, is_ostree: bool) -> Result<()> {
     let bootdir =
         crate::utils::open_dir_remount_rw(rootfs, BOOT.into()).context("Opening /boot")?;
 
-    if is_ostree {
-        // On ostree systems, the boot directory already has our desired format, we should only
-        // remove the bootupd-state.json file to avoid bootupctl complaining it already exists.
-        bootdir
-            .remove_file_optional("bootupd-state.json")
-            .context("removing bootupd-state.json")?;
-    } else {
-        // This should not remove /boot/efi note.
-        remove_all_in_dir_no_xdev(&bootdir, false).context("Emptying /boot")?;
-        // TODO: Discover the ESP the same way bootupd does it; we should also
-        // support not wiping the ESP.
-        if ARCH_USES_EFI {
-            if let Some(efidir) = bootdir
-                .open_dir_optional(crate::bootloader::EFI_DIR)
-                .context("Opening /boot/efi")?
-            {
-                remove_all_in_dir_no_xdev(&efidir, false)
-                    .context("Emptying EFI system partition")?;
-            }
+    if ARCH_USES_EFI {
+        // On booted FCOS, esp is not mounted by default
+        // Mount ESP part at /boot/efi before clean
+        crate::bootloader::mount_esp_part(&rootfs, &rootfs_path, is_ostree)?;
+    }
+
+    // This should not remove /boot/efi note.
+    remove_all_except_loader_dirs(&bootdir, is_ostree).context("Emptying /boot")?;
+
+    // TODO: we should also support not wiping the ESP.
+    if ARCH_USES_EFI {
+        if let Some(efidir) = bootdir
+            .open_dir_optional(crate::bootloader::EFI_DIR)
+            .context("Opening /boot/efi")?
+        {
+            remove_all_in_dir_no_xdev(&efidir, false).context("Emptying EFI system partition")?;
         }
     }
 
@@ -2082,6 +2134,18 @@ pub(crate) async fn install_to_filesystem(
         .context("Mounting host / to {ALONGSIDE_ROOT_MOUNT}")?;
     }
 
+    let target_root_path = fsopts.root_path.clone();
+    // Get a file descriptor for the root path /target
+    let target_rootfs_fd =
+        Dir::open_ambient_dir(&target_root_path, cap_std::ambient_authority())
+            .with_context(|| format!("Opening target root directory {target_root_path}"))?;
+
+    tracing::debug!("Target root filesystem: {target_root_path}");
+
+    if let Some(false) = target_rootfs_fd.is_mountpoint(".")? {
+        anyhow::bail!("Not a mountpoint: {target_root_path}");
+    }
+
     // Check that the target is a directory
     {
         let root_path = &fsopts.root_path;
@@ -2095,10 +2159,7 @@ pub(crate) async fn install_to_filesystem(
 
     // Check to see if this happens to be the real host root
     if !fsopts.acknowledge_destructive {
-        let root_path = &fsopts.root_path;
-        let rootfs_fd = Dir::open_ambient_dir(root_path, cap_std::ambient_authority())
-            .with_context(|| format!("Opening target root directory {root_path}"))?;
-        warn_on_host_root(&rootfs_fd)?;
+        warn_on_host_root(&target_rootfs_fd)?;
     }
 
     // If we're installing to an ostree root, then find the physical root from
@@ -2114,7 +2175,8 @@ pub(crate) async fn install_to_filesystem(
     };
 
     // Get a file descriptor for the root path
-    let rootfs_fd = {
+    // It will be /target/sysroot on ostree OS, or will be /target
+    let rootfs_fd = if is_already_ostree {
         let root_path = &fsopts.root_path;
         let rootfs_fd = Dir::open_ambient_dir(&fsopts.root_path, cap_std::ambient_authority())
             .with_context(|| format!("Opening target root directory {root_path}"))?;
@@ -2125,6 +2187,8 @@ pub(crate) async fn install_to_filesystem(
             anyhow::bail!("Not a mountpoint: {root_path}");
         }
         rootfs_fd
+    } else {
+        target_rootfs_fd.try_clone()?
     };
 
     match fsopts.replace {
@@ -2134,7 +2198,9 @@ pub(crate) async fn install_to_filesystem(
             tokio::task::spawn_blocking(move || remove_all_in_dir_no_xdev(&rootfs_fd, true))
                 .await??;
         }
-        Some(ReplaceMode::Alongside) => clean_boot_directories(&rootfs_fd, is_already_ostree)?,
+        Some(ReplaceMode::Alongside) => {
+            clean_boot_directories(&target_rootfs_fd, &target_root_path, is_already_ostree)?
+        }
         None => require_empty_rootdir(&rootfs_fd)?,
     }
 
@@ -2179,7 +2245,7 @@ pub(crate) async fn install_to_filesystem(
 
     let boot_is_mount = {
         let root_dev = rootfs_fd.dir_metadata()?.dev();
-        let boot_dev = rootfs_fd
+        let boot_dev = target_rootfs_fd
             .symlink_metadata_optional(BOOT)?
             .ok_or_else(|| {
                 anyhow!("No /{BOOT} directory found in root; this is is currently required")
@@ -2190,9 +2256,10 @@ pub(crate) async fn install_to_filesystem(
     };
     // Find the UUID of /boot because we need it for GRUB.
     let boot_uuid = if boot_is_mount {
-        let boot_path = fsopts.root_path.join(BOOT);
+        let boot_path = target_root_path.join(BOOT);
+        tracing::debug!("boot_path={boot_path}");
         let u = bootc_mount::inspect_filesystem(&boot_path)
-            .context("Inspecting /{BOOT}")?
+            .with_context(|| format!("Inspecting /{BOOT}"))?
             .uuid
             .ok_or_else(|| anyhow!("No UUID found for /{BOOT}"))?;
         Some(u)
@@ -2279,6 +2346,7 @@ pub(crate) async fn install_to_filesystem(
         device_info,
         physical_root_path: fsopts.root_path,
         physical_root: rootfs_fd,
+        target_root_path: Some(target_root_path.clone()),
         rootfs_uuid: inspect.uuid.clone(),
         boot,
         kargs,

crates/lib/src/install/baseline.rs

@@ -491,6 +491,7 @@ pub(crate) fn install_create_rootfs(
         device_info,
         physical_root_path,
         physical_root,
+        target_root_path: None,
         rootfs_uuid: Some(root_uuid.to_string()),
         boot,
         kargs,