review comments

Author: andrewjstone

bootstore/src/schemes/v0/storage.rs

@@ -88,35 +88,30 @@ impl PersistentFsmState {
     }
 
     /// Load the ledger for use by trust quorum
-    pub async fn load_for_trust_quorum(
+    pub async fn load_for_trust_quorum_upgrade(
         log: &Logger,
         paths: Vec<Utf8PathBuf>,
     ) -> Option<SharePkgCommon> {
-        if let Some(ledger) =
-            Ledger::<PersistentFsmState>::new(&log, paths).await
-        {
-            let persistent_state = ledger.into_inner();
-            info!(
-                log,
-                "Loaded LRTQ PersistentFsmState from ledger in state {} with generation {}",
-                persistent_state.state.name(),
-                persistent_state.generation
-            );
+        let ledger = Ledger::<PersistentFsmState>::new(&log, paths).await?;
+        let persistent_state = ledger.into_inner();
+        info!(
+            log,
+            "Loaded LRTQ PersistentFsmState from ledger in state {} with generation {}",
+            persistent_state.state.name(),
+            persistent_state.generation
+        );
 
-            match persistent_state.state {
-                State::Uninitialized | State::Learning => {
-                    warn!(
-                        log,
-                        "Unexpected LRTQ state: {}. No share available.",
-                        persistent_state.state.name()
-                    );
-                    None
-                }
-                State::InitialMember { pkg, .. } => Some(pkg.common.clone()),
-                State::Learned { pkg } => Some(pkg.common.clone()),
+        match persistent_state.state {
+            State::Uninitialized | State::Learning => {
+                warn!(
+                    log,
+                    "Unexpected LRTQ state: {}. No share available.",
+                    persistent_state.state.name()
+                );
+                None
             }
-        } else {
-            None
+            State::InitialMember { pkg, .. } => Some(pkg.common.clone()),
+            State::Learned { pkg } => Some(pkg.common.clone()),
         }
     }
 }

dev-tools/omdb/src/bin/omdb/nexus.rs

@@ -4541,10 +4541,8 @@ async fn cmd_nexus_trust_quorum_lrtq_upgrade(
     client: &nexus_lockstep_client::Client,
     _destruction_token: DestructiveOperationToken,
 ) -> Result<(), anyhow::Error> {
-    // We already know the uuid at Nexus. Don't make the user type it.
-    let fake_uuid = Uuid::nil();
     let epoch = client
-        .trust_quorum_lrtq_upgrade(&fake_uuid)
+        .trust_quorum_lrtq_upgrade()
         .await
         .context("lrtq upgrade")?
         .into_inner();

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

@@ -1643,6 +1643,9 @@ mod tests {
         assert!(read_config.members.iter().all(|(_, info)| {
             info.state == TrustQuorumMemberState::Unacked
         }));
+
+        db.terminate().await;
+        logctx.cleanup_successful();
     }
 
     #[tokio::test]

nexus/lockstep-api/src/lib.rs

@@ -569,7 +569,7 @@ pub trait NexusLockstepApi {
     // if no epoch is given
     #[endpoint {
         method = GET,
-        path = "/trust-quorum/{rack_id}/config",
+        path = "/trust-quorum/config/{rack_id}",
     }]
     async fn trust_quorum_get_config(
         rqctx: RequestContext<Self::Context>,
@@ -583,11 +583,10 @@ pub trait NexusLockstepApi {
     /// asynchronously.
     #[endpoint {
         method = POST,
-        path = "/trust-quorum/{rack_id}/lrtq-upgrade"
+        path = "/trust-quorum/lrtq-upgrade"
     }]
     async fn trust_quorum_lrtq_upgrade(
         rqctx: RequestContext<Self::Context>,
-        path_params: Path<RackPathParam>,
     ) -> Result<HttpResponseOk<Epoch>, HttpError>;
 }
 

nexus/src/lockstep_api/http_entrypoints.rs

@@ -1126,7 +1126,6 @@ impl NexusLockstepApi for NexusLockstepApiImpl {
     // know from Nexus.
     async fn trust_quorum_lrtq_upgrade(
         rqctx: RequestContext<Self::Context>,
-        _path_params: Path<RackPathParam>,
     ) -> Result<HttpResponseOk<Epoch>, HttpError> {
         let apictx = &rqctx.context().context;
         let nexus = &apictx.nexus;

openapi/nexus-lockstep.json

@@ -1389,7 +1389,7 @@
         }
       }
     },
-    "/trust-quorum/{rack_id}/config": {
+    "/trust-quorum/config/{rack_id}": {
       "get": {
         "summary": "Retrieve the trust quorum configuration for the given epoch, or latest",
         "operationId": "trust_quorum_get_config",
@@ -1434,22 +1434,11 @@
         }
       }
     },
-    "/trust-quorum/{rack_id}/lrtq-upgrade": {
+    "/trust-quorum/lrtq-upgrade": {
       "post": {
         "summary": "Initiate an LRTQ upgrade",
         "description": "Return the epoch of the proposed configuration, so it can be polled asynchronously.",
         "operationId": "trust_quorum_lrtq_upgrade",
-        "parameters": [
-          {
-            "in": "path",
-            "name": "rack_id",
-            "required": true,
-            "schema": {
-              "type": "string",
-              "format": "uuid"
-            }
-          }
-        ],
         "responses": {
           "200": {
             "description": "successful operation",

trust-quorum/src/task.rs

@@ -448,7 +448,7 @@ impl NodeTask {
                 ps_ledger.generation,
             )
         } else if let Some(lrtq_share_data) =
-            PersistentFsmState::load_for_trust_quorum(
+            PersistentFsmState::load_for_trust_quorum_upgrade(
                 &log,
                 config.lrtq_ledger_paths.clone(),
             )