Simplify OrderedComponent logic

Author: plotnick

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

@@ -1929,28 +1929,32 @@ impl<'a> BlueprintBuilder<'a> {
         let old_repo = self.input.old_repo();
         let new_artifact = Self::zone_image_artifact(new_repo, zone_kind);
         let old_artifact = Self::zone_image_artifact(old_repo, zone_kind);
-        if let Some(prev) = OrderedComponent::from(zone_kind).prev() {
-            if prev >= OrderedComponent::NonNexusOmicronZone
-                && self.sled_ids_with_zones().any(|sled_id| {
+        match OrderedComponent::from(zone_kind) {
+            // Nexus can only be updated if all non-Nexus zones have been updated.
+            OrderedComponent::NexusZone => {
+                if self.sled_ids_with_zones().any(|sled_id| {
                     self.current_sled_zones(
                         sled_id,
                         BlueprintZoneDisposition::is_in_service,
                     )
-                    .any(|z| {
-                        let kind = z.zone_type.kind();
-                        let old_artifact =
-                            Self::zone_image_artifact(old_repo, kind);
-                        OrderedComponent::from(kind) == prev
-                            && z.image_source == old_artifact
+                    .filter(|z| {
+                        OrderedComponent::from(z.zone_type.kind())
+                            == OrderedComponent::NonNexusOmicronZone
                     })
-                })
-            {
-                old_artifact
-            } else {
-                new_artifact
+                    .any(|z| z.image_source != new_artifact)
+                }) {
+                    // Some dependent zone is not up-to-date.
+                    old_artifact
+                } else {
+                    // All dependent zones are up-to-date.
+                    new_artifact
+                }
+            }
+            // It's always safe to use the new artifact for non-Nexus zones.
+            OrderedComponent::NonNexusOmicronZone => new_artifact,
+            OrderedComponent::HostOs | OrderedComponent::SpRot => {
+                unreachable!("can't get an OS or SP/RoT image from a zone")
             }
-        } else {
-            new_artifact
         }
     }
 }

nexus/reconfigurator/planning/src/planner.rs

@@ -4924,7 +4924,7 @@ pub(crate) mod test {
                             .all(|(_, zone)| match zone.zone_type.kind() {
                                 ZoneKind::Nexus => {
                                     if zone.image_source == image_source {
-                                        comp = comp.next().unwrap();
+                                        comp = OrderedComponent::NexusZone;
                                         true
                                     } else {
                                         zone.image_source

nexus/reconfigurator/planning/src/planner/update_sequence.rs

@@ -5,50 +5,17 @@
 //! Updatable components ordered by dependencies (RFD 565).
 
 use nexus_sled_agent_shared::inventory::ZoneKind;
-use strum::{EnumIter, IntoEnumIterator as _};
 
 /// Update sequence as defined by RFD 565 §6.
-#[derive(
-    Debug, Clone, Copy, PartialEq, Eq, Hash, EnumIter, PartialOrd, Ord,
-)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
+#[allow(dead_code)]
 pub enum OrderedComponent {
     HostOs,
     SpRot,
     NonNexusOmicronZone,
     NexusZone,
 }
 
-/// To implement the checks in RFD 565 §9, we need to go backwards
-/// (and mabye forwards) through the component ordering. The simple
-/// linear scans here are fine as long as `OrderedComponent` has only
-/// a few variants; if it starts getting large (e.g., programatically
-/// generated from `ls-apis`), we can switch to something faster.
-impl OrderedComponent {
-    #[allow(dead_code)]
-    pub fn next(&self) -> Option<Self> {
-        let mut found = false;
-        for comp in OrderedComponent::iter() {
-            if found {
-                return Some(comp);
-            } else if comp == *self {
-                found = true;
-            }
-        }
-        None
-    }
-
-    pub fn prev(&self) -> Option<Self> {
-        let mut prev = None;
-        for comp in OrderedComponent::iter() {
-            if comp == *self {
-                return prev;
-            }
-            prev = Some(comp);
-        }
-        prev
-    }
-}
-
 impl From<ZoneKind> for OrderedComponent {
     fn from(zone_kind: ZoneKind) -> Self {
         match zone_kind {
@@ -57,52 +24,3 @@ impl From<ZoneKind> for OrderedComponent {
         }
     }
 }
-
-#[cfg(test)]
-mod test {
-    use super::*;
-
-    #[test]
-    fn ordered_component_next_prev() {
-        // Exhaustive checks ok for a few variants, revisit if it grows.
-        assert_eq!(OrderedComponent::HostOs.prev(), None);
-        assert_eq!(
-            OrderedComponent::HostOs.next(),
-            Some(OrderedComponent::SpRot)
-        );
-        assert_eq!(
-            OrderedComponent::SpRot.prev(),
-            Some(OrderedComponent::HostOs)
-        );
-        assert_eq!(
-            OrderedComponent::SpRot.next(),
-            Some(OrderedComponent::NonNexusOmicronZone)
-        );
-        assert_eq!(
-            OrderedComponent::NonNexusOmicronZone.prev(),
-            Some(OrderedComponent::SpRot)
-        );
-        assert_eq!(
-            OrderedComponent::NonNexusOmicronZone.next(),
-            Some(OrderedComponent::NexusZone)
-        );
-        assert_eq!(
-            OrderedComponent::NexusZone.prev(),
-            Some(OrderedComponent::NonNexusOmicronZone)
-        );
-        assert_eq!(OrderedComponent::NexusZone.next(), None);
-        assert!(OrderedComponent::HostOs < OrderedComponent::NexusZone);
-    }
-
-    #[test]
-    fn ordered_component_from_zone_kind() {
-        assert!(matches!(
-            OrderedComponent::from(ZoneKind::CruciblePantry),
-            OrderedComponent::NonNexusOmicronZone
-        ));
-        assert!(matches!(
-            OrderedComponent::from(ZoneKind::Nexus),
-            OrderedComponent::NexusZone
-        ));
-    }
-}