Skip to content

Commit 5b6adf0

Browse files
jgallagherjgallagher
authored
PendingMgsUpdateDetails: use separate structs for each variant (#8848)
This is a small followup from #8737 (comment): use explicit structs for each kind of `...Details`, which allows us to drop some `unreachable!`s.
1 parent 61ad056 commit 5b6adf0

File tree

21 files changed

+507
-404
lines changed

21 files changed

+507
-404
lines changed

‎dev-tools/reconfigurator-cli/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ use nexus_reconfigurator_simulation::{BlueprintId, CollectionId, SimState};
2828
use nexus_reconfigurator_simulation::{SimStateBuilder, SimTufRepoSource};
2929
use nexus_reconfigurator_simulation::{SimTufRepoDescription, Simulator};
3030
use nexus_sled_agent_shared::inventory::ZoneKind;
31-
use nexus_types::deployment::PlanningInput;
3231
use nexus_types::deployment::SledFilter;
3332
use nexus_types::deployment::execution;
3433
use nexus_types::deployment::execution::blueprint_external_dns_config;
@@ -43,6 +42,7 @@ use nexus_types::deployment::{
4342
BlueprintZoneImageSource, PendingMgsUpdateDetails,
4443
};
4544
use nexus_types::deployment::{OmicronZoneNic, TargetReleaseDescription};
45+
use nexus_types::deployment::{PendingMgsUpdateSpDetails, PlanningInput};
4646
use nexus_types::external_api::views::SledPolicy;
4747
use nexus_types::external_api::views::SledProvisionPolicy;
4848
use nexus_types::inventory::CollectionDisplayCliFilter;
@@ -2026,10 +2026,10 @@ fn cmd_blueprint_edit(
20262026
SpUpdateComponent::Sp {
20272027
expected_active_version,
20282028
expected_inactive_version,
2029-
} => PendingMgsUpdateDetails::Sp {
2029+
} => PendingMgsUpdateDetails::Sp(PendingMgsUpdateSpDetails {
20302030
expected_active_version,
20312031
expected_inactive_version,
2032-
},
2032+
}),
20332033
};
20342034

20352035
let artifact_version = ArtifactVersion::new(version)

‎dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -692,11 +692,11 @@ to: blueprint a5a8f242-ffa5-473c-8efd-2acf2dc0b736
692692
PENDING MGS UPDATES:
693693

694694
Pending MGS-managed updates (all baseboards):
695-
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
696-
sp_type slot part_number serial_number artifact_hash artifact_version details
697-
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
698-
- sled 0 model0 serial0 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 1.1.0 Sp { expected_active_version: ArtifactVersion("1.0.0"), expected_inactive_version: Version(ArtifactVersion("1.0.1")) }
699-
- sled 1 model1 serial1 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 newest Sp { expected_active_version: ArtifactVersion("newer"), expected_inactive_version: Version(ArtifactVersion("older")) }
695+
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
696+
sp_type slot part_number serial_number artifact_hash artifact_version details
697+
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
698+
- sled 0 model0 serial0 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 1.1.0 Sp(PendingMgsUpdateSpDetails { expected_active_version: ArtifactVersion("1.0.0"), expected_inactive_version: Version(ArtifactVersion("1.0.1")) })
699+
- sled 1 model1 serial1 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 newest Sp(PendingMgsUpdateSpDetails { expected_active_version: ArtifactVersion("newer"), expected_inactive_version: Version(ArtifactVersion("older")) })
700700

701701

702702
internal DNS:

‎dev-tools/reconfigurator-cli/tests/output/cmds-set-mgs-updates-stdout

Lines changed: 45 additions & 45 deletions
Large diffs are not rendered by default.

‎dev-tools/reconfigurator-cli/tests/output/cmds-target-release-stdout

Lines changed: 76 additions & 76 deletions
Large diffs are not rendered by default.

