From f095c2eec0338ddf9c63cbcde142b2ca99b541d1 Mon Sep 17 00:00:00 2001
From: Omer Tuchfeld <omer@tuchfeld.dev>
Date: Fri, 21 Jun 2024 11:03:42 +0200
Subject: [PATCH] install: Make stateroot configurable

This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (https://github.com/containers/bootc/pull/617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
---
 lib/src/install.rs               | 28 +++++++++++++++++++++------
 tests-integration/src/install.rs | 33 ++++++++++++++++++++++++--------
 2 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/lib/src/install.rs b/lib/src/install.rs
index 607623f62..0b4449094 100644
--- a/lib/src/install.rs
+++ b/lib/src/install.rs
@@ -51,8 +51,6 @@ use crate::store::Storage;
 use crate::task::Task;
 use crate::utils::sigpolicy_from_opts;
 
-/// The default "stateroot" or "osname"; see https://github.com/ostreedev/ostree/issues/2794
-const STATEROOT_DEFAULT: &str = "default";
 /// The toplevel boot directory
 const BOOT: &str = "boot";
 /// Directory for transient runtime state
@@ -171,6 +169,10 @@ pub(crate) struct InstallConfigOpts {
     #[clap(long, hide = true)]
     #[serde(default)]
     pub(crate) skip_bound_images: bool,
+    
+    /// The stateroot name to use. Defaults to `default`.
+    #[clap(long, hide = true)]
+    pub(crate) stateroot: Option<String>,
 }
 
 #[derive(Debug, Clone, clap::Parser, Serialize, Deserialize, PartialEq, Eq)]
@@ -567,8 +569,12 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result
     // Another implementation: https://github.com/coreos/coreos-assembler/blob/3cd3307904593b3a131b81567b13a4d0b6fe7c90/src/create_disk.sh#L295
     crate::lsm::ensure_dir_labeled(rootfs_dir, "", Some("/".into()), 0o755.into(), sepolicy)?;
 
-    // TODO: make configurable?
-    let stateroot = STATEROOT_DEFAULT;
+    let stateroot = state
+        .config_opts
+        .stateroot
+        .as_deref()
+        .unwrap_or(ostree_ext::container::deploy::STATEROOT_DEFAULT);
+
     Task::new_and_run(
         "Initializing ostree layout",
         "ostree",
@@ -638,7 +644,11 @@ async fn install_container(
 ) -> Result<(ostree::Deployment, InstallAleph)> {
     let sepolicy = state.load_policy()?;
     let sepolicy = sepolicy.as_ref();
-    let stateroot = STATEROOT_DEFAULT;
+    let stateroot = state
+        .config_opts
+        .stateroot
+        .as_deref()
+        .unwrap_or(ostree_ext::container::deploy::STATEROOT_DEFAULT);
 
     let container_rootfs = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
 
@@ -1095,11 +1105,17 @@ pub(crate) fn setup_sys_mount(fstype: &str, fspath: &str) -> Result<()> {
         return Ok(());
     }
 
+    dbg!("before", std::fs::read_dir(fspath)?.next());
+
     // This means the host has this mounted, so we should mount it too
     Task::new(format!("Mounting {fstype} {fspath}"), "mount")
         .args(["-t", fstype, fstype, fspath])
         .quiet()
-        .run()
+        .run()?;
+
+    dbg!("after", std::fs::read_dir(fspath)?.next());
+
+    Ok(())
 }
 
 /// Verify that we can load the manifest of the target image
diff --git a/tests-integration/src/install.rs b/tests-integration/src/install.rs
index 1f40dde85..083b54f95 100644
--- a/tests-integration/src/install.rs
+++ b/tests-integration/src/install.rs
@@ -26,15 +26,23 @@ pub(crate) const BASE_ARGS: &[&str] = &[
 // Clear out and delete any ostree roots
 fn reset_root(sh: &Shell) -> Result<()> {
     // TODO fix https://github.com/containers/bootc/pull/137
-    if !Path::new("/ostree/deploy/default").exists() {
-        return Ok(());
+
+    for stateroot in ["default", "foo"] {
+        if !Path::new(&format!("/ostree/deploy/{stateroot}")).exists() {
+            continue;
+        }
+
+        if Path::new(&format!("/ostree/deploy/{stateroot}/deploy")).exists() {
+            cmd!(
+            sh,
+            // env var hack needed because cmd! doesn't interpolate inside single quotes
+            "sudo STATEROOT={stateroot} /bin/sh -c 'chattr -i /ostree/deploy/$STATEROOT/deploy/*'"
+        )
+            .run()?;
+        }
+
+        cmd!(sh, "sudo rm /ostree/deploy/{stateroot} -rf").run()?;
     }
-    cmd!(
-        sh,
-        "sudo /bin/sh -c 'chattr -i /ostree/deploy/default/deploy/*'"
-    )
-    .run()?;
-    cmd!(sh, "sudo rm /ostree/deploy/default -rf").run()?;
     Ok(())
 }
 
@@ -136,6 +144,15 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
             crate::selinux::verify_selinux_recurse(root, &mut path, false)?;
             Ok(())
         }),
+        Trial::test("Install to non-default stateroot", move || {
+            let sh = &xshell::Shell::new()?;
+            reset_root(sh)?;
+            let non_default_stateroot = "foo";
+            cmd!(sh, "sudo {BASE_ARGS...} {target_args...} {image} bootc install to-existing-root --stateroot {non_default_stateroot} --acknowledge-destructive {generic_inst_args...}").run()?;
+            generic_post_install_verification()?;
+            assert!(Utf8Path::new("/ostree/deploy/foo").try_exists()?);
+            Ok(())
+        }),
         Trial::test("without an install config", move || {
             let sh = &xshell::Shell::new()?;
             reset_root(sh)?;