[11/n] [sled-agent] move around ZoneImageFileSource logic (#8235)

* Move `ZoneImageFileSource` to illumos-utils and use it in the zone
image builder.
* Move default file name logic to sled-agent-zone-images, since it cares
about that in the context of mupdate override logic.

Depends on:

* #8190
* everything before that

Author: sunshowers

illumos-utils/src/running_zone.rs

@@ -891,9 +891,12 @@ pub enum InstallZoneError {
         err: crate::zone::AdmError,
     },
 
-    #[error("Failed to find zone image '{image}' from {paths:?}")]
-    ImageNotFound { image: String, paths: Vec<Utf8PathBuf> },
-
+    #[error(
+        "Failed to find zone image '{}' from {:?}",
+        file_source.file_name,
+        file_source.search_paths,
+    )]
+    ImageNotFound { file_source: ZoneImageFileSource },
     #[error("Attempted to call install() on underspecified ZoneBuilder")]
     IncompleteBuilder,
 }
@@ -1058,12 +1061,8 @@ pub struct ZoneBuilder<'a> {
     underlay_vnic_allocator: Option<&'a VnicAllocator<Etherstub>>,
     /// Filesystem path at which the installed zone will reside.
     zone_root_path: Option<PathInPool>,
-    /// The directories that will be searched for the image tarball for the
-    /// provided zone type ([`Self::with_zone_type`]).
-    zone_image_paths: Option<&'a [Utf8PathBuf]>,
-    /// The file name of the zone image to search for in [`Self::zone_image_paths`].
-    /// If unset, defaults to `{zone_type}.tar.gz`.
-    zone_image_file_name: Option<&'a str>,
+    /// The file source.
+    file_source: Option<&'a ZoneImageFileSource>,
     /// The name of the type of zone being created (e.g. "propolis-server")
     zone_type: Option<&'a str>,
     /// Unique ID of the instance of the zone being created. (optional)
@@ -1120,24 +1119,12 @@ impl<'a> ZoneBuilder<'a> {
         self
     }
 
