[39/n] prepare sled-agent reconciler for honoring mupdate overrides (#8688)

This PR prepares the sled-agent config reconciler to honor mupdate
overrides. In particular, the logic that requires bouncing zones needs
to be updated to consider zone image locations after mupdate overrides
have been considered, not before.

Doing this required some refactoring. The biggest one is that looking up
zone image sources is no longer done through the image resolver and/or
within sled-agent's `services.rs`, but rather by gathering and querying
the resolver status within the config reconciler. This is a nice
improvement overall, particularly because it means we grab the lock once
per reconciler run rather than each time we need to look up a zone's
image source.

We do not actually honor the mupdate override yet, though all the pieces
are now in place. We'll combine the PRs to honor the override and update
the blueprint logic into one, within #8456.

Author: sunshowers

Cargo.lock

@@ -31,6 +31,7 @@ pub mod progenitor_operation_retry;
 pub mod snake_case_result;
 pub mod update;
 pub mod vlan;
+pub mod zone_images;
 pub mod zpool_name;
 
 /// A type that allows adding file and line numbers to log messages

common/src/lib.rs

@@ -0,0 +1,18 @@
+// 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/.
+
+use camino::Utf8PathBuf;
+
+/// Places to look for a zone's image.
+#[derive(Clone, Debug, PartialEq, Eq)]
+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>,
+}

common/src/zone_images.rs

@@ -19,6 +19,7 @@ use camino_tempfile::Utf8TempDir;
 use debug_ignore::DebugIgnore;
 use ipnetwork::IpNetwork;
 use omicron_common::backoff;
+use omicron_common::zone_images::ZoneImageFileSource;
 use omicron_uuid_kinds::OmicronZoneUuid;
 pub use oxlog::is_oxide_smf_log_file;
 use slog::{Logger, error, info, o, warn};
@@ -1338,19 +1339,6 @@ impl<'a> ZoneBuilder<'a> {
     }
 }
 
-/// Places to look for a zone's image.
-#[derive(Clone, Debug, PartialEq, Eq)]
-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 {

illumos-utils/src/running_zone.rs

@@ -889,6 +889,7 @@ pub fn zone_image_resolver(
                 },
             },
         },
+        image_directory_override: None,
     };
 
     status.to_inventory()

nexus/inventory/src/examples.rs

@@ -54,6 +54,7 @@ proptest.workspace = true
 schemars.workspace = true
 scopeguard.workspace = true
 serde_json.workspace = true
+sled-agent-zone-images-examples.workspace = true
 sled-storage = { workspace = true, features = ["testing"] }
 test-strategy.workspace = true
 xshell.workspace = true

sled-agent/config-reconciler/Cargo.toml

@@ -6,6 +6,7 @@
 //! partitions.
 
 use crate::InternalDisks;
+use crate::ResolverStatusExt;
 use crate::SledAgentArtifactStore;
 use camino::Utf8Path;
 use camino::Utf8PathBuf;
@@ -15,9 +16,11 @@ use nexus_sled_agent_shared::inventory::BootPartitionDetails;
 use nexus_sled_agent_shared::inventory::HostPhase2DesiredContents;
 use nexus_sled_agent_shared::inventory::HostPhase2DesiredSlots;
 use omicron_common::disk::M2Slot;
+use sled_agent_types::zone_images::ResolverStatus;
 use sled_hardware::PooledDiskError;
 use slog::Logger;
 use slog::info;
+use slog::o;
 use slog::warn;
 use slog_error_chain::InlineErrorChain;
 use std::io;
