install: Support being passed --filesystem with no install config

This fixes a logical bug where for base images that don't have any install configuration at all, we errored out even if we were explicitly passed `--filesystem`. This is just about making it easier to test base images that don't have an install config. Signed-off-by: Colin Walters <[email protected]>
cgwalters · Apr 20, 2024 · ae05ac2 · ae05ac2
1 parent 36e70da
commit ae05ac2
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 24 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -148,3 +148,42 @@ jobs:
           sudo podman run --rm -ti --privileged --env RUST_LOG=debug -v /:/target -v /var/lib/containers:/var/lib/containers -v ./usr/bin/bootc:/usr/bin/bootc --pid=host --security-opt label=disable \
             ${image} bootc install to-existing-root
           sudo podman run --rm -ti --privileged -v /:/target -v ./usr/bin/bootc:/usr/bin/bootc --pid=host --security-opt label=disable ${image} bootc internal-tests verify-selinux /target/ostree --warn
+  install-to-existing-root:
+    name: "Test install-to-existing-root"
+    needs: [build-c9s]
+    runs-on: ubuntu-latest
+    steps:
+      - name: Download
+        uses: actions/download-artifact@v4
+        with:
+          name: bootc-c9s.tar.zst
+      - name: Install
+        run: tar -xvf bootc.tar.zst
+      - name: Integration tests
+        run: |
+          set -xeuo pipefail
+          # We should be able to install to-existing-root with no install config,
+          # so we bind mount an empty directory over /usr/lib/bootc/install.
+          empty=$(mktemp -d)
+          image=quay.io/centos-bootc/centos-bootc-dev:stream9
+          sudo podman run --rm -ti --privileged --env RUST_LOG=debug -v /:/target -v /var/lib/containers:/var/lib/containers -v ./usr/bin/bootc:/usr/bin/bootc -v ${empty}:/usr/lib/bootc/install --pid=host --security-opt label=disable \
+            ${image} bootc install to-existing-root
+  install-to-loopback:
+    name: "Test install to-disk --via-loopback"
+    needs: [build-c9s]
+    runs-on: ubuntu-latest
+    steps:
+      - name: Download
+        uses: actions/download-artifact@v4
+        with:
+          name: bootc-c9s.tar.zst
+      - name: Install
+        run: tar -xvf bootc.tar.zst
+      - name: Integration tests
+        run: |
+          set -xeuo pipefail
+          image=quay.io/centos-bootc/centos-bootc-dev:stream9
+          tmpdisk=$(mktemp -p /var/tmp)
+          truncate -s 20G ${tmpdisk}
+          sudo podman run --rm -ti --privileged --env RUST_LOG=debug -v /:/target -v /var/lib/containers:/var/lib/containers -v ./usr/bin/bootc:/usr/bin/bootc --pid=host --security-opt label=disable \
+            -v ${tmpdisk}:/disk ${image} bootc install to-disk --via-loopback /disk
diff --git a/lib/src/install.rs b/lib/src/install.rs
@@ -298,7 +298,7 @@ pub(crate) struct State {
     #[allow(dead_code)]
     pub(crate) config_opts: InstallConfigOpts,
     pub(crate) target_imgref: ostree_container::OstreeImageReference,
-    pub(crate) install_config: config::InstallConfiguration,
+    pub(crate) install_config: Option<config::InstallConfiguration>,
     /// The parsed contents of the authorized_keys (not the file path)
     pub(crate) root_ssh_authorized_keys: Option<String>,
 }
