[reconfigurator] Fix planner RoT active slot bug (#8893)

Fixes a bug where the planner was using the expected active slot as the
found active slot for doing status checks.

Closes: #8887

Author: karencfv

nexus/reconfigurator/planning/src/mgs_updates/mod.rs

@@ -338,45 +338,36 @@ fn mgs_update_status(
             expected_pending_persistent_boot_preference,
             expected_transient_boot_preference,
         }) => {
-            let active_caboose_which = match &expected_active_slot.slot {
-                RotSlot::A => CabooseWhich::RotSlotA,
-                RotSlot::B => CabooseWhich::RotSlotB,
-            };
-
-            let Some(active_caboose) =
-                inventory.caboose_for(active_caboose_which, baseboard_id)
-            else {
-                return Err(MgsUpdateStatusError::MissingActiveCaboose);
-            };
-
-            let found_inactive_version = inventory
-                .caboose_for(active_caboose_which.toggled_slot(), baseboard_id)
-                .map(|c| c.caboose.version.as_ref());
-
             let rot_state = inventory
                 .rots
                 .get(baseboard_id)
                 .ok_or(MgsUpdateStatusError::MissingRotState)?;
 
+            let active_slot = rot_state.active_slot;
+
+            let active_caboose_which = match &active_slot {
+                RotSlot::A => CabooseWhich::RotSlotA,
+                RotSlot::B => CabooseWhich::RotSlotB,
+            };
+
+            let active_caboose = inventory
+                .caboose_for(active_caboose_which, baseboard_id)
+                .ok_or(MgsUpdateStatusError::MissingActiveCaboose)?;
+
             let found_active_version =
                 ArtifactVersion::new(active_caboose.caboose.version.clone())
                     .map_err(|e| {
                         MgsUpdateStatusError::FailedArtifactVersionParse(e)
                     })?;
 
             let found_active_slot = ExpectedActiveRotSlot {
-                slot: rot_state.active_slot,
+                slot: active_slot,
                 version: found_active_version,
             };
 
-            let expected = RotUpdateState {
-                active_slot: expected_active_slot.clone(),
-                persistent_boot_preference:
-                    *expected_persistent_boot_preference,
-                pending_persistent_boot_preference:
-                    *expected_pending_persistent_boot_preference,
-                transient_boot_preference: *expected_transient_boot_preference,
-            };
+            let found_inactive_version = inventory
+                .caboose_for(active_caboose_which.toggled_slot(), baseboard_id)
+                .map(|c| c.caboose.version.as_ref());
 
             let found = RotUpdateState {
                 active_slot: found_active_slot,
@@ -387,6 +378,15 @@ fn mgs_update_status(
                 transient_boot_preference: rot_state.transient_boot_preference,
             };
 
+            let expected = RotUpdateState {
+                active_slot: expected_active_slot.clone(),
+                persistent_boot_preference:
+                    *expected_persistent_boot_preference,
+                pending_persistent_boot_preference:
+                    *expected_pending_persistent_boot_preference,
+                transient_boot_preference: *expected_transient_boot_preference,
+            };
+
             mgs_update_status_rot(
                 desired_version,
                 expected,
@@ -549,6 +549,7 @@ mod test {
     use dropshot::ConfigLogging;
     use dropshot::ConfigLoggingLevel;
     use gateway_client::types::SpType;
+    use gateway_types::rot::RotSlot;
     use nexus_types::deployment::ExpectedVersion;
     use nexus_types::deployment::PendingMgsUpdateDetails;
     use nexus_types::deployment::PendingMgsUpdateRotBootloaderDetails;
@@ -916,15 +917,22 @@ mod test {
         assert_eq!(updates, later_updates);
 
         // At this point, we're ready to test that when the first SpType update
-        // completes, then the second one *is* started.  This tests three
+        // completes, then the second one *is* started.  This tests four
         // different things: first that we noticed the first one completed,
-        // second that we noticed another thing needed an update, and third that
-        // the planner schedules the updates in the correct order: first RoT,
-        // and second SP.
+        // second that we noticed another thing needed an update, third that
+        // an update is correctly configured after the active slot has changed
+        // from the previous component update, and fourth that the planner
+        // schedules the updates in the correct order: first RoT, and second SP.
         let later_collection = test_boards
             .collection_builder()
             .sp_active_version_exception(SpType::Switch, 1, ARTIFACT_VERSION_1)
             .rot_active_version_exception(SpType::Switch, 1, ARTIFACT_VERSION_1)
+            .rot_active_slot_exception(SpType::Sled, 0, RotSlot::B)
+            .rot_persistent_boot_preference_exception(
+                SpType::Sled,
+                0,
+                RotSlot::B,
+            )
             .build();
         let PlannedMgsUpdates { pending_updates: later_updates, .. } =
             plan_mgs_updates(

nexus/reconfigurator/planning/src/mgs_updates/rot.rs

@@ -82,6 +82,7 @@ pub fn mgs_update_status_rot(
     // should not proceed. Once https://github.com/oxidecomputer/hubris/pull/2050
     // is implemented, we should revist this check
     if found_persistent_boot_preference != found_active_slot.slot
+        || expected_persistent_boot_preference != expected_active_slot.slot
         || found_transient_boot_preference.is_some()
         || expected_transient_boot_preference.is_some()
     {

nexus/reconfigurator/planning/src/mgs_updates/test_helpers.rs

@@ -744,6 +744,8 @@ pub(super) struct TestBoardCollectionBuilder<'a> {
     sp_active_version_exceptions: BTreeMap<SpIdentifier, ArtifactVersion>,
     rot_active_version_exceptions: BTreeMap<SpIdentifier, ArtifactVersion>,
     stage0_version_exceptions: BTreeMap<SpIdentifier, ArtifactVersion>,
+    rot_active_slot_exceptions: BTreeMap<SpIdentifier, RotSlot>,
+    rot_persistent_boot_preference_exceptions: BTreeMap<SpIdentifier, RotSlot>,
 
     // host exceptions are keyed only by slot; they only apply to sleds.
     host_exceptions: BTreeMap<u16, HostOsException>,
@@ -774,6 +776,8 @@ impl<'a> TestBoardCollectionBuilder<'a> {
             sp_active_version_exceptions: BTreeMap::new(),
             rot_active_version_exceptions: BTreeMap::new(),
             stage0_version_exceptions: BTreeMap::new(),
+            rot_active_slot_exceptions: BTreeMap::new(),
+            rot_persistent_boot_preference_exceptions: BTreeMap::new(),
             host_exceptions: BTreeMap::new(),
         }
     }
@@ -829,6 +833,28 @@ impl<'a> TestBoardCollectionBuilder<'a> {
         self
     }
 
+    pub fn rot_active_slot_exception(
+        mut self,
+        type_: SpType,
+        sp_slot: u16,
+        s: RotSlot,
+    ) -> Self {
+        self.rot_active_slot_exceptions
+            .insert(SpIdentifier { type_, slot: sp_slot }, s);
+        self
+    }
+
+    pub fn rot_persistent_boot_preference_exception(
+        mut self,
+        type_: SpType,
+        sp_slot: u16,
+        s: RotSlot,
+    ) -> Self {
+        self.rot_persistent_boot_preference_exceptions
+            .insert(SpIdentifier { type_, slot: sp_slot }, s);
+        self
+    }
+
     pub fn has_rot_active_version_exception(
         &self,
         type_: SpType,
@@ -906,29 +932,6 @@ impl<'a> TestBoardCollectionBuilder<'a> {
         let mut builder =
             nexus_inventory::CollectionBuilder::new(self.boards.test_name);
 
-        let dummy_sp_state = SpState {
-            base_mac_address: [0; 6],
-            hubris_archive_id: String::from("unused"),
-            model: String::from("unused"),
-            power_state: PowerState::A0,
-            revision: 0,
-            rot: RotState::V3 {
-                active: RotSlot::A,
-                pending_persistent_boot_preference: None,
-                persistent_boot_preference: RotSlot::A,
-                slot_a_error: None,
-                slot_a_fwid: Default::default(),
-                slot_b_error: None,
-                slot_b_fwid: Default::default(),
-                stage0_error: None,
-                stage0_fwid: Default::default(),
-                stage0next_error: None,
-                stage0next_fwid: Default::default(),
-                transient_boot_preference: None,
-            },
-            serial_number: String::from("unused"),
-        };
-
         for board in &self.boards.boards {
             let &TestBoard {
                 id: sp_id,
@@ -939,6 +942,40 @@ impl<'a> TestBoardCollectionBuilder<'a> {
                 rot_sign: rkth,
             } = board;
 
+            let rot_active_slot = self
+                .rot_active_slot_exceptions
+                .get(&sp_id)
+                .cloned()
+                .unwrap_or(RotSlot::A);
+            let rot_persistent_boot_preference = self
+                .rot_persistent_boot_preference_exceptions
+                .get(&sp_id)
+                .cloned()
+                .unwrap_or(RotSlot::A);
+
+            let dummy_sp_state = SpState {
+                base_mac_address: [0; 6],
+                hubris_archive_id: String::from("unused"),
+                model: String::from("unused"),
+                power_state: PowerState::A0,
+                revision: 0,
+                rot: RotState::V3 {
+                    active: rot_active_slot,
+                    pending_persistent_boot_preference: None,
+                    persistent_boot_preference: rot_persistent_boot_preference,
+                    slot_a_error: None,
+                    slot_a_fwid: Default::default(),
+                    slot_b_error: None,
+                    slot_b_fwid: Default::default(),
+                    stage0_error: None,
+                    stage0_fwid: Default::default(),
+                    stage0next_error: None,
+                    stage0next_fwid: Default::default(),
+                    transient_boot_preference: None,
+                },
+                serial_number: String::from("unused"),
+            };
+
             let sp_state = SpState {
                 model: format!("dummy_{}", sp_id.type_),
                 serial_number: serial.to_string(),
@@ -985,7 +1022,7 @@ impl<'a> TestBoardCollectionBuilder<'a> {
             builder
                 .found_caboose(
                     &baseboard_id,
-                    CabooseWhich::RotSlotA,
+                    CabooseWhich::from_rot_slot(rot_active_slot),
                     "test",
                     SpComponentCaboose {
                         board: caboose_rot_board.to_string(),
@@ -1040,7 +1077,7 @@ impl<'a> TestBoardCollectionBuilder<'a> {
                 builder
                     .found_caboose(
                         &baseboard_id,
-                        CabooseWhich::RotSlotB,
+                        CabooseWhich::from_rot_slot(rot_active_slot.toggled()),
                         "test",
                         SpComponentCaboose {
                             board: caboose_rot_board.to_string(),