cfs/stage: Compute etc diff before staging

Before we write the staged deployment file at `/run/composefs` and
before we create new boot entries, we now compute the etc diff in
advance to check whethere we will face any merge conflicts while
finalizing the deployment. If we find conflicts we exit with and error
and detailing where conflicts were found

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>

Author: Johan-Liebert1

Justfile

@@ -217,7 +217,7 @@ test-composefs bootloader filesystem boot_type seal_state *ARGS:
                 --seal-state={{seal_state}} \
                 --boot-type={{boot_type}} \
                 {{ARGS}} \
-                $(if [ "{{boot_type}}" = "uki" ] && [ "{{seal_state}}" = "sealed" ]; then echo "readonly image-upgrade-reboot"; else echo "integration"; fi)
+                $(if [ "{{boot_type}}" = "uki" ] && [ "{{seal_state}}" = "sealed" ]; then echo "readonly image-upgrade-reboot"; else echo "etc-merge-conflict"; fi)
 
 # Run upgrade test: boot VM from published base image (with tmt deps added),
 # upgrade to locally-built image, reboot, then run readonly tests to verify.

crates/etc-merge/src/lib.rs

@@ -80,8 +80,8 @@ fn stat_eq_ignore_mtime(this: &Stat, other: &Stat) -> bool {
 
 #[derive(Debug)]
 pub struct UnmergablePaths {
-    path: PathBuf,
-    reason: String,
+    pub path: PathBuf,
+    pub reason: String,
 }
 
 /// Represents the differences between two directory trees.
@@ -95,7 +95,7 @@ pub struct Diff {
     /// Paths that exist in the pristine /etc but not in the current one
     removed: Vec<PathBuf>,
     /// Paths that are unmergable
-    unmergable_paths: Vec<UnmergablePaths>,
+    pub unmergable_paths: Vec<UnmergablePaths>,
 }
 
 fn collect_all_files(
@@ -485,6 +485,19 @@ pub fn compute_diff(
     Ok(diff)
 }
 
+pub fn print_unmergable_paths(diff: &Diff, writer: &mut impl Write) {
+    use owo_colors::OwoColorize;
+
+    for unmergable in &diff.unmergable_paths {
+        let _ = writeln!(
+            writer,
+            "{} {}",
+            ModificationType::Unmergable.magenta(),
+            unmergable.reason
+        );
+    }
+}
+
 /// Prints a colorized summary of differences to standard output.
 pub fn print_diff(diff: &Diff, writer: &mut impl Write) {
     use owo_colors::OwoColorize;
@@ -501,14 +514,7 @@ pub fn print_diff(diff: &Diff, writer: &mut impl Write) {
         let _ = writeln!(writer, "{} {removed:?}", ModificationType::Removed.red());
     }
 
-    for unmergable in &diff.unmergable_paths {
-        let _ = writeln!(
-            writer,
-            "{} {}",
-            ModificationType::Unmergable.magenta(),
-            unmergable.reason
-        );
-    }
+    print_unmergable_paths(diff, writer);
 }
 
 #[context("Collecting xattrs")]

crates/lib/src/bootc_composefs/finalize.rs

@@ -14,12 +14,16 @@ use cap_std_ext::cap_std::{ambient_authority, fs::Dir};
 use cap_std_ext::dirext::CapStdExtDirExt;
 use composefs::generic_tree::{FileSystem, Stat};
 use composefs_ctl::composefs;
-use etc_merge::{compute_diff, merge, print_diff, traverse_etc};
+use etc_merge::{Diff, compute_diff, merge, traverse_etc};
 use rustix::fs::fsync;
 
 use fn_error_context::context;
 
-pub(crate) async fn get_etc_diff(storage: &Storage, booted_cfs: &BootedComposefs) -> Result<()> {
+pub(crate) async fn get_etc_diff(
+    storage: &Storage,
+    booted_cfs: &BootedComposefs,
+    new_etc: Option<&Dir>,
+) -> Result<Diff> {
     let host = get_composefs_status(storage, booted_cfs).await?;
     let booted_composefs = host.require_composefs_booted()?;
 
@@ -37,16 +41,18 @@ pub(crate) async fn get_etc_diff(storage: &Storage, booted_cfs: &BootedComposefs
         Dir::open_ambient_dir(erofs_tmp_mnt.dir.path().join("etc"), ambient_authority())?;
     let current_etc = Dir::open_ambient_dir("/etc", ambient_authority())?;
 
-    let (pristine_files, current_files, _) = traverse_etc(&pristine_etc, &current_etc, None)?;
+    let (pristine_files, current_files, new_etc_files) =
+        traverse_etc(&pristine_etc, &current_etc, new_etc)?;
+
     let diff = compute_diff(
         &pristine_files,
         &current_files,
-        &FileSystem::new(Stat::uninitialized()),
+        &new_etc_files
+            .as_ref()
+            .unwrap_or(&FileSystem::new(Stat::uninitialized())),
     )?;
 
-    print_diff(&diff, &mut std::io::stdout());
-
-    Ok(())
+    Ok(diff)
 }
 
 pub(crate) async fn composefs_backend_finalize(

crates/lib/src/bootc_composefs/update.rs

@@ -7,10 +7,12 @@ use composefs_ctl::composefs;
 use composefs_ctl::composefs_boot;
 use composefs_ctl::composefs_oci;
 use composefs_oci::image::create_filesystem;
+use etc_merge::print_unmergable_paths;
 use fn_error_context::context;
 use ocidir::cap_std::ambient_authority;
 use ostree_ext::container::ManifestDiff;
 
+use crate::bootc_composefs::finalize::get_etc_diff;
 use crate::bootc_composefs::gc::GCOpts;
 use crate::spec::BootloaderKind;
 use crate::{
@@ -288,6 +290,18 @@ pub(crate) async fn do_upgrade(
             .context("Failed to mount composefs image")?,
     )?;
 
+    // Check if three way etc merge is possible without conflicts
+    let new_etc = mounted_fs
+        .open_dir("etc")
+        .context("Opening deployment's etc")?;
+
+    let diff = get_etc_diff(storage, booted_cfs, Some(&new_etc)).await?;
+
+    if !diff.unmergable_paths.is_empty() {
+        print_unmergable_paths(&diff, &mut std::io::stderr());
+        anyhow::bail!("Merge conflicts found in etc");
+    }
+
     let boot_type = BootType::from(entry);
 
     let boot_digest = match boot_type {
@@ -307,14 +321,16 @@ pub(crate) async fn do_upgrade(
         )?,
     };
 
+    let staged_state = StagedDeployment {
+        depl_id: id.to_hex(),
+        finalization_locked: opts.download_only,
+    };
+
     write_composefs_state(
         &Utf8PathBuf::from("/sysroot"),
         &id,
         imgref,
-        Some(StagedDeployment {
-            depl_id: id.to_hex(),
-            finalization_locked: opts.download_only,
-        }),
+        Some(staged_state),
         boot_type,
         boot_digest,
         &manifest_digest,
@@ -393,16 +409,20 @@ pub(crate) async fn upgrade_composefs(
 
         start_finalize_stated_svc()?;
 
-        // Make the staged deployment not download_only
-        let new_staged = StagedDeployment {
-            depl_id: staged.require_composefs()?.verity.clone(),
-            finalization_locked: false,
-        };
-
         let staged_depl_dir =
             Dir::open_ambient_dir(COMPOSEFS_TRANSIENT_STATE_DIR, ambient_authority())
                 .context("Opening transient state directory")?;
 
+        let current = staged_depl_dir
+            .read_to_string(COMPOSEFS_STAGED_DEPLOYMENT_FNAME)
+            .context("Reading staged file")?;
+
+        let mut new_staged: StagedDeployment =
+            serde_json::from_str(&current).context("Deserialzing staged file")?;
+
+        // Make the staged deployment not download_only
+        new_staged.finalization_locked = false;
+
         staged_depl_dir
             .atomic_replace_with(
                 COMPOSEFS_STAGED_DEPLOYMENT_FNAME,

crates/lib/src/cli.rs

@@ -2455,7 +2455,9 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
                     anyhow::bail!("ConfigDiff is only supported for composefs backend")
                 }
                 BootedStorageKind::Composefs(booted_cfs) => {
-                    get_etc_diff(storage, &booted_cfs).await
+                    let diff = get_etc_diff(storage, &booted_cfs, None).await?;
+                    print_diff(&diff, &mut std::io::stdout());
+                    Ok(())
                 }
             }
         }

crates/lib/src/store/mod.rs

@@ -249,7 +249,6 @@ impl<'a> BootedOstree<'a> {
 }
 
 /// Represents a composefs-based boot environment
-#[allow(dead_code)]
 pub struct BootedComposefs {
     pub repo: Arc<ComposefsRepository>,
     pub cmdline: &'static ComposefsCmdline,