Check zone image source in inventory

Also remove now-superfluous `OrderedComponent` enum.

Author: plotnick

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

@@ -628,7 +628,7 @@ INFO sufficient InternalDns zones exist in plan, desired_count: 3, current_count
 INFO added zone to sled, sled_id: a88790de-5962-4871-8686-61c1fd5b7094, kind: ExternalDns
 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
-INFO zones not yet up-to-date, sled_id: a88790de-5962-4871-8686-61c1fd5b7094
+INFO some zones not yet up-to-date, sled_id: a88790de-5962-4871-8686-61c1fd5b7094
 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
 

nexus/reconfigurator/planning/src/blueprint_builder/builder.rs

@@ -14,7 +14,6 @@ use crate::blueprint_editor::ExternalSnatNetworkingChoice;
 use crate::blueprint_editor::NoAvailableDnsSubnets;
 use crate::blueprint_editor::SledEditError;
 use crate::blueprint_editor::SledEditor;
-use crate::planner::OrderedComponent;
 use crate::planner::ZoneExpungeReason;
 use crate::planner::rng::PlannerRng;
 use anyhow::Context as _;
@@ -1965,30 +1964,25 @@ impl<'a> BlueprintBuilder<'a> {
         zone_kind: ZoneKind,
         new_repo: Option<&TufRepoDescription>,
     ) -> bool {
-        match OrderedComponent::from(zone_kind) {
-            OrderedComponent::HostOs | OrderedComponent::SpRot => {
-                todo!("can't yet update Host OS or SP/RoT")
-            }
-            OrderedComponent::OmicronZone(kind) => match kind {
-                ZoneKind::Nexus => {
-                    // Nexus can only be updated if all non-Nexus zones have been updated,
-                    // i.e., their image source is an artifact from the new repo.
-                    self.sled_ids_with_zones().all(|sled_id| {
-                        self.current_sled_zones(
-                            sled_id,
-                            BlueprintZoneDisposition::is_in_service,
-                        )
-                        .filter(|z| z.zone_type.kind() != ZoneKind::Nexus)
-                        .all(|z| {
-                            z.image_source
-                                == Self::zone_image_artifact(new_repo, z.kind())
-                        })
+        match zone_kind {
+            ZoneKind::Nexus => {
+                // Nexus can only be updated if all non-Nexus zones have been updated,
+                // i.e., their image source is an artifact from the new repo.
+                self.sled_ids_with_zones().all(|sled_id| {
+                    self.current_sled_zones(
+                        sled_id,
+                        BlueprintZoneDisposition::is_in_service,
+                    )
+                    .filter(|z| z.zone_type.kind() != ZoneKind::Nexus)
+                    .all(|z| {
+                        z.image_source
+                            == Self::zone_image_artifact(new_repo, z.kind())
                     })
-                }
-                // <https://github.com/oxidecomputer/omicron/issues/6404>
-                // ZoneKind::CockroachDb => todo!("check cluster status in inventory"),
-                _ => true, // other zone kinds have no special dependencies
-            },
+                })
+            }
+            // <https://github.com/oxidecomputer/omicron/issues/6404>
+            // ZoneKind::CockroachDb => todo!("check cluster status in inventory"),
+            _ => true, // other zone kinds have no special dependencies
         }
     }
 

nexus/reconfigurator/planning/src/planner.rs

@@ -37,6 +37,7 @@ use omicron_uuid_kinds::PhysicalDiskUuid;
 use omicron_uuid_kinds::SledUuid;
 use slog::error;
 use slog::{Logger, info, warn};
+use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::str::FromStr;
 
@@ -45,11 +46,9 @@ use self::omicron_zone_placement::OmicronZonePlacement;
 use self::omicron_zone_placement::OmicronZonePlacementSledState;
 pub use self::rng::PlannerRng;
 pub use self::rng::SledPlannerRng;
-pub(crate) use self::update_sequence::OrderedComponent;
 
 mod omicron_zone_placement;
 pub(crate) mod rng;
-mod update_sequence;
 
 pub struct Planner<'a> {
     log: Logger,
@@ -905,34 +904,34 @@ impl<'a> Planner<'a> {
             .map(|(id, _details)| id)
             .collect::<Vec<_>>();
 
-        // Wait for all current zones to appear in the inventory.
-        // TODO-correctness: We should check their image source,
-        // but the inventory doesn't report that yet; see
-        // <https://github.com/oxidecomputer/omicron/issues/8084>.
+        // Wait for zones to appear up-to-date in the inventory.
         let inventory_zones = self
             .inventory
-            .all_omicron_zones()
-            .map(|z| z.id)
-            .collect::<BTreeSet<_>>();
+            .all_ledgered_omicron_zones()
+            .map(|z| (z.id, z.image_source.clone()))
+            .collect::<BTreeMap<_, _>>();
         for &sled_id in &sleds {
             if !self
                 .blueprint
                 .current_sled_zones(
                     sled_id,
                     BlueprintZoneDisposition::is_in_service,
                 )
-                .all(|zone| inventory_zones.contains(&zone.id))
+                .all(|zone| {
+                    let image_source = zone.image_source.clone().into();
+                    inventory_zones.get(&zone.id) == Some(&image_source)
+                })
             {
                 info!(
-                    self.log, "zones not yet up-to-date";
+                    self.log, "some zones not yet up-to-date";
                     "sled_id" => %sled_id,
                 );
                 return Ok(());
             }
         }
 
-        // Choose among the out-of-date zones one with the fewest dependencies.
-        let mut out_of_date_zones = sleds
+        // Update the first out-of-date zone.
+        let out_of_date_zones = sleds
             .into_iter()
             .flat_map(|sled_id| {
                 let blueprint = &self.blueprint;
@@ -949,8 +948,6 @@ impl<'a> Planner<'a> {
                     })
             })
             .collect::<Vec<(SledUuid, BlueprintZoneConfig)>>();
-
-        sort_zones_by_deps(&mut out_of_date_zones);
         if let Some((sled_id, zone)) = out_of_date_zones.first() {
             return self.update_or_expunge_zone(*sled_id, zone);
         }
@@ -1113,15 +1110,6 @@ impl<'a> Planner<'a> {
     }
 }
 
-/// Sort in place a vector of sled-specific zone configurations by API
-/// dependencies (RFD 565 §6).
-fn sort_zones_by_deps(zones: &mut Vec<(SledUuid, BlueprintZoneConfig)>) {
-    zones.sort_by(|a, b| {
-        OrderedComponent::from(a.1.zone_type.kind())
-            .cmp(&OrderedComponent::from(b.1.zone_type.kind()))
-    })
-}
-
 /// The reason a sled's zones need to be expunged.
 ///
 /// This is used only for introspection and logging -- it's not part of the
@@ -4511,53 +4499,20 @@ pub(crate) mod test {
         logctx.cleanup_successful();
     }
 
-    #[test]
-    fn test_sort_zones_by_deps() {
-        static TEST_NAME: &str = "sort_zones_by_deps";
-        let logctx = test_setup_log(TEST_NAME);
-        let log = logctx.log.clone();
-
-        // Collect zone configs from our example system.
-        let (_collection, _input, blueprint) = example(&log, TEST_NAME);
-        let mut zones = blueprint
-            .all_omicron_zones(BlueprintZoneDisposition::any)
-            .map(|(sled_id, z)| (sled_id, z.clone()))
-            .collect::<Vec<(SledUuid, BlueprintZoneConfig)>>();
-
-        // Sort them and verify the ordering constraints.
-        sort_zones_by_deps(&mut zones);
-        let mut nexus = false;
-        for kind in zones.iter().map(|(_, z)| z.zone_type.kind()) {
-            match kind {
-                ZoneKind::Nexus => {
-                    nexus = true;
-                }
-                _ => {
-                    assert!(!nexus);
-                }
-            }
-        }
-        assert!(nexus);
-
-        logctx.cleanup_successful();
-    }
-
     /// Manually update the example system's inventory collection's zones
     /// from a blueprint.
     fn update_collection_from_blueprint(
         example: &mut ExampleSystem,
         blueprint: &Blueprint,
     ) {
         for (&sled_id, config) in blueprint.sleds.iter() {
-            let sled_config = config.clone().into_in_service_sled_config();
-            let zones_config = OmicronZonesConfig {
-                generation: sled_config.generation,
-                zones: sled_config.zones.into_iter().collect(),
-            };
             example
                 .system
-                .sled_set_omicron_zones(sled_id, zones_config)
-                .expect("can't set omicron zones for sled");
+                .sled_set_omicron_config(
+                    sled_id,
+                    config.clone().into_in_service_sled_config(),
+                )
+                .expect("can't set sled config");
         }
         example.collection =
             example.system.to_collection_builder().unwrap().build();
@@ -4787,33 +4742,20 @@ pub(crate) mod test {
 
             parent = blueprint;
         }
+        let blueprint9 = parent;
 
         // All Crucible Pantries should now be updated.
         assert_eq!(
-            parent
+            blueprint9
                 .all_omicron_zones(BlueprintZoneDisposition::is_in_service)
                 .filter(|(_, z)| is_up_to_date_pantry(z))
                 .count(),
             CRUCIBLE_PANTRY_REDUNDANCY
         );
 
-        // One more iteration for the last old zone to be expunged.
-        update_collection_from_blueprint(&mut example, &parent);
-        let blueprint10 = Planner::new_based_on(
-            log.clone(),
-            &parent,
-            &input,
-            "last_blueprint",
-            &example.collection,
-        )
-        .expect("can't create planner")
-        .with_rng(PlannerRng::from_seed((TEST_NAME, "last_bp")))
-        .plan()
-        .expect("replan for last blueprint");
-
         // All old Pantry zones should now be expunged.
         assert_eq!(
-            blueprint10
+            blueprint9
                 .all_omicron_zones(BlueprintZoneDisposition::is_expunged)
                 .filter(|(_, z)| is_old_pantry(z))
                 .count(),
@@ -4823,17 +4765,17 @@ pub(crate) mod test {
         // Now we can update Nexus, because all of its dependent zones
         // are up-to-date w/r/t the new repo.
         assert_eq!(
-            parent
+            blueprint9
                 .all_omicron_zones(BlueprintZoneDisposition::is_in_service)
                 .filter(|(_, z)| is_old_nexus(z))
                 .count(),
             NEXUS_REDUNDANCY + 1,
         );
-        let mut parent = blueprint10;
-        for i in 11..=17 {
-            let blueprint_name = format!("blueprint_{i}");
+        let mut parent = blueprint9;
+        for i in 10..=17 {
             update_collection_from_blueprint(&mut example, &parent);
 
+            let blueprint_name = format!("blueprint{i}");
             let blueprint = Planner::new_based_on(
                 log.clone(),
                 &parent,
@@ -4847,7 +4789,6 @@ pub(crate) mod test {
             .unwrap_or_else(|_| panic!("can't re-plan after {i} iterations"));
 
             let summary = blueprint.diff_since_blueprint(&parent);
-            eprintln!("{}", summary.display());
             for sled in summary.diff.sleds.modified_values_diff() {
                 if i % 2 == 0 {
                     assert!(sled.zones.added.is_empty());
@@ -4965,8 +4906,9 @@ pub(crate) mod test {
 
         let mut parent = blueprint1;
         for i in 2..=MAX_PLANNING_ITERATIONS {
-            let blueprint_name = format!("blueprint_{i}");
             update_collection_from_blueprint(&mut example, &parent);
+
+            let blueprint_name = format!("blueprint{i}");
             let blueprint = Planner::new_based_on(
                 log.clone(),
                 &parent,