[sled-agent] move zone image source resolver into a new crate (#8079)

Author: sunshowers

Cargo.lock

@@ -127,6 +127,7 @@ members = [
     "sled-agent/config-reconciler",
     "sled-agent/repo-depot-api",
     "sled-agent/types",
+    "sled-agent/zone-images",
     "sled-diagnostics",
     "sled-hardware",
     "sled-hardware/types",
@@ -279,6 +280,7 @@ default-members = [
     "sled-agent/config-reconciler",
     "sled-agent/repo-depot-api",
     "sled-agent/types",
+    "sled-agent/zone-images",
     "sled-diagnostics",
     "sled-hardware",
     "sled-hardware/types",
@@ -666,6 +668,7 @@ sled-agent-api = { path = "sled-agent/api" }
 sled-agent-client = { path = "clients/sled-agent-client" }
 sled-agent-config-reconciler = { path = "sled-agent/config-reconciler" }
 sled-agent-types = { path = "sled-agent/types" }
+sled-agent-zone-images = { path = "sled-agent/zone-images" }
 sled-diagnostics = { path = "sled-diagnostics" }
 sled-hardware = { path = "sled-hardware" }
 sled-hardware-types = { path = "sled-hardware/types" }

Cargo.toml

@@ -85,6 +85,7 @@ sled-agent-api.workspace = true
 sled-agent-client.workspace = true
 sled-agent-config-reconciler.workspace = true
 sled-agent-types.workspace = true
+sled-agent-zone-images.workspace = true
 sled-diagnostics.workspace = true
 sled-hardware.workspace = true
 sled-hardware-types.workspace = true

sled-agent/Cargo.toml

@@ -102,14 +102,13 @@ use sled_agent_types::{
     sled::SWITCH_ZONE_BASEBOARD_FILE, time_sync::TimeSync,
     zone_bundle::ZoneBundleCause,
 };
+use sled_agent_zone_images::ZoneImageSourceResolver;
 use sled_hardware::SledMode;
 use sled_hardware::is_gimlet;
 use sled_hardware::underlay;
 use sled_hardware_types::Baseboard;
 use sled_storage::config::MountConfig;
-use sled_storage::dataset::{
-    CONFIG_DATASET, INSTALL_DATASET, M2_ARTIFACT_DATASET, ZONE_DATASET,
-};
+use sled_storage::dataset::{CONFIG_DATASET, ZONE_DATASET};
 use sled_storage::manager::StorageHandle;
 use slog::Logger;
 use slog_error_chain::InlineErrorChain;
@@ -779,8 +778,8 @@ pub struct ServiceManagerInner {
     switch_zone_bootstrap_address: Ipv6Addr,
     storage: StorageHandle,
     zone_bundler: ZoneBundler,
+    zone_image_resolver: ZoneImageSourceResolver,
     ledger_directory_override: OnceLock<Utf8PathBuf>,
-    image_directory_override: OnceLock<Utf8PathBuf>,
     system_api: Box<dyn SystemApi>,
 }
 
@@ -991,8 +990,8 @@ impl ServiceManager {
                     .switch_zone_bootstrap_ip,
                 storage,
                 zone_bundler,
+                zone_image_resolver: ZoneImageSourceResolver::new(),
                 ledger_directory_override: OnceLock::new(),
-                image_directory_override: OnceLock::new(),
                 system_api,
             }),
         }
@@ -1005,7 +1004,7 @@ impl ServiceManager {
 
     #[cfg(all(test, target_os = "illumos"))]
     fn override_image_directory(&self, path: Utf8PathBuf) {
-        self.inner.image_directory_override.set(path).unwrap();
+        self.inner.zone_image_resolver.override_image_directory(path);
     }
 
     pub(crate) fn ddm_reconciler(&self) -> &DdmReconciler {
@@ -1725,54 +1724,11 @@ impl ServiceManager {
             ZoneArgs::Omicron(zone_config) => &zone_config.zone.image_source,
             ZoneArgs::Switch(_) => &OmicronZoneImageSource::InstallDataset,
         };
-        let zone_image_file_name = match image_source {
-            OmicronZoneImageSource::InstallDataset => None,
-            OmicronZoneImageSource::Artifact { hash } => Some(hash.to_string()),
-        };
         let all_disks = self.inner.storage.get_latest_disks().await;
-        let zone_image_paths = match image_source {
-            OmicronZoneImageSource::InstallDataset => {
-                // Look for the image in the ramdisk first
-                let mut zone_image_paths =
-                    vec![Utf8PathBuf::from("/opt/oxide")];
-                // Inject an image path if requested by a test.
-                if let Some(path) = self.inner.image_directory_override.get() {
-                    zone_image_paths.push(path.clone());
-                };
-
-                // If the boot disk exists, look for the image in the "install"
-                // dataset there too.
-                if let Some((_, boot_zpool)) = all_disks.boot_disk() {
-                    zone_image_paths.push(boot_zpool.dataset_mountpoint(
-                        &all_disks.mount_config().root,
-                        INSTALL_DATASET,
-                    ));
-                }
-
-                zone_image_paths
-            }
-            OmicronZoneImageSource::Artifact { .. } => {
-                // Search both artifact datasets, but look on the boot disk first.
-                let boot_zpool =
-                    all_disks.boot_disk().map(|(_, boot_zpool)| boot_zpool);
-                // This iterator starts with the zpool for the boot disk (if it
-                // exists), and then is followed by all other zpools.
-                let zpool_iter = boot_zpool.into_iter().chain(
-                    all_disks
-                        .all_m2_zpools()
-                        .into_iter()
-                        .filter(|zpool| Some(zpool) != boot_zpool.as_ref()),
-                );
-                zpool_iter
-                    .map(|zpool| {
-                        zpool.dataset_mountpoint(
-                            &all_disks.mount_config().root,
-                            M2_ARTIFACT_DATASET,
-                        )
-                    })
-                    .collect()
-            }
-        };
+        let file_source = self
+            .inner
+            .zone_image_resolver
+            .file_source_for(image_source, &all_disks);
 
         let zone_type_str = match &request {
             ZoneArgs::Omicron(zone_config) => {
@@ -1796,14 +1752,14 @@ impl ServiceManager {
         if let Some(vnic) = bootstrap_vnic {
             zone_builder = zone_builder.with_bootstrap_vnic(vnic);
         }
-        if let Some(file_name) = &zone_image_file_name {
+        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(zone_image_paths.as_slice())
+            .with_zone_image_paths(file_source.search_paths.as_slice())
             .with_zone_type(zone_type_str)
             .with_datasets(datasets.as_slice())
             .with_filesystems(&filesystems)

sled-agent/src/services.rs

@@ -0,0 +1,15 @@
+[package]
+name = "sled-agent-zone-images"
+version = "0.1.0"
+edition = "2021"
+license = "MPL-2.0"
+
+[lints]
+workspace = true
+
+[dependencies]
+anyhow.workspace = true
+camino.workspace = true
+nexus-sled-agent-shared.workspace = true
+omicron-workspace-hack.workspace = true
+sled-storage.workspace = true

sled-agent/zone-images/Cargo.toml

@@ -0,0 +1,12 @@
+// 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/.
+
+//! Management of Omicron zone images within sled-agent.
+//!
+//! This contains a subset of zone image code at the moment: you're encouraged
+//! to move more code into this crate as appropriate.
+
+mod source_resolver;
+
+pub use source_resolver::*;

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

@@ -0,0 +1,107 @@
+// 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/.
+
+//! Zone image lookup.
+
+use camino::Utf8PathBuf;
+use nexus_sled_agent_shared::inventory::OmicronZoneImageSource;
+use sled_storage::dataset::INSTALL_DATASET;
+use sled_storage::dataset::M2_ARTIFACT_DATASET;
+use sled_storage::resources::AllDisks;
+use std::sync::OnceLock;
+
+/// 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.
+pub struct ZoneImageSourceResolver {
+    image_directory_override: OnceLock<Utf8PathBuf>,
+}
+
+impl ZoneImageSourceResolver {
+    pub fn new() -> Self {
+        ZoneImageSourceResolver { image_directory_override: OnceLock::new() }
+    }
+
+    /// Overrides the image directory with another one.
+    ///
+    /// Intended for testing.
+    pub fn override_image_directory(&self, path: Utf8PathBuf) {
+        self.image_directory_override.set(path).unwrap();
+    }
+
+    /// Returns a [`ZoneImageFileSource`] consisting of the file name, plus a
+    /// list of potential paths to search, for a zone image.
+    pub fn file_source_for(
+        &self,
+        image_source: &OmicronZoneImageSource,
+        all_disks: &AllDisks,
+    ) -> 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 {
+            OmicronZoneImageSource::InstallDataset => {
+                // Look for the image in the ramdisk first
+                let mut zone_image_paths =
+                    vec![Utf8PathBuf::from("/opt/oxide")];
+                // Inject an image path if requested by a test.
+                if let Some(path) = self.image_directory_override.get() {
+                    zone_image_paths.push(path.clone());
+                };
+
+                // If the boot disk exists, look for the image in the "install"
+                // dataset there too.
+                if let Some((_, boot_zpool)) = all_disks.boot_disk() {
+                    zone_image_paths.push(boot_zpool.dataset_mountpoint(
+                        &all_disks.mount_config().root,
+                        INSTALL_DATASET,
+                    ));
+                }
+
+                zone_image_paths
+            }
+            OmicronZoneImageSource::Artifact { .. } => {
+                // Search both artifact datasets, but look on the boot disk first.
+                let boot_zpool =
+                    all_disks.boot_disk().map(|(_, boot_zpool)| boot_zpool);
+                // This iterator starts with the zpool for the boot disk (if it
+                // exists), and then is followed by all other zpools.
+                let zpool_iter = boot_zpool.into_iter().chain(
+                    all_disks
+                        .all_m2_zpools()
+                        .into_iter()
+                        .filter(|zpool| Some(zpool) != boot_zpool.as_ref()),
+                );
+                zpool_iter
+                    .map(|zpool| {
+                        zpool.dataset_mountpoint(
+                            &all_disks.mount_config().root,
+                            M2_ARTIFACT_DATASET,
+                        )
+                    })
+                    .collect()
+            }
+        };
+
+        ZoneImageFileSource { file_name, search_paths }
+    }
+}