From 8bdf7fcb4fee0cf1403f766a907eb714ed53f241 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 19 Apr 2024 14:42:15 -0400 Subject: [PATCH] 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 --- .github/workflows/ci.yml | 39 ++++++++++++++++++++++++++++++ lib/src/install.rs | 10 +++++--- lib/src/install/baseline.rs | 47 +++++++++++++++++++++++-------------- lib/src/install/config.rs | 7 +++--- 4 files changed, 79 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 90504c07..69284c40 100644 --- 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 \ + ${image} bootc install to-disk --via-loopback "${tmpdisk}" diff --git a/lib/src/install.rs b/lib/src/install.rs index e74a52f6..10694a4e 100644 --- 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, /// The parsed contents of the authorized_keys (not the file path) pub(crate) root_ssh_authorized_keys: Option, } @@ -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 index 16d50cc6..52e2ab03 100644 --- 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 { 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::>(); mount::mount(&rootdev, &rootfs)?; diff --git a/lib/src/install/config.rs b/lib/src/install/config.rs index 4a326561..730d18a2 100644 --- 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 { +pub(crate) fn load_config() -> Result> { 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 = None; @@ -177,8 +177,9 @@ pub(crate) fn load_config() -> Result { 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) }