-    /// The directories that will be searched for the image tarball for the
-    /// provided zone type ([`Self::with_zone_type`]).
-    pub fn with_zone_image_paths(
-        mut self,
-        image_paths: &'a [Utf8PathBuf],
-    ) -> Self {
-        self.zone_image_paths = Some(image_paths);
-        self
-    }
-
-    /// The file name of the zone image to search for in the zone image
-    /// paths ([`Self::with_zone_image_paths`]). If unset, defaults to
-    /// `{zone_type}.tar.gz`.
-    pub fn with_zone_image_file_name(
+    /// The file name and image source.
+    pub fn with_file_source(
         mut self,
-        image_file_name: &'a str,
+        file_source: &'a ZoneImageFileSource,
     ) -> Self {
-        self.zone_image_file_name = Some(image_file_name);
+        self.file_source = Some(file_source);
         self
     }
 
@@ -1260,8 +1247,7 @@ impl<'a> ZoneBuilder<'a> {
             log: Some(log),
             underlay_vnic_allocator: Some(underlay_vnic_allocator),
             zone_root_path: Some(mut zone_root_path),
-            zone_image_paths: Some(zone_image_paths),
-            zone_image_file_name,
+            file_source: Some(file_source),
             zone_type: Some(zone_type),
             unique_name,
             datasets: Some(datasets),
@@ -1289,23 +1275,16 @@ impl<'a> ZoneBuilder<'a> {
         let full_zone_name =
             InstalledZone::get_zone_name(zone_type, unique_name);
 
-        // Looks for the image within `zone_image_path`, in order.
-        let image_file_name = match zone_image_file_name {
-            Some(image) => image,
-            None => &format!("{}.tar.gz", zone_type),
-        };
-        let zone_image_path = zone_image_paths
+        // Look for the image within `file_source.search_paths`, in order.
+        let zone_image_path = file_source
+            .search_paths
             .iter()
             .find_map(|image_path| {
-                let path = image_path.join(image_file_name);
+                let path = image_path.join(&file_source.file_name);
                 if path.exists() { Some(path) } else { None }
             })
             .ok_or_else(|| InstallZoneError::ImageNotFound {
-                image: image_file_name.to_string(),
-                paths: zone_image_paths
-                    .iter()
-                    .map(|p| p.to_path_buf())
-                    .collect(),
+                file_source: file_source.clone(),
             })?;
 
         let mut net_device_names: Vec<String> = opte_ports
@@ -1359,6 +1338,19 @@ impl<'a> ZoneBuilder<'a> {
     }
 }
 
+/// Places to look for a zone's image.
+#[derive(Clone, Debug)]
+pub struct ZoneImageFileSource {
+    /// The file name to look for.
+    pub file_name: String,
+
+    /// The paths to look for the zone image in.
+    ///
+    /// This represents a high-confidence belief, but not a guarantee, that the
+    /// zone image will be found in one of these locations.
+    pub search_paths: Vec<Utf8PathBuf>,
+}
+
 /// Return true if the service with the given FMRI appears to be an
 /// Oxide-managed service.
 pub fn is_oxide_smf_service(fmri: impl AsRef<str>) -> bool {

sled-agent/src/instance.rs

@@ -39,6 +39,7 @@ use rand::prelude::IteratorRandom;
 use sled_agent_config_reconciler::AvailableDatasetsReceiver;
 use sled_agent_types::instance::*;
 use sled_agent_types::zone_bundle::ZoneBundleCause;
+use sled_agent_zone_images::ramdisk_file_source;
 use slog::Logger;
 use std::net::IpAddr;
 use std::net::SocketAddr;
@@ -2003,7 +2004,7 @@ impl InstanceRunner {
             .with_log(self.log.clone())
             .with_underlay_vnic_allocator(&self.vnic_allocator)
             .with_zone_root_path(root)
-            .with_zone_image_paths(&["/opt/oxide".into()])
+            .with_file_source(&ramdisk_file_source("propolis-server"))
             .with_zone_type("propolis-server")
             .with_unique_name(OmicronZoneUuid::from_untyped_uuid(
                 self.propolis_id.into_untyped_uuid(),

sled-agent/src/probe_manager.rs

@@ -23,6 +23,7 @@ use sled_agent_config_reconciler::{
     AvailableDatasetsReceiver, CurrentlyManagedZpools,
     CurrentlyManagedZpoolsReceiver,
 };
+use sled_agent_zone_images::ramdisk_file_source;
 use slog::{Logger, error, warn};
 use std::collections::{HashMap, HashSet};
 use std::hash::{Hash, Hasher};
@@ -354,7 +355,7 @@ impl ProbeManagerInner {
             .with_log(self.log.clone())
             .with_underlay_vnic_allocator(&self.vnic_allocator)
             .with_zone_root_path(zone_root_path)
-            .with_zone_image_paths(&["/opt/oxide".into()])
+            .with_file_source(&ramdisk_file_source("probe"))
             .with_zone_type("probe")
             .with_unique_name(OmicronZoneUuid::from_untyped_uuid(probe.id))
             .with_datasets(&[])

sled-agent/src/services.rs

@@ -1355,6 +1355,13 @@ impl ServiceManager {
             .map(|d| zone::Device { name: d.to_string() })
             .collect();
 
+        let zone_type_str = match &request {
+            ZoneArgs::Omicron(zone_config) => {
+                zone_config.zone_type.kind().zone_prefix()
+            }
+            ZoneArgs::Switch(_) => "switch",
+        };
+
         // TODO: `InstallDataset` should be renamed to something more accurate
         // when all the major changes here have landed. Some zones are
         // distributed from the host OS image and are never placed in the
@@ -1366,17 +1373,11 @@ impl ServiceManager {
             ZoneArgs::Switch(_) => &OmicronZoneImageSource::InstallDataset,
         };
         let file_source = self.inner.zone_image_resolver.file_source_for(
+            zone_type_str,
             image_source,
             self.inner.internal_disks_rx.current(),
         );
 
-        let zone_type_str = match &request {
-            ZoneArgs::Omicron(zone_config) => {
-                zone_config.zone_type.kind().zone_prefix()
-            }
-            ZoneArgs::Switch(_) => "switch",
-        };
-
         // We use the fake initialiser for testing
         let mut zone_builder = match self.inner.system_api.fake_install_dir() {
             None => ZoneBuilderFactory::new().builder(),
@@ -1392,15 +1393,12 @@ impl ServiceManager {
         if let Some(vnic) = bootstrap_vnic {
             zone_builder = zone_builder.with_bootstrap_vnic(vnic);
         }
-        if let Some(file_name) = &file_source.file_name {
-            zone_builder = zone_builder.with_zone_image_file_name(file_name);
-        }
         let installed_zone = zone_builder
             .with_log(self.inner.log.clone())
             .with_underlay_vnic_allocator(&self.inner.underlay_vnic_allocator)
             .with_zone_root_path(zone_root_path)
-            .with_zone_image_paths(file_source.search_paths.as_slice())
             .with_zone_type(zone_type_str)
+            .with_file_source(&file_source)
             .with_datasets(datasets.as_slice())
             .with_filesystems(&filesystems)
             .with_data_links(&data_links)

sled-agent/zone-images/src/file_source.rs

@@ -0,0 +1,26 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+//! Utilities to construct `ZoneImageFileSource` instances.
+
+use illumos_utils::running_zone::ZoneImageFileSource;
+
+/// The location to look for images shipped with the RAM disk.
+pub const RAMDISK_IMAGE_PATH: &str = "/opt/oxide";
+
+/// Constructs a file source for the RAM disk.
+///
+/// This accepts a `zone_type` string rather than a `ZoneType` or `ZoneKind`
+/// enum because it is used to manage non-Omicron zones like propolis-server.
+pub fn ramdisk_file_source(zone_type: &str) -> ZoneImageFileSource {
+    ZoneImageFileSource {
+        file_name: install_dataset_file_name(zone_type),
+        search_paths: vec![RAMDISK_IMAGE_PATH.into()],
+    }
+}
+
+/// Returns the filename for install-dataset images.
+pub fn install_dataset_file_name(zone_type: &str) -> String {
+    format!("{}.tar.gz", zone_type)
+}

sled-agent/zone-images/src/lib.rs

@@ -8,6 +8,7 @@
 //! to move more code into this crate as appropriate.
 
 mod errors;
+mod file_source;
 mod install_dataset_metadata;
 mod mupdate_override;
 mod source_resolver;
@@ -16,6 +17,7 @@ mod test_utils;
 mod zone_manifest;
 
 pub use errors::*;
+pub use file_source::*;
 pub use install_dataset_metadata::*;
 pub use mupdate_override::*;
 pub use source_resolver::*;

sled-agent/zone-images/src/source_resolver.rs

@@ -7,29 +7,18 @@
 use crate::AllMupdateOverrides;
 use crate::AllZoneManifests;
 use crate::MupdateOverrideStatus;
+use crate::RAMDISK_IMAGE_PATH;
 use crate::ZoneManifestStatus;
+use crate::install_dataset_file_name;
 use camino::Utf8PathBuf;
+use illumos_utils::running_zone::ZoneImageFileSource;
 use nexus_sled_agent_shared::inventory::OmicronZoneImageSource;
 use sled_agent_config_reconciler::InternalDisks;
 use sled_agent_config_reconciler::InternalDisksWithBootDisk;
 use slog::o;
 use std::sync::Arc;
 use std::sync::Mutex;
 
-/// Places to look for an Omicron zone image.
-pub struct ZoneImageFileSource {
-    /// A custom file name to look for, if provided.
-    ///
-    /// The default file name is `<zone_type>.tar.gz`.
-    pub file_name: Option<String>,
-
-    /// The paths to look for the zone image in.
-    ///
-    /// This represents a high-confidence belief, but not a guarantee, that the
-    /// zone image will be found in one of these locations.
-    pub search_paths: Vec<Utf8PathBuf>,
-}
-
 /// Resolves [`OmicronZoneImageSource`] instances into file names and search
 /// paths.
 ///
@@ -67,11 +56,12 @@ impl ZoneImageSourceResolver {
     /// list of potential paths to search, for a zone image.
     pub fn file_source_for(
         &self,
+        zone_type: &str,
         image_source: &OmicronZoneImageSource,
         internal_disks: InternalDisks,
     ) -> ZoneImageFileSource {
         let inner = self.inner.lock().unwrap();
-        inner.file_source_for(image_source, internal_disks)
+        inner.file_source_for(zone_type, image_source, internal_disks)
     }
 }
 
@@ -119,22 +109,15 @@ impl ResolverInner {
 
     fn file_source_for(
         &self,
+        zone_type: &str,
         image_source: &OmicronZoneImageSource,
         internal_disks: InternalDisks,
     ) -> ZoneImageFileSource {
-        let file_name = match image_source {
-            OmicronZoneImageSource::InstallDataset => {
-                // Use the default file name for install-dataset lookups.
-                None
-            }
-            OmicronZoneImageSource::Artifact { hash } => Some(hash.to_string()),
-        };
-
-        let search_paths = match image_source {
+        match image_source {
             OmicronZoneImageSource::InstallDataset => {
                 // Look for the image in the ramdisk first
                 let mut zone_image_paths =
-                    vec![Utf8PathBuf::from("/opt/oxide")];
+                    vec![Utf8PathBuf::from(RAMDISK_IMAGE_PATH)];
                 // Inject an image path if requested by a test.
                 if let Some(path) = &self.image_directory_override {
                     zone_image_paths.push(path.clone());
@@ -146,16 +129,24 @@ impl ResolverInner {
                     zone_image_paths.push(path);
                 }
 
-                zone_image_paths
+                ZoneImageFileSource {
+                    file_name: install_dataset_file_name(zone_type),
+                    search_paths: zone_image_paths,
+                }
             }
-            OmicronZoneImageSource::Artifact { .. } => {
+            OmicronZoneImageSource::Artifact { hash } => {
                 // Search both artifact datasets. This iterator starts with the
                 // dataset for the boot disk (if it exists), and then is followed
                 // by all other disks.
-                internal_disks.all_artifact_datasets().collect()
+                let search_paths =
+                    internal_disks.all_artifact_datasets().collect();
+                ZoneImageFileSource {
+                    // Images in the artifact store are named by just their
+                    // hash.
+                    file_name: hash.to_string(),
+                    search_paths,
+                }
             }
-        };
-
-        ZoneImageFileSource { file_name, search_paths }
+        }
     }
 }