sled agent: implement OmicronZoneImageSource::Artifact (#7781)

Closes #7281.

Author: iliana

illumos-utils/src/running_zone.rs

@@ -1044,6 +1044,9 @@ pub struct ZoneBuilder<'a> {
     /// 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 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)
@@ -1110,6 +1113,17 @@ impl<'a> ZoneBuilder<'a> {
         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(
+        mut self,
+        image_file_name: &'a str,
+    ) -> Self {
+        self.zone_image_file_name = Some(image_file_name);
+        self
+    }
+
     /// The name of the type of zone being created (e.g. "propolis-server")
     pub fn with_zone_type(mut self, zone_type: &'a str) -> Self {
         self.zone_type = Some(zone_type);
@@ -1227,6 +1241,7 @@ impl<'a> ZoneBuilder<'a> {
             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,
             zone_type: Some(zone_type),
             unique_name,
             datasets: Some(datasets),
@@ -1255,15 +1270,18 @@ impl<'a> ZoneBuilder<'a> {
             InstalledZone::get_zone_name(zone_type, unique_name);
 
         // Looks for the image within `zone_image_path`, in order.
-        let image = format!("{}.tar.gz", zone_type);
+        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
             .iter()
             .find_map(|image_path| {
-                let path = image_path.join(&image);
+                let path = image_path.join(image_file_name);
                 if path.exists() { Some(path) } else { None }
             })
             .ok_or_else(|| InstallZoneError::ImageNotFound {
-                image: image.to_string(),
+                image: image_file_name.to_string(),
                 paths: zone_image_paths
                     .iter()
                     .map(|p| p.to_path_buf())

nexus-sled-agent-shared/src/inventory.rs

@@ -654,6 +654,18 @@ pub enum OmicronZoneImageSource {
     Artifact { hash: ArtifactHash },
 }
 
+impl OmicronZoneImageSource {
+    /// Return the artifact hash used for the zone image, if the zone's image
+    /// source is from the artifact store.
+    pub fn artifact_hash(&self) -> Option<ArtifactHash> {
+        if let OmicronZoneImageSource::Artifact { hash } = self {
+            Some(*hash)
+        } else {
+            None
+        }
+    }
+}
+
 // See `OmicronZoneConfig`. This is a separate function instead of being `impl
 // Default` because we don't want to accidentally use this default in Rust code.
 fn deserialize_image_source_default() -> OmicronZoneImageSource {

sled-agent/src/artifact_store.rs

@@ -17,6 +17,7 @@
 //! Operations that list or modify artifacts or the configuration are called by
 //! Nexus and handled by the Sled Agent API.
 
+use std::collections::BTreeMap;
 use std::future::Future;
 use std::io::{ErrorKind, Write};
 use std::net::SocketAddrV6;
@@ -48,6 +49,8 @@ use tokio::sync::{mpsc, oneshot, watch};
 use tokio::task::JoinSet;
 use tufaceous_artifact::ArtifactHash;
 
+use crate::services::ServiceManager;
+
 // These paths are defined under the artifact storage dataset. They
 // cannot conflict with any artifact paths because all artifact paths are
 // hexadecimal-encoded SHA-256 checksums.
@@ -87,7 +90,11 @@ pub(crate) struct ArtifactStore<T: DatasetsManager> {
 }
 
 impl<T: DatasetsManager> ArtifactStore<T> {
-    pub(crate) async fn new(log: &Logger, storage: T) -> ArtifactStore<T> {
+    pub(crate) async fn new(
+        log: &Logger,
+        storage: T,
+        services: Option<ServiceManager>,
+    ) -> ArtifactStore<T> {
         let log = log.new(slog::o!("component" => "ArtifactStore"));
 
         let mut ledger_paths = Vec::new();
@@ -126,6 +133,7 @@ impl<T: DatasetsManager> ArtifactStore<T> {
         tokio::task::spawn(ledger_manager(
             log.clone(),
             ledger_paths,
+            services,
             ledger_rx,
             config_tx,
         ));
@@ -244,15 +252,27 @@ impl<T: DatasetsManager> ArtifactStore<T> {
     pub(crate) async fn get(
         &self,
         sha256: ArtifactHash,
+    ) -> Result<File, Error> {
+        Self::get_from_storage(&self.storage, &self.log, sha256).await
+    }
+
+    /// Open an artifact file by hash from a storage handle.
+    ///
+    /// This is the same as [ArtifactStore::get], but can be called with only
+    /// a [StorageHandle].
+    pub(crate) async fn get_from_storage(
+        storage: &T,
+        log: &Logger,
+        sha256: ArtifactHash,
     ) -> Result<File, Error> {
         let sha256_str = sha256.to_string();
         let mut last_error = None;
-        for mountpoint in self.storage.artifact_storage_paths().await {
+        for mountpoint in storage.artifact_storage_paths().await {
             let path = mountpoint.join(&sha256_str);
             match File::open(&path).await {
                 Ok(file) => {
                     info!(
-                        &self.log,
+                        &log,
                         "Retrieved artifact";
                         "sha256" => &sha256_str,
                         "path" => path.as_str(),
@@ -261,7 +281,7 @@ impl<T: DatasetsManager> ArtifactStore<T> {
                 }
                 Err(err) if err.kind() == ErrorKind::NotFound => {}
                 Err(err) => {
-                    log_and_store!(last_error, &self.log, "open", path, err);
+                    log_and_store!(last_error, &log, "open", path, err);
                 }
             }
         }
@@ -430,9 +450,11 @@ type LedgerManagerRequest =
 async fn ledger_manager(
     log: Logger,
     ledger_paths: Vec<Utf8PathBuf>,
+    services: Option<ServiceManager>,
     mut rx: mpsc::Receiver<LedgerManagerRequest>,
     config_channel: watch::Sender<Option<ArtifactConfig>>,
 ) {
+    let services = services.as_ref();
     let handle_request = async |new_config: ArtifactConfig| {
         if ledger_paths.is_empty() {
             return Err(Error::NoUpdateDataset);
@@ -441,7 +463,25 @@ async fn ledger_manager(
             Ledger::<ArtifactConfig>::new(&log, ledger_paths.clone()).await
         {
             if new_config.generation > ledger.data().generation {
-                // New config generation; update the ledger.
+                // New config generation. First check that the configuration
+                // contains all artifacts that are presently in use.
+                let mut missing = BTreeMap::new();
+                // Check artifacts from the current zone configuration.
+                if let Some(services) = services {
+                    for zone in services.omicron_zones_list().await.zones {
+                        if let Some(hash) = zone.image_source.artifact_hash() {
+                            if !new_config.artifacts.contains(&hash) {
+                                missing
+                                    .insert(hash, "current zone configuration");
+                            }
+                        }
+                    }
+                }
+                if !missing.is_empty() {
+                    return Err(Error::InUseArtifactsMissing(missing));
+                }
+
+                // Everything looks okay; update the ledger.
                 *ledger.data_mut() = new_config;
                 ledger
             } else if new_config == *ledger.data() {
@@ -740,7 +780,7 @@ impl RepoDepotApi for RepoDepotImpl {
 }
 
 #[derive(Debug, thiserror::Error, SlogInlineError)]
-pub(crate) enum Error {
+pub enum Error {
     #[error("Error while reading request body")]
     Body(dropshot::HttpError),
 
@@ -784,6 +824,9 @@ pub(crate) enum Error {
     #[error("Digest mismatch: expected {expected}, actual {actual}")]
     HashMismatch { expected: ArtifactHash, actual: ArtifactHash },
 
+    #[error("Artifacts in use are not present in new config: {0:?}")]
+    InUseArtifactsMissing(BTreeMap<ArtifactHash, &'static str>),
+
     #[error("Blocking task failed")]
     Join(#[source] tokio::task::JoinError),
 
@@ -813,6 +856,7 @@ impl From<Error> for HttpError {
         match err {
             // 4xx errors
             Error::HashMismatch { .. }
+            | Error::InUseArtifactsMissing { .. }
             | Error::NoConfig
             | Error::NotInConfig { .. } => {
                 HttpError::for_bad_request(None, err.to_string())
@@ -951,7 +995,7 @@ mod test {
 
         let log = test_setup_log("generations");
         let backend = TestBackend::new(2);
-        let store = ArtifactStore::new(&log.log, backend).await;
+        let store = ArtifactStore::new(&log.log, backend, None).await;
 
         // get_config returns None
         assert!(store.get_config().is_none());
@@ -1004,7 +1048,7 @@ mod test {
     async fn list_get_put() {
         let log = test_setup_log("list_get_put");
         let backend = TestBackend::new(2);
-        let mut store = ArtifactStore::new(&log.log, backend).await;
+        let mut store = ArtifactStore::new(&log.log, backend, None).await;
 
         // get fails, because it doesn't exist yet
         assert!(matches!(
@@ -1126,7 +1170,7 @@ mod test {
 
         let log = test_setup_log("no_dataset");
         let backend = TestBackend::new(0);
-        let store = ArtifactStore::new(&log.log, backend).await;
+        let store = ArtifactStore::new(&log.log, backend, None).await;
 
         assert!(matches!(
             store.get(TEST_HASH).await,
@@ -1154,7 +1198,7 @@ mod test {
 
         let log = test_setup_log("wrong_hash");
         let backend = TestBackend::new(2);
-        let store = ArtifactStore::new(&log.log, backend).await;
+        let store = ArtifactStore::new(&log.log, backend, None).await;
         let mut config = ArtifactConfig {
             generation: 1u32.into(),
             artifacts: BTreeSet::new(),
@@ -1214,7 +1258,7 @@ mod test {
 
         let log = test_setup_log("issue_7796");
         let backend = TestBackend::new(2);
-        let store = ArtifactStore::new(&log.log, backend).await;
+        let store = ArtifactStore::new(&log.log, backend, None).await;
 
         let mut config = ArtifactConfig {
             generation: 1u32.into(),