Rename API to sled_add and take UninitializedSledId as param (#4704)

This is the second half of the fix for #4607

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

@@ -919,7 +919,7 @@ impl DataStore {
     pub async fn find_hw_baseboard_id(
         &self,
         opctx: &OpContext,
-        baseboard_id: BaseboardId,
+        baseboard_id: &BaseboardId,
     ) -> Result<Uuid, Error> {
         opctx.authorize(authz::Action::Read, &authz::INVENTORY).await?;
         let conn = self.pool_connection_authorized(opctx).await?;
@@ -1442,7 +1442,7 @@ mod test {
             part_number: "some-part".into(),
         };
         let err = datastore
-            .find_hw_baseboard_id(&opctx, baseboard_id)
+            .find_hw_baseboard_id(&opctx, &baseboard_id)
             .await
             .unwrap_err();
         assert!(matches!(err, Error::ObjectNotFound { .. }));

nexus/src/app/rack.rs

@@ -31,6 +31,7 @@ use nexus_types::external_api::params::LinkConfig;
 use nexus_types::external_api::params::LldpServiceConfig;
 use nexus_types::external_api::params::RouteConfig;
 use nexus_types::external_api::params::SwitchPortConfig;
+use nexus_types::external_api::params::UninitializedSledId;
 use nexus_types::external_api::params::{
     AddressLotCreate, BgpPeerConfig, LoopbackAddressCreate, Route, SiloCreate,
     SwitchPortSettingsCreate,
@@ -51,6 +52,7 @@ use omicron_common::api::external::ListResultVec;
 use omicron_common::api::external::LookupResult;
 use omicron_common::api::external::Name;
 use omicron_common::api::external::NameOrId;
+use omicron_common::api::external::ResourceType;
 use omicron_common::api::internal::shared::ExternalPortDiscovery;
 use sled_agent_client::types::AddSledRequest;
 use sled_agent_client::types::EarlyNetworkConfigBody;
@@ -69,6 +71,19 @@ use std::num::NonZeroU32;
 use std::str::FromStr;
 use uuid::Uuid;
 
+// A limit for querying the last inventory collection
+//
+// We set a limit of 200 here to give us some breathing room when
+// querying for cabooses and RoT pages, each of which is "4 per SP/RoT",
+// which in a single fully populated rack works out to (32 sleds + 2
+// switches + 1 psc) * 4 = 140.
+//
+// This feels bad and probably needs more thought; see
+// https://github.com/oxidecomputer/omicron/issues/4621 where this limit
+// being too low bit us, and it will link to a more general followup
+// issue.
+const INVENTORY_COLLECTION_LIMIT: u32 = 200;
+
 impl super::Nexus {
     pub(crate) async fn racks_list(
         &self,
@@ -872,17 +887,7 @@ impl super::Nexus {
     ) -> ListResultVec<UninitializedSled> {
         debug!(self.log, "Getting latest collection");
         // Grab the SPs from the last collection
-        //
-        // We set a limit of 200 here to give us some breathing room when
-        // querying for cabooses and RoT pages, each of which is "4 per SP/RoT",
-        // which in a single fully populated rack works out to (32 sleds + 2
-        // switches + 1 psc) * 4 = 140.
-        //
-        // This feels bad and probably needs more thought; see
-        // https://github.com/oxidecomputer/omicron/issues/4621 where this limit
-        // being too low bit us, and it will link to a more general followup
-        // issue.
-        let limit = NonZeroU32::new(200).unwrap();
+        let limit = NonZeroU32::new(INVENTORY_COLLECTION_LIMIT).unwrap();
         let collection = self
             .db_datastore
             .inventory_get_latest_collection(opctx, limit)
@@ -933,33 +938,58 @@ impl super::Nexus {
     }
 
     /// Add a sled to an intialized rack
-    pub(crate) async fn add_sled_to_initialized_rack(
+    pub(crate) async fn sled_add(
         &self,
         opctx: &OpContext,
-        sled: UninitializedSled,
+        sled: UninitializedSledId,
     ) -> Result<(), Error> {
-        let baseboard_id = sled.baseboard.clone().into();
-        let hw_baseboard_id =
-            self.db_datastore.find_hw_baseboard_id(opctx, baseboard_id).await?;
+        let baseboard_id = sled.clone().into();
+        let hw_baseboard_id = self
+            .db_datastore
+            .find_hw_baseboard_id(opctx, &baseboard_id)
+            .await?;
 
-        let subnet = self.db_datastore.rack_subnet(opctx, sled.rack_id).await?;
+        let subnet = self.db_datastore.rack_subnet(opctx, self.rack_id).await?;
         let rack_subnet =
             Ipv6Subnet::<RACK_PREFIX>::from(rack_subnet(Some(subnet))?);
 
         let allocation = self
             .db_datastore
             .allocate_sled_underlay_subnet_octets(
                 opctx,
-                sled.rack_id,
+                self.rack_id,
                 hw_baseboard_id,
             )
             .await?;
 
+        // Grab the SPs from the last collection
+        let limit = NonZeroU32::new(INVENTORY_COLLECTION_LIMIT).unwrap();
+        let collection = self
+            .db_datastore
+            .inventory_get_latest_collection(opctx, limit)
+            .await?;
+
+        // If there isn't a collection, we don't know about the sled
+        let Some(collection) = collection else {
+            return Err(Error::unavail("no inventory data available"));
+        };
+
+        // Find the revision
+        let Some(sp) = collection.sps.get(&baseboard_id) else {
+            return Err(Error::ObjectNotFound {
+                type_name: ResourceType::Sled,
+                lookup_type:
+                    omicron_common::api::external::LookupType::ByCompositeId(
+                        format!("{sled:?}"),
+                    ),
+            });
+        };
+
         // Convert the baseboard as necessary
         let baseboard = sled_agent_client::types::Baseboard::Gimlet {
-            identifier: sled.baseboard.serial.clone(),
-            model: sled.baseboard.part.clone(),
-            revision: sled.baseboard.revision,
+            identifier: sled.serial.clone(),
+            model: sled.part.clone(),
+            revision: sp.baseboard_revision.into(),
         };
 
         // Make the call to sled-agent
@@ -985,13 +1015,11 @@ impl super::Nexus {
             },
         };
         let sa = self.get_any_sled_agent(opctx).await?;
-        sa.add_sled_to_initialized_rack(&req).await.map_err(|e| {
-            Error::InternalError {
-                internal_message: format!(
-                    "failed to add sled with baseboard {:?} to rack {}: {e}",
-                    sled.baseboard, allocation.rack_id
-                ),
-            }
+        sa.sled_add(&req).await.map_err(|e| Error::InternalError {
+            internal_message: format!(
+                "failed to add sled with baseboard {:?} to rack {}: {e}",
+                sled, allocation.rack_id
+            ),
         })?;
 
         Ok(())

nexus/src/external_api/http_entrypoints.rs

@@ -6,7 +6,7 @@
 
 use super::{
     console_api, device_auth, params,
-    params::ProjectSelector,
+    params::{ProjectSelector, UninitializedSledId},
     shared::UninitializedSled,
     views::{
         self, Certificate, Group, IdentityProvider, Image, IpPool, IpPoolRange,
@@ -228,7 +228,7 @@ pub(crate) fn external_api() -> NexusApiDescription {
         api.register(switch_list)?;
         api.register(switch_view)?;
         api.register(sled_list_uninitialized)?;
-        api.register(add_sled_to_initialized_rack)?;
+        api.register(sled_add)?;
 
         api.register(user_builtin_list)?;
         api.register(user_builtin_view)?;
@@ -4689,15 +4689,15 @@ async fn sled_list_uninitialized(
     path = "/v1/system/hardware/sleds/",
     tags = ["system/hardware"]
 }]
-async fn add_sled_to_initialized_rack(
+async fn sled_add(
     rqctx: RequestContext<Arc<ServerContext>>,
-    sled: TypedBody<UninitializedSled>,
+    sled: TypedBody<UninitializedSledId>,
 ) -> Result<HttpResponseUpdatedNoContent, HttpError> {
     let apictx = rqctx.context();
     let nexus = &apictx.nexus;
     let handler = async {
         let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
-        nexus.add_sled_to_initialized_rack(&opctx, sled.into_inner()).await?;
+        nexus.sled_add(&opctx, sled.into_inner()).await?;
         Ok(HttpResponseUpdatedNoContent())
     };
     apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await

nexus/tests/integration_tests/endpoints.rs

@@ -20,10 +20,8 @@ use nexus_test_utils::SLED_AGENT_UUID;
 use nexus_test_utils::SWITCH_UUID;
 use nexus_types::external_api::params;
 use nexus_types::external_api::shared;
-use nexus_types::external_api::shared::Baseboard;
 use nexus_types::external_api::shared::IpRange;
 use nexus_types::external_api::shared::Ipv4Range;
-use nexus_types::external_api::shared::UninitializedSled;
 use omicron_common::api::external::AddressLotKind;
 use omicron_common::api::external::ByteCount;
 use omicron_common::api::external::IdentityMetadataCreateParams;
@@ -41,7 +39,6 @@ use once_cell::sync::Lazy;
 use std::net::IpAddr;
 use std::net::Ipv4Addr;
 use std::str::FromStr;
-use uuid::Uuid;
 
 pub static HARDWARE_RACK_URL: Lazy<String> =
     Lazy::new(|| format!("/v1/system/hardware/racks/{}", RACK_UUID));
@@ -69,15 +66,10 @@ pub static HARDWARE_SLED_DISK_URL: Lazy<String> = Lazy::new(|| {
 pub static SLED_INSTANCES_URL: Lazy<String> = Lazy::new(|| {
     format!("/v1/system/hardware/sleds/{}/instances", SLED_AGENT_UUID)
 });
-pub static DEMO_UNINITIALIZED_SLED: Lazy<UninitializedSled> =
-    Lazy::new(|| UninitializedSled {
-        baseboard: Baseboard {
-            serial: "demo-serial".to_string(),
-            part: "demo-part".to_string(),
-            revision: 6,
-        },
-        rack_id: Uuid::new_v4(),
-        cubby: 1,
+pub static DEMO_UNINITIALIZED_SLED: Lazy<params::UninitializedSledId> =
+    Lazy::new(|| params::UninitializedSledId {
+        serial: "demo-serial".to_string(),
+        part: "demo-part".to_string(),
     });
 
 // Global policy

nexus/tests/output/nexus_tags.txt

@@ -117,13 +117,13 @@ snapshot_view                            GET      /v1/snapshots/{snapshot}
 
 API operations found with tag "system/hardware"
 OPERATION ID                             METHOD   URL PATH
-add_sled_to_initialized_rack             POST     /v1/system/hardware/sleds
 networking_switch_port_apply_settings    POST     /v1/system/hardware/switch-port/{port}/settings
 networking_switch_port_clear_settings    DELETE   /v1/system/hardware/switch-port/{port}/settings
 networking_switch_port_list              GET      /v1/system/hardware/switch-port
 physical_disk_list                       GET      /v1/system/hardware/disks
 rack_list                                GET      /v1/system/hardware/racks
 rack_view                                GET      /v1/system/hardware/racks/{rack_id}
+sled_add                                 POST     /v1/system/hardware/sleds
 sled_instance_list                       GET      /v1/system/hardware/sleds/{sled_id}/instances
 sled_list                                GET      /v1/system/hardware/sleds
 sled_list_uninitialized                  GET      /v1/system/hardware/sleds-uninitialized

nexus/types/src/external_api/params.rs

@@ -47,6 +47,23 @@ macro_rules! id_path_param {
     };
 }
 
+/// The unique hardware ID for a sled
+#[derive(
+    Clone,
+    Debug,
+    Serialize,
+    Deserialize,
+    JsonSchema,
+    PartialOrd,
+    Ord,
+    PartialEq,
+    Eq,
+)]
+pub struct UninitializedSledId {
+    pub serial: String,
+    pub part: String,
+}
+
 path_param!(ProjectPath, project, "project");
 path_param!(InstancePath, instance, "instance");
 path_param!(NetworkInterfacePath, interface, "network interface");

nexus/types/src/inventory.rs

@@ -20,6 +20,7 @@ use std::sync::Arc;
 use strum::EnumIter;
 use uuid::Uuid;
 
+use crate::external_api::params::UninitializedSledId;
 use crate::external_api::shared::Baseboard;
 
 /// Results of collecting hardware/software inventory from various Omicron
@@ -139,6 +140,12 @@ impl From<Baseboard> for BaseboardId {
     }
 }
 
+impl From<UninitializedSledId> for BaseboardId {
+    fn from(value: UninitializedSledId) -> Self {
+        BaseboardId { part_number: value.part, serial_number: value.serial }
+    }
+}
+
 /// Caboose contents found during a collection
 ///
 /// These are normalized in the database.  Each distinct `Caboose` is assigned a

openapi/nexus.json

@@ -3765,12 +3765,12 @@
           "system/hardware"
         ],
         "summary": "Add a sled to an initialized rack",
-        "operationId": "add_sled_to_initialized_rack",
+        "operationId": "sled_add",
         "requestBody": {
           "content": {
             "application/json": {
               "schema": {
-                "$ref": "#/components/schemas/UninitializedSled"
+                "$ref": "#/components/schemas/UninitializedSledId"
               }
             }
           },
@@ -14909,6 +14909,22 @@
           "rack_id"
         ]
       },
+      "UninitializedSledId": {
+        "description": "The unique hardware ID for a sled",
+        "type": "object",
+        "properties": {
+          "part": {
+            "type": "string"
+          },
+          "serial": {
+            "type": "string"
+          }
+        },
+        "required": [
+          "part",
+          "serial"
+        ]
+      },
       "UninitializedSledResultsPage": {
         "description": "A single page of results",
         "type": "object",

openapi/sled-agent.json

@@ -576,7 +576,7 @@
     "/sleds": {
       "put": {
         "summary": "Add a sled to a rack that was already initialized via RSS",
-        "operationId": "add_sled_to_initialized_rack",
+        "operationId": "sled_add",
         "requestBody": {
           "content": {
             "application/json": {

sled-agent/src/http_entrypoints.rs

@@ -77,7 +77,7 @@ pub fn api() -> SledApiDescription {
         api.register(uplink_ensure)?;
         api.register(read_network_bootstore_config_cache)?;
         api.register(write_network_bootstore_config)?;
-        api.register(add_sled_to_initialized_rack)?;
+        api.register(sled_add)?;
         api.register(metrics_collect)?;
         api.register(host_os_write_start)?;
         api.register(host_os_write_status_get)?;
@@ -713,7 +713,7 @@ async fn write_network_bootstore_config(
     method = PUT,
     path = "/sleds"
 }]
-async fn add_sled_to_initialized_rack(
+async fn sled_add(
     rqctx: RequestContext<SledAgent>,
     body: TypedBody<AddSledRequest>,
 ) -> Result<HttpResponseUpdatedNoContent, HttpError> {
@@ -731,7 +731,7 @@ async fn add_sled_to_initialized_rack(
         ));
     }
 
-    crate::sled_agent::add_sled_to_initialized_rack(
+    crate::sled_agent::sled_add(
         sa.logger().clone(),
         request.sled_id,
         request.start_request,

sled-agent/src/sled_agent.rs

@@ -1105,7 +1105,7 @@ pub enum AddSledError {
 }
 
 /// Add a sled to an initialized rack.
-pub async fn add_sled_to_initialized_rack(
+pub async fn sled_add(
     log: Logger,
     sled_id: Baseboard,
     request: StartSledAgentRequest,