[reconfigurator] rewrite zones_currently_updating to be clearer (#8594)

This is semantically the same as the prior implementation but is
hopefully easier to understand.

Both @jgallagher and I were a bit confused about the previous
implementation, which is a sign that a rewrite might help.

Author: sunshowers

dev-tools/reconfigurator-cli/tests/output/cmds-example-stdout

@@ -506,7 +506,7 @@ INFO sufficient ExternalDns zones exist in plan, desired_count: 0, current_count
 WARN failed to place all new desired Nexus zones, placed: 0, wanted_to_place: 3
 INFO sufficient Oximeter zones exist in plan, desired_count: 0, current_count: 0
 WARN cannot issue more SP updates (no current artifacts)
-INFO some zones not yet up-to-date, sled_id: 89d02b1b-478c-401a-8e28-7a26f74fa41b, zones_currently_updating: [(b3c9c041-d2f0-4767-bdaf-0e52e9d7a013 (service), InternalNtp)]
+INFO some zones not yet up-to-date, sled_id: 89d02b1b-478c-401a-8e28-7a26f74fa41b, zones_currently_updating: [ZoneCurrentlyUpdating { zone_id: b3c9c041-d2f0-4767-bdaf-0e52e9d7a013 (service), zone_kind: InternalNtp, reason: MissingInInventory { bp_image_source: InstallDataset } }]
 INFO will ensure cockroachdb setting, setting: cluster.preserve_downgrade_option, value: DoNotModify
 generated blueprint 86db3308-f817-4626-8838-4085949a6a41 based on parent blueprint ade5749d-bdf3-4fab-a8ae-00bea01b3a5a
 

dev-tools/reconfigurator-cli/tests/output/cmds-expunge-newly-added-external-dns-stdout

@@ -981,7 +981,7 @@ INFO added zone to sled, sled_id: 711ac7f8-d19e-4572-bdb9-e9b50f6e362a, kind: Ex
 INFO sufficient Nexus zones exist in plan, desired_count: 3, current_count: 3
 INFO sufficient Oximeter zones exist in plan, desired_count: 0, current_count: 0
 WARN cannot issue more SP updates (no current artifacts)
-INFO some zones not yet up-to-date, sled_id: 711ac7f8-d19e-4572-bdb9-e9b50f6e362a, zones_currently_updating: [(fe2d5287-24e3-4071-b214-2640b097a759 (service), ExternalDns)]
+INFO some zones not yet up-to-date, sled_id: 711ac7f8-d19e-4572-bdb9-e9b50f6e362a, zones_currently_updating: [ZoneCurrentlyUpdating { zone_id: fe2d5287-24e3-4071-b214-2640b097a759 (service), zone_kind: ExternalDns, reason: MissingInInventory { bp_image_source: InstallDataset } }]
 INFO will ensure cockroachdb setting, setting: cluster.preserve_downgrade_option, value: DoNotModify
 generated blueprint 9c998c1d-1a7b-440a-ae0c-40f781dea6e2 based on parent blueprint 366b0b68-d80e-4bc1-abd3-dc69837847e0
 

dev-tools/reconfigurator-cli/tests/output/cmds-expunge-newly-added-internal-dns-stdout

@@ -1014,7 +1014,7 @@ INFO sufficient ExternalDns zones exist in plan, desired_count: 3, current_count
 INFO sufficient Nexus zones exist in plan, desired_count: 3, current_count: 3
 INFO sufficient Oximeter zones exist in plan, desired_count: 0, current_count: 0
 WARN cannot issue more SP updates (no current artifacts)
-INFO some zones not yet up-to-date, sled_id: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c, zones_currently_updating: [(e375dd21-320b-43b7-bc92-a2c3dac9d9e1 (service), InternalDns)]
+INFO some zones not yet up-to-date, sled_id: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c, zones_currently_updating: [ZoneCurrentlyUpdating { zone_id: e375dd21-320b-43b7-bc92-a2c3dac9d9e1 (service), zone_kind: InternalDns, reason: MissingInInventory { bp_image_source: InstallDataset } }]
 INFO will ensure cockroachdb setting, setting: cluster.preserve_downgrade_option, value: DoNotModify
 generated blueprint af934083-59b5-4bf6-8966-6fb5292c29e1 based on parent blueprint 58d5e830-0884-47d8-a7cd-b2b3751adeb4
 

nexus/reconfigurator/planning/src/planner.rs

@@ -16,6 +16,8 @@ use crate::blueprint_editor::SledEditError;
 use crate::mgs_updates::plan_mgs_updates;
 use crate::planner::omicron_zone_placement::PlacementError;
 use gateway_client::types::SpType;
+use nexus_sled_agent_shared::inventory::ConfigReconcilerInventoryResult;
+use nexus_sled_agent_shared::inventory::OmicronZoneImageSource;
 use nexus_sled_agent_shared::inventory::OmicronZoneType;
 use nexus_sled_agent_shared::inventory::ZoneKind;
 use nexus_types::deployment::Blueprint;
@@ -39,6 +41,7 @@ use nexus_types::external_api::views::SledState;
 use nexus_types::inventory::Collection;
 use omicron_common::policy::COCKROACHDB_REDUNDANCY;
 use omicron_common::policy::INTERNAL_DNS_REDUNDANCY;
+use omicron_uuid_kinds::OmicronZoneUuid;
 use omicron_uuid_kinds::PhysicalDiskUuid;
 use omicron_uuid_kinds::SledUuid;
 use slog::debug;
@@ -1224,22 +1227,101 @@ impl<'a> Planner<'a> {
         // Wait for zones to appear up-to-date in the inventory.
         let inventory_zones = self
             .inventory
-            .all_running_omicron_zones()
-            .map(|z| (z.id, z.image_source.clone()))
+            .all_reconciled_omicron_zones()
+            .map(|(z, sa_result)| (z.id, (&z.image_source, sa_result)))
             .collect::<BTreeMap<_, _>>();
+
+        #[derive(Debug)]
+        #[expect(dead_code)]
+        struct ZoneCurrentlyUpdating<'a> {
+            zone_id: OmicronZoneUuid,
+            zone_kind: ZoneKind,
+            reason: UpdatingReason<'a>,
+        }
+
+        #[derive(Debug)]
+        #[expect(dead_code)]
+        enum UpdatingReason<'a> {
+            ImageSourceMismatch {
+                bp_image_source: &'a BlueprintZoneImageSource,
+                inv_image_source: &'a OmicronZoneImageSource,
+            },
+            MissingInInventory {
+                bp_image_source: &'a BlueprintZoneImageSource,
+            },
+            ReconciliationError {
+                bp_image_source: &'a BlueprintZoneImageSource,
+                inv_image_source: &'a OmicronZoneImageSource,
+                message: &'a str,
+            },
+        }
+
         for &sled_id in &sleds {
+            // Build a list of zones currently in the blueprint but where
+            // inventory has a mismatch or does not know about the zone.
+            //
+            // What about the case where a zone is in inventory but not in the
+            // blueprint? See
+            // https://github.com/oxidecomputer/omicron/issues/8589.
             let zones_currently_updating = self
                 .blueprint
                 .current_sled_zones(
                     sled_id,
                     BlueprintZoneDisposition::is_in_service,
                 )
                 .filter_map(|zone| {
-                    let image_source = zone.image_source.clone().into();
-                    if inventory_zones.get(&zone.id) != Some(&image_source) {
-                        Some((zone.id, zone.zone_type.kind()))
-                    } else {
-                        None
+                    let bp_image_source =
+                        OmicronZoneImageSource::from(zone.image_source.clone());
+                    match inventory_zones.get(&zone.id) {
+                        Some((
+                            inv_image_source,
+                            ConfigReconcilerInventoryResult::Ok,
+                        )) if *inv_image_source == &bp_image_source => {
+                            // The inventory and blueprint image sources match
+                            // -- this means that the zone is up-to-date.
+                            None
+                        }
+                        Some((
+                            inv_image_source,
+                            ConfigReconcilerInventoryResult::Ok,
+                        )) => {
+                            // The inventory and blueprint image sources differ.
+                            Some(ZoneCurrentlyUpdating {
+                                zone_id: zone.id,
+                                zone_kind: zone.kind(),
+                                reason: UpdatingReason::ImageSourceMismatch {
+                                    bp_image_source: &zone.image_source,
+                                    inv_image_source,
+                                },
+                            })
+                        }
+                        Some((
+                            inv_image_source,
+                            ConfigReconcilerInventoryResult::Err { message },
+                        )) => {
+                            // The inventory reports this zone but there was an
+                            // error reconciling it (most likely an error
+                            // starting the zone).
+                            Some(ZoneCurrentlyUpdating {
+                                zone_id: zone.id,
+                                zone_kind: zone.kind(),
+                                reason: UpdatingReason::ReconciliationError {
+                                    bp_image_source: &zone.image_source,
+                                    inv_image_source,
+                                    message,
+                                },
+                            })
+                        }
+                        None => {
+                            // The blueprint has a zone that inventory does not have.
+                            Some(ZoneCurrentlyUpdating {
+                                zone_id: zone.id,
+                                zone_kind: zone.kind(),
+                                reason: UpdatingReason::MissingInInventory {
+                                    bp_image_source: &zone.image_source,
+                                },
+                            })
+                        }
                     }
                 })
                 .collect::<Vec<_>>();

nexus/types/src/inventory.rs

@@ -23,6 +23,7 @@ use iddqd::IdOrdItem;
 use iddqd::IdOrdMap;
 use iddqd::id_upcast;
 use nexus_sled_agent_shared::inventory::ConfigReconcilerInventory;
+use nexus_sled_agent_shared::inventory::ConfigReconcilerInventoryResult;
 use nexus_sled_agent_shared::inventory::ConfigReconcilerInventoryStatus;
 use nexus_sled_agent_shared::inventory::InventoryDataset;
 use nexus_sled_agent_shared::inventory::InventoryDisk;
@@ -197,6 +198,20 @@ impl Collection {
             .flat_map(|reconciliation| reconciliation.running_omicron_zones())
     }
 
+    /// Iterate over all the Omicron zones along with their statuses (as
+    /// reported by each sled-agent's last reconciliation attempt)
+    pub fn all_reconciled_omicron_zones(
+        &self,
+    ) -> impl Iterator<Item = (&OmicronZoneConfig, &ConfigReconcilerInventoryResult)>
+    {
+        self.sled_agents
+            .iter()
+            .filter_map(|sa| sa.last_reconciliation.as_ref())
+            .flat_map(|reconciliation| {
+                reconciliation.reconciled_omicron_zones()
+            })
+    }
+
     /// Iterate over the sled ids of sleds identified as Scrimlets
     pub fn scrimlets(&self) -> impl Iterator<Item = SledUuid> + '_ {
         self.sled_agents.iter().filter_map(|sa| {