From 33bd09502836ecf698c8079b5e867494959e88eb Mon Sep 17 00:00:00 2001 From: Omer Tuchfeld Date: Thu, 31 Oct 2024 13:49:25 +0100 Subject: [PATCH] install: Add support for pulling LBIs during install Partially solves #846 This adds a new `--bound-images` option to `bootc install` which will allow the user to choose how they want to handle the retrieval of LBIs into the target's container storage. The existing behavior, which will stay the default, is `--bound-images stored` which will resolve the LBIs and verify they exist in the source's container storage before copying them into the target's container storage. The new behavior is `--bound-images pull` which will skip the resolution step and directly pull the LBIs into the target's container storage. The older `--skip-bound-images` option (previously hidden) is now removed and replaced with the new (but still hidden) `--bound-images skip` option. Signed-off-by: Omer Tuchfeld Signed-off-by: Colin Walters --- lib/src/boundimage.rs | 6 ++- lib/src/cli.rs | 5 ++ lib/src/install.rs | 114 +++++++++++++++++++++++++++++++----------- 3 files changed, 94 insertions(+), 31 deletions(-) diff --git a/lib/src/boundimage.rs b/lib/src/boundimage.rs index f5b552c3c..bc78d538a 100644 --- a/lib/src/boundimage.rs +++ b/lib/src/boundimage.rs @@ -108,6 +108,7 @@ pub(crate) fn query_bound_images(root: &Dir) -> Result> { #[cfg(feature = "install")] impl ResolvedBoundImage { + #[context("resolving bound image {}", src.image)] pub(crate) async fn from_image(src: &BoundImage) -> Result { let proxy = containers_image_proxy::ImageProxy::new().await?; let img = proxy @@ -148,7 +149,10 @@ fn parse_container_file(file_contents: &tini::Ini) -> Result { } #[context("Pulling bound images")] -pub(crate) async fn pull_images(sysroot: &Storage, bound_images: Vec) -> Result<()> { +pub(crate) async fn pull_images( + sysroot: &Storage, + bound_images: Vec, +) -> Result<()> { tracing::debug!("Pulling bound images: {}", bound_images.len()); // Yes, the usage of NonZeroUsize here is...maybe odd looking, but I find // it an elegant way to divide (empty vector, non empty vector) since diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 1a9d47deb..9f19a6a67 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -1039,6 +1039,11 @@ fn test_parse_install_args() { }; assert!(o.target_opts.target_no_signature_verification); assert_eq!(o.filesystem_opts.root_path.as_str(), "/target"); + // Ensure we default to old bound images behavior + assert_eq!( + o.config_opts.bound_images, + crate::install::BoundImagesOpt::Stored + ); } #[test] diff --git a/lib/src/install.rs b/lib/src/install.rs index 1893c0a70..98857edf3 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -20,8 +20,7 @@ use std::str::FromStr; use std::sync::Arc; use std::time::Duration; -use anyhow::{anyhow, Context, Result}; -use anyhow::{ensure, Ok}; +use anyhow::{anyhow, ensure, Context, Result}; use bootc_utils::CommandRunExt; use camino::Utf8Path; use camino::Utf8PathBuf; @@ -44,6 +43,7 @@ use rustix::fs::{FileTypeExt, MetadataExt as _}; use serde::{Deserialize, Serialize}; use self::baseline::InstallBlockDeviceOpts; +use crate::boundimage::{BoundImage, ResolvedBoundImage}; use crate::containerenv::ContainerExecutionInfo; use crate::lsm; use crate::mount::Filesystem; @@ -131,6 +131,27 @@ pub(crate) struct InstallSourceOpts { pub(crate) source_imgref: Option, } +#[derive(ValueEnum, Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(rename_all = "kebab-case")] +pub(crate) enum BoundImagesOpt { + /// Bound images must exist in the source's root container storage (default) + #[default] + Stored, + #[clap(hide = true)] + /// Do not resolve any "logically bound" images at install time. + Skip, + // TODO: Once we implement https://github.com/containers/bootc/issues/863 update this comment + // to mention source's root container storage being used as lookaside cache + /// Bound images will be pulled and stored directly in the target's bootc container storage + Pull, +} + +impl std::fmt::Display for BoundImagesOpt { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.to_possible_value().unwrap().get_name().fmt(f) + } +} + #[derive(clap::Args, Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub(crate) struct InstallConfigOpts { /// Disable SELinux in the target (installed) system. @@ -166,10 +187,11 @@ pub(crate) struct InstallConfigOpts { #[serde(default)] pub(crate) generic_image: bool, - /// Do not pull any "logically bound" images at install time. - #[clap(long, hide = true)] + /// How should logically bound images be retrieved. + #[clap(long)] #[serde(default)] - pub(crate) skip_bound_images: bool, + #[arg(default_value_t)] + pub(crate) bound_images: BoundImagesOpt, /// The stateroot name to use. Defaults to `default`. #[clap(long)] @@ -1318,7 +1340,7 @@ async fn install_with_sysroot( rootfs: &RootSetup, sysroot: &Storage, boot_uuid: &str, - bound_images: &[crate::boundimage::ResolvedBoundImage], + bound_images: BoundImages, has_ostree: bool, ) -> Result<()> { // And actually set up the container in that root, returning a deployment and @@ -1346,18 +1368,66 @@ async fn install_with_sysroot( tracing::debug!("Installed bootloader"); tracing::debug!("Perfoming post-deployment operations"); - // Note that we *always* initialize this container storage, even - // if there are no bound images today. + + // Note that we *always* initialize this container storage, even if there are no bound images + // today. let imgstore = sysroot.get_ensure_imgstore()?; - // Now copy each bound image from the host's container storage into the target. - for image in bound_images { - let image = image.image.as_str(); - imgstore.pull_from_host_storage(image).await?; + + match bound_images { + BoundImages::Skip => {} + BoundImages::Resolved(resolved_bound_images) => { + // Now copy each bound image from the host's container storage into the target. + for image in resolved_bound_images { + let image = image.image.as_str(); + imgstore.pull_from_host_storage(image).await?; + } + } + BoundImages::Unresolved(bound_images) => { + crate::boundimage::pull_images(sysroot, bound_images) + .await + .context("pulling bound images")?; + } } Ok(()) } +enum BoundImages { + Skip, + Resolved(Vec), + Unresolved(Vec), +} + +impl BoundImages { + async fn from_state(state: &State) -> Result { + let bound_images = match state.config_opts.bound_images { + BoundImagesOpt::Skip => BoundImages::Skip, + others => { + let queried_images = crate::boundimage::query_bound_images(&state.container_root)?; + match others { + BoundImagesOpt::Stored => { + // Verify each bound image is present in the container storage + let mut r = Vec::with_capacity(queried_images.len()); + for image in queried_images { + let resolved = ResolvedBoundImage::from_image(&image).await?; + tracing::debug!("Resolved {}: {}", resolved.image, resolved.digest); + r.push(resolved) + } + BoundImages::Resolved(r) + } + BoundImagesOpt::Pull => { + // No need to resolve the images, we will pull them into the target later + BoundImages::Unresolved(queried_images) + } + BoundImagesOpt::Skip => anyhow::bail!("unreachable error"), + } + } + }; + + Ok(bound_images) + } +} + async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Result<()> { if matches!(state.selinux_state, SELinuxFinalState::ForceTargetDisabled) { rootfs.kargs.push("selinux=0".to_string()); @@ -1390,23 +1460,7 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re "Missing /dev mount to host /dev" ); - let bound_images = if state.config_opts.skip_bound_images { - Vec::new() - } else { - crate::boundimage::query_bound_images(&state.container_root)? - }; - tracing::debug!("bound images={bound_images:?}"); - - // Verify each bound image is present in the container storage - let bound_images = { - let mut r = Vec::with_capacity(bound_images.len()); - for image in bound_images { - let resolved = crate::boundimage::ResolvedBoundImage::from_image(&image).await?; - tracing::debug!("Resolved {}: {}", resolved.image, resolved.digest); - r.push(resolved) - } - r - }; + let bound_images = BoundImages::from_state(state).await?; // Initialize the ostree sysroot (repo, stateroot, etc.) { @@ -1416,7 +1470,7 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re rootfs, &sysroot, &boot_uuid, - &bound_images, + bound_images, has_ostree, ) .await?;