Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .packit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,28 @@ jobs:
context:
running_env: "packit"

# This plan runs the package-mode migration on one guest. Testing Farm's
# default pipeline provisions a single Artemis guest for this plan.
- 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"
package_source: "artifacts"

# ci/merge: full target set
- job: tests
trigger: pull_request
Expand All @@ -124,6 +146,28 @@ 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"
package_source: "artifacts"

- job: propose_downstream
trigger: release
dist_git_branches:
Expand Down
23 changes: 22 additions & 1 deletion Justfile
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ 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)
# 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
Expand Down Expand Up @@ -192,6 +193,26 @@ test-tmt *ARGS: build
@just _build-upgrade-image
@just test-tmt-nobuild {{ARGS}}

# The package-mode plan needs a local image archive to transfer into its guest.
# Run the plan using one TMT-provisioned guest.
[group('core')]
test-tmt-package-mode:
#!/bin/bash
set -euo pipefail
# Build the local image, ensure its tag, and export it for the guest.
just base={{base}} build
if [[ "{{base_img}}" != localhost/bootc ]]; then
podman tag {{base_img}} localhost/bootc
fi
podman save --format oci-archive \
--output target/package-mode-local.oci localhost/bootc
archive_path="$(realpath -- target/package-mode-local.oci)"
PACKAGE_MODE_IMAGE_ARCHIVE="$archive_path" tmt \
--context=running_env=packit \
--context=package_source=local \
--context="test_disk_image=${test_disk_image:-centos-stream-10}" \
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
Expand Down
172 changes: 160 additions & 12 deletions crates/etc-merge/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,28 @@ pub fn compute_diff(
pristine_etc_files: &FileSystem<CustomMetadata>,
current_etc_files: &FileSystem<CustomMetadata>,
new_etc_files: &FileSystem<CustomMetadata>,
) -> anyhow::Result<Diff> {
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<CustomMetadata>,
current_etc_files: &FileSystem<CustomMetadata>,
new_etc_files: &FileSystem<CustomMetadata>,
) -> anyhow::Result<Diff> {
compute_diff_impl(pristine_etc_files, current_etc_files, new_etc_files, false)
}

fn compute_diff_impl(
pristine_etc_files: &FileSystem<CustomMetadata>,
current_etc_files: &FileSystem<CustomMetadata>,
new_etc_files: &FileSystem<CustomMetadata>,
detect_deletions: bool,
) -> anyhow::Result<Diff> {
let mut diff = Diff {
added: vec![],
Expand All @@ -488,12 +510,14 @@ pub fn compute_diff(
&mut diff,
)?;

get_deletions(
&pristine_etc_files.root,
&current_etc_files.root,
PathBuf::new(),
&mut diff,
)?;
if detect_deletions {
get_deletions(
&pristine_etc_files.root,
&current_etc_files.root,
PathBuf::new(),
&mut diff,
)?;
}

Ok(diff)
}
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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, &current, Some(&new))?;
let new_tree = new_tree.unwrap();
let diff = compute_diff_without_deletions(&pristine_tree, &current_tree, &new_tree)?;

merge(&current, &current_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, &current, Some(&new))?;
let diff = compute_diff(&pristine_tree, &current_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()
Expand Down Expand Up @@ -1291,12 +1372,79 @@ mod tests {

let merge_res = merge(&c, &current_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, &current_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, &current_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, &current_tree, &new_tree)?;

merge(&c, &current_tree, &n, &new_tree, &diff)?;

assert_eq!(n.read("conflict")?, b"host contents");
assert!(n.metadata("conflict")?.is_file());

Ok(())
}
}
5 changes: 5 additions & 0 deletions crates/lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No GRUB rollback entry is written anymore; this line and the PR description still say so.

/// forward `/etc` customisations via a 3-way merge.
ToExistingRoot(crate::install::InstallToExistingRootOpts),
/// Nondestructively create a fresh installation state inside an existing bootc system.
///
Expand Down Expand Up @@ -2382,6 +2386,7 @@ async fn run_from_opt(opt: Opt) -> Result<CliExitStatus> {
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
Expand Down
Loading
Loading