From e5c5ae28f2e13592fbb0fe01cd339e74aebd9eec Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Mon, 17 Aug 2026 10:13:54 -0500 Subject: [PATCH 1/6] install: Add /var and /etc migration options Preserve existing-root data while providing a clear, reliable /etc merge path for migration. Assisted-by: AI Signed-off-by: ckyrouac --- crates/etc-merge/src/lib.rs | 172 +++++++- crates/lib/src/cli.rs | 5 + crates/lib/src/install.rs | 136 +++++- crates/lib/src/install/config.rs | 7 + crates/lib/src/install/migrate.rs | 395 ++++++++++++++++++ docs/src/bootc-installation.7.md | 20 +- .../man/bootc-install-to-existing-root.8.md | 43 +- 7 files changed, 740 insertions(+), 38 deletions(-) create mode 100644 crates/lib/src/install/migrate.rs diff --git a/crates/etc-merge/src/lib.rs b/crates/etc-merge/src/lib.rs index 66d74ad1be..daa87770f9 100644 --- a/crates/etc-merge/src/lib.rs +++ b/crates/etc-merge/src/lib.rs @@ -470,6 +470,28 @@ pub fn compute_diff( pristine_etc_files: &FileSystem, current_etc_files: &FileSystem, new_etc_files: &FileSystem, +) -> anyhow::Result { + compute_diff_impl(pristine_etc_files, current_etc_files, new_etc_files, true) +} + +/// Computes differences without treating paths absent from current `/etc` as deletions. +/// +/// This is intended for package-mode to image-mode migration, where the current +/// system may predate defaults in the new image. In that case, absence does not +/// imply that an administrator deleted the path. +pub fn compute_diff_without_deletions( + pristine_etc_files: &FileSystem, + current_etc_files: &FileSystem, + new_etc_files: &FileSystem, +) -> anyhow::Result { + compute_diff_impl(pristine_etc_files, current_etc_files, new_etc_files, false) +} + +fn compute_diff_impl( + pristine_etc_files: &FileSystem, + current_etc_files: &FileSystem, + new_etc_files: &FileSystem, + detect_deletions: bool, ) -> anyhow::Result { let mut diff = Diff { added: vec![], @@ -488,12 +510,14 @@ pub fn compute_diff( &mut diff, )?; - get_deletions( - &pristine_etc_files.root, - ¤t_etc_files.root, - PathBuf::new(), - &mut diff, - )?; + if detect_deletions { + get_deletions( + &pristine_etc_files.root, + ¤t_etc_files.root, + PathBuf::new(), + &mut diff, + )?; + } Ok(diff) } @@ -788,7 +812,10 @@ fn merge_leaf( }; if matches!(new_inode, Some(Inode::Directory(..))) { - anyhow::bail!("Modified config file {file:?} newly defaults to directory. Cannot merge") + anyhow::bail!( + "Cannot merge host path {file:?}: the new image contains a directory at this path; \ + resolve the conflict manually before retrying" + ); }; // If a new file with the same path exists, we delete it @@ -802,7 +829,13 @@ fn merge_leaf( } else { current_etc_fd .copy(&file, new_etc_fd, &file) - .with_context(|| format!("Copying file {file:?}"))?; + .with_context(|| { + format!( + "Merge conflict for host /etc path {file:?}: unable to copy it into the new /etc; \ + inspect and resolve conflicting path components (especially absolute symlinks \ + that escape /etc) so the host file can be copied, then retry" + ) + })?; }; rustix::fs::chownat( @@ -1049,6 +1082,54 @@ mod tests { Ok(()) } + #[test] + fn migration_diff_preserves_image_only_default() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let pristine = tempdir.open_dir("pristine_etc")?; + let current = tempdir.open_dir("current_etc")?; + let new = tempdir.open_dir("new_etc")?; + + pristine.write("image-only.conf", b"image default")?; + new.write("image-only.conf", b"image default")?; + + let (pristine_tree, current_tree, new_tree) = + traverse_etc(&pristine, ¤t, Some(&new))?; + let new_tree = new_tree.unwrap(); + let diff = compute_diff_without_deletions(&pristine_tree, ¤t_tree, &new_tree)?; + + merge(¤t, ¤t_tree, &new, &new_tree, &diff)?; + + assert_eq!(new.read("image-only.conf")?, b"image default"); + Ok(()) + } + + #[test] + fn compute_diff_still_detects_deletions() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let pristine = tempdir.open_dir("pristine_etc")?; + let current = tempdir.open_dir("current_etc")?; + let new = tempdir.open_dir("new_etc")?; + + pristine.write("deleted.conf", b"old default")?; + + let (pristine_tree, current_tree, new_tree) = + traverse_etc(&pristine, ¤t, Some(&new))?; + let diff = compute_diff(&pristine_tree, ¤t_tree, &new_tree.as_ref().unwrap())?; + + assert_eq!(diff.removed, [PathBuf::from("deleted.conf")]); + Ok(()) + } + fn compare_meta(meta1: Metadata, meta2: Metadata) -> bool { return meta1.is_file() == meta2.is_file() && meta1.is_dir() == meta2.is_dir() @@ -1291,12 +1372,79 @@ mod tests { let merge_res = merge(&c, ¤t_etc_files, &n, &new_etc_files.unwrap(), &diff); - assert!(merge_res.is_err()); - assert_eq!( - merge_res.unwrap_err().root_cause().to_string(), - "Modified config file \"file-to-dir\" newly defaults to directory. Cannot merge" + // The directory should still exist in new_etc (image's directory wins) + let error = merge_res.expect_err("file-to-directory conflict must fail migration"); + let error = format!("{error:#}"); + assert!(error.contains("Cannot merge host path")); + assert!(error.contains("resolve the conflict manually before retrying")); + + Ok(()) + } + + #[test] + fn regular_file_copy_conflict_returns_actionable_error() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let p = tempdir.open_dir("pristine_etc")?; + let c = tempdir.open_dir("current_etc")?; + let n = tempdir.open_dir("new_etc")?; + + p.write("conflict", "pristine contents")?; + c.write("conflict", "host contents")?; + n.write("conflict", "image contents")?; + + let (pristine_tree, current_tree, new_tree) = traverse_etc(&p, &c, Some(&n))?; + let new_tree = new_tree.unwrap(); + let diff = compute_diff(&pristine_tree, ¤t_tree, &new_tree)?; + + // Make the source path escape the /etc sandbox after traversal so the + // snapshot still identifies it as a modified regular host file. + c.remove_file("conflict")?; + symlinkat("/outside-etc", &c, "conflict")?; + + let error = merge(&c, ¤t_tree, &n, &new_tree, &diff) + .expect_err("copy through an absolute symlink outside current_etc must fail"); + let error = format!("{error:#}"); + assert!(error.contains("Merge conflict"), "{error}"); + assert!(error.contains("host /etc path \"conflict\""), "{error}"); + assert!(error.contains("absolute symlinks"), "{error}"); + assert!( + error.contains("resolve conflicting path components"), + "{error}" ); Ok(()) } + + #[test] + fn regular_host_file_replaces_new_image_symlink() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let p = tempdir.open_dir("pristine_etc")?; + let c = tempdir.open_dir("current_etc")?; + let n = tempdir.open_dir("new_etc")?; + + p.write("conflict", "pristine contents")?; + c.write("conflict", "host contents")?; + symlinkat("/outside-etc", &n, "conflict")?; + + let (pristine_tree, current_tree, new_tree) = traverse_etc(&p, &c, Some(&n))?; + let new_tree = new_tree.unwrap(); + let diff = compute_diff(&pristine_tree, ¤t_tree, &new_tree)?; + + merge(&c, ¤t_tree, &n, &new_tree, &diff)?; + + assert_eq!(n.read("conflict")?, b"host contents"); + assert!(n.metadata("conflict")?.is_file()); + + Ok(()) + } } diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 0a7c0e5987..fc0bef4482 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -346,6 +346,10 @@ pub(crate) enum InstallOpts { /// the running host root filesystem. Currently, the host root filesystem's `/boot` partition /// will be wiped, but the content of the existing root will otherwise be retained, and will /// need to be cleaned up if desired when rebooted into the new root. + /// + /// When migrating from a package-mode system, use `--preserve-var` to copy `/var` data + /// into the new deployment and write a GRUB rollback entry, and `--merge-etc` to carry + /// forward `/etc` customisations via a 3-way merge. ToExistingRoot(crate::install::InstallToExistingRootOpts), /// Nondestructively create a fresh installation state inside an existing bootc system. /// @@ -2382,6 +2386,7 @@ async fn run_from_opt(opt: Opt) -> Result { InstallOpts::ToFilesystem(opts) => { crate::install::install_to_filesystem(opts, false, crate::install::Cleanup::Skip) .await + .map(|_| ()) } InstallOpts::ToExistingRoot(opts) => { crate::install::install_to_existing_root(opts).await diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 03eeb22172..6889b8f3ec 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -144,6 +144,7 @@ mod aleph; pub(crate) mod baseline; pub(crate) mod completion; pub(crate) mod config; +pub(crate) mod migrate; mod osbuild; pub(crate) mod osconfig; @@ -620,6 +621,52 @@ pub(crate) struct InstallToExistingRootOpts { #[clap(flatten)] pub(crate) composefs_opts: InstallComposefsOpts, + + /// Preserve the running system's `/var` data into the new bootc deployment. + /// + /// After a plain `bootc install to-existing-root`, the new deployment's + /// `/var` is initially empty (ostree bind-mounts it from a fresh directory). + /// Passing this flag performs the following additional steps **after** the + /// core install completes: + /// + /// 1. `/var` content is copied into the new deployment: + /// - **Reflink copy** (btrfs / XFS): `cp --reflink=always` performs an + /// instantaneous copy-on-write clone — no extra disk space consumed. + /// - **Full copy** (filesystems without reflinks): data is copied into + /// the deployment's writable `/var` directory. + /// 2. Paths listed with `--preserve-var-skip` (or in install configuration) + /// are excluded. Rollback preservation is intentionally independent. + /// + /// The running system's `root_path` must be mounted (e.g. `-v /:/target`). + #[clap(long)] + pub(crate) preserve_var: bool, + + /// Relative paths below `/var` to leave out while preserving `/var`. + #[clap(long = "preserve-var-skip", value_name = "PATH")] + pub(crate) preserve_var_skip: Vec, + + /// Merge the running system's `/etc` customisations into the new deployment. + /// + /// Plain `bootc install to-existing-root` populates the new deployment's + /// `/etc` directly from the image. The running admin's customisations + /// (NIC profiles, SSH host keys, secrets, custom CA certificates, etc.) + /// remain at `/etc` but are not applied to the new deployment. + /// + /// Passing this flag runs a 3-way merge using the `etc-merge` algorithm + /// after the core install completes: + /// + /// A (pristine baseline) = `/usr/etc` — image's shipped defaults + /// B (current live) = `/etc` — running system's `/etc` + /// C (new deployment) = `/etc` — deploy target + /// + /// The diff A→B captures every file the admin changed relative to the image + /// defaults and applies those changes onto C. This is the same algorithm + /// bootc uses during `bootc upgrade`, applied at install time rather than + /// only at upgrade time. + /// + /// The running system's `root_path` must be mounted (e.g. `-v /:/target`). + #[clap(long)] + pub(crate) merge_etc: bool, } #[derive(Debug, clap::Parser, PartialEq, Eq)] @@ -2021,7 +2068,7 @@ async fn install_with_sysroot( boot_uuid: &str, bound_images: BoundImages, has_ostree: bool, -) -> Result<()> { +) -> Result { let ostree = storage.get_ostree()?; let c_storage = storage.get_ensure_imgstore()?; @@ -2088,7 +2135,7 @@ async fn install_with_sysroot( } } - Ok(()) + Ok(camino::Utf8PathBuf::from(deployment_path.to_string())) } enum BoundImages { @@ -2127,7 +2174,11 @@ impl BoundImages { } } -async fn ostree_install(state: &State, rootfs: &RootSetup, cleanup: Cleanup) -> Result<()> { +async fn ostree_install( + state: &State, + rootfs: &RootSetup, + cleanup: Cleanup, +) -> Result { // We verify this upfront because it's currently required by bootupd let boot_uuid = rootfs .get_boot_uuid()? @@ -2139,10 +2190,10 @@ async fn ostree_install(state: &State, rootfs: &RootSetup, cleanup: Cleanup) -> // Initialize the ostree sysroot (repo, stateroot, etc.) - { + let deployment_path = { let (sysroot, has_ostree) = initialize_ostree_root(state, rootfs).await?; - install_with_sysroot( + let deployment_path = install_with_sysroot( state, rootfs, &sysroot, @@ -2165,19 +2216,20 @@ async fn ostree_install(state: &State, rootfs: &RootSetup, cleanup: Cleanup) -> // We must drop the sysroot here in order to close any open file // descriptors. + deployment_path }; // Run this on every install as the penultimate step install_finalize(&rootfs.physical_root_path).await?; - Ok(()) + Ok(deployment_path) } async fn install_to_filesystem_impl( state: &State, rootfs: &mut RootSetup, cleanup: Cleanup, -) -> Result<()> { +) -> Result> { if matches!(state.selinux_state, SELinuxFinalState::ForceTargetDisabled) { rootfs.kargs.extend(&Cmdline::from("selinux=0")); } @@ -2199,7 +2251,7 @@ async fn install_to_filesystem_impl( } } - if state.composefs_options.composefs_backend { + let deployment_path = if state.composefs_options.composefs_backend { let fetch_ref = state.source.composefs_fetch_reference(); let manifest = get_container_manifest_and_config(&fetch_ref).await?; // A capable filesystem gets a strict provisional repository. The @@ -2269,8 +2321,9 @@ async fn install_to_filesystem_impl( ) .context("SELinux labeling of composefs objects")?; } + None } else { - ostree_install(state, rootfs, cleanup).await?; + let deployment_path = ostree_install(state, rootfs, cleanup).await?; // For s390x, we set zipl as the bootloader // this needs to be done after the ostree commit is deployed, @@ -2289,7 +2342,8 @@ async fn install_to_filesystem_impl( .run_capture_stderr() .context("Setting bootloader config to zipl")?; } - } + Some(deployment_path) + }; // As the very last step before filesystem finalization, do a full SELinux // relabel of the physical root filesystem. Any files that are already @@ -2311,7 +2365,7 @@ async fn install_to_filesystem_impl( } } - Ok(()) + Ok(deployment_path) } fn installation_complete() { @@ -2668,7 +2722,7 @@ pub(crate) async fn install_to_filesystem( opts: InstallToFilesystemOpts, targeting_host_root: bool, cleanup: Cleanup, -) -> Result<()> { +) -> Result> { // Log the installation operation to systemd journal const INSTALL_FILESYSTEM_JOURNAL_ID: &str = "9a8b7c6d5e4f3a2b1c0d9e8f7a6b5c4d3"; let source_image = opts @@ -2939,14 +2993,14 @@ pub(crate) async fn install_to_filesystem( skip_finalize, }; - install_to_filesystem_impl(&state, &mut rootfs, cleanup).await?; + let deployment_path = install_to_filesystem_impl(&state, &mut rootfs, cleanup).await?; // Drop all data about the root except the path to ensure any file descriptors etc. are closed. drop(rootfs); installation_complete(); - Ok(()) + Ok(deployment_path) } pub(crate) async fn install_to_existing_root(opts: InstallToExistingRootOpts) -> Result<()> { @@ -2979,7 +3033,22 @@ pub(crate) async fn install_to_existing_root(opts: InstallToExistingRootOpts) -> false => Cleanup::Skip, }; - let opts = InstallToFilesystemOpts { + // Extract migration flags before opts is consumed. + let preserve_var = opts.preserve_var; + let merge_etc = opts.merge_etc; + let root_path = std::path::PathBuf::from(opts.root_path.as_str()); + let mut preserve_var_skip = opts.preserve_var_skip; + if let Some(config) = config::load_config()? { + if let Some(config_skip) = config.preserve_var_skip { + preserve_var_skip.extend(config_skip); + } + } + let preserve_var_skip = migrate::validate_exclusions(&preserve_var_skip)?; + if (preserve_var || merge_etc) && opts.composefs_opts.composefs_backend { + anyhow::bail!("--preserve-var and --merge-etc require the ostree backend"); + } + + let fs_opts = InstallToFilesystemOpts { filesystem_opts: InstallTargetFilesystemOpts { root_path: opts.root_path, root_mount_spec: None, @@ -2994,7 +3063,42 @@ pub(crate) async fn install_to_existing_root(opts: InstallToExistingRootOpts) -> composefs_opts: opts.composefs_opts, }; - install_to_filesystem(opts, true, cleanup).await + let deployment_path = install_to_filesystem(fs_opts, true, cleanup).await?; + + // Post-install migration steps (run after the ostree deploy is complete). + if preserve_var { + println!(); + println!("Preserving /var..."); + let deployment_path = deployment_path + .as_ref() + .context("Install did not produce an ostree deployment")?; + let physical_root = if root_path.join("sysroot/ostree").exists() { + root_path.join("sysroot") + } else { + root_path.clone() + }; + let deploy_dir = physical_root.join(deployment_path); + let new_var = migrate::deployment_var_path(&deploy_dir)?; + migrate::preserve_var(&root_path.join("var"), &new_var, &preserve_var_skip) + .context("Post-install /var preservation")?; + } + + if merge_etc { + println!(); + println!("Merging running /etc into new deployment..."); + let deployment_path = deployment_path + .as_ref() + .context("Install did not produce an ostree deployment")?; + let physical_root = if root_path.join("sysroot/ostree").exists() { + root_path.join("sysroot") + } else { + root_path.clone() + }; + migrate::merge_etc_into_deployment(&root_path, &physical_root.join(deployment_path)) + .context("Post-install /etc merge")?; + } + + Ok(()) } /// Read the /boot entry from /etc/fstab, if it exists diff --git a/crates/lib/src/install/config.rs b/crates/lib/src/install/config.rs index 55fdd42f98..93da528344 100644 --- a/crates/lib/src/install/config.rs +++ b/crates/lib/src/install/config.rs @@ -132,6 +132,8 @@ pub(crate) struct InstallConfiguration { /// Enforce that the containers-storage stack has a non-default /// (i.e. not `insecureAcceptAnything`) container image signature policy. pub(crate) enforce_container_sigpolicy: Option, + /// Paths below `/var` which should not be copied by `--preserve-var`. + pub(crate) preserve_var_skip: Option>, } fn merge_basic(s: &mut Option, o: Option, _env: &EnvProperties) { @@ -226,6 +228,11 @@ impl Mergeable for InstallConfiguration { other.enforce_container_sigpolicy, env, ); + if let Some(other_skip) = other.preserve_var_skip { + self.preserve_var_skip + .get_or_insert_with(Default::default) + .extend(other_skip) + } if let Some(other_kargs) = other.kargs { self.kargs .get_or_insert_with(Default::default) diff --git a/crates/lib/src/install/migrate.rs b/crates/lib/src/install/migrate.rs new file mode 100644 index 0000000000..771da598ad --- /dev/null +++ b/crates/lib/src/install/migrate.rs @@ -0,0 +1,395 @@ +//! # Package-mode to image-mode migration helpers +//! +//! This module implements the post-install state-preservation steps that make +//! `bootc install to-existing-root` useful for converting a live package-mode +//! (RPM/DEB) system to a bootc image-mode deployment without losing data. +//! +//! These steps are triggered by passing `--preserve-var` and/or `--merge-etc` +//! to `bootc install to-existing-root`. They run **after** the core install +//! (ostree deploy + bootupd) completes but **before** the first reboot, while +//! the package-mode environment is still the running OS. +//! +//! ## `/var` preservation (`--preserve-var`) +//! +//! After a plain `bootc install to-existing-root`, the new deployment's `/var` +//! is bound from `/ostree/deploy//var/` — an initially-empty +//! directory. The old package-mode `/var` is stranded at `/var` +//! (inside the container, the host root is mounted at `root_path`). +//! +//! Two strategies are tried in order: +//! +//! - **Strategy C — reflink copy** (btrfs / XFS with reflinks): `cp --reflink=always` +//! performs an instantaneous copy-on-write clone. No extra disk space is used +//! until data diverges. +//! +//! - **Strategy D — plain copy** (ext4 and other non-reflink filesystems): +//! `cp -a` copies each non-ephemeral subdirectory of `/var` into the new +//! deployment's stateroot `var/`. This is correct but slow for large `/var` +//! trees, and **unsafe for live databases** (see the known-limitation comment +//! on `preserve_var_copy`). +//! +//! Exclusions such as `tmp`, `log/journal`, `lib/containers`, or `lib/rpm` +//! can be supplied through configuration or repeated `--preserve-var-skip` +//! options. +//! +//! ## `/etc` merge (`--merge-etc`) +//! +//! A plain `bootc install to-existing-root` populates the new deployment's `/etc` +//! from the image. The running system's admin customisations (NIC profiles, SSH +//! host keys, secrets, etc.) end up at `/etc` but are not applied to +//! the new `/etc`. +//! +//! `--merge-etc` runs the 3-way merge from the `etc-merge` crate at install time: +//! +//! | Input | Source | +//! |-------|--------| +//! | A — pristine baseline | `/usr/etc` (image's shipped defaults) | +//! | B — current live | `/etc` (running admin customisations) | +//! | C — new deployment | `/etc` (deploy target, written by image) | +//! +//! The diff A→B captures everything the admin changed relative to the image +//! defaults and applies those changes onto C. Machine-specific files (SSH host +//! keys, machine-id, NIC profiles) are included because they are precisely what +//! needs to be transferred to make the migrated system functional. +//! +//! Rollback preservation is intentionally independent from `/var` migration. + +use std::path::{Component, Path}; +use std::process::Stdio; + +use anyhow::{Context, Result}; +use cap_std_ext::cap_std::ambient_authority; +use cap_std_ext::cap_std::fs::Dir as CapStdDir; +use composefs_ctl::composefs::generic_tree::{FileSystem, Stat}; +use etc_merge::{compute_diff_without_deletions, merge, traverse_etc}; +use fn_error_context::context; + +// ── Top-level entry points ──────────────────────────────────────────────────── + +/// Run the 3-way `/etc` merge on the new deployment. +/// +/// Inputs: +/// - **A (pristine)** = `/usr/etc` +/// - **B (current)** = `/etc` +/// - **C (new)** = `/etc` +/// +/// `root_path` is the host root as seen from inside the install container. +#[context("Running 3-way /etc merge into new deployment")] +pub(crate) fn merge_etc_into_deployment(root_path: &Path, deploy_dir: &Path) -> Result<()> { + let deploy_usr_etc = deploy_dir.join("usr/etc"); + let deploy_etc = deploy_dir.join("etc"); + let host_etc = root_path.join("etc"); + + merge_etc(&host_etc, &deploy_usr_etc, &deploy_etc) +} + +// ── Internal helpers ────────────────────────────────────────────────────────── + +/// Preserve the running `/var` into the new deployment's `var/` directory. +/// +/// `src_var` is the running system's `/var` (at `/var`). +/// `new_var` is the new deployment's empty `var/` directory. +pub(crate) fn preserve_var(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { + std::fs::create_dir_all(new_var).with_context(|| format!("Creating {}", new_var.display()))?; + + let mut effective_exclusions = ["tmp", "cache", "log/journal", "lib/containers"] + .into_iter() + .map(String::from) + .collect::>(); + effective_exclusions.extend(exclusions.iter().cloned()); + + if reflinks_supported(src_var, new_var) { + println!(" Filesystem supports reflinks — using copy-on-write clone (Strategy C)"); + preserve_var_reflink(src_var, new_var, &effective_exclusions) + } else { + println!(" Filesystem does not support reflinks — falling back to full copy (Strategy D)"); + preserve_var_copy(src_var, new_var, &effective_exclusions) + } +} + +pub(crate) fn deployment_var_path(deploy_dir: &Path) -> Result { + let deploy_parent = deploy_dir + .parent() + .context("Deployment path has no parent")?; + let stateroot = deploy_parent + .parent() + .context("Deployment path has no stateroot")?; + Ok(stateroot.join("var")) +} + +/// Returns true if the filesystem hosting `new_var` supports reflinks. +/// +/// Probes by attempting a zero-byte reflink from `src_var` into `new_var`. +fn reflinks_supported(src_var: &Path, new_var: &Path) -> bool { + let probe_src = src_var.join(".bootc-reflink-probe-src"); + let probe_dst = new_var.join(".bootc-reflink-probe"); + + let _ = std::fs::write(&probe_src, b"probe"); + let result = std::process::Command::new("cp") + .args(["--reflink=always", "-a"]) + .arg(&probe_src) + .arg(&probe_dst) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false); + + let _ = std::fs::remove_file(&probe_src); + let _ = std::fs::remove_file(&probe_dst); + result +} + +/// Strategy C: reflink-copy each top-level entry under `src_var` into `new_var`. +/// +/// Skips well-known ephemeral subdirectories. +#[context("Reflink-copying /var into new deployment (Strategy C)")] +fn preserve_var_reflink(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { + let entries = + std::fs::read_dir(src_var).with_context(|| format!("Reading {}", src_var.display()))?; + + for entry in entries { + let entry = entry.with_context(|| format!("Reading entry in {}", src_var.display()))?; + let name = entry.file_name(); + let name_str = name.to_string_lossy(); + + if exclusions.iter().any(|s| s == name_str.as_ref()) { + println!( + " Skipping {} (ephemeral)", + src_var.join(name_str.as_ref()).display() + ); + continue; + } + + let src_entry = src_var.join(name_str.as_ref()); + let dst_entry = new_var.join(name_str.as_ref()); + + if let Some(skip) = exclusions.iter().find_map(|path| { + path.strip_prefix(&format!("{name_str}/")) + .filter(|rest| !rest.contains('/')) + }) { + copy_dir_skip_subdir(&src_entry, &dst_entry, skip, true)?; + continue; + } + + println!( + " Reflink-copying {} → {}", + src_entry.display(), + dst_entry.display() + ); + let status = std::process::Command::new("cp") + .args(["--reflink=always", "-a", "--no-clobber"]) + .arg(&src_entry) + .arg(new_var) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .status() + .with_context(|| format!("cp --reflink=always {}", src_entry.display()))?; + + anyhow::ensure!( + status.success(), + "cp --reflink=always failed for {}", + src_entry.display() + ); + } + + println!(" /var reflink copy complete."); + Ok(()) +} + +/// Copy a directory recursively, skipping one named subdirectory. +/// +/// Used by both Strategy C and Strategy D to copy `var/log/` while excluding +/// `var/log/journal/`. `reflink` selects whether `cp --reflink=always` or +/// plain `cp -a` is used. +fn copy_dir_skip_subdir(src: &Path, dst: &Path, skip_name: &str, reflink: bool) -> Result<()> { + std::fs::create_dir_all(dst).with_context(|| format!("Creating {}", dst.display()))?; + + let entries = std::fs::read_dir(src).with_context(|| format!("Reading {}", src.display()))?; + + for entry in entries { + let entry = entry.with_context(|| format!("Reading entry in {}", src.display()))?; + let name = entry.file_name(); + let name_str = name.to_string_lossy(); + + if name_str == skip_name { + println!(" Skipping {}/{} (ephemeral)", src.display(), name_str); + continue; + } + + let src_entry = src.join(name_str.as_ref()); + let mut cmd = std::process::Command::new("cp"); + if reflink { + cmd.args(["--reflink=always", "-a", "--no-clobber"]); + } else { + cmd.args(["-a", "--no-clobber"]); + } + let status = cmd + .arg(&src_entry) + .arg(dst) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .status() + .with_context(|| format!("cp -a {}", src_entry.display()))?; + + anyhow::ensure!(status.success(), "cp failed for {}", src_entry.display()); + } + + Ok(()) +} + +/// Strategy D: plain recursive copy of `/var` for filesystems without reflink support. +/// +/// # Known limitation +/// +/// This performs a full `cp -a` of every included subdirectory of the running +/// `/var` into the new deployment's ostree stateroot `var/`. For most workloads +/// this is fine, but it is **unsafe for databases and other applications that +/// keep open write handles into `/var`** (e.g. PostgreSQL in `/var/lib/pgsql`, +/// MySQL/MariaDB in `/var/lib/mysql`, SQLite databases under `/var/lib/*`). +/// Copying a live database with `cp -a` will almost certainly produce a +/// corrupted copy. +/// +/// The correct fix is to run this migration only after stopping all stateful +/// services that write to `/var`, or — better — to migrate the filesystem to +/// btrfs or XFS (which support reflinks, Strategy C) so that the copy is +/// instantaneous and atomic from the kernel's perspective. +/// +/// A future improvement would be to accept a user-supplied exclusion list so +/// that specific high-risk directories (e.g. `/var/lib/pgsql`) can be skipped +/// and migrated manually. For now, operators are responsible for stopping +/// affected services before running `bootc install to-existing-root --preserve-var` +/// on ext4 (or other non-reflink) filesystems. +/// +/// See: +#[context("Copying /var into new deployment (Strategy D)")] +fn preserve_var_copy(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { + let entries = + std::fs::read_dir(src_var).with_context(|| format!("Reading {}", src_var.display()))?; + + for entry in entries { + let entry = entry.with_context(|| format!("Reading entry in {}", src_var.display()))?; + let name = entry.file_name(); + let name_str = name.to_string_lossy(); + + if exclusions.iter().any(|s| s == name_str.as_ref()) { + println!( + " Skipping {} (ephemeral)", + src_var.join(name_str.as_ref()).display() + ); + continue; + } + + let src_entry = src_var.join(name_str.as_ref()); + let dst_entry = new_var.join(name_str.as_ref()); + + if let Some(skip) = exclusions.iter().find_map(|path| { + path.strip_prefix(&format!("{name_str}/")) + .filter(|rest| !rest.contains('/')) + }) { + copy_dir_skip_subdir(&src_entry, &dst_entry, skip, false)?; + continue; + } + + println!( + " Copying {} → {}", + src_entry.display(), + dst_entry.display() + ); + let status = std::process::Command::new("cp") + .args(["-a", "--no-clobber"]) + .arg(&src_entry) + .arg(new_var) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .status() + .with_context(|| format!("cp -a {}", src_entry.display()))?; + + anyhow::ensure!(status.success(), "cp -a failed for {}", src_entry.display()); + } + + println!(" /var copy complete."); + Ok(()) +} + +/// Validate exclusions before touching either tree. Paths are relative to `/var`. +pub(crate) fn validate_exclusions(exclusions: &[String]) -> Result> { + exclusions + .iter() + .map(|path| { + let path = path.trim_end_matches('/'); + anyhow::ensure!(!path.is_empty(), "empty /var exclusion"); + anyhow::ensure!( + Path::new(path) + .components() + .all(|component| matches!(component, Component::Normal(_))), + "invalid /var exclusion {path:?}; expected a relative path" + ); + anyhow::ensure!( + Path::new(path).components().count() <= 2, + "unsupported nested /var exclusion {path:?}; use at most one child path" + ); + Ok(path.to_string()) + }) + .collect() +} + +/// 3-way `/etc` merge. +/// +/// - `host_etc` = `/etc` (running system, input B) +/// - `deploy_usr_etc` = `/usr/etc` (image defaults, input A) +/// - `deploy_etc` = `/etc` (new deploy target, input C) +#[context("Merging running /etc into new deployment")] +fn merge_etc(host_etc: &Path, deploy_usr_etc: &Path, deploy_etc: &Path) -> Result<()> { + let pristine_fd = CapStdDir::open_ambient_dir(deploy_usr_etc, ambient_authority()) + .with_context(|| format!("Opening pristine etc: {}", deploy_usr_etc.display()))?; + let current_fd = CapStdDir::open_ambient_dir(host_etc, ambient_authority()) + .with_context(|| format!("Opening running etc: {}", host_etc.display()))?; + let new_fd = CapStdDir::open_ambient_dir(deploy_etc, ambient_authority()) + .with_context(|| format!("Opening deploy etc: {}", deploy_etc.display()))?; + + let (pristine_tree, current_tree, new_tree_opt) = + traverse_etc(&pristine_fd, ¤t_fd, Some(&new_fd)) + .context("Traversing /etc trees for 3-way merge")?; + + let new_tree = new_tree_opt.unwrap_or_else(|| FileSystem::new(Stat::uninitialized())); + + let diff = compute_diff_without_deletions(&pristine_tree, ¤t_tree, &new_tree) + .context("Computing /etc diff")?; + + println!(" /etc diff (changes being applied from running system):"); + etc_merge::print_diff(&diff, &mut std::io::stdout()); + + merge(¤t_fd, ¤t_tree, &new_fd, &new_tree, &diff) + .context("Applying /etc 3-way merge")?; + + println!(" /etc merge complete."); + Ok(()) +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn exclusions_are_relative_and_normalized() { + assert_eq!( + validate_exclusions(&["lib/rpm/".into()]).unwrap(), + ["lib/rpm"] + ); + assert!(validate_exclusions(&["../etc".into()]).is_err()); + assert!(validate_exclusions(&["/etc".into()]).is_err()); + assert!(validate_exclusions(&["lib/containers/storage".into()]).is_err()); + } + + #[test] + fn deployment_var_is_stateroot_var() { + let deploy = Path::new("/sysroot/ostree/deploy/default/deploy/checksum.0"); + assert_eq!( + deployment_var_path(deploy).unwrap(), + Path::new("/sysroot/ostree/deploy/default/var") + ); + } +} diff --git a/docs/src/bootc-installation.7.md b/docs/src/bootc-installation.7.md index 22f086adda..de243fdc95 100644 --- a/docs/src/bootc-installation.7.md +++ b/docs/src/bootc-installation.7.md @@ -294,6 +294,17 @@ and data from the old system (now accessible at `/sysroot`) to the new system. T useful for preserving network settings, user accounts, or application data from the previous installation. +By default, the old `/var` and `/etc` are not migrated into the new deployment. With +the ostree backend, `--preserve-var` opts into copying the old `/var` into the new +stateroot before reboot; this uses reflinks when supported and otherwise makes a full +copy, which can be unsafe while databases or other stateful services are running. +`/var/tmp`, `/var/cache`, `/var/log/journal`, and `/var/lib/containers` are always +excluded. Repeat `--preserve-var-skip PATH` to add further exclusions using paths +relative to `/var` (a path or one direct child path, such as `lib/rpm`). The +ostree-only `--merge-etc` option instead performs a three-way merge of the +host's `/etc` customizations into the new deployment before the first reboot. See the +[to-existing-root man page](man/bootc-install-to-existing-root.8.md) for details. + ##### Before reboot: Injecting new configuration After running `bootc install to-existing-root`, you may want to inject @@ -342,11 +353,10 @@ After rebooting into the new bootc system, the previous root filesystem is mounted at `/sysroot`. You can then migrate configuration and data from the old system to the new one. -**Important:** Any data from `/etc` that you want to use in the new system must be -manually copied from `/sysroot/etc` to `/etc` after rebooting into the new system. -There is currently no automated mechanism for migrating this configuration data. -This applies to network configurations, user accounts, application settings, and other -system configuration stored in `/etc`. +Without `--merge-etc`, configuration data from the old `/etc` remains at +`/sysroot/etc` and is not applied to the new system. Manually copy or merge any data +you need after rebooting. This applies to network configurations, user accounts, +application settings, and other system configuration stored in `/etc`. For example, after rebooting: diff --git a/docs/src/man/bootc-install-to-existing-root.8.md b/docs/src/man/bootc-install-to-existing-root.8.md index 3ff11166ed..508395ea98 100644 --- a/docs/src/man/bootc-install-to-existing-root.8.md +++ b/docs/src/man/bootc-install-to-existing-root.8.md @@ -24,6 +24,25 @@ configuration files: 1. **Before rebooting**: Injecting new configuration into the newly installed system 2. **After rebooting**: Migrating configuration from the old system to the new system +By default, installation does not migrate the running system's `/var` data or +customizations in `/etc`. The opt-in `--preserve-var` and `--merge-etc` options +provide these migrations on the **ostree backend only**. They run after the +installation completes and before the first reboot. + +`--preserve-var` copies the old `/var` into the new stateroot. Where supported, +the copy uses filesystem reflinks; otherwise it performs a full copy. A full copy +can be unsafe while databases or other stateful services are running, so stop +those services before installation when a full copy will be used. The paths +`tmp`, `cache`, `log/journal`, and `lib/containers` are always excluded. Use the +repeatable `--preserve-var-skip` option to add exclusions relative to `/var`; +each path may be one component or a direct child path (for example, `tmp` or +`lib/rpm`). + +`--merge-etc` performs a three-way merge that applies the running host's `/etc` +customizations to the new deployment before its first reboot. Without this +option, the new deployment's `/etc` comes from the image; there is no automatic +`/etc` migration by default. + ### Before reboot: Injecting new configuration If you need to inject new configuration files (such as custom `/etc/fstab` entries, @@ -76,9 +95,10 @@ After rebooting into the new bootc system, the previous root filesystem data is accessible at `/sysroot` (the "physical root"). This allows you to migrate data from the old system to the new one. -**Important:** Any configuration data from `/etc` that you want to use in the -new system must be **manually copied** from `/sysroot/etc` to `/etc` after -rebooting. There is currently no automated mechanism for migrating this data. +With the default installation behavior, configuration data from the old `/etc` +remains at `/sysroot/etc` and is not applied to the new system. Copy or merge +any needed configuration manually after reboot, unless you used `--merge-etc` +to apply the old host's customizations to the new deployment before rebooting. For example, to migrate configuration after rebooting: @@ -99,8 +119,9 @@ vipw # Carefully review and merge users from /sysroot/etc/passwd ``` This applies to network configurations, user accounts, application settings, -and other system configuration stored in `/etc`. Review files in `/sysroot/etc` -and manually copy or merge what you need into `/etc`. +and other system configuration stored in `/etc` when `--merge-etc` was not used. +Review files in `/sysroot/etc` and manually copy or merge what you need into +`/etc`. **Note:** A generated `/etc/resolv.conf` should not normally be copied or edited directly. It may be a dangling symlink after reboot or contain only a @@ -231,6 +252,18 @@ of migrating the fstab entries. See the "Injecting kernel arguments" section abo Name of the UKI addons to install without the ".efi.addon" suffix. This option can be provided multiple times if multiple addons are to be installed (composefs backend only) +**--preserve-var** + + Preserve the running system's `/var` data into the new bootc deployment + +**--preserve-var-skip**=*PATH* + + Relative paths below `/var` to leave out while preserving `/var` + +**--merge-etc** + + Merge the running system's `/etc` customisations into the new deployment + # VERSION From 8cff69cce99d5d278e514de8a10e09711521776a Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Thu, 1 Oct 2026 14:05:49 -0500 Subject: [PATCH 2/6] install: Optimize /var migration across directory capabilities Use directory-capability checks to preserve /var efficiently across filesystems and document migration exclusions. Assisted-by: AI Signed-off-by: ckyrouac --- crates/lib/src/install/migrate.rs | 542 ++++++++++++++---- crates/lib/src/store/mod.rs | 14 +- crates/lib/src/store/reflink.rs | 279 +++++++++ docs/src/bootc-installation.7.md | 8 +- .../man/bootc-install-to-existing-root.8.md | 14 +- 5 files changed, 721 insertions(+), 136 deletions(-) create mode 100644 crates/lib/src/store/reflink.rs diff --git a/crates/lib/src/install/migrate.rs b/crates/lib/src/install/migrate.rs index 771da598ad..8c40a0aae7 100644 --- a/crates/lib/src/install/migrate.rs +++ b/crates/lib/src/install/migrate.rs @@ -19,8 +19,9 @@ //! Two strategies are tried in order: //! //! - **Strategy C — reflink copy** (btrfs / XFS with reflinks): `cp --reflink=always` -//! performs an instantaneous copy-on-write clone. No extra disk space is used -//! until data diverges. +//! clones files copy-on-write, avoiding an immediate second copy of their data. +//! Files are cloned individually, so this is not an atomic snapshot of the +//! `/var` tree or of a live database. //! //! - **Strategy D — plain copy** (ext4 and other non-reflink filesystems): //! `cp -a` copies each non-ephemeral subdirectory of `/var` into the new @@ -55,15 +56,25 @@ //! Rollback preservation is intentionally independent from `/var` migration. use std::path::{Component, Path}; -use std::process::Stdio; +use std::{ + ffi::OsStr, + os::fd::AsFd, + path::PathBuf, + process::{Command, Stdio}, + sync::Arc, +}; use anyhow::{Context, Result}; use cap_std_ext::cap_std::ambient_authority; use cap_std_ext::cap_std::fs::Dir as CapStdDir; +use cap_std_ext::cap_std::io_lifetimes::AsFilelike; +use cap_std_ext::cmdext::{CapStdExtCommandExt, CmdFds}; use composefs_ctl::composefs::generic_tree::{FileSystem, Stat}; use etc_merge::{compute_diff_without_deletions, merge, traverse_etc}; use fn_error_context::context; +use crate::store::reflink::supports_reflink; + // ── Top-level entry points ──────────────────────────────────────────────────── /// Run the 3-way `/etc` merge on the new deployment. @@ -90,7 +101,9 @@ pub(crate) fn merge_etc_into_deployment(root_path: &Path, deploy_dir: &Path) -> /// `src_var` is the running system's `/var` (at `/var`). /// `new_var` is the new deployment's empty `var/` directory. pub(crate) fn preserve_var(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { - std::fs::create_dir_all(new_var).with_context(|| format!("Creating {}", new_var.display()))?; + let src_var = CapStdDir::open_ambient_dir(src_var, ambient_authority()) + .with_context(|| format!("Opening source {}", src_var.display()))?; + let new_var = open_or_create_var_dir(new_var)?; let mut effective_exclusions = ["tmp", "cache", "log/journal", "lib/containers"] .into_iter() @@ -98,13 +111,39 @@ pub(crate) fn preserve_var(src_var: &Path, new_var: &Path, exclusions: &[String] .collect::>(); effective_exclusions.extend(exclusions.iter().cloned()); - if reflinks_supported(src_var, new_var) { + if supports_reflink(&src_var, &new_var)? { println!(" Filesystem supports reflinks — using copy-on-write clone (Strategy C)"); - preserve_var_reflink(src_var, new_var, &effective_exclusions) + preserve_var_reflink(&src_var, &new_var, &effective_exclusions) } else { println!(" Filesystem does not support reflinks — falling back to full copy (Strategy D)"); - preserve_var_copy(src_var, new_var, &effective_exclusions) + preserve_var_copy(&src_var, &new_var, &effective_exclusions) + } +} + +/// Open the destination through its parent capability, creating just the +/// final `/var` directory if needed. The caller-supplied parent is the initial +/// ambient authority; all subsequent traversal is relative to opened handles. +fn open_or_create_var_dir(path: &Path) -> Result { + let parent = path.parent().context("Destination /var has no parent")?; + let name = path + .file_name() + .context("Destination /var has no final path component")?; + let parent = CapStdDir::open_ambient_dir(parent, ambient_authority()) + .with_context(|| format!("Opening destination parent {}", parent.display()))?; + match parent.create_dir(name) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + anyhow::ensure!( + parent.symlink_metadata(name)?.is_dir(), + "Destination {} exists and is not a directory", + path.display() + ); + } + Err(e) => return Err(e).with_context(|| format!("Creating {}", path.display())), } + parent + .open_dir(name) + .with_context(|| format!("Opening destination {}", path.display())) } pub(crate) fn deployment_var_path(deploy_dir: &Path) -> Result { @@ -117,79 +156,102 @@ pub(crate) fn deployment_var_path(deploy_dir: &Path) -> Result bool { - let probe_src = src_var.join(".bootc-reflink-probe-src"); - let probe_dst = new_var.join(".bootc-reflink-probe"); - - let _ = std::fs::write(&probe_src, b"probe"); - let result = std::process::Command::new("cp") - .args(["--reflink=always", "-a"]) - .arg(&probe_src) - .arg(&probe_dst) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status() - .map(|s| s.success()) - .unwrap_or(false); +/// Run GNU cp with directory handles inherited as fixed descriptors. Each +/// operand is `/proc/self/fd/N[/entry]`; cp's top-level paths therefore remain +/// rooted in the opened capabilities even if the original paths are renamed. +/// GNU cp still performs its own recursive traversal beneath each operand, so +/// this is not race-safe against a concurrent writer replacing entries while +/// the copy is in progress. +fn run_cp( + src_root: &CapStdDir, + src_names: &[&OsStr], + dst_root: &CapStdDir, + dst_name: Option<&OsStr>, + options: &[&str], + stdout: Stdio, + stderr: Stdio, +) -> Result { + let src_fd = Arc::new( + src_root + .as_filelike_view::() + .as_fd() + .try_clone_to_owned()?, + ); + let dst_fd = Arc::new( + dst_root + .as_filelike_view::() + .as_fd() + .try_clone_to_owned()?, + ); + let mut fds = CmdFds::new(); + fds.take_fd_n(src_fd, 3); + fds.take_fd_n(dst_fd, 4); + + let src_paths = src_names.iter().map(|name| { + let mut path = PathBuf::from("/proc/self/fd/3"); + path.push(name); + path + }); + let mut dst_path = PathBuf::from("/proc/self/fd/4"); + if let Some(name) = dst_name { + dst_path.push(name); + } - let _ = std::fs::remove_file(&probe_src); - let _ = std::fs::remove_file(&probe_dst); - result + let status = Command::new("cp") + .args(options) + .arg("--") + .args(src_paths) + .arg(dst_path) + .stdout(stdout) + .stderr(stderr) + .take_fds(fds) + .status() + .context("Running cp")?; + Ok(status) } /// Strategy C: reflink-copy each top-level entry under `src_var` into `new_var`. /// /// Skips well-known ephemeral subdirectories. #[context("Reflink-copying /var into new deployment (Strategy C)")] -fn preserve_var_reflink(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { - let entries = - std::fs::read_dir(src_var).with_context(|| format!("Reading {}", src_var.display()))?; +fn preserve_var_reflink( + src_var: &CapStdDir, + new_var: &CapStdDir, + exclusions: &[String], +) -> Result<()> { + let entries = src_var.read_dir(".").context("Reading source /var")?; for entry in entries { - let entry = entry.with_context(|| format!("Reading entry in {}", src_var.display()))?; + let entry = entry.context("Reading entry in source /var")?; let name = entry.file_name(); let name_str = name.to_string_lossy(); - if exclusions.iter().any(|s| s == name_str.as_ref()) { - println!( - " Skipping {} (ephemeral)", - src_var.join(name_str.as_ref()).display() - ); + if exclusions.iter().any(|s| name.to_str() == Some(s.as_str())) { + println!(" Skipping {} (ephemeral)", name_str); continue; } - let src_entry = src_var.join(name_str.as_ref()); - let dst_entry = new_var.join(name_str.as_ref()); - - if let Some(skip) = exclusions.iter().find_map(|path| { - path.strip_prefix(&format!("{name_str}/")) - .filter(|rest| !rest.contains('/')) - }) { - copy_dir_skip_subdir(&src_entry, &dst_entry, skip, true)?; + let skips = nested_exclusions(&name, exclusions); + if !skips.is_empty() && src_var.symlink_metadata(&name)?.is_dir() { + copy_dir_skip_subdir(src_var, new_var, &name, &skips, true)?; continue; } - println!( - " Reflink-copying {} → {}", - src_entry.display(), - dst_entry.display() - ); - let status = std::process::Command::new("cp") - .args(["--reflink=always", "-a", "--no-clobber"]) - .arg(&src_entry) - .arg(new_var) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .status() - .with_context(|| format!("cp --reflink=always {}", src_entry.display()))?; + println!(" Reflink-copying {} → {}", name_str, name_str); + let status = run_cp( + src_var, + &[&name], + new_var, + None, + &["--reflink=always", "-a", "--no-clobber"], + Stdio::inherit(), + Stdio::inherit(), + )?; anyhow::ensure!( status.success(), "cp --reflink=always failed for {}", - src_entry.display() + name_str ); } @@ -197,42 +259,70 @@ fn preserve_var_reflink(src_var: &Path, new_var: &Path, exclusions: &[String]) - Ok(()) } -/// Copy a directory recursively, skipping one named subdirectory. +/// Copy a directory recursively, skipping selected child directories. /// /// Used by both Strategy C and Strategy D to copy `var/log/` while excluding /// `var/log/journal/`. `reflink` selects whether `cp --reflink=always` or -/// plain `cp -a` is used. -fn copy_dir_skip_subdir(src: &Path, dst: &Path, skip_name: &str, reflink: bool) -> Result<()> { - std::fs::create_dir_all(dst).with_context(|| format!("Creating {}", dst.display()))?; - - let entries = std::fs::read_dir(src).with_context(|| format!("Reading {}", src.display()))?; - +/// plain `cp -a` is used. Included sibling entries are passed in one cp process +/// so hardlinks between them remain linked. As in the previous implementation, +/// this filtered path does not copy metadata from the containing directory +/// itself; ordinary (unfiltered) entries are copied wholly by cp -a. +fn copy_dir_skip_subdir( + src_root: &CapStdDir, + dst_root: &CapStdDir, + dir_name: &OsStr, + skip_names: &[&str], + reflink: bool, +) -> Result<()> { + let src = open_child_dir_nofollow(src_root, dir_name) + .with_context(|| format!("Opening /var/{}", dir_name.to_string_lossy()))?; + let dst = open_or_create_child_dir(dst_root, dir_name)?; + let entries = src + .read_dir(".") + .context("Reading excluded-copy source directory")?; + + let mut included_entries = Vec::new(); for entry in entries { - let entry = entry.with_context(|| format!("Reading entry in {}", src.display()))?; + let entry = entry.context("Reading entry in source child directory")?; let name = entry.file_name(); let name_str = name.to_string_lossy(); - if name_str == skip_name { - println!(" Skipping {}/{} (ephemeral)", src.display(), name_str); + if skip_names.iter().any(|skip| name == OsStr::new(skip)) { + println!( + " Skipping {}/{} (ephemeral)", + dir_name.to_string_lossy(), + name_str + ); continue; } + included_entries.push(name); + } - let src_entry = src.join(name_str.as_ref()); - let mut cmd = std::process::Command::new("cp"); - if reflink { - cmd.args(["--reflink=always", "-a", "--no-clobber"]); + if !included_entries.is_empty() { + let names = included_entries + .iter() + .map(|name| name.as_os_str()) + .collect::>(); + let options = if reflink { + &["--reflink=always", "-a", "--no-clobber"][..] } else { - cmd.args(["-a", "--no-clobber"]); - } - let status = cmd - .arg(&src_entry) - .arg(dst) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .status() - .with_context(|| format!("cp -a {}", src_entry.display()))?; - - anyhow::ensure!(status.success(), "cp failed for {}", src_entry.display()); + &["-a", "--no-clobber"][..] + }; + let status = run_cp( + &src, + &names, + &dst, + None, + options, + Stdio::inherit(), + Stdio::inherit(), + )?; + + anyhow::ensure!( + status.success(), + "cp failed while copying {}", + dir_name.to_string_lossy() + ); } Ok(()) @@ -252,66 +342,108 @@ fn copy_dir_skip_subdir(src: &Path, dst: &Path, skip_name: &str, reflink: bool) /// /// The correct fix is to run this migration only after stopping all stateful /// services that write to `/var`, or — better — to migrate the filesystem to -/// btrfs or XFS (which support reflinks, Strategy C) so that the copy is -/// instantaneous and atomic from the kernel's perspective. +/// btrfs or XFS (which support reflinks, Strategy C) to avoid copying every +/// file's data immediately. Reflinks clone files individually; they do not +/// provide an atomic snapshot of the `/var` tree or of a live database. /// -/// A future improvement would be to accept a user-supplied exclusion list so -/// that specific high-risk directories (e.g. `/var/lib/pgsql`) can be skipped -/// and migrated manually. For now, operators are responsible for stopping -/// affected services before running `bootc install to-existing-root --preserve-var` -/// on ext4 (or other non-reflink) filesystems. +/// Exclusions can be configured with install configuration or repeated +/// `--preserve-var-skip` options. Paths are relative to `/var` and may be one +/// component (such as `lib`) or exactly one child (such as `lib/pgsql`). `/var` +/// itself and deeper paths are not accepted. Excluded state is not migrated by +/// this command and must be handled separately. Exclusions do not make a live database copy +/// consistent: any database state that is copied while it is being written can +/// still be corrupted. Stop stateful services before copying their data, or use +/// a consistent snapshot. /// /// See: #[context("Copying /var into new deployment (Strategy D)")] -fn preserve_var_copy(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { - let entries = - std::fs::read_dir(src_var).with_context(|| format!("Reading {}", src_var.display()))?; +fn preserve_var_copy( + src_var: &CapStdDir, + new_var: &CapStdDir, + exclusions: &[String], +) -> Result<()> { + let entries = src_var.read_dir(".").context("Reading source /var")?; for entry in entries { - let entry = entry.with_context(|| format!("Reading entry in {}", src_var.display()))?; + let entry = entry.context("Reading entry in source /var")?; let name = entry.file_name(); let name_str = name.to_string_lossy(); - if exclusions.iter().any(|s| s == name_str.as_ref()) { - println!( - " Skipping {} (ephemeral)", - src_var.join(name_str.as_ref()).display() - ); + if exclusions.iter().any(|s| name.to_str() == Some(s.as_str())) { + println!(" Skipping {} (ephemeral)", name_str); continue; } - let src_entry = src_var.join(name_str.as_ref()); - let dst_entry = new_var.join(name_str.as_ref()); - - if let Some(skip) = exclusions.iter().find_map(|path| { - path.strip_prefix(&format!("{name_str}/")) - .filter(|rest| !rest.contains('/')) - }) { - copy_dir_skip_subdir(&src_entry, &dst_entry, skip, false)?; + let skips = nested_exclusions(&name, exclusions); + if !skips.is_empty() && src_var.symlink_metadata(&name)?.is_dir() { + copy_dir_skip_subdir(src_var, new_var, &name, &skips, false)?; continue; } - println!( - " Copying {} → {}", - src_entry.display(), - dst_entry.display() - ); - let status = std::process::Command::new("cp") - .args(["-a", "--no-clobber"]) - .arg(&src_entry) - .arg(new_var) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .status() - .with_context(|| format!("cp -a {}", src_entry.display()))?; - - anyhow::ensure!(status.success(), "cp -a failed for {}", src_entry.display()); + println!(" Copying {} → {}", name_str, name_str); + let status = run_cp( + src_var, + &[&name], + new_var, + None, + &["-a", "--no-clobber"], + Stdio::inherit(), + Stdio::inherit(), + )?; + + anyhow::ensure!(status.success(), "cp -a failed for {}", name_str); } println!(" /var copy complete."); Ok(()) } +fn nested_exclusions<'a>(name: &OsStr, exclusions: &'a [String]) -> Vec<&'a str> { + let Some(name) = name.to_str() else { + return Vec::new(); + }; + let prefix = format!("{name}/"); + exclusions + .iter() + .filter_map(|path| { + path.strip_prefix(&prefix) + .filter(|rest| !rest.contains('/')) + }) + .collect() +} + +/// Open a single source child directory without following a symlink swapped +/// into place after the caller's metadata check. The entry name comes directly +/// from `read_dir`, so this is one path component beneath the capability root. +fn open_child_dir_nofollow(parent: &CapStdDir, name: &OsStr) -> std::io::Result { + use rustix::fs::{Mode, OFlags}; + + let fd = rustix::fs::openat( + parent.as_filelike_view::().as_fd(), + name, + OFlags::CLOEXEC | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::RDONLY, + Mode::empty(), + )?; + Ok(CapStdDir::from_std_file(std::fs::File::from(fd))) +} + +fn open_or_create_child_dir(parent: &CapStdDir, name: &OsStr) -> Result { + match parent.create_dir(name) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + anyhow::ensure!( + parent.symlink_metadata(name)?.is_dir(), + "Destination child {} exists and is not a directory", + name.to_string_lossy() + ); + } + Err(e) => return Err(e).context("Creating destination child directory"), + } + parent + .open_dir(name) + .context("Opening destination child directory") +} + /// Validate exclusions before touching either tree. Paths are relative to `/var`. pub(crate) fn validate_exclusions(exclusions: &[String]) -> Result> { exclusions @@ -372,6 +504,16 @@ fn merge_etc(host_etc: &Path, deploy_usr_etc: &Path, deploy_etc: &Path) -> Resul #[cfg(test)] mod tests { use super::*; + use cap_std_ext::dirext::CapStdExtDirExt; + use std::{ + fs, + os::unix::fs::{MetadataExt, PermissionsExt, symlink}, + time::{Duration, SystemTime}, + }; + + fn cap_dir(path: &Path) -> CapStdDir { + CapStdDir::open_ambient_dir(path, ambient_authority()).unwrap() + } #[test] fn exclusions_are_relative_and_normalized() { @@ -392,4 +534,160 @@ mod tests { Path::new("/sysroot/ostree/deploy/default/var") ); } + + #[test] + fn plain_copy_skips_multiple_children_under_parent() -> Result<()> { + let exclusions = ["tmp".into(), "log/journal".into(), "log/private".into()]; + assert_eq!( + nested_exclusions(OsStr::new("log"), &exclusions), + ["journal", "private"] + ); + + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir_all(src_path.join("tmp"))?; + fs::create_dir_all(src_path.join("log/journal"))?; + fs::create_dir_all(src_path.join("log/private"))?; + fs::create_dir_all(src_path.join("log/keep"))?; + fs::write(src_path.join("tmp/ignored"), b"tmp")?; + fs::write(src_path.join("log/journal/ignored"), b"journal")?; + fs::write(src_path.join("log/private/ignored"), b"private")?; + fs::write(src_path.join("log/keep/preserved"), b"keep")?; + fs::hard_link( + src_path.join("log/keep/preserved"), + src_path.join("log/keep/preserved-hardlink"), + )?; + fs::create_dir(&dst_path)?; + + preserve_var_copy(&cap_dir(&src_path), &cap_dir(&dst_path), &exclusions)?; + + assert!(!dst_path.join("tmp").exists()); + assert!(!dst_path.join("log/journal").exists()); + assert!(!dst_path.join("log/private").exists()); + assert_eq!(fs::read(dst_path.join("log/keep/preserved"))?, b"keep"); + assert_eq!( + fs::metadata(dst_path.join("log/keep/preserved"))?.ino(), + fs::metadata(dst_path.join("log/keep/preserved-hardlink"))?.ino() + ); + Ok(()) + } + + #[test] + fn reflink_strategy_skips_multiple_children_under_parent() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir_all(src_path.join("log/journal"))?; + fs::create_dir_all(src_path.join("log/private"))?; + fs::create_dir_all(src_path.join("log/keep"))?; + fs::write(src_path.join("log/journal/ignored"), b"journal")?; + fs::write(src_path.join("log/private/ignored"), b"private")?; + fs::write(src_path.join("log/keep/preserved"), b"keep")?; + fs::create_dir(&dst_path)?; + let src = cap_dir(&src_path); + let dst = cap_dir(&dst_path); + let supports_reflink = supports_reflink(&src, &dst)?; + if !supports_reflink { + fs::remove_file(src_path.join("log/keep/preserved"))?; + fs::remove_dir(src_path.join("log/keep"))?; + } + + // Exercise Strategy C's exclusion routing on every filesystem and, if + // available, its reflink copy of a retained sibling. + preserve_var_reflink(&src, &dst, &["log/journal".into(), "log/private".into()])?; + + assert!(!dst_path.join("log/journal").exists()); + assert!(!dst_path.join("log/private").exists()); + assert_eq!( + dst_path.join("log/keep/preserved").exists(), + supports_reflink + ); + Ok(()) + } + + #[test] + fn copy_preserves_symlink_without_traversing_target() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + let outside = temp.path().join("outside"); + fs::create_dir(&src_path)?; + fs::create_dir(&dst_path)?; + fs::create_dir(&outside)?; + fs::write(outside.join("sentinel"), b"outside")?; + symlink(&outside, src_path.join("escape"))?; + + preserve_var_copy( + &cap_dir(&src_path), + &cap_dir(&dst_path), + &["escape/sentinel".into()], + )?; + + let copied = fs::symlink_metadata(dst_path.join("escape"))?; + assert!(copied.file_type().is_symlink()); + assert_eq!(fs::read_link(dst_path.join("escape"))?, outside); + // The target remains outside the destination tree; it was not copied. + assert!(!dst_path.join("sentinel").exists()); + Ok(()) + } + + #[test] + fn copy_preserves_archive_metadata_and_hardlinks() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + let src_state = src_path.join("state"); + fs::create_dir_all(&src_state)?; + fs::create_dir(&dst_path)?; + fs::write(src_state.join("one"), b"contents")?; + fs::hard_link(src_state.join("one"), src_state.join("two"))?; + fs::set_permissions(&src_state, fs::Permissions::from_mode(0o751))?; + fs::set_permissions(src_state.join("one"), fs::Permissions::from_mode(0o640))?; + let src_cap = cap_dir(&src_path); + let dst_cap = cap_dir(&dst_path); + let xattr_supported = src_cap + .setxattr("state/one", "user.bootc_migrate_test", b"xattr") + .is_ok(); + let expected_mtime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_700_000_000); + fs::File::open(&src_state)?.set_times(fs::FileTimes::new().set_modified(expected_mtime))?; + + preserve_var_copy(&src_cap, &dst_cap, &[])?; + + let copied_dir = fs::metadata(dst_path.join("state"))?; + let copied_one = fs::metadata(dst_path.join("state/one"))?; + let copied_two = fs::metadata(dst_path.join("state/two"))?; + assert_eq!(copied_dir.permissions().mode() & 0o7777, 0o751); + assert_eq!(copied_one.permissions().mode() & 0o7777, 0o640); + assert_eq!(copied_dir.modified()?, expected_mtime); + assert_eq!(copied_one.ino(), copied_two.ino()); + assert_eq!(copied_one.nlink(), 2); + assert_eq!(copied_one.uid(), fs::metadata(src_state.join("one"))?.uid()); + assert_eq!(copied_one.gid(), fs::metadata(src_state.join("one"))?.gid()); + if xattr_supported { + assert_eq!( + dst_cap.getxattr("state/one", "user.bootc_migrate_test")?, + Some(b"xattr".to_vec()) + ); + } + Ok(()) + } + + #[test] + fn copy_keeps_existing_collisions_untouched() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir_all(src_path.join("state"))?; + fs::create_dir_all(dst_path.join("state"))?; + fs::write(src_path.join("state/existing"), b"source")?; + fs::write(src_path.join("state/new"), b"new")?; + fs::write(dst_path.join("state/existing"), b"destination")?; + + preserve_var_copy(&cap_dir(&src_path), &cap_dir(&dst_path), &[])?; + + assert_eq!(fs::read(dst_path.join("state/existing"))?, b"destination"); + assert_eq!(fs::read(dst_path.join("state/new"))?, b"new"); + Ok(()) + } } diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index e517bc9637..c2d945a1a3 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -32,7 +32,7 @@ //! The composefs → ostree step (arrow 2) was proven by the `composefs-to-ostree` //! spike branch. The planned implementation for the ostree backend will: //! -//! 1. Perform a lazy cached probe (`reflinks_supported`) at install time. +//! 1. Probe the relevant source/destination filesystem pair as needed. //! 2. Pull into containers-storage first (Stage 1). //! 3. Use `composefs_oci::pull` with `LocalFetchOpt::ZeroCopy` to populate composefs (Stage 2). //! 4. Finally, synthesize the ostree commit by walking the composefs tree, @@ -66,12 +66,10 @@ //! //! ## Reflink probe //! -//! The reflink probe is performed lazily and cached. It creates -//! two anonymous temporary files (via `O_TMPFILE`, no -//! cleanup needed), writes one byte to the source, and attempts -//! `ioctl(FICLONE)`. Returns `true` on success, `false` on `EOPNOTSUPP` or -//! `EXDEV`. The probe directory is `composefs/objects` if it already exists, -//! otherwise the physical root itself. +//! [`reflink::supports_reflink`] probes a specific source/destination directory +//! pair using temporary files and descriptor-level `ioctl(FICLONE)`. The +//! result is not cached: reflink support can differ across filesystems, so the +//! source/destination pair is the unit of the probe. //! //! # OSTree //! @@ -126,6 +124,8 @@ use crate::podstorage::CStorage; use crate::spec::{BootloaderKind, ImageStatus}; use crate::utils::{EfiError, deployment_fd, open_dir_remount_rw, read_uefi_var}; +pub(crate) mod reflink; + /// See pub type ComposefsRepository = composefs::repository::Repository; diff --git a/crates/lib/src/store/reflink.rs b/crates/lib/src/store/reflink.rs new file mode 100644 index 0000000000..148a40153b --- /dev/null +++ b/crates/lib/src/store/reflink.rs @@ -0,0 +1,279 @@ +//! Pairwise reflink capability probing for storage directories. + +use std::io::Write as _; + +use anyhow::{Context, Result}; +use cap_std_ext::cap_std::fs::{Dir as CapStdDir, File, OpenOptions}; +use cap_std_ext::cap_std::io_lifetimes::AsFilelike; +use rustix::fd::AsFd as _; + +/// Returns whether a file in `src` can be reflinked into `dst`. +/// +/// The result is scoped to this source/destination pair; it is deliberately +/// not cached. Temporary files are created exclusively beneath the supplied +/// directory capabilities and removed before returning. +pub(crate) fn supports_reflink(src: &CapStdDir, dst: &CapStdDir) -> Result { + probe_with(src, dst, |destination, source| { + let source_view = source.as_filelike_view::(); + let destination_view = destination.as_filelike_view::(); + let source_fd = source_view.as_fd(); + let destination_fd = destination_view.as_fd(); + rustix::fs::ioctl_ficlone(destination_fd, source_fd) + }) +} + +fn probe_with( + src: &CapStdDir, + dst: &CapStdDir, + clone: impl FnOnce(&File, &File) -> rustix::io::Result<()>, +) -> Result { + let mut source = match create_probe_file(src) { + Ok(source) => source, + Err(error) => return source_probe_failure(error), + }; + let mut destination = match create_probe_file(dst) { + Ok(destination) => destination, + Err(error) => { + source + .cleanup() + .context("Removing reflink probe source after destination creation failed")?; + return Err(error); + } + }; + + let probe_result = match source + .file + .as_mut() + .expect("probe source file is open") + .write_all(b"bootc reflink probe") + { + Err(error) => { + source_probe_failure(anyhow::Error::from(error).context("Writing reflink probe source")) + } + Ok(()) => match clone( + destination + .file + .as_ref() + .expect("probe destination file is open"), + source.file.as_ref().expect("probe source file is open"), + ) { + Ok(()) => Ok(true), + Err(error) if reflink_unsupported(error) => Ok(false), + Err(error) => Err(anyhow::Error::from(error).context("Probing FICLONE support")), + }, + }; + + // Attempt both removals even if one fails. Drop provides a best-effort + // fallback if the operation above returned early (for example, a write + // error). + let source_cleanup = source.cleanup(); + let destination_cleanup = destination.cleanup(); + source_cleanup.context("Removing reflink probe source")?; + destination_cleanup.context("Removing reflink probe destination")?; + probe_result +} + +fn reflink_unsupported(error: rustix::io::Errno) -> bool { + // Both files are newly created regular files, so EINVAL here indicates + // that FICLONE is unsupported rather than an invalid source type. + matches!( + error, + rustix::io::Errno::OPNOTSUPP | rustix::io::Errno::XDEV | rustix::io::Errno::INVAL + ) +} + +fn source_probe_access_error(error: &anyhow::Error) -> bool { + error.chain().any(|cause| { + cause.downcast_ref::().is_some_and(|error| { + matches!( + error.kind(), + std::io::ErrorKind::PermissionDenied | std::io::ErrorKind::ReadOnlyFilesystem + ) + }) + }) +} + +fn source_probe_failure(error: anyhow::Error) -> Result { + if source_probe_access_error(&error) { + tracing::debug!("Cannot prepare reflink probe source, using full copy: {error:#}"); + Ok(false) + } else { + Err(error) + } +} + +struct ProbeFile<'a> { + dir: &'a CapStdDir, + name: String, + file: Option, + removed: bool, +} + +impl ProbeFile<'_> { + fn cleanup(&mut self) -> std::io::Result<()> { + self.cleanup_with(|dir, name| dir.remove_file(name)) + } + + fn cleanup_with( + &mut self, + remove_file: impl FnOnce(&CapStdDir, &str) -> std::io::Result<()>, + ) -> std::io::Result<()> { + drop(self.file.take()); + match remove_file(self.dir, &self.name) { + Ok(()) => { + self.removed = true; + Ok(()) + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + self.removed = true; + Ok(()) + } + Err(error) => Err(error), + } + } +} + +impl Drop for ProbeFile<'_> { + fn drop(&mut self) { + if !self.removed { + let _ = self.cleanup(); + } + } +} + +fn create_probe_file(dir: &CapStdDir) -> Result> { + loop { + let name = format!(".bootc-reflink-probe-{}", uuid::Uuid::new_v4()); + match create_probe_file_with_name(dir, &name) { + Ok(file) => return Ok(file), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(error) => return Err(error).context("Creating reflink probe file"), + } + } +} + +fn create_probe_file_with_name<'a>( + dir: &'a CapStdDir, + name: &str, +) -> std::io::Result> { + let file = dir.open_with( + name, + OpenOptions::new().read(true).write(true).create_new(true), + )?; + Ok(ProbeFile { + dir, + name: name.to_owned(), + file: Some(file), + removed: false, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use cap_std_ext::cap_std::ambient_authority; + use std::fs; + + fn cap_dir(path: &std::path::Path) -> CapStdDir { + CapStdDir::open_ambient_dir(path, ambient_authority()).unwrap() + } + + #[test] + fn probe_reports_success_and_cleans_up() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir(&src_path)?; + fs::create_dir(&dst_path)?; + + let supported = probe_with(&cap_dir(&src_path), &cap_dir(&dst_path), |_, _| Ok(()))?; + assert!(supported); + assert!(fs::read_dir(&src_path)?.next().is_none()); + assert!(fs::read_dir(&dst_path)?.next().is_none()); + Ok(()) + } + + #[test] + fn probe_classifies_unsupported_and_propagates_unexpected_errors() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir(&src_path)?; + fs::create_dir(&dst_path)?; + let src = cap_dir(&src_path); + let dst = cap_dir(&dst_path); + + assert!(!probe_with(&src, &dst, |_, _| { + Err(rustix::io::Errno::OPNOTSUPP) + })?); + assert!(!probe_with(&src, &dst, |_, _| { + Err(rustix::io::Errno::INVAL) + })?); + assert!(fs::read_dir(&src_path)?.next().is_none()); + assert!(fs::read_dir(&dst_path)?.next().is_none()); + + assert!(probe_with(&src, &dst, |_, _| Err(rustix::io::Errno::IO)).is_err()); + assert!(fs::read_dir(&src_path)?.next().is_none()); + assert!(fs::read_dir(&dst_path)?.next().is_none()); + Ok(()) + } + + #[test] + fn classifies_only_known_unsupported_errors() { + assert!(reflink_unsupported(rustix::io::Errno::OPNOTSUPP)); + assert!(reflink_unsupported(rustix::io::Errno::XDEV)); + assert!(reflink_unsupported(rustix::io::Errno::INVAL)); + assert!(!reflink_unsupported(rustix::io::Errno::IO)); + } + + #[test] + fn source_permission_and_read_only_errors_select_full_copy() { + for error in [ + std::io::Error::from(std::io::ErrorKind::PermissionDenied), + std::io::Error::from(std::io::ErrorKind::ReadOnlyFilesystem), + ] { + let error = anyhow::Error::from(error).context("Creating reflink probe file"); + assert!(source_probe_access_error(&error)); + assert!(!source_probe_failure(error).unwrap()); + } + assert!(source_probe_failure(anyhow::anyhow!("unrelated error")).is_err()); + } + + #[test] + fn cleanup_retries_unlink_on_drop_after_failure() -> Result<()> { + let temp = tempfile::tempdir()?; + let path = temp.path().join("src"); + fs::create_dir(&path)?; + let dir = cap_dir(&path); + let name = ".bootc-reflink-probe-retry"; + let mut probe = create_probe_file_with_name(&dir, name)?; + + let error = probe + .cleanup_with(|_, _| Err(std::io::Error::from(std::io::ErrorKind::PermissionDenied))) + .unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied); + assert!(path.join(name).exists()); + + drop(probe); + assert!(!path.join(name).exists()); + Ok(()) + } + + #[test] + fn probe_file_creation_never_overwrites_existing_files() -> Result<()> { + let temp = tempfile::tempdir()?; + let path = temp.path().join("src"); + fs::create_dir(&path)?; + let existing = path.join(".bootc-reflink-probe-collision"); + fs::write(&existing, b"leave this alone")?; + + let error = + match create_probe_file_with_name(&cap_dir(&path), ".bootc-reflink-probe-collision") { + Ok(_) => panic!("create_new unexpectedly replaced an existing probe file"), + Err(error) => error, + }; + assert_eq!(error.kind(), std::io::ErrorKind::AlreadyExists); + assert_eq!(fs::read(existing)?, b"leave this alone"); + Ok(()) + } +} diff --git a/docs/src/bootc-installation.7.md b/docs/src/bootc-installation.7.md index de243fdc95..6a2cad0502 100644 --- a/docs/src/bootc-installation.7.md +++ b/docs/src/bootc-installation.7.md @@ -297,10 +297,14 @@ previous installation. By default, the old `/var` and `/etc` are not migrated into the new deployment. With the ostree backend, `--preserve-var` opts into copying the old `/var` into the new stateroot before reboot; this uses reflinks when supported and otherwise makes a full -copy, which can be unsafe while databases or other stateful services are running. +copy of non-excluded data. Reflinks clone individual files, but the directory-tree copy is not an atomic +database snapshot; concurrent changes can still result in inconsistent data. Stop +databases and other stateful services before copying. `/var/tmp`, `/var/cache`, `/var/log/journal`, and `/var/lib/containers` are always excluded. Repeat `--preserve-var-skip PATH` to add further exclusions using paths -relative to `/var` (a path or one direct child path, such as `lib/rpm`). The +relative to `/var`: each exclusion must be either one component (such as `lib`) or +exactly one child (such as `lib/pgsql`). `/var` itself and deeper paths are not +accepted. Excluded data is not migrated and must be handled separately. The ostree-only `--merge-etc` option instead performs a three-way merge of the host's `/etc` customizations into the new deployment before the first reboot. See the [to-existing-root man page](man/bootc-install-to-existing-root.8.md) for details. diff --git a/docs/src/man/bootc-install-to-existing-root.8.md b/docs/src/man/bootc-install-to-existing-root.8.md index 508395ea98..5290bd6a9e 100644 --- a/docs/src/man/bootc-install-to-existing-root.8.md +++ b/docs/src/man/bootc-install-to-existing-root.8.md @@ -30,13 +30,17 @@ provide these migrations on the **ostree backend only**. They run after the installation completes and before the first reboot. `--preserve-var` copies the old `/var` into the new stateroot. Where supported, -the copy uses filesystem reflinks; otherwise it performs a full copy. A full copy -can be unsafe while databases or other stateful services are running, so stop -those services before installation when a full copy will be used. The paths +the copy uses filesystem reflinks; otherwise it performs a regular copy of the +non-excluded contents. A regular copy +can be unsafe while databases or other stateful services are running. Reflinks +clone individual files, but the directory-tree copy is not an atomic database +snapshot; concurrent changes can still result in inconsistent data. Stop those +services before copying. The paths `tmp`, `cache`, `log/journal`, and `lib/containers` are always excluded. Use the repeatable `--preserve-var-skip` option to add exclusions relative to `/var`; -each path may be one component or a direct child path (for example, `tmp` or -`lib/rpm`). +each path must be either one component (for example, `lib`) or exactly one child +(for example, `lib/pgsql`). `/var` itself and deeper paths such as `lib/pgsql/data` +are not accepted. Excluded data is not migrated and must be handled separately. `--merge-etc` performs a three-way merge that applies the running host's `/etc` customizations to the new deployment before its first reboot. Without this From 381a4df3b50b9357df00df56083e4af82526509a Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Fri, 25 Sep 2026 10:54:45 -0500 Subject: [PATCH 3/6] tmt: Add package-mode existing-root migration coverage Exercise package-mode migration flags through a focused TMT path with predictable Fedora defaults. Assisted-by: AI Signed-off-by: ckyrouac --- .packit.yaml | 41 +++++++++ CONTRIBUTING.md | 21 +++++ Justfile | 5 ++ hack/provision-packit.sh | 1 - tmt/plans/copy-test-image.yml | 11 +++ tmt/plans/fetch-test-image.yml | 9 ++ tmt/plans/package-mode.fmf | 88 +++++++++++++++++++ ...to-existing-root-preserve-var-merge-etc.sh | 67 ++++++++++++++ ...o-existing-root-preserve-var-merge-etc.fmf | 11 +++ 9 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 tmt/plans/copy-test-image.yml create mode 100644 tmt/plans/fetch-test-image.yml create mode 100644 tmt/plans/package-mode.fmf create mode 100644 tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh create mode 100644 tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf diff --git a/.packit.yaml b/.packit.yaml index e2f7430571..8c868248f3 100644 --- a/.packit.yaml +++ b/.packit.yaml @@ -102,6 +102,26 @@ jobs: context: running_env: "packit" + # This plan uses an image-mode harness plus a separate package-mode target. + - job: tests + trigger: pull_request + identifier: package-mode-tier-1 + require: + label: + present: + - ci/tier-1 + targets: + - centos-stream-10-x86_64 + - centos-stream-10-aarch64 + - fedora-latest-stable-x86_64 + - fedora-latest-stable-aarch64 + tmt_plan: /tmt/plans/package-mode + tf_extra_params: + environments: + - tmt: + context: + running_env: "packit" + # ci/merge: full target set - job: tests trigger: pull_request @@ -124,6 +144,27 @@ jobs: context: running_env: "packit" + - job: tests + trigger: pull_request + identifier: package-mode-full + require: + label: + present: + - ci/merge + targets: + - centos-stream-9-x86_64 + - centos-stream-9-aarch64 + - centos-stream-10-x86_64 + - centos-stream-10-aarch64 + - fedora-all-x86_64 + - fedora-all-aarch64 + tmt_plan: /tmt/plans/package-mode + tf_extra_params: + environments: + - tmt: + context: + running_env: "packit" + - job: propose_downstream trigger: release dist_git_branches: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 298b76cf4d..46ece91e59 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -88,6 +88,27 @@ To run a specific test: just test-tmt readonly ``` +The package-mode tests use a separate TMT plan with two disposable VMs. By +default, `just test-tmt-package-mode` uses TMT's `fedora` image alias (the latest +released Fedora image). To use a different image, set `test_disk_image`: + +```bash +# Uses the latest released Fedora image. +just test-tmt-package-mode + +# Optional: use another TMT-compatible image. +test_disk_image=/path/to/package-mode-test-image.qcow2 just test-tmt-package-mode +``` + +This runs the `/tmt/plans/package-mode` plan with `running_env=packit`; unlike +`just test-tmt`, it does not build the regular integration-test images. The +package-mode test performs a destructive migration inside its target VM, so use +only a disposable image/environment. TMT and its virtual provisioning backend +must be installed and configured locally. Note that the plan also expects +Testing Farm artifacts (`/var/share/test-artifacts/*.src.rpm`, the +`test-artifacts` repository configuration, and `/var/ARTIFACTS`) in the harness +VM; a stock Fedora image alone does not provide them. + ### Faster iteration cycles The test cycle currently builds a disk image and creates a new ephemeral diff --git a/Justfile b/Justfile index afcb3a1984..d7dbd771d2 100644 --- a/Justfile +++ b/Justfile @@ -192,6 +192,11 @@ test-tmt *ARGS: build @just _build-upgrade-image @just test-tmt-nobuild {{ARGS}} +# Run the package-mode TMT plan using its two TMT-provisioned guests. +[group('core')] +test-tmt-package-mode: + @tmt --context=running_env=packit --context="test_disk_image=${test_disk_image:-fedora}" run plans --name '^/tmt/plans/package-mode$' + # Split out from `test-container` because, unlike the container integration tests, # unit tests don't depend on variant/filesystem/bootloader/boot_type/seal_state, so # CI runs this once per OS (in the `package` job) instead of once per test-integration diff --git a/hack/provision-packit.sh b/hack/provision-packit.sh index 9ec9a144ae..6b6a40b65f 100755 --- a/hack/provision-packit.sh +++ b/hack/provision-packit.sh @@ -91,6 +91,5 @@ podman build --jobs=4 --from "$BASE" -v "$BOOTC_TEMPDIR":/bootc-test:z -t localh # Keep these in sync with what's used in hack/lbi podman pull -q --retry 5 --retry-delay 5s quay.io/curl/curl:latest quay.io/curl/curl-base:latest registry.access.redhat.com/ubi9/podman:latest -# Run system-reinstall-bootc # TODO make it more scriptable instead of expect + send ./system-reinstall-bootc.exp diff --git a/tmt/plans/copy-test-image.yml b/tmt/plans/copy-test-image.yml new file mode 100644 index 0000000000..ab1eef15d7 --- /dev/null +++ b/tmt/plans/copy-test-image.yml @@ -0,0 +1,11 @@ +--- +- name: Copy build-under-test image archive from the TMT controller to target + hosts: target + tasks: + - name: Copy the same image archive to the target + ansible.builtin.copy: + src: /tmp/bootc-to-existing-root-preserve-var-merge-etc.oci + dest: /var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci + owner: root + group: root + mode: '0600' diff --git a/tmt/plans/fetch-test-image.yml b/tmt/plans/fetch-test-image.yml new file mode 100644 index 0000000000..1b50126c09 --- /dev/null +++ b/tmt/plans/fetch-test-image.yml @@ -0,0 +1,9 @@ +--- +- name: Fetch build-under-test image archive to the TMT controller + hosts: harness + tasks: + - name: Fetch the image archive from the harness + ansible.builtin.fetch: + src: /var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci + dest: /tmp/bootc-to-existing-root-preserve-var-merge-etc.oci + flat: true diff --git a/tmt/plans/package-mode.fmf b/tmt/plans/package-mode.fmf new file mode 100644 index 0000000000..da7a6a5e28 --- /dev/null +++ b/tmt/plans/package-mode.fmf @@ -0,0 +1,88 @@ +summary: Test package-mode to-existing-root migration flags +provision: + - name: harness + role: harness + how: virtual + image: $@{test_disk_image} + user: root + - name: target + role: target + how: virtual + image: $@{test_disk_image} + user: root +prepare: + # This is the normal integration guest setup, restricted to the harness. + - how: install + order: 97 + package: + - podman + - skopeo + - jq + - bootc + - system-reinstall-bootc + - zstd + - cpio + - rpm-build + - expect + - ansible-core + - parted + - lvm2 + - dosfstools + - e2fsprogs + where: harness + when: running_env != image_mode + - how: shell + order: 98 + script: + - mkdir -p bootc && cp /var/share/test-artifacts/*.src.rpm bootc + - cd bootc && rpm2cpio *.src.rpm > pkg.cpio && cpio -idmv < pkg.cpio && rm pkg.cpio && rm -f *-vendor.tar.zstd && zstd -d *.tar.zstd && tar -xvf *.tar -C . --strip-components=1 && ls -al + - cd bootc/hack && ./provision-packit.sh + - | + workdir=$(mktemp -d) + trap 'rm -rf -- "$workdir"' EXIT + cat > "$workdir/Containerfile" <<'EOF' + FROM localhost/bootc + RUN mkdir -p /etc/bootc-tmt-merge && \ + printf '%s\n' image-default > /etc/bootc-tmt-merge/default.conf && \ + printf '%s\n' image-only > /etc/bootc-tmt-merge/image-only.conf + EOF + podman build -t localhost/bootc-to-existing-root-preserve-var-merge-etc:latest "$workdir" + podman save --format oci-archive \ + --output /var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci \ + localhost/bootc-to-existing-root-preserve-var-merge-etc:latest + where: harness + when: running_env != image_mode + - how: install + order: 97 + package: + - podman + - jq + - bootc + where: target + when: running_env != image_mode + # Reboot only the image-mode harness after system-reinstall-bootc. + - how: ansible + order: 99 + playbook: + - https://github.com/bootc-dev/bootc/raw/refs/heads/main/hack/packit-reboot.yml + where: harness + when: running_env != image_mode + # Fetch to the TMT controller, then copy that exact archive to the target. + # Separate ordered steps avoid relying on shared storage or guest ordering. + - how: ansible + order: 100 + playbook: fetch-test-image.yml + where: harness + when: running_env != image_mode + - how: ansible + order: 101 + playbook: copy-test-image.yml + where: target + when: running_env != image_mode +execute: + how: tmt +discover: + how: fmf + where: target + test: + - /tmt/tests/test-50-to-existing-root-preserve-var-merge-etc diff --git a/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh b/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh new file mode 100644 index 0000000000..d00875de63 --- /dev/null +++ b/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh @@ -0,0 +1,67 @@ +#!/bin/bash +set -xeuo pipefail + +target_image=localhost/bootc-to-existing-root-preserve-var-merge-etc:latest +target_image_archive=/var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci +var_marker=/var/lib/bootc-tmt-preserve-var/sentinel +etc_dir=/etc/bootc-tmt-merge + +case ${TMT_REBOOT_COUNT:-0} in + 0) + # The target guest must still be the original package-mode system. + test ! -e /run/ostree-booted + + # The image was built from the harness's build-under-test image and + # transferred as an archive; do not rebuild a potentially different image. + podman load --input "$target_image_archive" + podman image exists "$target_image" + + # Exercise both an admin-created file and an admin modification of an + # image default. The derived image below supplies the pristine values. + mkdir -p "$etc_dir" "${var_marker%/*}" + + # TMT connects as root. Package-mode hosts keep root's home at /root, + # while bootc images link /root to /var/roothome, so preserve its key + # under /var for TMT to reconnect after the migration reboot. + install -d -m 0700 /var/roothome/.ssh + install -m 0600 /root/.ssh/authorized_keys /var/roothome/.ssh/authorized_keys + restorecon -RF /var/roothome/.ssh + + printf '%s\n' admin-modified | tee "$etc_dir/default.conf" + printf '%s\n' admin-only | tee "$etc_dir/admin-only.conf" + printf '%s\n' preserved-var | tee "$var_marker" + + # The flags require the OSTree backend. Run the current build-under-test + # as a privileged installer, with the package-mode root mounted at /target. + podman run --rm --privileged --pid=host --user=root:root \ + -v /dev:/dev \ + -v /:/target \ + -v /var/lib/containers:/var/lib/containers \ + --security-opt label=type:unconfined_t \ + "$target_image" \ + bootc install to-existing-root \ + --preserve-var \ + --merge-etc \ + --acknowledge-destructive + + bootc status + # Ask the guest to reboot itself; this is more reliable than + # testcloud's ACPI soft-poweroff for a full bootc deployment reboot. + tmt-reboot -c 'systemctl reboot' + ;; + 1) + test -e /run/ostree-booted + + booted_image=$(bootc status --json | jq -r '.status.booted.image.image.image') + test "$booted_image" = "$target_image" + + test "$(<"$var_marker")" = preserved-var + test "$(<"$etc_dir/default.conf")" = admin-modified + test "$(<"$etc_dir/admin-only.conf")" = admin-only + test "$(<"$etc_dir/image-only.conf")" = image-only + ;; + *) + printf 'Unexpected TMT_REBOOT_COUNT=%s\n' "$TMT_REBOOT_COUNT" >&2 + exit 1 + ;; +esac diff --git a/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf b/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf new file mode 100644 index 0000000000..04be7c53c3 --- /dev/null +++ b/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf @@ -0,0 +1,11 @@ +# The two-guest package-mode plan builds a test image on a separate harness VM, +# reboots that harness into image mode, and transfers the OCI archive to target. +# This test installs that image onto the package-mode target, reboots it, then +# verifies the preserved /var data and merged /etc configuration. +summary: Package-mode /var preservation and /etc merge +duration: 45m +adjust: + - when: running_env == image_mode + enabled: false + because: this migration test requires a package-mode target +test: bash booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh From f20e5ae71d735250cf1c38f09c5626234fbce471 Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Wed, 7 Oct 2026 15:36:13 -0400 Subject: [PATCH 4/6] tmt: Support local and Packit package-mode runs Make package-mode TMT runs work locally and in Packit by consistently resolving artifacts, playbooks, and workdirs. Assisted-by: AI Signed-off-by: ckyrouac --- .packit.yaml | 2 + CONTRIBUTING.md | 33 +++++-- Justfile | 27 +++++- hack/provision-packit.sh | 5 +- tmt/plans/copy-local-test-image.yml | 11 +++ tmt/plans/forward-test-artifacts.yml | 75 +++++++++++++++ tmt/plans/package-mode.fmf | 95 ++++++++++++++----- ...o-existing-root-preserve-var-merge-etc.fmf | 9 +- 8 files changed, 215 insertions(+), 42 deletions(-) create mode 100644 tmt/plans/copy-local-test-image.yml create mode 100644 tmt/plans/forward-test-artifacts.yml diff --git a/.packit.yaml b/.packit.yaml index 8c868248f3..ede69c1395 100644 --- a/.packit.yaml +++ b/.packit.yaml @@ -121,6 +121,7 @@ jobs: - tmt: context: running_env: "packit" + package_source: "artifacts" # ci/merge: full target set - job: tests @@ -164,6 +165,7 @@ jobs: - tmt: context: running_env: "packit" + package_source: "artifacts" - job: propose_downstream trigger: release diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 46ece91e59..79e45f0f8f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -89,8 +89,12 @@ just test-tmt readonly ``` The package-mode tests use a separate TMT plan with two disposable VMs. By -default, `just test-tmt-package-mode` uses TMT's `fedora` image alias (the latest -released Fedora image). To use a different image, set `test_disk_image`: +default, `just test-tmt-package-mode` builds `localhost/bootc` from the latest +Fedora bootc base and uses TMT's `fedora` image alias (the latest released +Fedora image). This performs a full local image build, so it takes longer and +uses more disk/network than simply launching the VM test. The build archive is +stored under the ignored `target/` directory and copied to the harness VM. +To use a different guest image, set `test_disk_image`: ```bash # Uses the latest released Fedora image. @@ -100,14 +104,23 @@ just test-tmt-package-mode test_disk_image=/path/to/package-mode-test-image.qcow2 just test-tmt-package-mode ``` -This runs the `/tmt/plans/package-mode` plan with `running_env=packit`; unlike -`just test-tmt`, it does not build the regular integration-test images. The -package-mode test performs a destructive migration inside its target VM, so use -only a disposable image/environment. TMT and its virtual provisioning backend -must be installed and configured locally. Note that the plan also expects -Testing Farm artifacts (`/var/share/test-artifacts/*.src.rpm`, the -`test-artifacts` repository configuration, and `/var/ARTIFACTS`) in the harness -VM; a stock Fedora image alone does not provide them. +The local build base can be overridden with `BOOTC_base`; when changing the +guest image, choose a matching bootc base (for example, Fedora guests should use +the corresponding `quay.io/fedora/fedora-bootc` release). The package-mode test +performs a destructive migration inside its target VM, so use only a disposable +image/environment. TMT and its virtual provisioning backend must be installed +and configured locally. + +Packit runs the same plan with `package_source=artifacts`: the TMT controller +must provide `/var/share/test-artifacts/` with exactly one top-level `.src.rpm` +and `/etc/yum.repos.d/test-artifacts.repo`. `/var/ARTIFACTS/` is forwarded to +the harness when present, but is an optional Testing Farm pass-through, not a +test prerequisite. Local `just test-tmt-package-mode` runs use +`package_source=local` and need none of those controller artifact paths; they +skip Packit provisioning and the harness reboot while retaining the shared +migration test and archive transfer. Local mode tests migration behavior using +the image built from the checkout; it does not validate the SRPM, repository, or +Packit provisioning path. ### Faster iteration cycles diff --git a/Justfile b/Justfile index d7dbd771d2..690d7d5a2e 100644 --- a/Justfile +++ b/Justfile @@ -48,7 +48,17 @@ erofs_version := env("BOOTC_erofs_version", "v1") # Baseconfigs to inject into the image for testing (e.g. "etc-transient" or "root-transient") baseconfigs := env("BOOTC_baseconfigs", "") # Base container image to build from -base := env("BOOTC_base", "quay.io/centos-bootc/centos-bootc:stream10") +default_base := "quay.io/centos-bootc/centos-bootc:stream10" +base := env("BOOTC_base", default_base) +# Package-mode tests default to Fedora to match test_disk_image=fedora. +# Honor BOOTC_base or a Justfile `base=` override when explicitly provided. +package_mode_base := if base != default_base { + base +} else if env("BOOTC_base", "") != "" { + base +} else { + "quay.io/fedora/fedora-bootc:latest" +} # Buildroot base image buildroot_base := env("BOOTC_buildroot_base", "quay.io/centos/centos:stream10") # Optional: path to extra source (e.g. composefs-rs) for local development @@ -195,7 +205,20 @@ test-tmt *ARGS: build # Run the package-mode TMT plan using its two TMT-provisioned guests. [group('core')] test-tmt-package-mode: - @tmt --context=running_env=packit --context="test_disk_image=${test_disk_image:-fedora}" run plans --name '^/tmt/plans/package-mode$' + #!/bin/bash + set -euo pipefail + just base={{package_mode_base}} build + if [[ "{{base_img}}" != localhost/bootc ]]; then + podman tag {{base_img}} localhost/bootc + fi + mkdir -p target + podman save --format oci-archive \ + --output target/package-mode-local.oci localhost/bootc + tmt \ + --context=running_env=packit \ + --context=package_source=local \ + --context="test_disk_image=${test_disk_image:-fedora}" \ + run plans --name '^/tmt/plans/package-mode$' # Split out from `test-container` because, unlike the container integration tests, # unit tests don't depend on variant/filesystem/bootloader/boot_type/seal_state, so diff --git a/hack/provision-packit.sh b/hack/provision-packit.sh index 6b6a40b65f..9c72091403 100755 --- a/hack/provision-packit.sh +++ b/hack/provision-packit.sh @@ -15,7 +15,10 @@ trap 'rm -rf -- "$BOOTC_TEMPDIR"' EXIT cp -a . "$BOOTC_TEMPDIR" # Keep testing farm run folder -cp -r /var/ARTIFACTS "$BOOTC_TEMPDIR" +if [[ -d /var/ARTIFACTS ]]; then + cp -a /var/ARTIFACTS "$BOOTC_TEMPDIR" +fi +mkdir -p "$BOOTC_TEMPDIR/ARTIFACTS" # Copy bootc repo cp -r /var/share/test-artifacts "$BOOTC_TEMPDIR" diff --git a/tmt/plans/copy-local-test-image.yml b/tmt/plans/copy-local-test-image.yml new file mode 100644 index 0000000000..dc8a2e7c0c --- /dev/null +++ b/tmt/plans/copy-local-test-image.yml @@ -0,0 +1,11 @@ +--- +- name: Copy locally built package-mode image archive to the harness + hosts: harness + tasks: + - name: Copy the local image archive from the repository checkout + ansible.builtin.copy: + src: "{{ playbook_dir }}/../../target/package-mode-local.oci" + dest: /var/tmp/bootc-package-mode-local.oci + owner: root + group: root + mode: '0600' diff --git a/tmt/plans/forward-test-artifacts.yml b/tmt/plans/forward-test-artifacts.yml new file mode 100644 index 0000000000..c8aa95fcd4 --- /dev/null +++ b/tmt/plans/forward-test-artifacts.yml @@ -0,0 +1,75 @@ +--- +- name: Forward Testing Farm artifacts from the TMT controller to the harness + hosts: harness + gather_facts: false + tasks: + - name: Check the controller artifact repository exists + ansible.builtin.stat: + path: /var/share/test-artifacts + delegate_to: localhost + register: artifact_repository + + - name: Check the controller artifact repository configuration exists + ansible.builtin.stat: + path: /etc/yum.repos.d/test-artifacts.repo + delegate_to: localhost + register: artifact_repository_config + + - name: Check whether the controller ARTIFACTS directory exists + ansible.builtin.stat: + path: /var/ARTIFACTS + delegate_to: localhost + register: artifacts_directory + + - name: Validate controller artifact paths + ansible.builtin.assert: + that: + - artifact_repository.stat.exists + - artifact_repository.stat.isdir + - artifact_repository_config.stat.exists + - artifact_repository_config.stat.isreg + fail_msg: >- + Cannot forward package-mode test artifacts: the TMT controller must + provide the /var/share/test-artifacts/ directory, + and /etc/yum.repos.d/test-artifacts.repo file. + + - name: Find source RPMs in the controller artifact repository + ansible.builtin.find: + paths: /var/share/test-artifacts + patterns: '*.src.rpm' + recurse: false + file_type: file + delegate_to: localhost + register: source_rpms + + - name: Validate the controller artifact repository contains a source RPM + ansible.builtin.assert: + that: + - source_rpms.matched | int == 1 + fail_msg: >- + Cannot forward package-mode test artifacts: exactly one top-level + .src.rpm file must be present in /var/share/test-artifacts/ on the + TMT controller. + + - name: Copy the full artifact repository to the harness + ansible.builtin.copy: + src: /var/share/test-artifacts/ + dest: /var/share/test-artifacts/ + owner: root + group: root + + - name: Copy the artifact repository configuration to the harness + ansible.builtin.copy: + src: /etc/yum.repos.d/test-artifacts.repo + dest: /etc/yum.repos.d/test-artifacts.repo + owner: root + group: root + mode: '0644' + + - name: Copy ARTIFACTS to the harness + ansible.builtin.copy: + src: /var/ARTIFACTS + dest: /var/ + owner: root + group: root + when: artifacts_directory.stat.exists and artifacts_directory.stat.isdir diff --git a/tmt/plans/package-mode.fmf b/tmt/plans/package-mode.fmf index da7a6a5e28..a3656f7c54 100644 --- a/tmt/plans/package-mode.fmf +++ b/tmt/plans/package-mode.fmf @@ -1,4 +1,6 @@ summary: Test package-mode to-existing-root migration flags +environment: + PACKAGE_SOURCE: $@{package_source} provision: - name: harness role: harness @@ -11,7 +13,32 @@ provision: image: $@{test_disk_image} user: root prepare: - # This is the normal integration guest setup, restricted to the harness. + # Reject missing/unknown sources before any mode-specific setup runs. + - how: shell + order: 95 + script: | + case "${PACKAGE_SOURCE:-}" in + artifacts|local) ;; + *) + printf 'package_source must be artifacts or local (got %q)\n' "${PACKAGE_SOURCE:-}" >&2 + exit 1 + ;; + esac + where: harness + # Forward Testing Farm artifacts before harness setup consumes them. + - how: ansible + order: 96 + playbook: tmt/plans/forward-test-artifacts.yml + where: harness + when: package_source == artifacts + # The local archive is copied from this checkout, independent of any + # Testing Farm controller paths. + - how: ansible + order: 96 + playbook: tmt/plans/copy-local-test-image.yml + where: harness + when: package_source == local + # Packit provisioning needs the full integration toolchain. - how: install order: 97 package: @@ -30,28 +57,46 @@ prepare: - dosfstools - e2fsprogs where: harness - when: running_env != image_mode + when: package_source == artifacts + # Local mode only needs Podman to derive and save the marker image. + - how: install + order: 97 + package: + - podman + where: harness + when: package_source == local - how: shell order: 98 script: - mkdir -p bootc && cp /var/share/test-artifacts/*.src.rpm bootc - cd bootc && rpm2cpio *.src.rpm > pkg.cpio && cpio -idmv < pkg.cpio && rm pkg.cpio && rm -f *-vendor.tar.zstd && zstd -d *.tar.zstd && tar -xvf *.tar -C . --strip-components=1 && ls -al - cd bootc/hack && ./provision-packit.sh - - | - workdir=$(mktemp -d) - trap 'rm -rf -- "$workdir"' EXIT - cat > "$workdir/Containerfile" <<'EOF' - FROM localhost/bootc - RUN mkdir -p /etc/bootc-tmt-merge && \ - printf '%s\n' image-default > /etc/bootc-tmt-merge/default.conf && \ - printf '%s\n' image-only > /etc/bootc-tmt-merge/image-only.conf - EOF - podman build -t localhost/bootc-to-existing-root-preserve-var-merge-etc:latest "$workdir" - podman save --format oci-archive \ - --output /var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci \ - localhost/bootc-to-existing-root-preserve-var-merge-etc:latest where: harness - when: running_env != image_mode + when: package_source == artifacts + - how: shell + order: 98 + script: podman load --input /var/tmp/bootc-package-mode-local.oci + where: harness + when: package_source == local + # Both sources now provide localhost/bootc. Derive the marker image once so + # the migration test consumes the same archive in either execution mode. + - how: shell + order: 99 + script: | + workdir=$(mktemp -d) + trap 'rm -rf -- "$workdir"' EXIT + cat > "$workdir/Containerfile" <<'EOF' + FROM localhost/bootc + RUN mkdir -p /etc/bootc-tmt-merge && \ + printf '%s\n' image-default > /etc/bootc-tmt-merge/default.conf && \ + printf '%s\n' image-only > /etc/bootc-tmt-merge/image-only.conf + EOF + podman build -t localhost/bootc-to-existing-root-preserve-var-merge-etc:latest "$workdir" + podman save --format oci-archive \ + --output /var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci \ + localhost/bootc-to-existing-root-preserve-var-merge-etc:latest + where: harness + when: package_source == artifacts or package_source == local - how: install order: 97 package: @@ -59,26 +104,26 @@ prepare: - jq - bootc where: target - when: running_env != image_mode + when: package_source == artifacts or package_source == local # Reboot only the image-mode harness after system-reinstall-bootc. - how: ansible - order: 99 + order: 100 playbook: - https://github.com/bootc-dev/bootc/raw/refs/heads/main/hack/packit-reboot.yml where: harness - when: running_env != image_mode + when: package_source == artifacts # Fetch to the TMT controller, then copy that exact archive to the target. # Separate ordered steps avoid relying on shared storage or guest ordering. - how: ansible - order: 100 - playbook: fetch-test-image.yml + order: 101 + playbook: tmt/plans/fetch-test-image.yml where: harness - when: running_env != image_mode + when: package_source == artifacts or package_source == local - how: ansible - order: 101 - playbook: copy-test-image.yml + order: 102 + playbook: tmt/plans/copy-test-image.yml where: target - when: running_env != image_mode + when: package_source == artifacts or package_source == local execute: how: tmt discover: diff --git a/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf b/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf index 04be7c53c3..823ffdfcff 100644 --- a/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf +++ b/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf @@ -1,7 +1,8 @@ -# The two-guest package-mode plan builds a test image on a separate harness VM, -# reboots that harness into image mode, and transfers the OCI archive to target. -# This test installs that image onto the package-mode target, reboots it, then -# verifies the preserved /var data and merged /etc configuration. +# The two-guest package-mode plan prepares a test image on a separate harness +# VM and transfers its OCI archive to target. Packit reboots the harness into +# image mode; local runs load a locally built image instead. This test installs +# the image onto the package-mode target, reboots it, then verifies preserved +# /var data and merged /etc configuration. summary: Package-mode /var preservation and /etc merge duration: 45m adjust: From 12dddec654efa1c1349dd1423d99a0a3e19943ad Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Thu, 8 Oct 2026 10:25:16 -0400 Subject: [PATCH 5/6] tmt: Stream and clean package-mode image transfers Stream package-mode image transfers and make staging, cleanup, and version validation reliable across test execution. Assisted-by: AI Signed-off-by: ckyrouac --- CONTRIBUTING.md | 30 ++++ Justfile | 24 ++- tmt/plans/check-transfer-prerequisites.yml | 57 +++++++ tmt/plans/cleanup-test-image.yml | 69 ++++++++ tmt/plans/copy-local-test-image.yml | 62 ++++++- tmt/plans/copy-test-image.yml | 11 -- tmt/plans/fetch-test-image.yml | 158 +++++++++++++++++- tmt/plans/forward-test-artifacts.yml | 108 ++++++++++-- tmt/plans/package-mode.fmf | 35 +++- ...to-existing-root-preserve-var-merge-etc.sh | 31 +++- ...o-existing-root-preserve-var-merge-etc.fmf | 8 +- 11 files changed, 540 insertions(+), 53 deletions(-) create mode 100644 tmt/plans/check-transfer-prerequisites.yml create mode 100644 tmt/plans/cleanup-test-image.yml delete mode 100644 tmt/plans/copy-test-image.yml diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 79e45f0f8f..38539702bd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -94,6 +94,21 @@ Fedora bootc base and uses TMT's `fedora` image alias (the latest released Fedora image). This performs a full local image build, so it takes longer and uses more disk/network than simply launching the VM test. The build archive is stored under the ignored `target/` directory and copied to the harness VM. +For local Just runs, TMT uses a temporary metadata root containing only +`.fmf/` and `tmt/`, so ignored build output is not synchronized to either VM; +the OCI archive stays in `target/` and is synchronized separately. +TMT's run workdir defaults to +`${XDG_CACHE_HOME:-$HOME/.cache}/bootc/tmt-package-mode-runs`. It must be outside +the checkout and temporary metadata root, and visible at the same absolute path +to both the TMT container and the host session-libvirt process so libvirt can +access seed images created by TMT. Set `TMT_WORKDIR_ROOT` to use a different +shared directory that meets these requirements: + +```bash +TMT_WORKDIR_ROOT=/shared/tmt-package-mode-runs just test-tmt-package-mode +``` + +The target creates the directory if needed and passes its absolute path to TMT. To use a different guest image, set `test_disk_image`: ```bash @@ -122,6 +137,21 @@ migration test and archive transfer. Local mode tests migration behavior using the image built from the checkout; it does not validate the SRPM, repository, or Packit provisioning path. +Package-mode OCI archives and artifact directories are transferred from the +harness with `ansible.posix.synchronize` (rsync over TMT's forwarded SSH +connection). This avoids Ansible `fetch`/`copy` falling back to a piped SSH +transfer that can buffer a 1 GB+ archive in controller memory; the original +SFTP/SCP errors were not recorded. The TMT controller must have `rsync` and the +`ansible.posix` collection installed. TMT 1.79 or newer is required on both the +local and Testing Farm controllers. The plan installs `rsync` on both guests +from their configured base repositories before synchronizing any payloads. +The verified archive and its SHA256 sidecar are staged under the run-specific +controller `TMT_PLAN_DATA/package-mode-transfer` directory. TMT 1.79 pushes +controller plan data to the selected test guest before execute, where the test +checks the sidecar and loads the archive. A finish playbook removes controller +staging after test success or failure; the fetch playbook still cleans up if +staging or checksum verification fails during prepare. + ### Faster iteration cycles The test cycle currently builds a disk image and creates a new ephemeral diff --git a/Justfile b/Justfile index 690d7d5a2e..26f27473f7 100644 --- a/Justfile +++ b/Justfile @@ -207,18 +207,36 @@ test-tmt *ARGS: build test-tmt-package-mode: #!/bin/bash set -euo pipefail + repo_root="$(git rev-parse --show-toplevel)" + repo_root="$(realpath -- "$repo_root")" + metadata_root="$(realpath -- "$(mktemp -d "${TMPDIR:-/tmp}/bootc-tmt-package-mode.XXXXXXXX")")" + trap 'rm -rf -- "$metadata_root"' EXIT + cp -a -- "$repo_root/.fmf" "$metadata_root/" + cp -a -- "$repo_root/tmt" "$metadata_root/" + workdir_root="${TMT_WORKDIR_ROOT:-${XDG_CACHE_HOME:-$HOME/.cache}/bootc/tmt-package-mode-runs}" + workdir_root="$(realpath -m -- "$workdir_root")" + if [[ "$workdir_root" == "$repo_root" || "$workdir_root" == "$repo_root/"* ]]; then + echo "TMT workdir root must be outside the checkout (fmf root): $workdir_root" >&2 + exit 1 + fi + if [[ "$workdir_root" == "$metadata_root" || "$workdir_root" == "$metadata_root/"* ]]; then + echo "TMT workdir root must be outside the temporary metadata root: $workdir_root" >&2 + exit 1 + fi + mkdir -p -- "$workdir_root" just base={{package_mode_base}} build if [[ "{{base_img}}" != localhost/bootc ]]; then podman tag {{base_img}} localhost/bootc fi - mkdir -p target podman save --format oci-archive \ --output target/package-mode-local.oci localhost/bootc - tmt \ + archive_path="$(realpath -- target/package-mode-local.oci)" + PACKAGE_MODE_IMAGE_ARCHIVE="$archive_path" tmt \ + --root "$metadata_root" \ --context=running_env=packit \ --context=package_source=local \ --context="test_disk_image=${test_disk_image:-fedora}" \ - run plans --name '^/tmt/plans/package-mode$' + run --workdir-root "$workdir_root" plans --name '^/tmt/plans/package-mode$' # Split out from `test-container` because, unlike the container integration tests, # unit tests don't depend on variant/filesystem/bootloader/boot_type/seal_state, so diff --git a/tmt/plans/check-transfer-prerequisites.yml b/tmt/plans/check-transfer-prerequisites.yml new file mode 100644 index 0000000000..c190fee78d --- /dev/null +++ b/tmt/plans/check-transfer-prerequisites.yml @@ -0,0 +1,57 @@ +--- +- name: Check package-mode transfer prerequisites + hosts: harness + gather_facts: false + tasks: + - name: Check TMT is available on the controller + ansible.builtin.command: tmt --version + delegate_to: localhost + register: controller_tmt_version_output + changed_when: false + failed_when: false + + - name: Require TMT version output on the controller + ansible.builtin.assert: + that: + - controller_tmt_version_output.rc == 0 + fail_msg: >- + Unable to run tmt --version on the TMT controller. Install TMT 1.79.0 + or newer and rerun the test. + + - name: Parse the TMT controller version + ansible.builtin.set_fact: + controller_tmt_version: >- + {{ controller_tmt_version_output.stdout + | regex_search('(? 0 + fail_msg: >- + Could not parse a semantic version from TMT controller output: + {{ controller_tmt_version_output.stdout | default('') }}. + TMT 1.79.0 or newer is required. + + - name: Require TMT 1.79.0 or newer on the controller + ansible.builtin.assert: + that: + - controller_tmt_version is version('1.79.0', '>=', version_type='semver') + fail_msg: >- + TMT {{ controller_tmt_version }} is too old on the TMT controller; + version 1.79.0 or newer is required to push plan data before execute. + + - name: Check rsync is available on the TMT controller + ansible.builtin.command: rsync --version + delegate_to: localhost + register: controller_rsync + changed_when: false + failed_when: false + + - name: Require rsync on the TMT controller + ansible.builtin.assert: + that: + - controller_rsync.rc == 0 + fail_msg: >- + Package-mode image and artifact transfers require rsync on the TMT + controller. Install rsync and rerun the test. diff --git a/tmt/plans/cleanup-test-image.yml b/tmt/plans/cleanup-test-image.yml new file mode 100644 index 0000000000..b2944f886d --- /dev/null +++ b/tmt/plans/cleanup-test-image.yml @@ -0,0 +1,69 @@ +--- +- name: Clean up the package-mode archive staging directory + hosts: harness + gather_facts: false + tasks: + - name: Read TMT plan data directory + ansible.builtin.set_fact: + package_mode_plan_data: "{{ lookup('ansible.builtin.env', 'TMT_PLAN_DATA') }}" + + - name: Validate TMT plan data directory + ansible.builtin.assert: + that: + - package_mode_plan_data is match('^/(?:[^/]+/)*run-[^/]+/tmt/plans/package-mode/data$') + fail_msg: >- + TMT_PLAN_DATA must be an absolute path ending in + /run-/tmt/plans/package-mode/data before removing package-mode + archive staging. + + - name: Canonicalize TMT plan data directory on the controller + ansible.builtin.command: + argv: + - realpath + - -e + - -- + - "{{ package_mode_plan_data }}" + delegate_to: localhost + register: canonical_plan_data + changed_when: false + failed_when: false + + - name: Require the canonical TMT plan data directory + ansible.builtin.assert: + that: + - canonical_plan_data.rc == 0 + - canonical_plan_data.stdout == package_mode_plan_data + - canonical_plan_data.stdout is match('^/(?:[^/]+/)*run-[^/]+/tmt/plans/package-mode/data$') + - canonical_plan_data.stdout is search('/tmt/plans/package-mode/data$') + fail_msg: >- + TMT_PLAN_DATA must be the canonical, existing controller path ending + in /tmt/plans/package-mode/data; symlinks and non-canonical paths are + not allowed. + + - name: Set the run-specific controller staging directory + ansible.builtin.set_fact: + package_mode_staging_dir: "{{ package_mode_plan_data }}/package-mode-transfer" + + - name: Inspect the controller staging path + ansible.builtin.stat: + path: "{{ package_mode_staging_dir }}" + delegate_to: localhost + register: package_mode_staging + + - name: Require a safe controller staging path + ansible.builtin.assert: + that: + - not (package_mode_staging.stat.islnk | default(false)) + - >- + not (package_mode_staging.stat.exists | default(false)) or + (package_mode_staging.stat.isdir | default(false)) + fail_msg: >- + The package-mode staging path must be absent or a real directory; + symlinks and non-directories are not allowed. + + - name: Remove controller staging after test completion + ansible.builtin.file: + path: "{{ package_mode_staging_dir }}" + state: absent + delegate_to: localhost + when: package_mode_staging.stat.exists | default(false) diff --git a/tmt/plans/copy-local-test-image.yml b/tmt/plans/copy-local-test-image.yml index dc8a2e7c0c..23d8b77364 100644 --- a/tmt/plans/copy-local-test-image.yml +++ b/tmt/plans/copy-local-test-image.yml @@ -1,11 +1,61 @@ --- - name: Copy locally built package-mode image archive to the harness hosts: harness + gather_facts: false tasks: - - name: Copy the local image archive from the repository checkout - ansible.builtin.copy: - src: "{{ playbook_dir }}/../../target/package-mode-local.oci" + - name: Read the local image archive path from the controller environment + ansible.builtin.set_fact: + local_archive_path: "{{ lookup('ansible.builtin.env', 'PACKAGE_MODE_IMAGE_ARCHIVE') }}" + + - name: Require an absolute local image archive path + ansible.builtin.assert: + that: + - local_archive_path | length > 0 + - local_archive_path is match('^/') + fail_msg: >- + PACKAGE_MODE_IMAGE_ARCHIVE must be set to the absolute path of the + local package-mode OCI archive by the Just invocation. + delegate_to: localhost + + - name: Inspect the local image archive on the controller + ansible.builtin.stat: + path: "{{ local_archive_path }}" + checksum_algorithm: sha256 + delegate_to: localhost + register: local_archive + + - name: Require the local image archive + ansible.builtin.assert: + that: + - local_archive.stat.exists + - local_archive.stat.isreg + fail_msg: >- + The local package-mode OCI archive was not found at + {{ local_archive_path }}. Build it with just + test-tmt-package-mode before running the TMT plan. + delegate_to: localhost + + - name: Stream the local image archive to the harness + ansible.posix.synchronize: + src: "{{ local_archive_path }}" dest: /var/tmp/bootc-package-mode-local.oci - owner: root - group: root - mode: '0600' + mode: push + checksum: true + owner: false + group: false + rsync_opts: + - --chmod=F600 + use_ssh_args: true + + - name: Inspect the copied image archive on the harness + ansible.builtin.stat: + path: /var/tmp/bootc-package-mode-local.oci + checksum_algorithm: sha256 + register: harness_archive + + - name: Verify the local image archive transfer + ansible.builtin.assert: + that: + - harness_archive.stat.exists + - harness_archive.stat.checksum == local_archive.stat.checksum + fail_msg: The local image archive SHA256 differs after transfer to the harness. diff --git a/tmt/plans/copy-test-image.yml b/tmt/plans/copy-test-image.yml deleted file mode 100644 index ab1eef15d7..0000000000 --- a/tmt/plans/copy-test-image.yml +++ /dev/null @@ -1,11 +0,0 @@ ---- -- name: Copy build-under-test image archive from the TMT controller to target - hosts: target - tasks: - - name: Copy the same image archive to the target - ansible.builtin.copy: - src: /tmp/bootc-to-existing-root-preserve-var-merge-etc.oci - dest: /var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci - owner: root - group: root - mode: '0600' diff --git a/tmt/plans/fetch-test-image.yml b/tmt/plans/fetch-test-image.yml index 1b50126c09..262d14b038 100644 --- a/tmt/plans/fetch-test-image.yml +++ b/tmt/plans/fetch-test-image.yml @@ -1,9 +1,157 @@ --- - name: Fetch build-under-test image archive to the TMT controller hosts: harness + gather_facts: false tasks: - - name: Fetch the image archive from the harness - ansible.builtin.fetch: - src: /var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci - dest: /tmp/bootc-to-existing-root-preserve-var-merge-etc.oci - flat: true + - name: Read TMT plan data directory + ansible.builtin.set_fact: + package_mode_plan_data: "{{ lookup('ansible.builtin.env', 'TMT_PLAN_DATA') }}" + + - name: Validate TMT plan data directory + ansible.builtin.assert: + that: + - package_mode_plan_data is match('^/(?:[^/]+/)*run-[^/]+/tmt/plans/package-mode/data$') + fail_msg: >- + TMT_PLAN_DATA must be an absolute path ending in + /run-/tmt/plans/package-mode/data before staging the package-mode + archive. + + - name: Canonicalize TMT plan data directory on the controller + ansible.builtin.command: + argv: + - realpath + - -e + - -- + - "{{ package_mode_plan_data }}" + delegate_to: localhost + register: canonical_plan_data + changed_when: false + failed_when: false + + - name: Require the canonical TMT plan data directory + ansible.builtin.assert: + that: + - canonical_plan_data.rc == 0 + - canonical_plan_data.stdout == package_mode_plan_data + - canonical_plan_data.stdout is match('^/(?:[^/]+/)*run-[^/]+/tmt/plans/package-mode/data$') + - canonical_plan_data.stdout is search('/tmt/plans/package-mode/data$') + fail_msg: >- + TMT_PLAN_DATA must be the canonical, existing controller path ending + in /tmt/plans/package-mode/data; symlinks and non-canonical paths are + not allowed. + + - name: Set the run-specific controller staging directory + ansible.builtin.set_fact: + package_mode_staging_dir: "{{ package_mode_plan_data }}/package-mode-transfer" + + - name: Inspect the controller staging path + ansible.builtin.stat: + path: "{{ package_mode_staging_dir }}" + delegate_to: localhost + register: package_mode_staging + + - name: Require a safe controller staging path + ansible.builtin.assert: + that: + - not (package_mode_staging.stat.islnk | default(false)) + - >- + not (package_mode_staging.stat.exists | default(false)) or + (package_mode_staging.stat.isdir | default(false)) + fail_msg: >- + The package-mode staging path must be absent or a real directory; + symlinks and non-directories are not allowed. + + - name: Remove stale controller staging directory + ansible.builtin.file: + path: "{{ package_mode_staging_dir }}" + state: absent + delegate_to: localhost + when: package_mode_staging.stat.exists | default(false) + + - name: Stage and verify the generated image archive + block: + - name: Inspect the generated image archive on the harness + ansible.builtin.stat: + path: /var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci + checksum_algorithm: sha256 + ignore_unreachable: true + register: harness_archive + + - name: Require the generated image archive + ansible.builtin.assert: + that: + - >- + not (harness_archive.unreachable | default(false)) and + harness_archive.stat.exists and harness_archive.stat.isreg + fail_msg: The generated package-mode image archive is missing on the harness. + + - name: Create run-specific controller staging directory + ansible.builtin.file: + path: "{{ package_mode_staging_dir }}" + state: directory + mode: '0700' + delegate_to: localhost + + - name: Stream the generated image archive to controller staging + ansible.posix.synchronize: + mode: pull + src: /var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci + dest: "{{ package_mode_staging_dir }}/bootc-to-existing-root-preserve-var-merge-etc.oci" + checksum: true + archive: false + use_ssh_args: true + ignore_unreachable: true + register: archive_transfer + + - name: Require a reachable harness transfer + ansible.builtin.assert: + that: + - not (archive_transfer.unreachable | default(false)) + fail_msg: The harness became unreachable while transferring the package-mode archive. + + - name: Inspect the staged image archive on the controller + ansible.builtin.stat: + path: "{{ package_mode_staging_dir }}/bootc-to-existing-root-preserve-var-merge-etc.oci" + checksum_algorithm: sha256 + delegate_to: localhost + register: staged_archive + + - name: Verify the generated image archive transfer + ansible.builtin.assert: + that: + - staged_archive.stat.exists + - staged_archive.stat.checksum == harness_archive.stat.checksum + fail_msg: The generated image archive SHA256 differs after transfer to the controller. + + - name: Create the staged archive SHA256 sidecar + ansible.builtin.command: + argv: + - sha256sum + - -- + - bootc-to-existing-root-preserve-var-merge-etc.oci + chdir: "{{ package_mode_staging_dir }}" + delegate_to: localhost + register: staged_archive_checksum + changed_when: false + + - name: Save the staged archive SHA256 sidecar + ansible.builtin.copy: + content: "{{ staged_archive_checksum.stdout }}\n" + dest: "{{ package_mode_staging_dir }}/bootc-to-existing-root-preserve-var-merge-etc.oci.sha256" + mode: '0600' + delegate_to: localhost + + - name: Mark the staged archive and checksum ready for TMT execute + ansible.builtin.set_fact: + package_mode_fetch_succeeded: true + rescue: + - name: Report the failed archive fetch + ansible.builtin.fail: + msg: Fetching or verifying the package-mode image archive failed. + always: + - name: Remove controller staging after a failed fetch + ansible.builtin.file: + path: "{{ package_mode_staging_dir }}" + state: absent + delegate_to: localhost + when: not (package_mode_fetch_succeeded | default(false)) diff --git a/tmt/plans/forward-test-artifacts.yml b/tmt/plans/forward-test-artifacts.yml index c8aa95fcd4..7711b73e7a 100644 --- a/tmt/plans/forward-test-artifacts.yml +++ b/tmt/plans/forward-test-artifacts.yml @@ -51,12 +51,54 @@ .src.rpm file must be present in /var/share/test-artifacts/ on the TMT controller. - - name: Copy the full artifact repository to the harness - ansible.builtin.copy: + - name: Calculate SHA256 checksums for controller artifact files + ansible.builtin.find: + paths: /var/share/test-artifacts + recurse: true + file_type: file + get_checksum: true + checksum_algorithm: sha256 + delegate_to: localhost + register: source_artifacts + + - name: Create the artifact repository directory on the harness + ansible.builtin.file: + path: /var/share/test-artifacts + state: directory + mode: '0755' + + - name: Stream the full artifact repository to the harness + ansible.posix.synchronize: src: /var/share/test-artifacts/ dest: /var/share/test-artifacts/ - owner: root - group: root + checksum: true + owner: false + group: false + use_ssh_args: true + + - name: Calculate SHA256 checksums for harness artifact files + ansible.builtin.find: + paths: /var/share/test-artifacts + recurse: true + file_type: file + get_checksum: true + checksum_algorithm: sha256 + register: harness_artifacts + + - name: Verify the artifact repository transfer + ansible.builtin.assert: + that: + - >- + (source_artifacts.files | map(attribute='path') + | map('regex_replace', '^/var/share/test-artifacts/', '') + | zip(source_artifacts.files | map(attribute='checksum')) + | list | sort) + == + (harness_artifacts.files | map(attribute='path') + | map('regex_replace', '^/var/share/test-artifacts/', '') + | zip(harness_artifacts.files | map(attribute='checksum')) + | list | sort) + fail_msg: The artifact repository SHA256 manifest differs after transfer to the harness. - name: Copy the artifact repository configuration to the harness ansible.builtin.copy: @@ -66,10 +108,56 @@ group: root mode: '0644' - - name: Copy ARTIFACTS to the harness - ansible.builtin.copy: - src: /var/ARTIFACTS - dest: /var/ - owner: root - group: root + - name: Calculate SHA256 checksums for controller ARTIFACTS files + ansible.builtin.find: + paths: /var/ARTIFACTS + recurse: true + file_type: file + get_checksum: true + checksum_algorithm: sha256 + delegate_to: localhost + register: source_artifacts_directory + when: artifacts_directory.stat.exists and artifacts_directory.stat.isdir + + - name: Create the ARTIFACTS directory on the harness + ansible.builtin.file: + path: /var/ARTIFACTS + state: directory + mode: '0755' + when: artifacts_directory.stat.exists and artifacts_directory.stat.isdir + + - name: Stream optional ARTIFACTS to the harness + ansible.posix.synchronize: + src: /var/ARTIFACTS/ + dest: /var/ARTIFACTS/ + checksum: true + owner: false + group: false + use_ssh_args: true + when: artifacts_directory.stat.exists and artifacts_directory.stat.isdir + + - name: Calculate SHA256 checksums for harness ARTIFACTS files + ansible.builtin.find: + paths: /var/ARTIFACTS + recurse: true + file_type: file + get_checksum: true + checksum_algorithm: sha256 + register: harness_artifacts_directory + when: artifacts_directory.stat.exists and artifacts_directory.stat.isdir + + - name: Verify the optional ARTIFACTS transfer + ansible.builtin.assert: + that: + - >- + (source_artifacts_directory.files | map(attribute='path') + | map('regex_replace', '^/var/ARTIFACTS/', '') + | zip(source_artifacts_directory.files | map(attribute='checksum')) + | list | sort) + == + (harness_artifacts_directory.files | map(attribute='path') + | map('regex_replace', '^/var/ARTIFACTS/', '') + | zip(harness_artifacts_directory.files | map(attribute='checksum')) + | list | sort) + fail_msg: The ARTIFACTS SHA256 manifest differs after transfer to the harness. when: artifacts_directory.stat.exists and artifacts_directory.stat.isdir diff --git a/tmt/plans/package-mode.fmf b/tmt/plans/package-mode.fmf index a3656f7c54..9a55865f20 100644 --- a/tmt/plans/package-mode.fmf +++ b/tmt/plans/package-mode.fmf @@ -13,6 +13,26 @@ provision: image: $@{test_disk_image} user: root prepare: + # rsync is used for all large transfers. Install it before any of the + # package-source-specific synchronization steps below. + - how: install + order: 93 + package: + - rsync + where: harness + when: package_source == artifacts or package_source == local + - how: install + order: 93 + package: + - rsync + where: target + when: package_source == artifacts or package_source == local + # synchronize runs rsync on the TMT controller as well as each guest. + - how: ansible + order: 94 + playbook: tmt/plans/check-transfer-prerequisites.yml + where: harness + when: package_source == artifacts or package_source == local # Reject missing/unknown sources before any mode-specific setup runs. - how: shell order: 95 @@ -112,18 +132,14 @@ prepare: - https://github.com/bootc-dev/bootc/raw/refs/heads/main/hack/packit-reboot.yml where: harness when: package_source == artifacts - # Fetch to the TMT controller, then copy that exact archive to the target. - # Separate ordered steps avoid relying on shared storage or guest ordering. + # Stage and verify the archive on the controller. TMT pushes plan data to the + # selected test guest before execute, so no separate controller-to-target + # transfer is needed. - how: ansible order: 101 playbook: tmt/plans/fetch-test-image.yml where: harness when: package_source == artifacts or package_source == local - - how: ansible - order: 102 - playbook: tmt/plans/copy-test-image.yml - where: target - when: package_source == artifacts or package_source == local execute: how: tmt discover: @@ -131,3 +147,8 @@ discover: where: target test: - /tmt/tests/test-50-to-existing-root-preserve-var-merge-etc +finish: + - how: ansible + playbook: tmt/plans/cleanup-test-image.yml + where: harness + when: package_source == artifacts or package_source == local diff --git a/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh b/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh index d00875de63..a4f811b6bd 100644 --- a/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh +++ b/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh @@ -2,20 +2,37 @@ set -xeuo pipefail target_image=localhost/bootc-to-existing-root-preserve-var-merge-etc:latest -target_image_archive=/var/tmp/bootc-to-existing-root-preserve-var-merge-etc.oci +archive_name=bootc-to-existing-root-preserve-var-merge-etc.oci +archive_dir="${TMT_PLAN_DATA:-}/package-mode-transfer" +target_archive="$archive_dir/$archive_name" +target_archive_checksum="$target_archive.sha256" var_marker=/var/lib/bootc-tmt-preserve-var/sentinel etc_dir=/etc/bootc-tmt-merge case ${TMT_REBOOT_COUNT:-0} in 0) + # TMT pushes plan data to the selected test guest before execute. Fail + # clearly if either staged file did not arrive with that data. + if [[ -z "${TMT_PLAN_DATA:-}" ]]; then + echo 'TMT_PLAN_DATA is missing in the package-mode test guest' >&2 + exit 1 + fi + if [[ ! -f "$target_archive" || -L "$target_archive" || ! -s "$target_archive" ]]; then + echo "Package-mode image archive is missing or invalid: $target_archive" >&2 + exit 1 + fi + if [[ ! -f "$target_archive_checksum" || -L "$target_archive_checksum" ]]; then + echo "Package-mode image archive checksum is missing or invalid: $target_archive_checksum" >&2 + exit 1 + fi + (cd "$archive_dir" && sha256sum -c -- "$archive_name.sha256") + podman --remote=false load < "$target_archive" + podman --remote=false image exists "$target_image" + rm -- "$target_archive" "$target_archive_checksum" + # The target guest must still be the original package-mode system. test ! -e /run/ostree-booted - # The image was built from the harness's build-under-test image and - # transferred as an archive; do not rebuild a potentially different image. - podman load --input "$target_image_archive" - podman image exists "$target_image" - # Exercise both an admin-created file and an admin modification of an # image default. The derived image below supplies the pristine values. mkdir -p "$etc_dir" "${var_marker%/*}" @@ -33,7 +50,7 @@ case ${TMT_REBOOT_COUNT:-0} in # The flags require the OSTree backend. Run the current build-under-test # as a privileged installer, with the package-mode root mounted at /target. - podman run --rm --privileged --pid=host --user=root:root \ + podman --remote=false run --rm --privileged --pid=host --user=root:root \ -v /dev:/dev \ -v /:/target \ -v /var/lib/containers:/var/lib/containers \ diff --git a/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf b/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf index 823ffdfcff..93f49a1526 100644 --- a/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf +++ b/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf @@ -1,8 +1,8 @@ # The two-guest package-mode plan prepares a test image on a separate harness -# VM and transfers its OCI archive to target. Packit reboots the harness into -# image mode; local runs load a locally built image instead. This test installs -# the image onto the package-mode target, reboots it, then verifies preserved -# /var data and merged /etc configuration. +# VM and stages its OCI archive for TMT's plan-data push to the target guest. +# Packit reboots the harness into image mode; local runs load a locally built +# image instead. This test installs the image onto the package-mode target, +# reboots it, then verifies preserved /var data and merged /etc configuration. summary: Package-mode /var preservation and /etc merge duration: 45m adjust: From e04ef8a302c4c23a26c4a82ec6f7ce1416386f1a Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Thu, 8 Oct 2026 13:58:56 -0400 Subject: [PATCH 6/6] tmt: Prepare the package-mode boot test target Prepare the target's boot filesystem and image state required by the package-mode boot test. Assisted-by: AI Signed-off-by: ckyrouac --- CONTRIBUTING.md | 3 +++ tmt/plans/package-mode.fmf | 14 +++++++++++++- ...test-to-existing-root-preserve-var-merge-etc.sh | 1 + 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 38539702bd..6c70247076 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -137,6 +137,9 @@ migration test and archive transfer. Local mode tests migration behavior using the image built from the checkout; it does not validate the SRPM, repository, or Packit provisioning path. +In both package-source modes, the target preloads the test's bound container +images because the installer resolves those references from target-side storage. + Package-mode OCI archives and artifact directories are transferred from the harness with `ansible.posix.synchronize` (rsync over TMT's forwarded SSH connection). This avoids Ansible `fetch`/`copy` falling back to a piped SSH diff --git a/tmt/plans/package-mode.fmf b/tmt/plans/package-mode.fmf index 9a55865f20..70c8ca4564 100644 --- a/tmt/plans/package-mode.fmf +++ b/tmt/plans/package-mode.fmf @@ -107,7 +107,9 @@ prepare: trap 'rm -rf -- "$workdir"' EXIT cat > "$workdir/Containerfile" <<'EOF' FROM localhost/bootc - RUN mkdir -p /etc/bootc-tmt-merge && \ + # Fedora Cloud and fedora-bootc have different /etc/ssl/certs baselines. + RUN rm -rf /etc/ssl/certs && \ + mkdir -p /etc/bootc-tmt-merge && \ printf '%s\n' image-default > /etc/bootc-tmt-merge/default.conf && \ printf '%s\n' image-only > /etc/bootc-tmt-merge/image-only.conf EOF @@ -125,6 +127,16 @@ prepare: - bootc where: target when: package_source == artifacts or package_source == local + # The installer resolves stored LBI references against target-side Podman + # storage, so preload every image used by the shared migration test. + - how: shell + order: 98 + script: | + podman --remote=false pull quay.io/curl/curl:latest + podman --remote=false pull quay.io/curl/curl-base:latest + podman --remote=false pull registry.access.redhat.com/ubi9/podman:latest + where: target + when: package_source == artifacts or package_source == local # Reboot only the image-mode harness after system-reinstall-bootc. - how: ansible order: 100 diff --git a/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh b/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh index a4f811b6bd..9be193675c 100644 --- a/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh +++ b/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh @@ -53,6 +53,7 @@ case ${TMT_REBOOT_COUNT:-0} in podman --remote=false run --rm --privileged --pid=host --user=root:root \ -v /dev:/dev \ -v /:/target \ + -v /boot:/target/boot \ -v /var/lib/containers:/var/lib/containers \ --security-opt label=type:unconfined_t \ "$target_image" \