@@ -515,7 +515,7 @@ impl SourceInfo {
 }
 
 pub(crate) fn print_configuration() -> Result<()> {
-    let mut install_config = config::load_config()?;
+    let mut install_config = config::load_config()?.unwrap_or_default();
     install_config.filter_to_external();
     let stdout = std::io::stdout().lock();
     serde_json::to_writer(stdout, &install_config).map_err(Into::into)
@@ -1128,7 +1128,11 @@ async fn prepare_install(
     }
 
     let install_config = config::load_config()?;
-    tracing::debug!("Loaded install configuration");
+    if install_config.is_some() {
+        tracing::debug!("Loaded install configuration");
+    } else {
+        tracing::debug!("No install configuration found");
+    }
 
     // Eagerly read the file now to ensure we error out early if e.g. it doesn't exist,
     // instead of much later after we're 80% of the way through an install.

diff --git a/lib/src/install/baseline.rs b/lib/src/install/baseline.rs
@@ -146,6 +146,15 @@ pub(crate) fn install_create_rootfs(
     opts: InstallBlockDeviceOpts,
 ) -> Result<RootSetup> {
     let luks_name = "root";
+    // Ensure we have a root filesystem upfornt
+    let root_filesystem = opts
+        .filesystem
+        .or(state
+            .install_config
+            .as_ref()
+            .and_then(|c| c.filesystem_root())
+            .and_then(|r| r.fstype))
+        .ok_or_else(|| anyhow::anyhow!("No root filesystem specified"))?;
     // Verify that the target is empty (if not already wiped in particular, but it's
     // also good to verify that the wipe worked)
     let device = crate::blockdev::list_dev(&opts.device)?;
@@ -296,9 +305,18 @@ pub(crate) fn install_create_rootfs(
     };
 
     let base_rootdev = findpart(ROOTPN)?;
-    let block_setup = state
-        .install_config
-        .get_block_setup(opts.block_setup.as_ref().copied())?;
+    // Use the install configuration to find the block setup, if we have one
+    let block_setup = if let Some(config) = state.install_config.as_ref() {
+        config.get_block_setup(opts.block_setup.as_ref().copied())?
+    } else if opts.filesystem.is_some() {
+        // Otherwise, if a filesystem is specified then we default to whatever was
+        // specified via --block-setup, or the default
+        opts.block_setup.clone().unwrap_or_default()
+    } else {
+        // If there was no default filesystem, then there's no default block setup,
+        // and we need to error out.
+        anyhow::bail!("No install configuration found, and no filesystem specified")
+    };
     let (rootdev, root_blockdev_kargs) = match block_setup {
         BlockSetup::Direct => (base_rootdev, None),
         BlockSetup::Tpm2Luks => {
@@ -342,13 +360,6 @@ pub(crate) fn install_create_rootfs(
     let boot_uuid = mkfs(bootdev, bootfs_type, Some("boot"), []).context("Initializing /boot")?;
 
     // Initialize rootfs
-    let root_filesystem = opts
-        .filesystem
-        .or(state
-            .install_config
-            .filesystem_root()
-            .and_then(|r| r.fstype))
-        .ok_or_else(|| anyhow::anyhow!("No root filesystem specified"))?;
     let root_uuid = mkfs(&rootdev, root_filesystem, Some("root"), [])?;
     let rootarg = format!("root=UUID={root_uuid}");
     let bootsrc = format!("UUID={boot_uuid}");
@@ -359,18 +370,18 @@ pub(crate) fn install_create_rootfs(
         fstype: MountSpec::AUTO.into(),
         options: Some("ro".into()),
     };
+    let install_config_kargs = state
+        .install_config
+        .as_ref()
+        .and_then(|c| c.kargs.as_ref())
+        .into_iter()
+        .flatten()
+        .map(ToOwned::to_owned);
     let kargs = root_blockdev_kargs
         .into_iter()
         .flatten()
         .chain([rootarg, RW_KARG.to_string(), bootarg].into_iter())
-        .chain(
-            state
-                .install_config
-                .kargs
-                .iter()
-                .flatten()
-                .map(ToOwned::to_owned),
-        )
+        .chain(install_config_kargs)
         .collect::<Vec<_>>();
 
     mount::mount(&rootdev, &rootfs)?;

diff --git a/lib/src/install/config.rs b/lib/src/install/config.rs
@@ -153,7 +153,7 @@ impl InstallConfiguration {
 
 #[context("Loading configuration")]
 /// Load the install configuration, merging all found configuration files.
-pub(crate) fn load_config() -> Result<InstallConfiguration> {
+pub(crate) fn load_config() -> Result<Option<InstallConfiguration>> {
     const SYSTEMD_CONVENTIONAL_BASES: &[&str] = &["/usr/lib", "/usr/local/lib", "/etc", "/run"];
     let fragments = liboverdrop::scan(SYSTEMD_CONVENTIONAL_BASES, "bootc/install", &["toml"], true);
     let mut config: Option<InstallConfiguration> = None;
@@ -177,8 +177,9 @@ pub(crate) fn load_config() -> Result<InstallConfiguration> {
             config = c.install;
         }
     }
-    let mut config = config.ok_or_else(|| anyhow::anyhow!("No bootc/install config found; this operating system must define a default configuration to be installable"))?;
-    config.canonicalize();
+    if let Some(config) = config.as_mut() {
+        config.canonicalize();
+    }
     Ok(config)
 }