Merge pull request #712 from cgwalters/bound-install-time

Bound install time

lib/src/boundimage.rs

@@ -18,15 +18,27 @@ use ostree_ext::sysroot::SysrootLock;
 /// symbolic links to `.container` or `.image` files.
 const BOUND_IMAGE_DIR: &str = "usr/lib/bootc-experimental/bound-images.d";
 
+/// A subset of data parsed from a `.image` or `.container` file with
+/// the minimal information necessary to fetch the image.
+///
+/// In the future this may be extended to include e.g. certificates or
+/// other pull options.
+#[derive(PartialEq, Eq)]
+pub(crate) struct BoundImage {
+    image: String,
+    auth_file: Option<String>,
+}
+
 /// Given a deployment, pull all container images it references.
 pub(crate) fn pull_bound_images(sysroot: &SysrootLock, deployment: &Deployment) -> Result<()> {
     let deployment_root = &crate::utils::deployment_fd(sysroot, deployment)?;
-    let bound_images = parse_spec_dir(deployment_root, BOUND_IMAGE_DIR)?;
+    let bound_images = query_bound_images(deployment_root)?;
     pull_images(deployment_root, bound_images)
 }
 
-#[context("parse bound image spec dir")]
-fn parse_spec_dir(root: &Dir, spec_dir: &str) -> Result<Vec<BoundImage>> {
+#[context("Querying bound images")]
+pub(crate) fn query_bound_images(root: &Dir) -> Result<Vec<BoundImage>> {
+    let spec_dir = BOUND_IMAGE_DIR;
     let Some(bound_images_dir) = root.open_dir_optional(spec_dir)? else {
         tracing::debug!("Missing {spec_dir}");
         return Ok(Default::default());
@@ -101,7 +113,7 @@ fn parse_container_file(file_contents: &tini::Ini) -> Result<BoundImage> {
 }
 
 #[context("pull bound images")]
-fn pull_images(_deployment_root: &Dir, bound_images: Vec<BoundImage>) -> Result<()> {
+pub(crate) fn pull_images(_deployment_root: &Dir, bound_images: Vec<BoundImage>) -> Result<()> {
     tracing::debug!("Pulling bound images: {}", bound_images.len());
     //TODO: do this in parallel
     for bound_image in bound_images {
@@ -117,17 +129,6 @@ fn pull_images(_deployment_root: &Dir, bound_images: Vec<BoundImage>) -> Result<
     Ok(())
 }
 
-/// A subset of data parsed from a `.image` or `.container` file with
-/// the minimal information necessary to fetch the image.
-///
-/// In the future this may be extended to include e.g. certificates or
-/// other pull options.
-#[derive(PartialEq, Eq)]
-struct BoundImage {
-    image: String,
-    auth_file: Option<String>,
-}
-
 impl BoundImage {
     fn new(image: String, auth_file: Option<String>) -> Result<BoundImage> {
         let image = parse_spec_value(&image).context("Invalid image value")?;
@@ -178,12 +179,12 @@ mod tests {
 
         // Empty dir should return an empty vector
         let td = &cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
-        let images = parse_spec_dir(td, &BOUND_IMAGE_DIR).unwrap();
+        let images = query_bound_images(td).unwrap();
         assert_eq!(images.len(), 0);
 
         td.create_dir_all(BOUND_IMAGE_DIR).unwrap();
         td.create_dir_all(CONTAINER_IMAGE_DIR).unwrap();
-        let images = parse_spec_dir(td, &BOUND_IMAGE_DIR).unwrap();
+        let images = query_bound_images(td).unwrap();
         assert_eq!(images.len(), 0);
 
         // Should return BoundImages
@@ -215,7 +216,7 @@ mod tests {
         )
         .unwrap();
 
-        let mut images = parse_spec_dir(td, &BOUND_IMAGE_DIR).unwrap();
+        let mut images = query_bound_images(td).unwrap();
         images.sort_by(|a, b| a.image.as_str().cmp(&b.image.as_str()));
         assert_eq!(images.len(), 2);
         assert_eq!(images[0].image, "quay.io/bar/bar:latest");
@@ -224,13 +225,13 @@ mod tests {
         // Invalid symlink should return an error
         td.symlink("./blah", format!("{BOUND_IMAGE_DIR}/blah.image"))
             .unwrap();
-        assert!(parse_spec_dir(td, &BOUND_IMAGE_DIR).is_err());
+        assert!(query_bound_images(td).is_err());
 
         // Invalid image contents should return an error
         td.write("error.image", "[Image]\n").unwrap();
         td.symlink_contents("/error.image", format!("{BOUND_IMAGE_DIR}/error.image"))
             .unwrap();
-        assert!(parse_spec_dir(td, &BOUND_IMAGE_DIR).is_err());
+        assert!(query_bound_images(td).is_err());
 
         Ok(())
     }

lib/src/install.rs

@@ -35,6 +35,7 @@ use ostree_ext::container as ostree_container;
 use ostree_ext::oci_spec;
 use ostree_ext::ostree;
 use ostree_ext::prelude::Cast;
+use ostree_ext::sysroot::SysrootLock;
 use rustix::fs::{FileTypeExt, MetadataExt as _};
 use serde::{Deserialize, Serialize};
 
@@ -160,6 +161,11 @@ pub(crate) struct InstallConfigOpts {
     #[clap(long)]
     #[serde(default)]
     pub(crate) generic_image: bool,
+
+    /// Do not pull any "logically bound" images at install time.
+    #[clap(long, hide = true)]
+    #[serde(default)]
+    pub(crate) skip_bound_images: bool,
 }
 
 #[derive(Debug, Clone, clap::Parser, Serialize, Deserialize, PartialEq, Eq)]
@@ -1219,23 +1225,21 @@ async fn prepare_install(
     Ok(state)
 }
 
-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());
-    }
-
-    // We verify this upfront because it's currently required by bootupd
-    let boot_uuid = rootfs
-        .get_boot_uuid()?
-        .or(rootfs.rootfs_uuid.as_deref())
-        .ok_or_else(|| anyhow!("No uuid for boot/root"))?;
-    tracing::debug!("boot uuid={boot_uuid}");
-
-    // Initialize the ostree sysroot (repo, stateroot, etc.)
-    let sysroot = initialize_ostree_root(state, rootfs).await?;
+/// Given a baseline root filesystem with an ostree sysroot initialized:
+/// - install the container to that root
+/// - install the bootloader
+/// - Other post operations, such as pulling bound images
+async fn install_with_sysroot(
+    state: &State,
+    rootfs: &RootSetup,
+    sysroot: &ostree::Sysroot,
+    boot_uuid: &str,
+) -> Result<()> {
+    let sysroot = SysrootLock::new_from_sysroot(&sysroot).await?;
     // And actually set up the container in that root, returning a deployment and
     // the aleph state (see below).
     let (deployment, aleph) = install_container(state, rootfs, &sysroot).await?;
+    let stateroot = deployment.osname();
     // Write the aleph data that captures the system state at the time of provisioning for aid in future debugging.
     rootfs
         .rootfs_fd
@@ -1244,6 +1248,7 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re
             anyhow::Ok(())
         })
         .context("Writing aleph version")?;
+
     if cfg!(target_arch = "s390x") {
         // TODO: Integrate s390x support into install_via_bootupd
         crate::bootloader::install_via_zipl(&rootfs.device_info, boot_uuid)?;
@@ -1254,12 +1259,62 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re
             &state.config_opts,
         )?;
     }
+    tracing::debug!("Installed bootloader");
 
-    // After this point, we need to drop all open references to the filesystem
-    drop(deployment);
-    drop(sysroot);
+    tracing::debug!("Perfoming post-deployment operations");
+    let deployment_root = crate::utils::deployment_fd(&sysroot, &deployment)?;
+    let bound_images = if state.config_opts.skip_bound_images {
+        Vec::new()
+    } else {
+        crate::boundimage::query_bound_images(&deployment_root)?
+    };
+    if !bound_images.is_empty() {
+        // TODO: We only do this dance to initialize `/var` at install time if
+        // there are bound images today; it minimizes side effects.
+        // However going forward we really do need to handle a separate /var partition...
+        // and to do that we may in the general case need to run the `var.mount`
+        // target from the new root.
+        let varpath = format!("ostree/deploy/{stateroot}/var");
+        let var = rootfs
+            .rootfs_fd
+            .open_dir(&varpath)
+            .with_context(|| format!("Opening {varpath}"))?;
+        Task::new("Mounting deployment /var", "mount")
+            .args(["--bind", ".", "/var"])
+            .cwd(&var)?
+            .run()?;
+        // podman needs this
+        Task::new("Initializing /var/tmp", "systemd-tmpfiles")
+            .args(["--create", "--boot", "--prefix=/var/tmp"])
+            .verbose()
+            .run()?;
+        crate::boundimage::pull_images(&deployment_root, bound_images)?;
+    }
 
-    tracing::debug!("Installed bootloader");
+    Ok(())
+}
+
+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());
+    }
+    // Drop exclusive ownership since we're done with mutation
+    let rootfs = &*rootfs;
+
+    // We verify this upfront because it's currently required by bootupd
+    let boot_uuid = rootfs
+        .get_boot_uuid()?
+        .or(rootfs.rootfs_uuid.as_deref())
+        .ok_or_else(|| anyhow!("No uuid for boot/root"))?;
+    tracing::debug!("boot uuid={boot_uuid}");
+
+    // Initialize the ostree sysroot (repo, stateroot, etc.)
+    {
+        let sysroot = initialize_ostree_root(state, rootfs).await?;
+        install_with_sysroot(state, rootfs, &sysroot, &boot_uuid).await?;
+        // We must drop the sysroot here in order to close any open file
+        // descriptors.
+    }
 
     // Finalize mounted filesystems
     if !rootfs.skip_finalize {