diff --git a/nexus/mgs-updates/src/driver_update.rs b/nexus/mgs-updates/src/driver_update.rs index 5ec96d19ca3..4f0ebefc48e 100644 --- a/nexus/mgs-updates/src/driver_update.rs +++ b/nexus/mgs-updates/src/driver_update.rs @@ -630,11 +630,14 @@ async fn wait_for_update_done( mod test { use super::ApplyUpdateError; use crate::test_util::test_artifacts::TestArtifacts; + use crate::test_util::updates::ExpectedSpComponent; use crate::test_util::updates::UpdateDescription; use assert_matches::assert_matches; use gateway_client::types::SpType; use gateway_messages::SpPort; use gateway_test_utils::setup::GatewayTestContext; + use gateway_types::rot::RotSlot; + use nexus_types::deployment::ExpectedActiveRotSlot; use nexus_types::deployment::ExpectedVersion; use nexus_types::internal_api::views::UpdateAttemptStatus; use nexus_types::internal_api::views::UpdateCompletedHow; @@ -655,7 +658,7 @@ mod test { let artifacts = TestArtifacts::new(log).await.unwrap(); // Basic case: normal update - run_one_successful_update( + run_one_successful_sp_update( &gwtestctx, &artifacts, SpType::Sled, @@ -666,7 +669,7 @@ mod test { .await; // Basic case: attempted update, found no changes needed - run_one_successful_update( + run_one_successful_sp_update( &gwtestctx, &artifacts, SpType::Sled, @@ -677,7 +680,7 @@ mod test { .await; // Run the same two tests for a switch SP. - run_one_successful_update( + run_one_successful_sp_update( &gwtestctx, &artifacts, SpType::Switch, @@ -686,7 +689,7 @@ mod test { UpdateCompletedHow::CompletedUpdate, ) .await; - run_one_successful_update( + run_one_successful_sp_update( &gwtestctx, &artifacts, SpType::Switch, @@ -700,7 +703,7 @@ mod test { gwtestctx.teardown().await; } - async fn run_one_successful_update( + async fn run_one_successful_sp_update( gwtestctx: &GatewayTestContext, artifacts: &TestArtifacts, sp_type: SpType, @@ -715,14 +718,103 @@ mod test { slot_id, artifact_hash, override_baseboard_id: None, - override_expected_active: None, - override_expected_inactive: None, + override_expected_sp_component: ExpectedSpComponent::Sp { + override_expected_active: None, + override_expected_inactive: None, + }, + override_progress_timeout: None, + }; + + let in_progress = desc.setup().await; + let finished = in_progress.finish().await; + finished.expect_sp_success(expected_result); + } + + /// Tests several happy-path cases of updating an RoT + #[tokio::test] + async fn test_rot_update_basic() { + let gwtestctx = gateway_test_utils::setup::test_setup( + "test_rot_update_basic", + SpPort::One, + ) + .await; + let log = &gwtestctx.logctx.log; + let artifacts = TestArtifacts::new(log).await.unwrap(); + + // Basic case: normal update + run_one_successful_rot_update( + &gwtestctx, + &artifacts, + SpType::Sled, + 1, + &artifacts.rot_gimlet_artifact_hash, + UpdateCompletedHow::CompletedUpdate, + ) + .await; + + // Basic case: attempted update, found no changes needed + run_one_successful_rot_update( + &gwtestctx, + &artifacts, + SpType::Sled, + 1, + &artifacts.rot_gimlet_artifact_hash, + UpdateCompletedHow::FoundNoChangesNeeded, + ) + .await; + + // Run the same two tests for a switch RoT. + run_one_successful_rot_update( + &gwtestctx, + &artifacts, + SpType::Switch, + 0, + &artifacts.rot_sidecar_artifact_hash, + UpdateCompletedHow::CompletedUpdate, + ) + .await; + run_one_successful_rot_update( + &gwtestctx, + &artifacts, + SpType::Switch, + 0, + &artifacts.rot_sidecar_artifact_hash, + UpdateCompletedHow::FoundNoChangesNeeded, + ) + .await; + + artifacts.teardown().await; + gwtestctx.teardown().await; + } + + async fn run_one_successful_rot_update( + gwtestctx: &GatewayTestContext, + artifacts: &TestArtifacts, + sp_type: SpType, + slot_id: u32, + artifact_hash: &ArtifactHash, + expected_result: UpdateCompletedHow, + ) { + let desc = UpdateDescription { + gwtestctx, + artifacts, + sp_type, + slot_id, + artifact_hash, + override_baseboard_id: None, + override_expected_sp_component: ExpectedSpComponent::Rot { + override_expected_active_slot: None, + override_expected_inactive_version: None, + override_expected_persistent_boot_preference: None, + override_expected_pending_persistent_boot_preference: None, + override_expected_transient_boot_preference: None, + }, override_progress_timeout: None, }; let in_progress = desc.setup().await; let finished = in_progress.finish().await; - finished.expect_success(expected_result); + finished.expect_rot_success(expected_result); } /// Tests the case where two updates run concurrently. One notices another @@ -756,8 +848,10 @@ mod test { slot_id: 1, artifact_hash: &artifacts.sp_gimlet_artifact_hash, override_baseboard_id: None, - override_expected_active: None, - override_expected_inactive: None, + override_expected_sp_component: ExpectedSpComponent::Sp { + override_expected_active: None, + override_expected_inactive: None, + }, override_progress_timeout: None, }; @@ -768,19 +862,23 @@ mod test { slot_id: 1, artifact_hash: &artifacts.sp_gimlet_artifact_hash, override_baseboard_id: None, - override_expected_active: None, - override_expected_inactive: Some(ExpectedVersion::Version( - std::str::from_utf8( - artifacts - .deployed_caboose(&artifacts.sp_gimlet_artifact_hash) - .expect("deployed caboose for generated artifact") - .version() - .expect("valid version"), - ) - .expect("version is UTF-8") - .parse() - .expect("version is valid"), - )), + override_expected_sp_component: ExpectedSpComponent::Sp { + override_expected_active: None, + override_expected_inactive: Some(ExpectedVersion::Version( + std::str::from_utf8( + artifacts + .deployed_caboose( + &artifacts.sp_gimlet_artifact_hash, + ) + .expect("deployed caboose for generated artifact") + .version() + .expect("valid version"), + ) + .expect("version is UTF-8") + .parse() + .expect("version is valid"), + )), + }, override_progress_timeout: None, }; @@ -799,8 +897,9 @@ mod test { let finished2 = in_progress2.finish().await; // Both should succeed, but with different codes. - finished1.expect_success(UpdateCompletedHow::CompletedUpdate); - finished2.expect_success(UpdateCompletedHow::WaitedForConcurrentUpdate); + finished1.expect_sp_success(UpdateCompletedHow::CompletedUpdate); + finished2 + .expect_sp_success(UpdateCompletedHow::WaitedForConcurrentUpdate); artifacts.teardown().await; gwtestctx.teardown().await; @@ -825,8 +924,10 @@ mod test { slot_id: 1, artifact_hash: &artifacts.sp_gimlet_artifact_hash, override_baseboard_id: None, - override_expected_active: None, - override_expected_inactive: None, + override_expected_sp_component: ExpectedSpComponent::Sp { + override_expected_active: None, + override_expected_inactive: None, + }, override_progress_timeout: None, }; @@ -837,19 +938,23 @@ mod test { slot_id: 1, artifact_hash: &artifacts.sp_gimlet_artifact_hash, override_baseboard_id: None, - override_expected_active: None, - override_expected_inactive: Some(ExpectedVersion::Version( - std::str::from_utf8( - artifacts - .deployed_caboose(&artifacts.sp_gimlet_artifact_hash) - .expect("deployed caboose for generated artifact") - .version() - .expect("valid version"), - ) - .expect("version is UTF-8") - .parse() - .expect("version is valid"), - )), + override_expected_sp_component: ExpectedSpComponent::Sp { + override_expected_active: None, + override_expected_inactive: Some(ExpectedVersion::Version( + std::str::from_utf8( + artifacts + .deployed_caboose( + &artifacts.sp_gimlet_artifact_hash, + ) + .expect("deployed caboose for generated artifact") + .version() + .expect("valid version"), + ) + .expect("version is UTF-8") + .parse() + .expect("version is valid"), + )), + }, // This timeout (10 seconds) seeks to balance being long enough to // be relevant without making the tests take too long. (It's // assumed that 10 seconds here is not a huge deal because this is @@ -870,7 +975,8 @@ mod test { // This time, resume the second update first. It will take over the // first one. let finished2 = in_progress2.finish().await; - finished2.expect_success(UpdateCompletedHow::TookOverConcurrentUpdate); + finished2 + .expect_sp_success(UpdateCompletedHow::TookOverConcurrentUpdate); // Now if we resume the first update, it will find that the SP has been // reset out from under it. This is the closest thing we have to a test @@ -911,8 +1017,10 @@ mod test { part_number: String::from("i86pc"), serial_number: String::from("SimGimlet0"), }), - override_expected_active: None, - override_expected_inactive: None, + override_expected_sp_component: ExpectedSpComponent::Sp { + override_expected_active: None, + override_expected_inactive: None, + }, override_progress_timeout: None, }; @@ -938,8 +1046,10 @@ mod test { slot_id: 1, artifact_hash: &artifacts.sp_gimlet_artifact_hash, override_baseboard_id: None, - override_expected_active: Some("not-right".parse().unwrap()), - override_expected_inactive: None, + override_expected_sp_component: ExpectedSpComponent::Sp { + override_expected_active: Some("not-right".parse().unwrap()), + override_expected_inactive: None, + }, override_progress_timeout: None, }; @@ -965,8 +1075,12 @@ mod test { slot_id: 1, artifact_hash: &artifacts.sp_gimlet_artifact_hash, override_baseboard_id: None, - override_expected_active: None, - override_expected_inactive: Some(ExpectedVersion::NoValidVersion), + override_expected_sp_component: ExpectedSpComponent::Sp { + override_expected_active: None, + override_expected_inactive: Some( + ExpectedVersion::NoValidVersion, + ), + }, override_progress_timeout: None, }; @@ -992,10 +1106,12 @@ mod test { slot_id: 1, artifact_hash: &artifacts.sp_gimlet_artifact_hash, override_baseboard_id: None, - override_expected_active: None, - override_expected_inactive: Some(ExpectedVersion::Version( - "something-else".parse().unwrap(), - )), + override_expected_sp_component: ExpectedSpComponent::Sp { + override_expected_active: None, + override_expected_inactive: Some(ExpectedVersion::Version( + "something-else".parse().unwrap(), + )), + }, override_progress_timeout: None, }; @@ -1022,8 +1138,222 @@ mod test { slot_id: 1, artifact_hash: &artifacts.sp_gimlet_artifact_hash, override_baseboard_id: None, - override_expected_active: None, - override_expected_inactive: None, + override_expected_sp_component: ExpectedSpComponent::Sp { + override_expected_active: None, + override_expected_inactive: None, + }, + override_progress_timeout: None, + }; + let in_progress = desc.setup().await; + artifacts.teardown().await; + in_progress.finish().await.expect_failure(&|error, sp1, sp2| { + assert_matches!(error, ApplyUpdateError::FetchArtifact(..)); + // No changes should have been made in this case. + assert_eq!(sp1, sp2); + }); + + gwtestctx.teardown().await; + } + + /// Tests several RoT update easy fast-failure cases. + #[tokio::test] + async fn test_rot_basic_failures() { + let gwtestctx = gateway_test_utils::setup::test_setup( + "test_rot_basic_failures", + SpPort::One, + ) + .await; + let log = &gwtestctx.logctx.log; + let artifacts = TestArtifacts::new(log).await.unwrap(); + + // Test a case of mistaken identity (reported baseboard does not match + // the one that we expect). + let desc = UpdateDescription { + gwtestctx: &gwtestctx, + artifacts: &artifacts, + sp_type: SpType::Sled, + slot_id: 1, + artifact_hash: &artifacts.sp_gimlet_artifact_hash, + override_baseboard_id: Some(BaseboardId { + part_number: String::from("i86pc"), + serial_number: String::from("SimGimlet0"), + }), + override_expected_sp_component: ExpectedSpComponent::Rot { + override_expected_active_slot: None, + override_expected_inactive_version: None, + override_expected_persistent_boot_preference: None, + override_expected_pending_persistent_boot_preference: None, + override_expected_transient_boot_preference: None, + }, + override_progress_timeout: None, + }; + + desc.setup().await.finish().await.expect_failure(&|error, sp1, sp2| { + assert_matches!(error, ApplyUpdateError::PreconditionFailed(..)); + let message = InlineErrorChain::new(error).to_string(); + eprintln!("{}", message); + assert!(message.contains( + "in sled slot 1, expected to find part \"i86pc\" serial \ + \"SimGimlet0\", but found part \"i86pc\" serial \ + \"SimGimlet01\"", + )); + + // No changes should have been made in this case. + assert_eq!(sp1, sp2); + }); + + // Test a case where the active version doesn't match what we expect. + let desc = UpdateDescription { + gwtestctx: &gwtestctx, + artifacts: &artifacts, + sp_type: SpType::Sled, + slot_id: 1, + artifact_hash: &artifacts.sp_gimlet_artifact_hash, + override_baseboard_id: None, + override_expected_sp_component: ExpectedSpComponent::Rot { + override_expected_active_slot: Some(ExpectedActiveRotSlot { + slot: RotSlot::A, + version: "not-right".parse().unwrap(), + }), + override_expected_inactive_version: None, + override_expected_persistent_boot_preference: None, + override_expected_pending_persistent_boot_preference: None, + override_expected_transient_boot_preference: None, + }, + override_progress_timeout: None, + }; + + desc.setup().await.finish().await.expect_failure(&|error, sp1, sp2| { + assert_matches!(error, ApplyUpdateError::PreconditionFailed(..)); + let message = InlineErrorChain::new(error).to_string(); + eprintln!("{}", message); + assert!(message.contains( + "expected to find active version \"not-right\", but \ + found \"0.0.4\"" + )); + + // No changes should have been made in this case. + assert_eq!(sp1, sp2); + }); + + // Test a case where the active slot doesn't match what we expect. + let desc = UpdateDescription { + gwtestctx: &gwtestctx, + artifacts: &artifacts, + sp_type: SpType::Sled, + slot_id: 1, + artifact_hash: &artifacts.sp_gimlet_artifact_hash, + override_baseboard_id: None, + override_expected_sp_component: ExpectedSpComponent::Rot { + override_expected_active_slot: Some(ExpectedActiveRotSlot { + slot: RotSlot::B, + version: "0.0.4".parse().unwrap(), + }), + override_expected_inactive_version: None, + override_expected_persistent_boot_preference: None, + override_expected_pending_persistent_boot_preference: None, + override_expected_transient_boot_preference: None, + }, + override_progress_timeout: None, + }; + + desc.setup().await.finish().await.expect_failure(&|error, sp1, sp2| { + assert_matches!(error, ApplyUpdateError::PreconditionFailed(..)); + let message = InlineErrorChain::new(error).to_string(); + eprintln!("{}", message); + assert!( + message.contains("expected to find active slot B, but found A") + ); + + // No changes should have been made in this case. + assert_eq!(sp1, sp2); + }); + + // Test a case where the inactive version doesn't match what it should + // (expected invalid, found something else). + let desc = UpdateDescription { + gwtestctx: &gwtestctx, + artifacts: &artifacts, + sp_type: SpType::Sled, + slot_id: 1, + artifact_hash: &artifacts.sp_gimlet_artifact_hash, + override_baseboard_id: None, + override_expected_sp_component: ExpectedSpComponent::Rot { + override_expected_active_slot: None, + override_expected_inactive_version: Some( + ExpectedVersion::NoValidVersion, + ), + override_expected_persistent_boot_preference: None, + override_expected_pending_persistent_boot_preference: None, + override_expected_transient_boot_preference: None, + }, + override_progress_timeout: None, + }; + + desc.setup().await.finish().await.expect_failure(&|error, sp1, sp2| { + assert_matches!(error, ApplyUpdateError::PreconditionFailed(..)); + let message = InlineErrorChain::new(error).to_string(); + eprintln!("{}", message); + assert!(message.contains( + "expected to find inactive version NoValidVersion, \ + but found Version(\"0.0.3\")" + )); + + // No changes should have been made in this case. + assert_eq!(sp1, sp2); + }); + + // Now test a case where the inactive version doesn't match what it + // should (expected a different valid version). + let desc = UpdateDescription { + gwtestctx: &gwtestctx, + artifacts: &artifacts, + sp_type: SpType::Sled, + slot_id: 1, + artifact_hash: &artifacts.sp_gimlet_artifact_hash, + override_baseboard_id: None, + override_expected_sp_component: ExpectedSpComponent::Rot { + override_expected_active_slot: None, + override_expected_inactive_version: Some( + ExpectedVersion::Version("something-else".parse().unwrap()), + ), + override_expected_persistent_boot_preference: None, + override_expected_pending_persistent_boot_preference: None, + override_expected_transient_boot_preference: None, + }, + override_progress_timeout: None, + }; + + desc.setup().await.finish().await.expect_failure(&|error, sp1, sp2| { + assert_matches!(error, ApplyUpdateError::PreconditionFailed(..)); + let message = InlineErrorChain::new(error).to_string(); + eprintln!("{}", message); + assert!(message.contains( + "expected to find inactive version \ + Version(ArtifactVersion(\"something-else\")), but found \ + Version(\"0.0.3\")" + )); + + // No changes should have been made in this case. + assert_eq!(sp1, sp2); + }); + + // Test a case where we fail to fetch the artifact. We simulate this by + // tearing down our artifact server before the update starts. + let desc = UpdateDescription { + gwtestctx: &gwtestctx, + artifacts: &artifacts, + sp_type: SpType::Sled, + slot_id: 1, + artifact_hash: &artifacts.sp_gimlet_artifact_hash, + override_baseboard_id: None, + override_expected_sp_component: ExpectedSpComponent::Rot { + override_expected_active_slot: None, + override_expected_inactive_version: None, + override_expected_persistent_boot_preference: None, + override_expected_pending_persistent_boot_preference: None, + override_expected_transient_boot_preference: None, + }, override_progress_timeout: None, }; let in_progress = desc.setup().await; diff --git a/nexus/mgs-updates/src/rot_updater.rs b/nexus/mgs-updates/src/rot_updater.rs index b36a2a4b0cd..d7c5e7caf40 100644 --- a/nexus/mgs-updates/src/rot_updater.rs +++ b/nexus/mgs-updates/src/rot_updater.rs @@ -270,14 +270,6 @@ impl SpComponentUpdateHelper for ReconfiguratorRotUpdater { }, }; - // If the active slot does not match the expected active slot, there is - // likely another update happening. Bail out. - if expected_active_slot.slot() != active { - return Err(PrecheckError::WrongActiveSlot { - expected: expected_active_slot.slot, found: *active - }) - } - // Fetch the caboose from the currently active slot. let caboose = mgs_clients .try_all_serially(log, move |mgs_client| async move { @@ -286,7 +278,7 @@ impl SpComponentUpdateHelper for ReconfiguratorRotUpdater { update.sp_type, update.slot_id, &SpComponent::ROT.to_string(), - expected_active_slot.slot().to_u16(), + active.to_u16(), ) .await }) @@ -300,6 +292,14 @@ impl SpComponentUpdateHelper for ReconfiguratorRotUpdater { return Ok(PrecheckStatus::UpdateComplete); } + // If the active slot does not match the expected active slot, it is possible + // another update is happening. Bail out. + if expected_active_slot.slot() != active { + return Err(PrecheckError::WrongActiveSlot { + expected: expected_active_slot.slot, found: *active + }) + } + // Otherwise, if the version in the currently active slot does not // match what we expect to find, bail out. It may be that somebody // else has come along and completed a subsequent update and we diff --git a/nexus/mgs-updates/src/test_util/sp_test_state.rs b/nexus/mgs-updates/src/test_util/sp_test_state.rs index 88d43798904..9c9198a4d5c 100644 --- a/nexus/mgs-updates/src/test_util/sp_test_state.rs +++ b/nexus/mgs-updates/src/test_util/sp_test_state.rs @@ -10,6 +10,8 @@ use gateway_client::types::SpComponentCaboose; use gateway_client::types::SpState; use gateway_client::types::SpType; use gateway_messages::RotBootInfo; +use gateway_types::rot::RotSlot; +use nexus_types::deployment::ExpectedActiveRotSlot; use nexus_types::deployment::ExpectedVersion; use nexus_types::inventory::BaseboardId; use slog_error_chain::InlineErrorChain; @@ -132,6 +134,21 @@ impl SpTestState { self.caboose_rot_b.as_ref().expect("ROT slot B caboose") } + pub fn expect_caboose_rot_active(&self) -> &SpComponentCaboose { + match self.expect_rot_active_slot() { + RotSlot::A => self.expect_caboose_rot_a(), + RotSlot::B => self.expect_caboose_rot_b(), + } + } + + pub fn expect_caboose_rot_inactive(&self) -> &SpComponentCaboose { + let slot = self.expect_rot_active_slot().toggled(); + match slot { + RotSlot::A => self.expect_caboose_rot_a(), + RotSlot::B => self.expect_caboose_rot_b(), + } + } + pub fn baseboard_id(&self) -> BaseboardId { BaseboardId { part_number: self.sp_state.model.clone(), @@ -154,6 +171,90 @@ impl SpTestState { None => ExpectedVersion::NoValidVersion, } } + + pub fn expect_rot_state(&self) -> RotState { + self.sp_boot_info.clone() + } + + pub fn expect_rot_active_slot(&self) -> RotSlot { + match self.expect_rot_state() { + RotState::V2 { active, .. } | RotState::V3 { active, .. } => active, + RotState::CommunicationFailed { .. } => panic!("ROT active slot"), + } + } + + pub fn expect_rot_persistent_boot_preference(&self) -> RotSlot { + match self.expect_rot_state() { + RotState::V2 { persistent_boot_preference, .. } + | RotState::V3 { persistent_boot_preference, .. } => { + persistent_boot_preference + } + RotState::CommunicationFailed { .. } => { + panic!("ROT persistent boot preference") + } + } + } + + pub fn expect_rot_pending_persistent_boot_preference( + &self, + ) -> Option { + match self.expect_rot_state() { + RotState::V2 { pending_persistent_boot_preference, .. } + | RotState::V3 { pending_persistent_boot_preference, .. } => { + pending_persistent_boot_preference + } + RotState::CommunicationFailed { .. } => { + panic!("ROT pending persistent boot preference") + } + } + } + + pub fn expect_rot_transient_boot_preference(&self) -> Option { + match self.expect_rot_state() { + RotState::V2 { transient_boot_preference, .. } + | RotState::V3 { transient_boot_preference, .. } => { + transient_boot_preference + } + RotState::CommunicationFailed { .. } => { + panic!("ROT pending persistent boot preference") + } + } + } + + pub fn expected_active_rot_slot(&self) -> ExpectedActiveRotSlot { + let slot = self.expect_rot_active_slot(); + let version = match slot { + RotSlot::A => self + .expect_caboose_rot_a() + .version + .parse() + .expect("valid artifact version"), + RotSlot::B => self + .expect_caboose_rot_b() + .version + .parse() + .expect("valid artifact version"), + }; + ExpectedActiveRotSlot { slot, version } + } + + pub fn expect_rot_inactive_version(&self) -> ExpectedVersion { + let slot = self.expect_rot_active_slot().toggled(); + match slot { + RotSlot::A => match &self.caboose_rot_a { + Some(v) => ExpectedVersion::Version( + v.version.parse().expect("valid SP inactive slot version"), + ), + None => ExpectedVersion::NoValidVersion, + }, + RotSlot::B => match &self.caboose_rot_b { + Some(v) => ExpectedVersion::Version( + v.version.parse().expect("valid SP inactive slot version"), + ), + None => ExpectedVersion::NoValidVersion, + }, + } + } } fn ignore_invalid_caboose_error( diff --git a/nexus/mgs-updates/src/test_util/test_artifacts.rs b/nexus/mgs-updates/src/test_util/test_artifacts.rs index 5e3fac4bcd5..33cb4853009 100644 --- a/nexus/mgs-updates/src/test_util/test_artifacts.rs +++ b/nexus/mgs-updates/src/test_util/test_artifacts.rs @@ -34,6 +34,8 @@ type InMemoryRepoDepotServerContext = Arc; pub struct TestArtifacts { pub sp_gimlet_artifact_hash: ArtifactHash, pub sp_sidecar_artifact_hash: ArtifactHash, + pub rot_gimlet_artifact_hash: ArtifactHash, + pub rot_sidecar_artifact_hash: ArtifactHash, pub artifact_cache: Arc, deployed_cabooses: BTreeMap, resolver: FixedResolver, @@ -74,10 +76,44 @@ impl TestArtifacts { ArtifactHash(digest.finalize().into()) }; + // Make an RoT update artifact for SimGimlet. + let rot_gimlet_artifact_caboose = CabooseBuilder::default() + .git_commit("fake-git-commit") + .board(SIM_GIMLET_BOARD) + .version("0.0.0") + .name("fake-name") + .build(); + let mut builder = HubrisArchiveBuilder::with_fake_image(); + builder.write_caboose(rot_gimlet_artifact_caboose.as_slice()).unwrap(); + let rot_gimlet_artifact = builder.build_to_vec().unwrap(); + let rot_gimlet_artifact_hash = { + let mut digest = sha2::Sha256::default(); + digest.update(&rot_gimlet_artifact); + ArtifactHash(digest.finalize().into()) + }; + + // Make an RoT update artifact for SimSidecar + let rot_sidecar_artifact_caboose = CabooseBuilder::default() + .git_commit("fake-git-commit") + .board(SIM_SIDECAR_BOARD) + .version("0.0.0") + .name("fake-name") + .build(); + let mut builder = HubrisArchiveBuilder::with_fake_image(); + builder.write_caboose(rot_sidecar_artifact_caboose.as_slice()).unwrap(); + let rot_sidecar_artifact = builder.build_to_vec().unwrap(); + let rot_sidecar_artifact_hash = { + let mut digest = sha2::Sha256::default(); + digest.update(&rot_sidecar_artifact); + ArtifactHash(digest.finalize().into()) + }; + // Assemble a map of artifact hash to artifact contents. let artifact_data = [ (sp_gimlet_artifact_hash, sp_gimlet_artifact), (sp_sidecar_artifact_hash, sp_sidecar_artifact), + (rot_gimlet_artifact_hash, rot_gimlet_artifact), + (rot_sidecar_artifact_hash, rot_sidecar_artifact), ] .into_iter() .collect(); @@ -86,6 +122,8 @@ impl TestArtifacts { let deployed_cabooses = [ (sp_gimlet_artifact_hash, sp_gimlet_artifact_caboose), (sp_sidecar_artifact_hash, sp_sidecar_artifact_caboose), + (rot_gimlet_artifact_hash, rot_gimlet_artifact_caboose), + (rot_sidecar_artifact_hash, rot_sidecar_artifact_caboose), ] .into_iter() .collect(); @@ -115,6 +153,8 @@ impl TestArtifacts { Ok(TestArtifacts { sp_gimlet_artifact_hash, sp_sidecar_artifact_hash, + rot_gimlet_artifact_hash, + rot_sidecar_artifact_hash, deployed_cabooses, artifact_cache, resolver, diff --git a/nexus/mgs-updates/src/test_util/updates.rs b/nexus/mgs-updates/src/test_util/updates.rs index 081cdd0c3af..f59986c6315 100644 --- a/nexus/mgs-updates/src/test_util/updates.rs +++ b/nexus/mgs-updates/src/test_util/updates.rs @@ -7,11 +7,14 @@ //! These are factored to make it easy to write a variety of different kinds of //! tests without having to put together too much boilerplate in each test. +use crate::SpComponentUpdateHelper; use crate::driver::UpdateAttemptStatusUpdater; use crate::driver_update::ApplyUpdateError; use crate::driver_update::PROGRESS_TIMEOUT; use crate::driver_update::SpComponentUpdate; use crate::driver_update::apply_update; +use crate::rot_bootloader_updater::ReconfiguratorRotBootloaderUpdater; +use crate::rot_updater::ReconfiguratorRotUpdater; use crate::sp_updater::ReconfiguratorSpUpdater; use crate::test_util::cabooses_equal; use crate::test_util::sp_test_state::SpTestState; @@ -21,6 +24,8 @@ use crate::test_util::test_artifacts::TestArtifacts; use futures::FutureExt; use gateway_client::types::SpType; use gateway_test_utils::setup::GatewayTestContext; +use gateway_types::rot::RotSlot; +use nexus_types::deployment::ExpectedActiveRotSlot; use nexus_types::deployment::ExpectedVersion; use nexus_types::deployment::PendingMgsUpdate; use nexus_types::deployment::PendingMgsUpdateDetails; @@ -37,6 +42,23 @@ use tokio::sync::watch; use tufaceous_artifact::ArtifactHash; use tufaceous_artifact::ArtifactVersion; +pub enum ExpectedSpComponent { + Sp { + override_expected_active: Option, + override_expected_inactive: Option, + }, + Rot { + override_expected_active_slot: Option, + override_expected_inactive_version: Option, + override_expected_persistent_boot_preference: Option, + override_expected_pending_persistent_boot_preference: Option, + override_expected_transient_boot_preference: Option, + }, + // TODO-K: Remove once fully implemented + #[allow(dead_code)] + RotBootloader {}, +} + /// Describes an update operation that can later be executed any number of times pub struct UpdateDescription<'a> { // Execution information @@ -53,9 +75,8 @@ pub struct UpdateDescription<'a> { // If `None`, the correct value is determined automatically. These are // overridable in order to induce specific kinds of failures. pub override_baseboard_id: Option, - pub override_expected_active: Option, - pub override_expected_inactive: Option, pub override_progress_timeout: Option, + pub override_expected_sp_component: ExpectedSpComponent, } impl UpdateDescription<'_> { @@ -77,14 +98,63 @@ impl UpdateDescription<'_> { .clone() .unwrap_or_else(|| sp1.baseboard_id()), ); - let expected_active_version = self - .override_expected_active - .clone() - .unwrap_or_else(|| sp1.expect_sp_active_version()); - let expected_inactive_version = self - .override_expected_inactive - .clone() - .unwrap_or_else(|| sp1.expect_sp_inactive_version()); + + let details = match &self.override_expected_sp_component { + ExpectedSpComponent::Sp { + override_expected_active, + override_expected_inactive, + } => { + let expected_active_version = override_expected_active + .clone() + .unwrap_or_else(|| sp1.expect_sp_active_version()); + let expected_inactive_version = override_expected_inactive + .clone() + .unwrap_or_else(|| sp1.expect_sp_inactive_version()); + + PendingMgsUpdateDetails::Sp { + expected_active_version, + expected_inactive_version, + } + } + ExpectedSpComponent::Rot { + override_expected_active_slot, + override_expected_inactive_version, + override_expected_persistent_boot_preference, + override_expected_pending_persistent_boot_preference, + override_expected_transient_boot_preference, + } => { + let expected_active_slot = override_expected_active_slot + .clone() + .unwrap_or_else(|| sp1.expected_active_rot_slot()); + let expected_inactive_version = + override_expected_inactive_version + .clone() + .unwrap_or_else(|| sp1.expect_rot_inactive_version()); + let expected_persistent_boot_preference = + override_expected_persistent_boot_preference + .unwrap_or_else(|| { + sp1.expect_rot_persistent_boot_preference() + }); + let expected_pending_persistent_boot_preference = + override_expected_pending_persistent_boot_preference + .or_else(|| { + sp1.expect_rot_pending_persistent_boot_preference() + }); + let expected_transient_boot_preference = + override_expected_transient_boot_preference + .or_else(|| sp1.expect_rot_transient_boot_preference()); + + PendingMgsUpdateDetails::Rot { + expected_active_slot, + expected_inactive_version, + expected_persistent_boot_preference, + expected_pending_persistent_boot_preference, + expected_transient_boot_preference, + } + } + ExpectedSpComponent::RotBootloader {} => unimplemented!(), + }; + let deployed_caboose = self .artifacts .deployed_caboose(self.artifact_hash) @@ -96,10 +166,7 @@ impl UpdateDescription<'_> { baseboard_id: baseboard_id.clone(), sp_type: self.sp_type, slot_id: self.slot_id, - details: PendingMgsUpdateDetails::Sp { - expected_active_version, - expected_inactive_version, - }, + details, artifact_hash: *self.artifact_hash, artifact_version: std::str::from_utf8( deployed_caboose.version().unwrap(), @@ -144,7 +211,19 @@ impl UpdateDescription<'_> { &request, update_id, ); - let sp_update_helper = Box::new(ReconfiguratorSpUpdater {}); + let sp_update_helper: Box< + dyn SpComponentUpdateHelper + Send + Sync, + > = match request.details { + PendingMgsUpdateDetails::Sp { .. } => { + Box::new(ReconfiguratorSpUpdater {}) + } + PendingMgsUpdateDetails::Rot { .. } => { + Box::new(ReconfiguratorRotUpdater {}) + } + PendingMgsUpdateDetails::RotBootloader { .. } => { + Box::new(ReconfiguratorRotBootloaderUpdater {}) + } + }; apply_update( artifact_cache, &sp_update, @@ -318,8 +397,8 @@ impl FinishedUpdateAttempt { FinishedUpdateAttempt { result, deployed_caboose, sp1, sp2 } } - /// Asserts various conditions associated with successful updates. - pub fn expect_success(&self, expected_result: UpdateCompletedHow) { + /// Asserts various conditions associated with successful SP updates. + pub fn expect_sp_success(&self, expected_result: UpdateCompletedHow) { let how = match self.result { Ok(how) if how == expected_result => how, _ => { @@ -360,6 +439,59 @@ impl FinishedUpdateAttempt { } } + /// Asserts various conditions associated with successful RoT updates. + pub fn expect_rot_success(&self, expected_result: UpdateCompletedHow) { + let how = match self.result { + Ok(how) if how == expected_result => how, + _ => { + panic!( + "unexpected result from apply_update(): {:?}", + self.result, + ); + } + }; + + eprintln!("apply_update() -> {:?}", how); + let sp2 = &self.sp2; + + // The active slot should contain what we just updated to. + let deployed_caboose = &self.deployed_caboose; + assert!(cabooses_equal( + &sp2.expect_caboose_rot_active(), + &deployed_caboose + )); + + // SP information should not have changed + let sp1 = &self.sp1; + assert_eq!( + sp1.expect_caboose_sp_inactive(), + sp2.expect_caboose_sp_inactive(), + ); + assert_eq!( + sp1.expect_caboose_sp_active(), + sp2.expect_caboose_sp_active(), + ); + + if how == UpdateCompletedHow::FoundNoChangesNeeded { + assert_eq!(sp1.expect_caboose_rot_a(), sp2.expect_caboose_rot_a()); + assert_eq!(sp1.expect_caboose_rot_b(), sp2.expect_caboose_rot_b()); + assert_eq!(sp1.sp_state, sp2.sp_state); + assert_eq!(sp1.sp_boot_info, sp2.sp_boot_info); + } else { + // An update was completed. The active slot should be what was the + // inactive one before and the inactive slot should contain what was + // in the active slot before + assert_eq!( + sp1.expect_rot_active_slot().toggled(), + sp2.expect_rot_active_slot() + ); + assert_eq!( + sp1.expect_caboose_rot_active(), + sp2.expect_caboose_rot_inactive() + ); + } + } + /// Asserts that the update failed and invokes `assert_error(error, /// initial_sp_state, final_sp_state)` for you to make your own assertions /// about why it failed and what state things were left in.