Skip to content

Commit 7a132fe

Browse files
andrewjstoneandrewjstone
committed
more review fixes
1 parent 8fe7c9a commit 7a132fe

File tree

1 file changed

+68
-165
lines changed

1 file changed

+68
-165
lines changed

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

Lines changed: 68 additions & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ use omicron_common::api::external::Error;
2626
use omicron_common::api::external::ListResultVec;
2727
use omicron_common::api::external::OptionalLookupResult;
2828
use omicron_common::bail_unless;
29-
use omicron_uuid_kinds::GenericUuid;
3029
use omicron_uuid_kinds::RackKind;
3130
use omicron_uuid_kinds::RackUuid;
3231
use sled_agent_types::sled::BaseboardId;
@@ -392,9 +391,10 @@ impl DataStore {
392391
let acked_prepares = acked_prepares.clone();
393392
async move {
394393
// First, retrieve our configuration if there is one.
395-
let latest = Self::tq_get_latest_config_conn(opctx, &c, rack_id)
396-
.await
397-
.map_err(|txn_error| txn_error.into_diesel(&err))?;
394+
let latest =
395+
Self::tq_get_latest_config_conn(opctx, &c, rack_id)
396+
.await
397+
.map_err(|txn_error| txn_error.into_diesel(&err))?;
398398

399399
let Some(db_config) = latest else {
400400
bail_txn!(
@@ -437,26 +437,16 @@ impl DataStore {
437437

438438
// Then get any members associated with the configuration
439439
let db_members = Self::tq_get_members_conn(
440-
opctx,
441-
&c,
442-
rack_id,
443-
db_config.epoch,
444-
)
445-
.await
446-
.map_err(|txn_error| txn_error.into_diesel(&err))?;
447-
448-
// We only update the configuration in the database if:
449-
// 1. This is the first time we have seen encrypted rack secrets
450-
// 2. We are transitioning from preparing to committed state.
451-
let should_write_secrets =
452-
db_config.encrypted_rack_secrets_salt.is_none()
453-
&& config.encrypted_rack_secrets.is_some();
440+
opctx,
441+
&c,
442+
rack_id,
443+
db_config.epoch,
444+
)
445+
.await
446+
.map_err(|txn_error| txn_error.into_diesel(&err))?;
454447

455448
let mut total_acks = 0;
456449
for (mut member, hw_id) in db_members {
457-
let mut update_share_digest = false;
458-
let mut update_prepared = false;
459-
460450
let baseboard_id: BaseboardId = hw_id.into();
461451

462452
// Set the share digest for the member if we just learned it
@@ -471,99 +461,68 @@ impl DataStore {
471461
baseboard_id
472462
);
473463
};
474-
member.share_digest = Some(hex::encode(digest.0));
475-
update_share_digest = true;
464+
member.share_digest = Some(hex::encode(digest.0));
465+
Self::update_tq_member_share_digest_in_txn(
466+
opctx,
467+
conn,
468+
member.clone(),
469+
)
470+
.await
471+
.map_err(|txn_error| txn_error.into_diesel(&err))?;
476472
}
477473

478474
// Set the state of this member
479475
if acked_prepares.contains(&baseboard_id)
480476
&& member.state == DbTrustQuorumMemberState::Unacked
481477
{
482-
update_prepared = true;
483478
total_acks += 1;
479+
Self::update_tq_member_state_prepared_in_txn(
480+
opctx,
481+
conn,
482+
member.clone(),
483+
)
484+
.await
485+
.map_err(|txn_error| txn_error.into_diesel(&err))?;
484486
}
485487

486488
if member.state == DbTrustQuorumMemberState::Prepared {
487489
total_acks += 1;
488490
}
489-
490-
// Write each member that has been modified
491-
match (update_share_digest, update_prepared) {
492-
(true, true) => {
493-
Self::update_tq_member_share_digest_and_state_prepared_in_txn(
494-
opctx,
495-
conn,
496-
member
497-
)
498-
.await
499-
.map_err(|txn_error| txn_error.into_diesel(&err))?;
500-
}
501-
(true, false) => {
502-
Self::update_tq_member_share_digest_in_txn(
503-
opctx,
504-
conn,
505-
member
506-
)
507-
.await
508-
.map_err(|txn_error| txn_error.into_diesel(&err))?;
509-
}
510-
(false, true) => {
511-
Self::update_tq_member_state_prepared_in_txn(
512-
opctx,
513-
conn,
514-
member
515-
)
516-
.await
517-
.map_err(|txn_error| txn_error.into_diesel(&err))?;
518-
}
519-
(false, false) => {
520-
// Nothing to do
521-
}
522-
}
523491
}
524492

493+
// We only update the configuration in the database if:
494+
// 1. This is the first time we have seen encrypted rack secrets
495+
// 2. We are transitioning from preparing to committed state.
496+
497+
// Should we write secrets?
498+
if db_config.encrypted_rack_secrets_salt.is_none()
499+
&& config.encrypted_rack_secrets.is_some()
500+
{
501+
Self::update_tq_encrypted_rack_secrets_in_txn(
502+
opctx,
503+
conn,
504+
db_config.rack_id,
505+
db_config.epoch,
506+
config.encrypted_rack_secrets.unwrap(),
507+
)
508+
.await
509+
.map_err(|txn_error| txn_error.into_diesel(&err))?;
510+
}
525511

526512
// Do we have enough acks to commit?
527-
let should_commit = total_acks
513+
if total_acks
528514
>= (db_config.threshold.0
529515
+ db_config.commit_crash_tolerance.0)
530-
as usize;
531-
532-
match (should_write_secrets, should_commit) {
533-
(true, true) => {
534-
Self::update_tq_encrypted_rack_secrets_and_commit_in_txn(
535-
opctx,
536-
conn,
537-
db_config.rack_id,
538-
db_config.epoch,
539-
config.encrypted_rack_secrets.unwrap(),
540-
)
541-
.await
542-
.map_err(|txn_error| txn_error.into_diesel(&err))?;
543-
}
544-
(true, false) => {
545-
Self::update_tq_encrypted_rack_secrets_in_txn(
546-
opctx,
547-
conn,
548-
db_config.rack_id,
549-
db_config.epoch,
550-
config.encrypted_rack_secrets.unwrap(),
551-
)
552-
.await
553-
.map_err(|txn_error| txn_error.into_diesel(&err))?;
554-
}
555-
(false, true) => {
556-
Self::update_tq_commit_state_in_txn(
557-
opctx,
558-
conn,
559-
db_config.rack_id,
560-
db_config.epoch)
561-
.await
562-
.map_err(|txn_error| txn_error.into_diesel(&err))?;
563-
}
564-
(false, false) => {
565-
// Nothing to do
566-
}
516+
as usize
517+
{
518+
Self::update_tq_state_committing_in_txn(
519+
opctx,
520+
conn,
521+
db_config.rack_id,
522+
db_config.epoch,
523+
)
524+
.await
525+
.map_err(|txn_error| txn_error.into_diesel(&err))?;
567526
}
568527

569528
Ok(())
@@ -821,30 +780,6 @@ impl DataStore {
821780
Ok(())
822781
}
823782

824-
async fn update_tq_member_share_digest_and_state_prepared_in_txn(
825-
opctx: &OpContext,
826-
conn: &async_bb8_diesel::Connection<DbConnection>,
827-
member: DbTrustQuorumMember,
828-
) -> Result<(), TransactionError<Error>> {
829-
opctx.authorize(authz::Action::Modify, &authz::FLEET).await?;
830-
use nexus_db_schema::schema::trust_quorum_member::dsl;
831-
832-
diesel::update(dsl::trust_quorum_member)
833-
.filter(dsl::rack_id.eq(member.rack_id))
834-
.filter(dsl::epoch.eq(member.epoch))
835-
.filter(dsl::hw_baseboard_id.eq(member.hw_baseboard_id))
836-
.filter(dsl::share_digest.is_null())
837-
.filter(dsl::state.eq(DbTrustQuorumMemberState::Unacked))
838-
.set((
839-
dsl::share_digest.eq(member.share_digest),
840-
dsl::state.eq(DbTrustQuorumMemberState::Prepared),
841-
))
842-
.execute_async(conn)
843-
.await?;
844-
845-
Ok(())
846-
}
847-
848783
async fn update_tq_members_state_commit_in_txn(
849784
opctx: &OpContext,
850785
conn: &async_bb8_diesel::Connection<DbConnection>,
@@ -950,41 +885,8 @@ impl DataStore {
950885
Ok(())
951886
}
952887

953-
async fn update_tq_encrypted_rack_secrets_and_commit_in_txn(
954-
opctx: &OpContext,
955-
conn: &async_bb8_diesel::Connection<DbConnection>,
956-
rack_id: DbTypedUuid<RackKind>,
957-
epoch: i64,
958-
encrypted_rack_secrets: EncryptedRackSecrets,
959-
) -> Result<(), TransactionError<Error>> {
960-
opctx.authorize(authz::Action::Modify, &authz::FLEET).await?;
961-
let salt = Some(hex::encode(encrypted_rack_secrets.salt.0));
962-
let secrets: Option<Vec<u8>> = Some(encrypted_rack_secrets.data.into());
963-
964-
use nexus_db_schema::schema::trust_quorum_configuration::dsl;
965-
966-
diesel::update(dsl::trust_quorum_configuration)
967-
.filter(dsl::rack_id.eq(rack_id))
968-
.filter(dsl::epoch.eq(epoch))
969-
.filter(dsl::encrypted_rack_secrets_salt.is_null())
970-
.filter(dsl::encrypted_rack_secrets.is_null())
971-
.filter(dsl::state.eq_any(vec![
972-
DbTrustQuorumConfigurationState::Preparing,
973-
DbTrustQuorumConfigurationState::PreparingLrtqUpgrade,
974-
]))
975-
.set((
976-
dsl::encrypted_rack_secrets_salt.eq(salt),
977-
dsl::encrypted_rack_secrets.eq(secrets),
978-
dsl::state.eq(DbTrustQuorumConfigurationState::Committing),
979-
))
980-
.execute_async(conn)
981-
.await?;
982-
983-
Ok(())
984-
}
985-
986888
/// Returns the number of rows update
987-
async fn update_tq_commit_state_in_txn(
889+
async fn update_tq_state_committing_in_txn(
988890
opctx: &OpContext,
989891
conn: &async_bb8_diesel::Connection<DbConnection>,
990892
rack_id: DbTypedUuid<RackKind>,
@@ -1060,7 +962,7 @@ impl DataStore {
1060962
opctx.authorize(authz::Action::Read, &authz::FLEET).await?;
1061963
use nexus_db_schema::schema::trust_quorum_configuration::dsl;
1062964
let Some(latest_epoch) = dsl::trust_quorum_configuration
1063-
.filter(dsl::rack_id.eq(rack_id.into_untyped_uuid()))
965+
.filter(dsl::rack_id.eq(DbTypedUuid::<RackKind>::from(rack_id)))
1064966
.order_by(dsl::epoch.desc())
1065967
.select(dsl::epoch)
1066968
.first_async::<i64>(conn)
@@ -1083,7 +985,7 @@ impl DataStore {
1083985
use nexus_db_schema::schema::trust_quorum_configuration::dsl;
1084986

1085987
let latest = dsl::trust_quorum_configuration
1086-
.filter(dsl::rack_id.eq(rack_id.into_untyped_uuid()))
988+
.filter(dsl::rack_id.eq(DbTypedUuid::<RackKind>::from(rack_id)))
1087989
.order_by(dsl::epoch.desc())
1088990
.first_async::<DbTrustQuorumConfiguration>(conn)
1089991
.await
@@ -1107,7 +1009,7 @@ impl DataStore {
11071009
use nexus_db_schema::schema::trust_quorum_member::dsl;
11081010

11091011
let members = dsl::trust_quorum_member
1110-
.filter(dsl::rack_id.eq(rack_id.into_untyped_uuid()))
1012+
.filter(dsl::rack_id.eq(DbTypedUuid::<RackKind>::from(rack_id)))
11111013
.filter(dsl::epoch.eq(epoch))
11121014
.inner_join(
11131015
hw_baseboard_id_dsl::hw_baseboard_id
@@ -1549,14 +1451,15 @@ mod tests {
15491451
// (This is not directly callable from a public API).
15501452
{
15511453
let conn = datastore.pool_connection_for_tests().await.unwrap();
1552-
let num_rows_updated = DataStore::update_tq_commit_state_in_txn(
1553-
opctx,
1554-
&conn,
1555-
config.rack_id.into(),
1556-
config.epoch.0 as i64,
1557-
)
1558-
.await
1559-
.unwrap();
1454+
let num_rows_updated =
1455+
DataStore::update_tq_state_committing_in_txn(
1456+
opctx,
1457+
&conn,
1458+
config.rack_id.into(),
1459+
config.epoch.0 as i64,
1460+
)
1461+
.await
1462+
.unwrap();
15601463
assert_eq!(num_rows_updated, 0);
15611464
}
15621465

0 commit comments

Comments
 (0)