@@ -128,27 +131,35 @@ pub(crate) struct BootPartitionReconciler {
 impl BootPartitionReconciler {
     pub(crate) async fn reconcile<T: SledAgentArtifactStore>(
         &mut self,
+        resolver_status: &ResolverStatus,
         internal_disks: &InternalDisks,
         desired: &HostPhase2DesiredSlots,
         artifact_store: &T,
-        // TODO We should also consider the mupdate override, once that work
-        // lands. Never overwrite the boot partitions while we're in that state.
         log: &Logger,
     ) -> BootPartitionContents {
+        let prepared_slot_a = resolver_status.prepare_host_phase_2_contents(
+            &log.new(o!("slot" => "A")),
+            &desired.slot_a,
+        );
+        let prepared_slot_b = resolver_status.prepare_host_phase_2_contents(
+            &log.new(o!("slot" => "B")),
+            &desired.slot_b,
+        );
+
         let (slot_a, slot_b) = futures::join!(
             Self::reconcile_slot::<_, RawDiskReader, RawDiskWriter>(
                 M2Slot::A,
                 internal_disks,
                 &mut self.cached_slot_a,
-                &desired.slot_a,
+                prepared_slot_a.desired_contents(),
                 artifact_store,
                 log,
             ),
             Self::reconcile_slot::<_, RawDiskReader, RawDiskWriter>(
                 M2Slot::B,
                 internal_disks,
                 &mut self.cached_slot_b,
-                &desired.slot_b,
+                prepared_slot_b.desired_contents(),
                 artifact_store,
                 log,
             ),
@@ -337,6 +348,27 @@ impl BootPartitionReconciler {
     }
 }
 
+pub enum HostPhase2PreparedContents<'a> {
+    /// No mupdate override was found, so the desired host phase 2 contents were
+    /// used.
+    NoMupdateOverride(&'a HostPhase2DesiredContents),
+
+    /// A mupdate override was found, so the contents are always set to
+    /// `CurrentContents`.
+    WithMupdateOverride,
+}
+
+impl<'a> HostPhase2PreparedContents<'a> {
+    pub fn desired_contents(&self) -> &'a HostPhase2DesiredContents {
+        match self {
+            Self::NoMupdateOverride(contents) => contents,
+            Self::WithMupdateOverride => {
+                &HostPhase2DesiredContents::CurrentContents
+            }
+        }
+    }
+}
+
 /// Traits to allow unit tests to run without accessing real raw disks; these
 /// are trivially implemented by the real types below and by unit tests inside
 /// the test module.

sled-agent/config-reconciler/src/host_phase_2.rs

@@ -53,6 +53,7 @@ mod handle;
 mod host_phase_2;
 mod internal_disks;
 mod ledger;
+mod mupdate_override;
 mod raw_disks;
 mod reconciler_task;
 mod sled_agent_facilities;
@@ -74,6 +75,7 @@ pub use internal_disks::InternalDisksWithBootDisk;
 pub use ledger::LedgerArtifactConfigError;
 pub use ledger::LedgerNewConfigError;
 pub use ledger::LedgerTaskError;
+pub use mupdate_override::ResolverStatusExt;
 pub use raw_disks::RawDisksSender;
 pub use reconciler_task::CurrentlyManagedZpools;
 pub use reconciler_task::CurrentlyManagedZpoolsReceiver;

sled-agent/config-reconciler/src/lib.rs

@@ -0,0 +1,183 @@
+// 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/.
+
+//! Behaviors of the config reconciler in the presence of mupdate overrides.
+
+use crate::InternalDisks;
+use crate::host_phase_2::HostPhase2PreparedContents;
+use camino::Utf8PathBuf;
+use nexus_sled_agent_shared::inventory::HostPhase2DesiredContents;
+use nexus_sled_agent_shared::inventory::OmicronZoneConfig;
+use nexus_sled_agent_shared::inventory::OmicronZoneImageSource;
+use nexus_sled_agent_shared::inventory::ZoneKind;
+use omicron_common::zone_images::ZoneImageFileSource;
+use sled_agent_types::zone_images::OmicronZoneFileSource;
+use sled_agent_types::zone_images::OmicronZoneImageLocation;
+use sled_agent_types::zone_images::PreparedOmicronZone;
+use sled_agent_types::zone_images::RAMDISK_IMAGE_PATH;
+use sled_agent_types::zone_images::ResolverStatus;
+use sled_agent_types::zone_images::ZoneImageLocationError;
+use slog::error;
+use slog_error_chain::InlineErrorChain;
+use tufaceous_artifact::ArtifactHash;
+
+/// An extension trait for `ResolverStatus`.
+///
+/// This trait refers to types that aren't available within `sled-agent-types`.
+pub trait ResolverStatusExt {
+    /// Look up the file source for an Omicron zone.
+    fn omicron_file_source(
+        &self,
+        log: &slog::Logger,
+        zone_kind: ZoneKind,
+        image_source: &OmicronZoneImageSource,
+        internal_disks: &InternalDisks,
+    ) -> OmicronZoneFileSource;
+
+    /// Prepare an Omicron zone for installation.
+    fn prepare_omicron_zone<'a>(
+        &self,
+        log: &slog::Logger,
+        zone_config: &'a OmicronZoneConfig,
+        internal_disks: &InternalDisks,
+    ) -> PreparedOmicronZone<'a> {
+        let file_source = self.omicron_file_source(
+            log,
+            zone_config.zone_type.kind(),
+            &zone_config.image_source,
+            internal_disks,
+        );
+        PreparedOmicronZone::new(zone_config, file_source)
+    }
+
+    fn prepare_host_phase_2_contents<'a>(
+        &self,
+        log: &slog::Logger,
+        desired: &'a HostPhase2DesiredContents,
+    ) -> HostPhase2PreparedContents<'a>;
+}
+
+impl ResolverStatusExt for ResolverStatus {
+    fn omicron_file_source(
+        &self,
+        log: &slog::Logger,
+        zone_kind: ZoneKind,
+        image_source: &OmicronZoneImageSource,
+        internal_disks: &InternalDisks,
+    ) -> OmicronZoneFileSource {
+        match image_source {
+            OmicronZoneImageSource::InstallDataset => {
+                let file_name = zone_kind.artifact_in_install_dataset();
+
+                // There's always at least one image path (the RAM disk below).
+                let mut search_paths = Vec::with_capacity(1);
+
+                // Inject an image path if requested by a test.
+                if let Some(path) = &self.image_directory_override {
+                    search_paths.push(path.clone());
+                };
+
+                // Any zones not part of the RAM disk are managed via the zone
+                // manifest.
+                let hash = install_dataset_hash(
+                    log,
+                    self,
+                    zone_kind,
+                    internal_disks,
+                    |path| search_paths.push(path),
+                );
+
+                // Look for the image in the RAM disk as a fallback. Note that
+                // install dataset images are not stored on the RAM disk in
+                // production, just in development or test workflows.
+                search_paths.push(Utf8PathBuf::from(RAMDISK_IMAGE_PATH));
+
+                OmicronZoneFileSource {
+                    location: OmicronZoneImageLocation::InstallDataset { hash },
+                    file_source: ZoneImageFileSource {
+                        file_name: file_name.to_owned(),
+                        search_paths,
+                    },
+                }
+            }
+            OmicronZoneImageSource::Artifact { hash } => {
+                // TODO: implement mupdate override here.
+                //
+                // 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.
+                let search_paths =
+                    internal_disks.all_artifact_datasets().collect();
+                OmicronZoneFileSource {
+                    // TODO: with mupdate overrides, return InstallDataset here
+                    location: OmicronZoneImageLocation::Artifact {
+                        hash: Ok(*hash),
+                    },
+                    file_source: ZoneImageFileSource {
+                        file_name: hash.to_string(),
+                        search_paths,
+                    },
+                }
+            }
+        }
+    }
+
+    fn prepare_host_phase_2_contents<'a>(
+        &self,
+        #[expect(unused)] log: &slog::Logger,
+        desired: &'a HostPhase2DesiredContents,
+    ) -> HostPhase2PreparedContents<'a> {
+        // TODO: Implement mupdate override logic.
+        HostPhase2PreparedContents::NoMupdateOverride(desired)
+    }
+}
+
+fn install_dataset_hash<F>(
+    log: &slog::Logger,
+    resolver_status: &ResolverStatus,
+    zone_kind: ZoneKind,
+    internal_disks: &InternalDisks,
+    mut search_paths_cb: F,
+) -> Result<ArtifactHash, ZoneImageLocationError>
+where
+    F: FnMut(Utf8PathBuf),
+{
+    // XXX: we ask for the boot zpool to be passed in here. But
+    // `ResolverStatus` also caches the boot zpool. How should we
+    // reconcile the two?
+    let hash = if let Some(path) = internal_disks.boot_disk_install_dataset() {
+        let hash = resolver_status.zone_manifest.zone_hash(zone_kind);
+        match hash {
+            Ok(hash) => {
+                search_paths_cb(path);
+                Ok(hash)
+            }
+            Err(error) => {
+                error!(
+                    log,
+                    "zone {} not found in the boot disk zone manifest, \
+                     not returning it as a source",
+                    zone_kind.report_str();
+                    "file_name" => zone_kind.artifact_in_install_dataset(),
+                    "error" => InlineErrorChain::new(&error),
+                );
+                Err(ZoneImageLocationError::ZoneHash(error))
+            }
+        }
+    } else {
+        // The boot disk is not available, so we cannot add the
+        // install dataset path from it.
+        error!(
+            log,
+            "boot disk install dataset not available, \
+             not returning it as a source";
+            "zone_kind" => zone_kind.report_str(),
+        );
+        Err(ZoneImageLocationError::BootDiskMissing)
+    };
+    hash
+}
+
+// Tests for this module live inside sled-agent-zone-images. (This is a bit
+// weird and should probably be addressed at some point.)