Skip to content

[reconfigurator] Fix planner RoT active slot bug #8893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 25, 2025
+99 −53
Merged

[reconfigurator] Fix planner RoT active slot bug #8893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
028d681
[reconfigurator] Fix planner RoT active slot bug
karencfv Aug 22, 2025
b8acd51
Improve planner RoT tests
karencfv Aug 23, 2025
79269af
fix SP component
karencfv Aug 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 36 additions & 28 deletions nexus/reconfigurator/planning/src/mgs_updates/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

@jgallagher jgallagher Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little surprised this didn't break some (incorrect) tests - can we add a test somewhere that only passes with this change? How did the reconfiguator-cli target-release test pass before?

Copy link
Contributor Author

@karencfv karencfv Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Franky, I'm surprised too. But yeah! I'll add a test, good call

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,
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions nexus/reconfigurator/planning/src/mgs_updates/rot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
87 changes: 62 additions & 25 deletions nexus/reconfigurator/planning/src/mgs_updates/test_helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,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>,
Expand Down Expand Up @@ -753,6 +755,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(),
}
}
Expand Down Expand Up @@ -808,6 +812,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,
Expand Down Expand Up @@ -885,29 +911,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,
Expand All @@ -918,6 +921,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(),
Expand Down Expand Up @@ -964,7 +1001,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(),
Expand Down Expand Up @@ -1019,7 +1056,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(),
Expand Down
Loading