‎dev-tools/reconfigurator-sp-updater/src/main.rs

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ use nexus_types::deployment::ExpectedVersion;
2222
use nexus_types::deployment::PendingMgsUpdate;
2323
use nexus_types::deployment::PendingMgsUpdateDetails;
2424
use nexus_types::deployment::PendingMgsUpdateHostPhase1Details;
25+
use nexus_types::deployment::PendingMgsUpdateRotBootloaderDetails;
26+
use nexus_types::deployment::PendingMgsUpdateRotDetails;
27+
use nexus_types::deployment::PendingMgsUpdateSpDetails;
2528
use nexus_types::deployment::PendingMgsUpdates;
2629
use nexus_types::internal_api::views::MgsUpdateDriverStatus;
2730
use nexus_types::inventory::BaseboardId;
@@ -320,24 +323,24 @@ fn cmd_config(
320323
update.artifact_version,
321324
);
322325
match &update.details {
323-
PendingMgsUpdateDetails::Sp {
326+
PendingMgsUpdateDetails::Sp(PendingMgsUpdateSpDetails {
324327
expected_active_version,
325328
expected_inactive_version,
326-
} => {
329+
}) => {
327330
swriteln!(
328331
s,
329332
" preconditions: active slot {:?}, inactive slot {:?}",
330333
expected_active_version,
331334
expected_inactive_version,
332335
);
333336
}
334-
PendingMgsUpdateDetails::Rot {
337+
PendingMgsUpdateDetails::Rot(PendingMgsUpdateRotDetails {
335338
expected_active_slot,
336339
expected_inactive_version,
337340
expected_persistent_boot_preference,
338341
expected_pending_persistent_boot_preference,
339342
expected_transient_boot_preference,
340-
} => {
343+
}) => {
341344
swriteln!(
342345
s,
343346
" preconditions: expected active slot {:?}
@@ -352,10 +355,12 @@ fn cmd_config(
352355
expected_transient_boot_preference,
353356
);
354357
}
355-
PendingMgsUpdateDetails::RotBootloader {
356-
expected_stage0_version,
357-
expected_stage0_next_version,
358-
} => {
358+
PendingMgsUpdateDetails::RotBootloader(
359+
PendingMgsUpdateRotBootloaderDetails {
360+
expected_stage0_version,
361+
expected_stage0_next_version,
362+
},
363+
) => {
359364
swriteln!(
360365
s,
361366
" preconditions: stage 0 {:?}, stage 0 next {:?}",
@@ -488,10 +493,10 @@ fn cmd_set(
488493
Component::Sp {
489494
expected_active_version,
490495
expected_inactive_version,
491-
} => PendingMgsUpdateDetails::Sp {
496+
} => PendingMgsUpdateDetails::Sp(PendingMgsUpdateSpDetails {
492497
expected_active_version,
493498
expected_inactive_version,
494-
},
499+
}),
495500
Component::Rot {
496501
expected_active_slot,
497502
expected_slot_a_version,
@@ -519,7 +524,7 @@ fn cmd_set(
519524
}
520525
};
521526

522-
PendingMgsUpdateDetails::Rot {
527+
PendingMgsUpdateDetails::Rot(PendingMgsUpdateRotDetails {
523528
expected_active_slot: ExpectedActiveRotSlot {
524529
slot: expected_active_slot,
525530
version: expected_active_version,
@@ -530,15 +535,17 @@ fn cmd_set(
530535
.unwrap_or(expected_active_slot),
531536
expected_pending_persistent_boot_preference,
532537
expected_transient_boot_preference,
533-
}
538+
})
534539
}
535540
Component::RotBootloader {
536541
expected_stage0_version,
537542
expected_stage0_next_version,
538-
} => PendingMgsUpdateDetails::RotBootloader {
539-
expected_stage0_version,
540-
expected_stage0_next_version,
541-
},
543+
} => PendingMgsUpdateDetails::RotBootloader(
544+
PendingMgsUpdateRotBootloaderDetails {
545+
expected_stage0_version,
546+
expected_stage0_next_version,
547+
},
548+
),
542549
Component::HostPhase1 {
543550
expected_active_phase_1_slot,
544551
expected_boot_disk,

‎nexus/db-model/src/deployment.rs

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ use nexus_types::deployment::CockroachDbPreserveDowngrade;
3939
use nexus_types::deployment::ExpectedActiveRotSlot;
4040
use nexus_types::deployment::PendingMgsUpdate;
4141
use nexus_types::deployment::PendingMgsUpdateDetails;
42+
use nexus_types::deployment::PendingMgsUpdateRotBootloaderDetails;
43+
use nexus_types::deployment::PendingMgsUpdateRotDetails;
44+
use nexus_types::deployment::PendingMgsUpdateSpDetails;
4245
use nexus_types::deployment::{
4346
BlueprintArtifactVersion, BlueprintDatasetConfig, OximeterReadMode,
4447
};
@@ -1339,16 +1342,18 @@ impl BpPendingMgsUpdateComponent for BpPendingMgsUpdateRotBootloader {
13391342
slot_id: **self.sp_slot,
13401343
artifact_hash: self.artifact_sha256.into(),
13411344
artifact_version: (*self.artifact_version).clone(),
1342-
details: PendingMgsUpdateDetails::RotBootloader {
1343-
expected_stage0_version: (*self.expected_stage0_version)
1344-
.clone(),
1345-
expected_stage0_next_version: match self
1346-
.expected_stage0_next_version
1347-
{
1348-
Some(v) => ExpectedVersion::Version((*v).clone()),
1349-
None => ExpectedVersion::NoValidVersion,
1345+
details: PendingMgsUpdateDetails::RotBootloader(
1346+
PendingMgsUpdateRotBootloaderDetails {
1347+
expected_stage0_version: (*self.expected_stage0_version)
1348+
.clone(),
1349+
expected_stage0_next_version: match self
1350+
.expected_stage0_next_version
1351+
{
1352+
Some(v) => ExpectedVersion::Version((*v).clone()),
1353+
None => ExpectedVersion::NoValidVersion,
1354+
},
13501355
},
1351-
},
1356+
),
13521357
}
13531358
}
13541359
}
@@ -1378,15 +1383,15 @@ impl BpPendingMgsUpdateComponent for BpPendingMgsUpdateSp {
13781383
slot_id: **self.sp_slot,
13791384
artifact_hash: self.artifact_sha256.into(),
13801385
artifact_version: (*self.artifact_version).clone(),
1381-
details: PendingMgsUpdateDetails::Sp {
1386+
details: PendingMgsUpdateDetails::Sp(PendingMgsUpdateSpDetails {
13821387
expected_active_version: (*self.expected_active_version)
13831388
.clone(),
13841389
expected_inactive_version: match self.expected_inactive_version
13851390
{
13861391
Some(v) => ExpectedVersion::Version((*v).clone()),
13871392
None => ExpectedVersion::NoValidVersion,
13881393
},
1389-
},
1394+
}),
13901395
}
13911396
}
13921397
}
@@ -1420,7 +1425,7 @@ impl BpPendingMgsUpdateComponent for BpPendingMgsUpdateRot {
14201425
slot_id: **self.sp_slot,
14211426
artifact_hash: self.artifact_sha256.into(),
14221427
artifact_version: (*self.artifact_version).clone(),
1423-
details: PendingMgsUpdateDetails::Rot {
1428+
details: PendingMgsUpdateDetails::Rot(PendingMgsUpdateRotDetails {
14241429
expected_active_slot: ExpectedActiveRotSlot {
14251430
slot: self.expected_active_slot.into(),
14261431
version: (*self.expected_active_version).clone(),
@@ -1438,7 +1443,7 @@ impl BpPendingMgsUpdateComponent for BpPendingMgsUpdateRot {
14381443
expected_transient_boot_preference: self
14391444
.expected_transient_boot_preference
14401445
.map(|s| s.into()),
1441-
},
1446+
}),
14421447
}
14431448
}
14441449
}

‎nexus/db-queries/src/db/datastore/deployment.rs

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ use nexus_types::deployment::OximeterReadMode;
8585
use nexus_types::deployment::PendingMgsUpdate;
8686
use nexus_types::deployment::PendingMgsUpdateDetails;
8787
use nexus_types::deployment::PendingMgsUpdateHostPhase1Details;
88+
use nexus_types::deployment::PendingMgsUpdateRotBootloaderDetails;
89+
use nexus_types::deployment::PendingMgsUpdateRotDetails;
90+
use nexus_types::deployment::PendingMgsUpdateSpDetails;
8891
use nexus_types::deployment::PendingMgsUpdates;
8992
use nexus_types::deployment::PlanningReport;
9093
use nexus_types::inventory::BaseboardId;
@@ -2055,10 +2058,10 @@ async fn insert_pending_mgs_update(
20552058
log: &Logger,
20562059
) -> Result<(), InsertTxnError> {
20572060
match &update.details {
2058-
PendingMgsUpdateDetails::Sp {
2061+
PendingMgsUpdateDetails::Sp(PendingMgsUpdateSpDetails {
20592062
expected_active_version,
20602063
expected_inactive_version,
2061-
} => {
2064+
}) => {
20622065
let db_blueprint_id = DbTypedUuid::from(blueprint_id)
20632066
.into_sql::<diesel::sql_types::Uuid>();
20642067
let db_sp_type =
@@ -2168,13 +2171,13 @@ async fn insert_pending_mgs_update(
21682171
_expected_inactive_version,
21692172
) = update_dsl::bp_pending_mgs_update_sp::all_columns();
21702173
}
2171-
PendingMgsUpdateDetails::Rot {
2174+
PendingMgsUpdateDetails::Rot(PendingMgsUpdateRotDetails {
21722175
expected_active_slot,
21732176
expected_inactive_version,
21742177
expected_persistent_boot_preference,
21752178
expected_pending_persistent_boot_preference,
21762179
expected_transient_boot_preference,
2177-
} => {
2180+
}) => {
21782181
let db_blueprint_id = DbTypedUuid::from(blueprint_id)
21792182
.into_sql::<diesel::sql_types::Uuid>();
21802183
let db_sp_type =
@@ -2298,10 +2301,12 @@ async fn insert_pending_mgs_update(
22982301
_expected_transient_boot_preference,
22992302
) = update_dsl::bp_pending_mgs_update_rot::all_columns();
23002303
}
2301-
PendingMgsUpdateDetails::RotBootloader {
2302-
expected_stage0_version,
2303-
expected_stage0_next_version,
2304-
} => {
2304+
PendingMgsUpdateDetails::RotBootloader(
2305+
PendingMgsUpdateRotBootloaderDetails {
2306+
expected_stage0_version,
2307+
expected_stage0_next_version,
2308+
},
2309+
) => {
23052310
let db_blueprint_id = DbTypedUuid::from(blueprint_id)
23062311
.into_sql::<diesel::sql_types::Uuid>();
23072312
let db_sp_type =
@@ -3578,10 +3583,10 @@ mod tests {
35783583
baseboard_id: baseboard_id.clone(),
35793584
sp_type: sp.sp_type,
35803585
slot_id: sp.sp_slot,
3581-
details: PendingMgsUpdateDetails::Sp {
3586+
details: PendingMgsUpdateDetails::Sp(PendingMgsUpdateSpDetails {
35823587
expected_active_version: "1.0.0".parse().unwrap(),
35833588
expected_inactive_version: ExpectedVersion::NoValidVersion,
3584-
},
3589+
}),
35853590
artifact_hash: ArtifactHash([72; 32]),
35863591
artifact_version: "2.0.0".parse().unwrap(),
35873592
});
@@ -3705,7 +3710,7 @@ mod tests {
37053710
baseboard_id: baseboard_id.clone(),
37063711
sp_type: sp.sp_type,
37073712
slot_id: sp.sp_slot,
3708-
details: PendingMgsUpdateDetails::Rot {
3713+
details: PendingMgsUpdateDetails::Rot(PendingMgsUpdateRotDetails {
37093714
expected_active_slot: ExpectedActiveRotSlot {
37103715
slot: RotSlot::A,
37113716
version: "1.0.0".parse().unwrap(),
@@ -3714,7 +3719,7 @@ mod tests {
37143719
expected_persistent_boot_preference: RotSlot::A,
37153720
expected_pending_persistent_boot_preference: None,
37163721
expected_transient_boot_preference: None,
3717-
},
3722+
}),
37183723
artifact_hash: ArtifactHash([72; 32]),
37193724
artifact_version: "2.0.0".parse().unwrap(),
37203725
});
@@ -3762,10 +3767,13 @@ mod tests {
37623767
baseboard_id: baseboard_id.clone(),
37633768
sp_type: sp.sp_type,
37643769
slot_id: sp.sp_slot,
3765-
details: PendingMgsUpdateDetails::RotBootloader {
3766-
expected_stage0_version: "1.0.0".parse().unwrap(),
3767-
expected_stage0_next_version: ExpectedVersion::NoValidVersion,
3768-
},
3770+
details: PendingMgsUpdateDetails::RotBootloader(
3771+
PendingMgsUpdateRotBootloaderDetails {
3772+
expected_stage0_version: "1.0.0".parse().unwrap(),
3773+
expected_stage0_next_version:
3774+
ExpectedVersion::NoValidVersion,
3775+
},
3776+
),
37693777
artifact_hash: ArtifactHash([72; 32]),
37703778
artifact_version: "2.0.0".parse().unwrap(),
37713779
});

‎nexus/mgs-updates/src/common_sp_update.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -299,15 +299,15 @@ impl SpComponentUpdateHelper {
299299
pub fn new(details: &PendingMgsUpdateDetails) -> Self {
300300
let inner: Box<dyn SpComponentUpdateHelperImpl + Send + Sync> =
301301
match details {
302-
PendingMgsUpdateDetails::Sp { .. } => {
303-
Box::new(ReconfiguratorSpUpdater {})
302+
PendingMgsUpdateDetails::Sp(details) => {
303+
Box::new(ReconfiguratorSpUpdater::new(details.clone()))
304304
}
305-
PendingMgsUpdateDetails::Rot { .. } => {
306-
Box::new(ReconfiguratorRotUpdater {})
307-
}
308-
PendingMgsUpdateDetails::RotBootloader { .. } => {
309-
Box::new(ReconfiguratorRotBootloaderUpdater {})
305+
PendingMgsUpdateDetails::Rot(details) => {
306+
Box::new(ReconfiguratorRotUpdater::new(details.clone()))
310307
}
308+
PendingMgsUpdateDetails::RotBootloader(details) => Box::new(
309+
ReconfiguratorRotBootloaderUpdater::new(details.clone()),
310+
),
311311
PendingMgsUpdateDetails::HostPhase1(details) => Box::new(
312312
ReconfiguratorHostPhase1Updater::new(details.clone()),
313313
),

‎nexus/mgs-updates/src/driver_update.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,15 @@ impl SpComponentUpdate {
109109
firmware_slot: 0,
110110
update_id,
111111
},
112-
PendingMgsUpdateDetails::Rot { expected_active_slot, .. } => {
112+
PendingMgsUpdateDetails::Rot(details) => {
113113
SpComponentUpdate {
114114
log: log.clone(),
115115
component: SpComponent::ROT,
116116
target_sp_type: request.sp_type,
117117
target_sp_slot: request.slot_id,
118118
// Like the SP, we request an update to the inactive slot
119-
firmware_slot: expected_active_slot
119+
firmware_slot: details
120+
.expected_active_slot
120121
.slot()
121122
.toggled()
122123
.to_u16(),
@@ -130,9 +131,9 @@ impl SpComponentUpdate {
130131
target_sp_type: request.sp_type,
131132
target_sp_slot: request.slot_id,
132133
// The RoT bootloader has two firmware slots, stage0 and
133-
// stage0next. We always request an update to stage0next, which
134-
// is the staging area for the bootloader and in this context
135-
// means "the inactive slot".
134+
// stage0next. We always request an update to stage0next,
135+
// which is the staging area for the bootloader and in this
136+
// context means "the inactive slot".
136137
firmware_slot: 1,
137138
update_id,
138139
}

0 commit comments

Comments
 (0)