From edc851b4a7a45c32370a3f5d43fadb1d288109fd Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Thu, 2 Apr 2026 22:06:40 +0000 Subject: [PATCH 01/26] Add support for re-adopting physical disks This change implements the determinations in RFD 693. It allows re-adopting physical disks in the control plane after the control plane level disk in the `physical_disk` table is expunged. It does this by forcing manual adoption of disks by an operator, where requests are placed in the `physical_disk_adoption_request` table. A disk will now only be adopted or re-adopted by the disk adoption background task if its physical vendor/model/serial information is present in a `physical_disk_adoption_request` row. The typical flow for an operator is to list unitialized disks and then issue an adoption request via the external API. --- nexus/db-model/src/physical_disk.rs | 37 ++ .../src/db/datastore/physical_disk.rs | 442 +++++++++++++++++- nexus/db-schema/src/schema.rs | 12 + nexus/external-api/output/nexus_tags.txt | 3 + nexus/external-api/src/lib.rs | 47 ++ .../tasks/physical_disk_adoption.rs | 8 +- nexus/src/app/sled.rs | 32 ++ nexus/src/external_api/http_entrypoints.rs | 93 +++- nexus/tests/integration_tests/endpoints.rs | 35 +- nexus/types/versions/src/latest.rs | 5 + nexus/types/versions/src/lib.rs | 2 + .../versions/src/manual_disk_adoption/mod.rs | 10 + .../src/manual_disk_adoption/physical_disk.rs | 45 ++ .../nexus-2026032500.0.0-cf1221.json.gitstub | 1 + ....json => nexus-2026040200.0.0-430bef.json} | 273 ++++++++++- openapi/nexus/nexus-latest.json | 2 +- .../crdb/add-disk-adoption-requests/up01.sql | 8 + .../crdb/add-disk-adoption-requests/up02.sql | 6 + schema/crdb/dbinit.sql | 19 + 19 files changed, 1064 insertions(+), 16 deletions(-) create mode 100644 nexus/types/versions/src/manual_disk_adoption/mod.rs create mode 100644 nexus/types/versions/src/manual_disk_adoption/physical_disk.rs create mode 100644 openapi/nexus/nexus-2026032500.0.0-cf1221.json.gitstub rename openapi/nexus/{nexus-2026032500.0.0-cf1221.json => nexus-2026040200.0.0-430bef.json} (99%) create mode 100644 schema/crdb/add-disk-adoption-requests/up01.sql create mode 100644 schema/crdb/add-disk-adoption-requests/up02.sql diff --git a/nexus/db-model/src/physical_disk.rs b/nexus/db-model/src/physical_disk.rs index 5f52f151a99..da014329371 100644 --- a/nexus/db-model/src/physical_disk.rs +++ b/nexus/db-model/src/physical_disk.rs @@ -74,6 +74,16 @@ impl PhysicalDisk { } } +impl From<&PhysicalDisk> for physical_disk_types::PhysicalDiskId { + fn from(value: &PhysicalDisk) -> Self { + Self { + vendor: value.vendor.clone(), + serial: value.serial.clone(), + model: value.model.clone(), + } + } +} + impl From for physical_disk_types::PhysicalDisk { fn from(disk: PhysicalDisk) -> Self { Self { @@ -89,6 +99,33 @@ impl From for physical_disk_types::PhysicalDisk { } } +/// A request to adopt a physical disk into the control plane. +#[derive(Queryable, Insertable, Debug, Clone, Selectable)] +#[diesel(table_name = nexus_db_schema::schema::physical_disk_adoption_request)] +pub struct PhysicalDiskAdoptionRequest { + pub id: uuid::Uuid, + pub vendor: String, + pub serial: String, + pub model: String, + pub time_created: DateTime, + pub time_deleted: Option>, +} + +impl From + for physical_disk_types::PhysicalDiskAdoptionRequest +{ + fn from(req: PhysicalDiskAdoptionRequest) -> Self { + Self { + id: req.id, + vendor: req.vendor, + serial: req.serial, + model: req.model, + time_created: req.time_created, + time_deleted: req.time_deleted, + } + } +} + impl DatastoreCollectionConfig for PhysicalDisk { type CollectionId = DbTypedUuid; type GenerationNumberColumn = physical_disk::dsl::rcgen; diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index e9958d18498..bd852eb521e 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -12,6 +12,7 @@ use crate::db::collection_insert::DatastoreCollection; use crate::db::model::ApplySledFilterExt; use crate::db::model::InvPhysicalDisk; use crate::db::model::PhysicalDisk; +use crate::db::model::PhysicalDiskAdoptionRequest; use crate::db::model::PhysicalDiskKind; use crate::db::model::PhysicalDiskPolicy; use crate::db::model::PhysicalDiskState; @@ -29,6 +30,7 @@ use nexus_db_errors::public_error_from_diesel; use nexus_db_lookup::DbConnection; use nexus_db_model::ApplyPhysicalDiskFilterExt; use nexus_types::deployment::{DiskFilter, SledFilter}; +use nexus_types::external_api::physical_disk::PhysicalDiskId; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; @@ -36,6 +38,7 @@ use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupType; use omicron_common::api::external::ResourceType; +use omicron_common::bail_unless; use omicron_uuid_kinds::CollectionUuid; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::PhysicalDiskUuid; @@ -43,7 +46,10 @@ use omicron_uuid_kinds::SledUuid; use uuid::Uuid; impl DataStore { - /// Inserts a physical disk and zpool together in a transaction + /// Inserts a physical disk and zpool together in a transaction if there is + /// a valid adoption request. + /// + /// Soft-deletes the adoption request. pub async fn physical_disk_and_zpool_insert( &self, opctx: &OpContext, @@ -73,6 +79,15 @@ impl DataStore { .await .map_err(|txn_error| txn_error.into_diesel(&err))?; + // Delete the adoption request if it exists. If it doesn't exist + // this will return an error. + Self::physical_disk_adoption_request_delete_on_connection( + &conn, + (&disk).into(), + ) + .await + .map_err(|txn_error| txn_error.into_diesel(&err))?; + Self::physical_disk_insert_on_connection( &conn, opctx, disk, ) @@ -97,6 +112,89 @@ impl DataStore { Ok(()) } + /// Create a row in `physical_disk_adoption_request` for a given `disk_id` + /// as long as a non-expunged disk isn't present in the `physical_disk` + /// table for the same disk_id. + /// + /// This request is idempotent. If the disk is already adopted, or an + /// adoption request exists, success is returned. + pub async fn physical_disk_adopt( + &self, + opctx: &OpContext, + disk_id: PhysicalDiskId, + ) -> Result<(), Error> { + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; + + use nexus_db_schema::schema::physical_disk::dsl as physical_disk_dsl; + use nexus_db_schema::schema::physical_disk_adoption_request::dsl as adoption_dsl; + + let conn = &*self.pool_connection_authorized(opctx).await?; + self.transaction_retry_wrapper("physical_disk_adopt") + .transaction(&conn, |conn| { + let vendor = disk_id.vendor.clone(); + let serial = disk_id.serial.clone(); + let model = disk_id.model.clone(); + async move { + // Check for an active (non-expunged) physical disk with + // the same vendor/serial/model. + let active_disk_exists = physical_disk_dsl::physical_disk + .filter(physical_disk_dsl::vendor.eq(vendor.clone())) + .filter(physical_disk_dsl::serial.eq(serial.clone())) + .filter(physical_disk_dsl::model.eq(model.clone())) + .filter(physical_disk_dsl::time_deleted.is_null()) + .filter( + physical_disk_dsl::disk_policy + .ne(PhysicalDiskPolicy::Expunged), + ) + .select(physical_disk_dsl::id) + .first_async::(&conn) + .await + .optional()?; + + if active_disk_exists.is_some() { + return Ok(()); + } + + // Check for an active adoption request with the same + // vendor/serial/model. + let active_request_exists = + adoption_dsl::physical_disk_adoption_request + .filter(adoption_dsl::vendor.eq(vendor.clone())) + .filter(adoption_dsl::serial.eq(serial.clone())) + .filter(adoption_dsl::model.eq(model.clone())) + .filter(adoption_dsl::time_deleted.is_null()) + .select(adoption_dsl::id) + .first_async::(&conn) + .await + .optional()?; + + if active_request_exists.is_some() { + return Ok(()); + } + + // Insert the adoption request. + diesel::insert_into( + adoption_dsl::physical_disk_adoption_request, + ) + .values(( + adoption_dsl::id.eq(Uuid::new_v4()), + adoption_dsl::vendor.eq(vendor), + adoption_dsl::serial.eq(serial), + adoption_dsl::model.eq(model), + adoption_dsl::time_created.eq(Utc::now()), + )) + .execute_async(&conn) + .await?; + + Ok(()) + } + }) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + Ok(()) + } + /// Stores a new physical disk in the database. /// /// - If the Vendor, Serial, and Model fields are the same as an existing @@ -189,11 +287,71 @@ impl DataStore { Ok(()) } + /// Returns all physical disks which are eligible for adoption. + /// + /// These disks: + /// + /// - Appear on in-service sleds + /// - Appear in inventory + /// - Have adoption requests + /// + /// If "inventory_collection_id" is not associated with a collection, this + /// function returns an empty list, rather than failing. + pub async fn physical_disk_adoptable_list( + &self, + opctx: &OpContext, + inventory_collection_id: CollectionUuid, + ) -> ListResultVec { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + + use nexus_db_schema::schema::inv_physical_disk::dsl as inv_physical_disk_dsl; + use nexus_db_schema::schema::physical_disk_adoption_request::dsl as adoption_request_dsl; + use nexus_db_schema::schema::sled::dsl as sled_dsl; + + sled_dsl::sled + // If the sled is not in-service, drop the list immediately. + .filter(sled_dsl::time_deleted.is_null()) + .sled_filter(SledFilter::InService) + // Look up all inventory physical disks that could match this sled + .inner_join( + inv_physical_disk_dsl::inv_physical_disk.on( + inv_physical_disk_dsl::inv_collection_id + .eq(inventory_collection_id.into_untyped_uuid()) + .and(inv_physical_disk_dsl::sled_id.eq(sled_dsl::id)) + .and( + inv_physical_disk_dsl::variant + .eq(PhysicalDiskKind::U2), + ), + ), + ) + // Ensure that each inventory disk has a valid adoption request + .inner_join( + adoption_request_dsl::physical_disk_adoption_request.on( + adoption_request_dsl::vendor + .eq(inv_physical_disk_dsl::vendor) + .and( + adoption_request_dsl::model + .eq(inv_physical_disk_dsl::model), + ) + .and( + adoption_request_dsl::serial + .eq(inv_physical_disk_dsl::serial), + ) + .and(adoption_request_dsl::time_deleted.is_null()), + ), + ) + .select(InvPhysicalDisk::as_select()) + .get_results_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + /// Returns all physical disks which: /// /// - Appear on in-service sleds /// - Appear in inventory - /// - Do not have any records of expungement + /// - Do not have adoption requests + /// - Do not have active physical disk records /// /// If "inventory_collection_id" is not associated with a collection, this /// function returns an empty list, rather than failing. @@ -206,6 +364,7 @@ impl DataStore { use nexus_db_schema::schema::inv_physical_disk::dsl as inv_physical_disk_dsl; use nexus_db_schema::schema::physical_disk::dsl as physical_disk_dsl; + use nexus_db_schema::schema::physical_disk_adoption_request::dsl as adoption_request_dsl; use nexus_db_schema::schema::sled::dsl as sled_dsl; sled_dsl::sled @@ -224,13 +383,18 @@ impl DataStore { ), ), ) - // Filter out any disks in the inventory for which we have ever had - // a control plane disk. + // Filter out any disks in the inventory where we have a control + // plane disk that is active. .filter(diesel::dsl::not(diesel::dsl::exists( physical_disk_dsl::physical_disk .select(0.into_sql::()) .filter(physical_disk_dsl::sled_id.eq(sled_dsl::id)) .filter(physical_disk_dsl::variant.eq(PhysicalDiskKind::U2)) + .filter(physical_disk_dsl::time_deleted.is_null()) + .filter( + physical_disk_dsl::disk_policy + .ne(PhysicalDiskPolicy::Expunged), + ) .filter( physical_disk_dsl::vendor .eq(inv_physical_disk_dsl::vendor), @@ -244,6 +408,25 @@ impl DataStore { .eq(inv_physical_disk_dsl::serial), ), ))) + // Filter out physical disks that exist in `physical_disk_adoption_request`. This means + // they are in the process of being adopted already. + .filter(diesel::dsl::not(diesel::dsl::exists( + adoption_request_dsl::physical_disk_adoption_request + .select(0.into_sql::()) + .filter(adoption_request_dsl::time_deleted.is_null()) + .filter( + adoption_request_dsl::vendor + .eq(inv_physical_disk_dsl::vendor), + ) + .filter( + adoption_request_dsl::model + .eq(inv_physical_disk_dsl::model), + ) + .filter( + adoption_request_dsl::serial + .eq(inv_physical_disk_dsl::serial), + ), + ))) .select(InvPhysicalDisk::as_select()) .get_results_async(&*self.pool_connection_authorized(opctx).await?) .await @@ -267,6 +450,22 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + /// Return a page of active physical disk adoption requests. + pub async fn physical_disk_adoption_request_list( + &self, + opctx: &OpContext, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + use nexus_db_schema::schema::physical_disk_adoption_request::dsl; + paginated(dsl::physical_disk_adoption_request, dsl::id, pagparams) + .filter(dsl::time_deleted.is_null()) + .select(PhysicalDiskAdoptionRequest::as_select()) + .load_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + /// Return a page of physical disks for a given sled id pub async fn sled_list_physical_disks( &self, @@ -330,6 +529,33 @@ impl DataStore { .map(|_rows_modified| ()) .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + + // Delete an adoption request from the database + async fn physical_disk_adoption_request_delete_on_connection( + conn: &async_bb8_diesel::Connection, + id: PhysicalDiskId, + ) -> Result<(), TransactionError> { + let now = Utc::now(); + use nexus_db_schema::schema::physical_disk_adoption_request::dsl; + let rows_modified = diesel::update(dsl::physical_disk_adoption_request) + .filter(dsl::vendor.eq(id.vendor.clone())) + .filter(dsl::serial.eq(id.serial.clone())) + .filter(dsl::model.eq(id.model.clone())) + .filter(dsl::time_deleted.is_null()) + .set(dsl::time_deleted.eq(now)) + .execute_async(conn) + .await?; + + bail_unless!( + rows_modified == 1, + "No adoption request for physical disk: {}:{}:{}", + id.vendor, + id.serial, + id.model + ); + + Ok(()) + } } #[cfg(test)] @@ -797,6 +1023,13 @@ mod test { // We can insert a disk into a sled that is not yet expunged let inv_disk = create_inv_disk("serial-001".to_string(), 1); let (disk, zpool) = create_disk_zpool_combo(sled.id(), &inv_disk); + + // We need an adoption request before we can adopt the disk + // Adoption is idempotent + datastore + .physical_disk_adopt(&opctx, (&disk).into()) + .await + .expect("adoption succeeds"); datastore .physical_disk_and_zpool_insert(&opctx, disk, zpool) .await @@ -814,6 +1047,12 @@ mod test { // Now that the sled is expunged, inserting the disk should fail let inv_disk = create_inv_disk("serial-002".to_string(), 2); let (disk, zpool) = create_disk_zpool_combo(sled.id(), &inv_disk); + // We don't want it to fail because it doesn't have an adoption request + // though + datastore + .physical_disk_adopt(&opctx, (&disk).into()) + .await + .expect("adoption succeeds"); let err = datastore .physical_disk_and_zpool_insert(&opctx, disk, zpool) .await @@ -903,20 +1142,36 @@ mod test { // // This creates disks for: 001, 002, and 101. // It leaves the following uninitialized: 003, 102, 103 + // + // We make sure we submit an adoption request before we attempt to + // create the disk and zpools as we don't want automatic adoption in the + // background task that uses this datastore method. let (disk_001, zpool) = create_disk_zpool_combo(sled_a.id(), &disks_a[0]); + datastore + .physical_disk_adopt(&opctx, (&disk_001).into()) + .await + .expect("adoption request succeeds"); datastore .physical_disk_and_zpool_insert(&opctx, disk_001, zpool) .await .unwrap(); let (disk_002, zpool) = create_disk_zpool_combo(sled_a.id(), &disks_a[1]); + datastore + .physical_disk_adopt(&opctx, (&disk_002).into()) + .await + .expect("adoption request succeeds"); datastore .physical_disk_and_zpool_insert(&opctx, disk_002, zpool) .await .unwrap(); let (disk_101, zpool) = create_disk_zpool_combo(sled_b.id(), &disks_b[0]); + datastore + .physical_disk_adopt(&opctx, (&disk_101).into()) + .await + .expect("adoption request succeeds"); datastore .physical_disk_and_zpool_insert(&opctx, disk_101, zpool) .await @@ -955,18 +1210,30 @@ mod test { // Observe no remaining uninitialized disks. let (disk_003, zpool) = create_disk_zpool_combo(sled_a.id(), &disks_a[2]); + datastore + .physical_disk_adopt(&opctx, (&disk_003).into()) + .await + .expect("adoption request succeeds"); datastore .physical_disk_and_zpool_insert(&opctx, disk_003.clone(), zpool) .await .unwrap(); let (disk_102, zpool) = create_disk_zpool_combo(sled_b.id(), &disks_b[1]); + datastore + .physical_disk_adopt(&opctx, (&disk_102).into()) + .await + .expect("adoption request succeeds"); datastore .physical_disk_and_zpool_insert(&opctx, disk_102.clone(), zpool) .await .unwrap(); let (disk_103, zpool) = create_disk_zpool_combo(sled_b.id(), &disks_b[2]); + datastore + .physical_disk_adopt(&opctx, (&disk_103).into()) + .await + .expect("adoption request succeeds"); datastore .physical_disk_and_zpool_insert(&opctx, disk_103.clone(), zpool) .await @@ -978,8 +1245,8 @@ mod test { .expect("Failed to list uninitialized disks"); assert_eq!(uninitialized_disks.len(), 0); - // Expunge some disks, observe that they do not re-appear as - // initialized. + // Expunge some disks, observe that they are now uninitialized again. This allows + // re-adding them to the control plane. use nexus_db_schema::schema::physical_disk::dsl; // Set a disk to "deleted". @@ -1005,12 +1272,171 @@ mod test { .await .unwrap(); - // The set of uninitialized disks should remain at zero + // The set of uninitialized disks should include the expunged/deleted + // disks let uninitialized_disks = datastore .physical_disk_uninitialized_list(&opctx, collection_id) .await .expect("Failed to list uninitialized disks"); - assert_eq!(uninitialized_disks.len(), 0); + assert_eq!(uninitialized_disks.len(), 2); + + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn physical_disk_adoptable_list() { + let logctx = dev::test_setup_log("physical_disk_adoptable_list"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + let conn = datastore.pool_connection_for_tests().await.unwrap(); + + let sled_a = create_test_sled(&datastore).await; + let sled_b = create_test_sled(&datastore).await; + + // No inventory -> No adoptable disks + let adoptable_disks = datastore + .physical_disk_adoptable_list( + &opctx, + CollectionUuid::new_v4(), // Collection that does not exist + ) + .await + .expect("Failed to look up adoptable disks"); + assert!(adoptable_disks.is_empty()); + + // Create inventory disks for both sleds + let mut builder = nexus_inventory::CollectionBuilder::new("test"); + let disks_a = vec![ + create_inv_disk("serial-001".to_string(), 1), + create_inv_disk("serial-002".to_string(), 2), + create_inv_disk("serial-003".to_string(), 3), + ]; + let disks_b = vec![ + create_inv_disk("serial-101".to_string(), 1), + create_inv_disk("serial-102".to_string(), 2), + create_inv_disk("serial-103".to_string(), 3), + ]; + add_sled_to_inventory(&mut builder, &sled_a, disks_a.clone()); + add_sled_to_inventory(&mut builder, &sled_b, disks_b.clone()); + let collection = builder.build(); + let collection_id = collection.id; + datastore + .inventory_insert_collection(&opctx, &collection) + .await + .expect("failed to insert collection"); + + // We have inventory, but no adoption requests so we should still list + // 0 disks. + let adoptable_disks = datastore + .physical_disk_adoptable_list(&opctx, collection_id) + .await + .expect("Failed to look up adoptable disks"); + assert!(adoptable_disks.is_empty()); + + // All 6 disks should be uninitialized + let uninitialized_disks = datastore + .physical_disk_uninitialized_list(&opctx, collection_id) + .await + .expect("Failed to list uninitialized disks"); + assert_eq!(uninitialized_disks.len(), 6); + + // Make two disks adoptable + datastore + .physical_disk_adopt(&opctx, disks_a[0].identity.clone().into()) + .await + .expect("adoption request succeeds"); + datastore + .physical_disk_adopt(&opctx, disks_b[0].identity.clone().into()) + .await + .expect("adoption request succeeds"); + + // Adoption is idempotent + datastore + .physical_disk_adopt(&opctx, disks_b[0].identity.clone().into()) + .await + .expect("adoption request succeeds"); + + // We should now have 2 adoptable disks + let adoptable_disks = datastore + .physical_disk_adoptable_list(&opctx, collection_id) + .await + .expect("Failed to look up adoptable disks"); + assert_eq!(adoptable_disks.len(), 2); + + // The remaining 4 disks should be uninitialized + let uninitialized_disks = datastore + .physical_disk_uninitialized_list(&opctx, collection_id) + .await + .expect("Failed to list uninitialized disks"); + assert_eq!(uninitialized_disks.len(), 4); + + // soft-deleting both adoptable disks makes them unadoptable again + DataStore::physical_disk_adoption_request_delete_on_connection( + &conn, + disks_a[0].identity.clone().into(), + ) + .await + .expect("successful soft delete"); + DataStore::physical_disk_adoption_request_delete_on_connection( + &conn, + disks_b[0].identity.clone().into(), + ) + .await + .expect("successful soft delete"); + let adoptable_disks = datastore + .physical_disk_adoptable_list(&opctx, collection_id) + .await + .expect("Failed to look up adoptable disks"); + assert!(adoptable_disks.is_empty()); + + // All 6 disks should be uninitialized again + let uninitialized_disks = datastore + .physical_disk_uninitialized_list(&opctx, collection_id) + .await + .expect("Failed to list uninitialized disks"); + assert_eq!(uninitialized_disks.len(), 6); + + // Adding one of the same disks back again should make it adoptable + datastore + .physical_disk_adopt(&opctx, disks_a[0].identity.clone().into()) + .await + .expect("adoption request succeeds"); + let adoptable_disks = datastore + .physical_disk_adoptable_list(&opctx, collection_id) + .await + .expect("Failed to look up adoptable disks"); + assert_eq!(adoptable_disks.len(), 1); + + // We should now only have 5 unininitialized disks + let uninitialized_disks = datastore + .physical_disk_uninitialized_list(&opctx, collection_id) + .await + .expect("Failed to list uninitialized disks"); + assert_eq!(uninitialized_disks.len(), 5); + + // Making a disk adoption reqeust for a disk that is not in inventory + // will not make the disk show up as adoptable. We allow the request to + // succeed but not the adoption. This means an operator can insert the + // disk after the fact. + let mut disk_not_in_inventory: PhysicalDiskId = + disks_a[0].identity.clone().into(); + disk_not_in_inventory.vendor = "some-other-vendor".to_string(); + datastore + .physical_disk_adopt(&opctx, disk_not_in_inventory) + .await + .expect("adoption request succeeds"); + let adoptable_disks = datastore + .physical_disk_adoptable_list(&opctx, collection_id) + .await + .expect("Failed to look up adoptable disks"); + assert_eq!(adoptable_disks.len(), 1); + + // We should still have only 5 unininitialized disks + let uninitialized_disks = datastore + .physical_disk_uninitialized_list(&opctx, collection_id) + .await + .expect("Failed to list uninitialized disks"); + assert_eq!(uninitialized_disks.len(), 5); db.terminate().await; logctx.cleanup_successful(); diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 4442584dd0b..2f9ed788884 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -1146,6 +1146,17 @@ table! { } } +table! { + physical_disk_adoption_request (id) { + id -> Uuid, + vendor -> Text, + model -> Text, + serial -> Text, + time_created -> Timestamptz, + time_deleted -> Nullable, + } +} + table! { certificate (id) { id -> Uuid, @@ -2662,6 +2673,7 @@ allow_tables_to_appear_in_same_query!( instance_network_interface, inv_physical_disk, inv_nvme_disk_firmware, + physical_disk_adoption_request, service_network_interface, oximeter, physical_disk, diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index 348798bc282..f4b528309f6 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -217,7 +217,10 @@ networking_switch_port_apply_settings POST /v1/system/hardware/switch-por networking_switch_port_clear_settings DELETE /v1/system/hardware/switch-port/{port}/settings networking_switch_port_list GET /v1/system/hardware/switch-port networking_switch_port_status GET /v1/system/hardware/switch-port/{port}/status +physical_disk_adopt PUT /v1/system/hardware/disks physical_disk_list GET /v1/system/hardware/disks +physical_disk_list_adoption_requests GET /v1/system/hardware/disk-adoption-requests +physical_disk_list_uninitialized GET /v1/system/hardware/disks-uninitialized physical_disk_view GET /v1/system/hardware/disks/{disk_id} rack_list GET /v1/system/hardware/racks rack_view GET /v1/system/hardware/racks/{rack_id} diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index e6416f360df..5f8841eaf6d 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -79,6 +79,7 @@ api_versions!([ // | date-based version should be at the top of the list. // v // (next_yyyy_mm_dd_nn, IDENT), + (2026_04_02_00, MANUAL_DISK_ADOPTION), (2026_03_25_00, SUBNET_POOL_UTILIZATION_REMAINING), (2026_03_24_00, ADD_ICMPV6_FIREWALL_SUPPORT), (2026_03_23_00, RENAME_PREFIX_LEN), @@ -6682,6 +6683,52 @@ pub trait NexusExternalApi { path_params: Path, ) -> Result, HttpError>; + /// List uninitialized physical disks + #[endpoint { + method = GET, + path = "/v1/system/hardware/disks-uninitialized", + tags = ["system/hardware"], + versions = VERSION_MANUAL_DISK_ADOPTION.. + }] + async fn physical_disk_list_uninitialized( + rqctx: RequestContext, + query: Query>, + ) -> Result< + HttpResponseOk< + ResultsPage, + >, + HttpError, + >; + + /// List physical disks adoption requests + #[endpoint { + method = GET, + path = "/v1/system/hardware/disk-adoption-requests", + tags = ["system/hardware"], + versions = VERSION_MANUAL_DISK_ADOPTION.. + }] + async fn physical_disk_list_adoption_requests( + rqctx: RequestContext, + query_params: Query, + ) -> Result< + HttpResponseOk< + ResultsPage, + >, + HttpError, + >; + + /// Adopt a physical disk for usage by the control plane + #[endpoint { + method = PUT, + path = "/v1/system/hardware/disks", + tags = ["system/hardware"], + versions = VERSION_MANUAL_DISK_ADOPTION.. + }] + async fn physical_disk_adopt( + rqctx: RequestContext, + req: TypedBody, + ) -> Result; + // Switches /// List switches diff --git a/nexus/src/app/background/tasks/physical_disk_adoption.rs b/nexus/src/app/background/tasks/physical_disk_adoption.rs index 89ea56180bb..ddae786425a 100644 --- a/nexus/src/app/background/tasks/physical_disk_adoption.rs +++ b/nexus/src/app/background/tasks/physical_disk_adoption.rs @@ -124,11 +124,11 @@ impl BackgroundTask for PhysicalDiskAdoption { let result = self .datastore - .physical_disk_uninitialized_list(opctx, collection_id) + .physical_disk_adoptable_list(opctx, collection_id) .await; - let uninitialized = match result { - Ok(uninitialized) => uninitialized, + let adoptable = match result { + Ok(adoptable) => adoptable, Err(err) => { let err = InlineErrorChain::new(&err); warn!( @@ -142,7 +142,7 @@ impl BackgroundTask for PhysicalDiskAdoption { } }; - for inv_disk in uninitialized { + for inv_disk in adoptable { let disk = PhysicalDisk::new( PhysicalDiskUuid::new_v4(), inv_disk.vendor, diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index e204e95cf69..e575cc17941 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -283,6 +283,38 @@ impl super::Nexus { .await } + pub(crate) async fn physical_disk_list_uninitialized( + &self, + opctx: &OpContext, + ) -> ListResultVec { + let collection_id = + self.db_datastore.inventory_get_latest_collection_id(opctx).await?; + let Some(collection_id) = collection_id else { + return Ok(vec![]); + }; + self.db_datastore + .physical_disk_uninitialized_list(opctx, collection_id) + .await + } + + pub(crate) async fn physical_disk_adoption_request_list( + &self, + opctx: &OpContext, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + self.db_datastore + .physical_disk_adoption_request_list(opctx, pagparams) + .await + } + + pub(crate) async fn physical_disk_adopt( + &self, + opctx: &OpContext, + disk_id: nexus_types::external_api::physical_disk::PhysicalDiskId, + ) -> Result<(), Error> { + self.db_datastore.physical_disk_adopt(opctx, disk_id).await + } + /// Inserts a physical disk into the database unless it already exists. /// /// NOTE: I'd like to re-work this to avoid the upsert-like behavior - can diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index fe8a5ca4321..9c5cadc518f 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -56,7 +56,10 @@ use nexus_types::external_api::identity_provider::IdentityProvider; use nexus_types::external_api::image::Image; use nexus_types::external_api::ip_pool::{IpPool, IpPoolRange}; use nexus_types::external_api::metrics::SystemMetricsPathParam; -use nexus_types::external_api::physical_disk::PhysicalDisk; +use nexus_types::external_api::physical_disk::{ + PhysicalDisk, PhysicalDiskAdoptionRequest, PhysicalDiskId, + UninitializedPhysicalDisk, +}; use nexus_types::external_api::probe::ProbeInfo; use nexus_types::external_api::project::Project; use nexus_types::external_api::rack::{Rack, RackMembershipStatus}; @@ -6695,6 +6698,94 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } + async fn physical_disk_list_uninitialized( + rqctx: RequestContext, + query: Query>, + ) -> Result>, HttpError> + { + let apictx = rqctx.context(); + let pag_params = query.into_inner(); + if let dropshot::WhichPage::Next(last_seen) = &pag_params.page { + return Err(Error::invalid_value( + last_seen.clone(), + "bad page token", + ) + .into()); + } + let handler = async { + let nexus = &apictx.context.nexus; + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let disks = nexus + .physical_disk_list_uninitialized(&opctx) + .await? + .into_iter() + .map(|d| UninitializedPhysicalDisk { + sled_id: d.sled_id.into(), + slot: d.slot as u64, + variant: d.variant.into(), + disk_id: PhysicalDiskId { + vendor: d.vendor, + serial: d.serial, + model: d.model, + }, + }) + .collect(); + Ok(HttpResponseOk(ResultsPage { items: disks, next_page: None })) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn physical_disk_list_adoption_requests( + rqctx: RequestContext, + query_params: Query, + ) -> Result< + HttpResponseOk>, + HttpError, + > { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let query = query_params.into_inner(); + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let requests = nexus + .physical_disk_adoption_request_list( + &opctx, + &data_page_params_for(&rqctx, &query)?, + ) + .await? + .into_iter() + .map(|r| r.into()) + .collect(); + Ok(HttpResponseOk(ScanById::results_page( + &query, + requests, + &|_, req: &PhysicalDiskAdoptionRequest| req.id, + )?)) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn physical_disk_adopt( + rqctx: RequestContext, + req: TypedBody, + ) -> Result { + audit_and_time(&rqctx, |opctx, nexus| async move { + nexus.physical_disk_adopt(&opctx, req.into_inner()).await?; + Ok(HttpResponseUpdatedNoContent()) + }) + .await + } + // Switches async fn switch_list( diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 758365e223d..0b6c6d0037d 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -34,6 +34,7 @@ use nexus_types::external_api::ip_pool; use nexus_types::external_api::multicast; use nexus_types::external_api::networking; use nexus_types::external_api::path_params; +use nexus_types::external_api::physical_disk; use nexus_types::external_api::policy; use nexus_types::external_api::project; use nexus_types::external_api::rack; @@ -120,6 +121,20 @@ pub static DEMO_SLED_PROVISION_POLICY: LazyLock< pub static HARDWARE_SWITCH_URL: LazyLock = LazyLock::new(|| format!("/v1/system/hardware/switches/{}", SWITCH_UUID)); + +pub static DEMO_HARDWARE_PHYSICAL_DISK_ID: LazyLock< + physical_disk::PhysicalDiskId, +> = LazyLock::new(|| physical_disk::PhysicalDiskId { + vendor: "test".into(), + serial: "test".into(), + model: "test".into(), +}); + +pub static HARDWARE_DISK_ADOPTION_REQUESTS_URL: &'static str = + "/v1/system/hardware/disk-adoption-requests"; +pub static HARDWARE_DISKS_UNINITIALIZED_URL: &'static str = + "/v1/system/hardware/disks-uninitialized"; + pub const HARDWARE_DISKS_URL: &'static str = "/v1/system/hardware/disks"; pub static HARDWARE_DISK_URL: LazyLock = LazyLock::new(|| { format!("/v1/system/hardware/disks/{}", PHYSICAL_DISK_UUID) @@ -2985,7 +3000,13 @@ pub static VERIFY_ENDPOINTS: LazyLock> = LazyLock::new( url: &HARDWARE_DISKS_URL, visibility: Visibility::Public, unprivileged_access: UnprivilegedAccess::None, - allowed_methods: vec![AllowedMethod::Get], + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(&*DEMO_HARDWARE_PHYSICAL_DISK_ID) + .unwrap(), + ), + ], }, VerifyEndpoint { url: &HARDWARE_DISK_URL, @@ -2993,6 +3014,18 @@ pub static VERIFY_ENDPOINTS: LazyLock> = LazyLock::new( unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![AllowedMethod::Get], }, + VerifyEndpoint { + url: &HARDWARE_DISKS_UNINITIALIZED_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], + }, + VerifyEndpoint { + url: &HARDWARE_DISK_ADOPTION_REQUESTS_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], + }, VerifyEndpoint { url: &HARDWARE_SLED_DISK_URL, visibility: Visibility::Public, diff --git a/nexus/types/versions/src/latest.rs b/nexus/types/versions/src/latest.rs index 7aab6ceaa2f..580c2d59564 100644 --- a/nexus/types/versions/src/latest.rs +++ b/nexus/types/versions/src/latest.rs @@ -447,6 +447,11 @@ pub mod physical_disk { pub use crate::v2025_11_20_00::physical_disk::PhysicalDiskKind; pub use crate::v2025_11_20_00::physical_disk::PhysicalDiskPolicy; pub use crate::v2025_11_20_00::physical_disk::PhysicalDiskState; + + // Types from MANUAL_DISK_ADOPTION. + pub use crate::v2026_04_02_00::physical_disk::PhysicalDiskAdoptionRequest; + pub use crate::v2026_04_02_00::physical_disk::PhysicalDiskId; + pub use crate::v2026_04_02_00::physical_disk::UninitializedPhysicalDisk; } pub mod rack { diff --git a/nexus/types/versions/src/lib.rs b/nexus/types/versions/src/lib.rs index ae02c281c59..b04d2f01a46 100644 --- a/nexus/types/versions/src/lib.rs +++ b/nexus/types/versions/src/lib.rs @@ -77,3 +77,5 @@ pub mod v2026_03_14_00; pub mod v2026_03_23_00; #[path = "subnet_pool_utilization_remaining/mod.rs"] pub mod v2026_03_25_00; +#[path = "manual_disk_adoption/mod.rs"] +pub mod v2026_04_02_00; diff --git a/nexus/types/versions/src/manual_disk_adoption/mod.rs b/nexus/types/versions/src/manual_disk_adoption/mod.rs new file mode 100644 index 00000000000..32af74959d8 --- /dev/null +++ b/nexus/types/versions/src/manual_disk_adoption/mod.rs @@ -0,0 +1,10 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Version `MANUAL_DISK_ADOPTION` of the Nexus external API. +//! +//! Allows listing of uninitialized physical disks, and forces manual rather +//! than automatic adoption of those disks. + +pub mod physical_disk; diff --git a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs new file mode 100644 index 00000000000..061163dbadb --- /dev/null +++ b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs @@ -0,0 +1,45 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Physical disk types for version MANUAL_DISK_ADOPTION. + +use omicron_uuid_kinds::SledUuid; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use crate::v2025_11_20_00::physical_disk::PhysicalDiskKind; + +/// A physical disk that has not yet been adopted by the control plane +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] +pub struct UninitializedPhysicalDisk { + pub sled_id: SledUuid, + pub slot: u64, + pub variant: PhysicalDiskKind, + pub disk_id: PhysicalDiskId, +} + +/// The unique identity of a physical disk +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] +pub struct PhysicalDiskId { + pub vendor: String, + pub serial: String, + pub model: String, +} + +impl From for PhysicalDiskId { + fn from(value: omicron_common::disk::DiskIdentity) -> Self { + Self { vendor: value.vendor, serial: value.serial, model: value.model } + } +} + +/// A request to adopt a physical disk into the control plane +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] +pub struct PhysicalDiskAdoptionRequest { + pub id: uuid::Uuid, + pub vendor: String, + pub serial: String, + pub model: String, + pub time_created: chrono::DateTime, + pub time_deleted: Option>, +} diff --git a/openapi/nexus/nexus-2026032500.0.0-cf1221.json.gitstub b/openapi/nexus/nexus-2026032500.0.0-cf1221.json.gitstub new file mode 100644 index 00000000000..2914b100282 --- /dev/null +++ b/openapi/nexus/nexus-2026032500.0.0-cf1221.json.gitstub @@ -0,0 +1 @@ +254a0c51bc0beecb79c8a9dfccce8e7bc35b5ca4:openapi/nexus/nexus-2026032500.0.0-cf1221.json diff --git a/openapi/nexus/nexus-2026032500.0.0-cf1221.json b/openapi/nexus/nexus-2026040200.0.0-430bef.json similarity index 99% rename from openapi/nexus/nexus-2026032500.0.0-cf1221.json rename to openapi/nexus/nexus-2026040200.0.0-430bef.json index 72d5a2e5d37..2c44a8cbe98 100644 --- a/openapi/nexus/nexus-2026032500.0.0-cf1221.json +++ b/openapi/nexus/nexus-2026040200.0.0-430bef.json @@ -7,7 +7,7 @@ "url": "https://oxide.computer", "email": "api@oxide.computer" }, - "version": "2026032500.0.0" + "version": "2026040200.0.0" }, "paths": { "/device/auth": { @@ -7695,6 +7695,65 @@ } } }, + "/v1/system/hardware/disk-adoption-requests": { + "get": { + "tags": [ + "system/hardware" + ], + "summary": "List physical disks adoption requests", + "operationId": "physical_disk_list_adoption_requests", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/IdSortMode" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PhysicalDiskAdoptionRequestResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": { + "required": [] + } + } + }, "/v1/system/hardware/disks": { "get": { "tags": [ @@ -7752,6 +7811,34 @@ "x-dropshot-pagination": { "required": [] } + }, + "put": { + "tags": [ + "system/hardware" + ], + "summary": "Adopt a physical disk for usage by the control plane", + "operationId": "physical_disk_adopt", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PhysicalDiskId" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "resource updated" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } } }, "/v1/system/hardware/disks/{disk_id}": { @@ -7793,6 +7880,58 @@ } } }, + "/v1/system/hardware/disks-uninitialized": { + "get": { + "tags": [ + "system/hardware" + ], + "summary": "List uninitialized physical disks", + "operationId": "physical_disk_list_uninitialized", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UninitializedPhysicalDiskResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": { + "required": [] + } + } + }, "/v1/system/hardware/rack-switch-port/{rack_id}/{switch_slot}/{port}/lldp/neighbors": { "get": { "tags": [ @@ -25182,6 +25321,82 @@ "vendor" ] }, + "PhysicalDiskAdoptionRequest": { + "description": "A request to adopt a physical disk into the control plane", + "type": "object", + "properties": { + "id": { + "type": "string", + "format": "uuid" + }, + "model": { + "type": "string" + }, + "serial": { + "type": "string" + }, + "time_created": { + "type": "string", + "format": "date-time" + }, + "time_deleted": { + "nullable": true, + "type": "string", + "format": "date-time" + }, + "vendor": { + "type": "string" + } + }, + "required": [ + "id", + "model", + "serial", + "time_created", + "vendor" + ] + }, + "PhysicalDiskAdoptionRequestResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/PhysicalDiskAdoptionRequest" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, + "PhysicalDiskId": { + "description": "The unique identity of a physical disk", + "type": "object", + "properties": { + "model": { + "type": "string" + }, + "serial": { + "type": "string" + }, + "vendor": { + "type": "string" + } + }, + "required": [ + "model", + "serial", + "vendor" + ] + }, "PhysicalDiskKind": { "description": "Describes the form factor of physical disks.", "type": "string", @@ -27830,6 +28045,15 @@ } ] }, + "SledUuid": { + "x-rust-type": { + "crate": "omicron-uuid-kinds", + "path": "omicron_uuid_kinds::SledUuid", + "version": "*" + }, + "type": "string", + "format": "uuid" + }, "Snapshot": { "description": "View of a Snapshot", "type": "object", @@ -29661,6 +29885,53 @@ } } }, + "UninitializedPhysicalDisk": { + "description": "A physical disk that has not yet been adopted by the control plane", + "type": "object", + "properties": { + "disk_id": { + "$ref": "#/components/schemas/PhysicalDiskId" + }, + "sled_id": { + "$ref": "#/components/schemas/SledUuid" + }, + "slot": { + "type": "integer", + "format": "uint64", + "minimum": 0 + }, + "variant": { + "$ref": "#/components/schemas/PhysicalDiskKind" + } + }, + "required": [ + "disk_id", + "sled_id", + "slot", + "variant" + ] + }, + "UninitializedPhysicalDiskResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/UninitializedPhysicalDisk" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, "UninitializedSled": { "description": "A sled that has not been added to an initialized rack yet", "type": "object", diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index bb7ce584356..4aacdb7d880 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026032500.0.0-cf1221.json \ No newline at end of file +nexus-2026040200.0.0-430bef.json \ No newline at end of file diff --git a/schema/crdb/add-disk-adoption-requests/up01.sql b/schema/crdb/add-disk-adoption-requests/up01.sql new file mode 100644 index 00000000000..8d23de3c40f --- /dev/null +++ b/schema/crdb/add-disk-adoption-requests/up01.sql @@ -0,0 +1,8 @@ +CREATE TABLE IF NOT EXISTS physical_disk_adoption_request ( + id UUID PRIMARY KEY, + vendor STRING(63) NOT NULL, + model STRING(63) NOT NULL, + serial STRING(63) NOT NULL, + time_created TIMESTAMPTZ NOT NULL, + time_deleted TIMESTAMPTZ +); diff --git a/schema/crdb/add-disk-adoption-requests/up02.sql b/schema/crdb/add-disk-adoption-requests/up02.sql new file mode 100644 index 00000000000..599b6c4fbfa --- /dev/null +++ b/schema/crdb/add-disk-adoption-requests/up02.sql @@ -0,0 +1,6 @@ +CREATE UNIQUE INDEX IF NOT EXISTS physical_disk_adoption_request_by_physical_id +ON physical_disk_adoption_request ( + vendor, + model, + serial +) WHERE time_deleted IS NULL; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index fd9c57e22a1..1b3ff6b84bc 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -504,6 +504,25 @@ CREATE UNIQUE INDEX IF NOT EXISTS lookup_physical_disk_by_sled ON omicron.public id ); +-- Any disk present in this table with a NON-NULL time_deleted column +-- is eligible for adoption. +CREATE TABLE IF NOT EXISTS physical_disk_adoption_request ( + id UUID PRIMARY KEY, + vendor STRING(63) NOT NULL, + model STRING(63) NOT NULL, + serial STRING(63) NOT NULL, + time_created TIMESTAMPTZ NOT NULL, + time_deleted TIMESTAMPTZ +); + +-- Only one eligible disk with the same physical id may be present at a time +CREATE UNIQUE INDEX IF NOT EXISTS physical_disk_adoption_request_by_physical_id +ON physical_disk_adoption_request ( + vendor, + model, + serial +) WHERE time_deleted IS NULL; + -- x509 certificates which may be used by services CREATE TABLE IF NOT EXISTS omicron.public.certificate ( -- Identity metadata (resource) From 3ee2cc125525ee788d1b2e4fd73d93d3c839d84c Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 7 Apr 2026 21:47:40 +0000 Subject: [PATCH 02/26] fix tests --- nexus/db-model/src/schema_versions.rs | 3 ++- nexus/tests/integration_tests/endpoints.rs | 4 ++-- schema/crdb/dbinit.sql | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 58fad5d6345..0aa7939703c 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: Version = Version::new(249, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(250, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ pub static KNOWN_VERSIONS: LazyLock> = LazyLock::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(250, "add-disk-adoption-requests"), KnownVersion::new(249, "fm-support-bundle-request"), KnownVersion::new(248, "cleanup-orphaned-subnet-pool-silo-links"), KnownVersion::new(247, "remove-tuf-base-url"), diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 0b6c6d0037d..51ea5732911 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -3016,13 +3016,13 @@ pub static VERIFY_ENDPOINTS: LazyLock> = LazyLock::new( }, VerifyEndpoint { url: &HARDWARE_DISKS_UNINITIALIZED_URL, - visibility: Visibility::Protected, + visibility: Visibility::Public, unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![AllowedMethod::Get], }, VerifyEndpoint { url: &HARDWARE_DISK_ADOPTION_REQUESTS_URL, - visibility: Visibility::Protected, + visibility: Visibility::Public, unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![AllowedMethod::Get], }, diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 1b3ff6b84bc..670eb6ece57 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -8431,7 +8431,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '249.0.0', NULL) + (TRUE, NOW(), NOW(), '250.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; From 260a348e6c4235f80f9230e4cfb829e7d38650d2 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 7 Apr 2026 22:34:29 +0000 Subject: [PATCH 03/26] expectorate --- schema/crdb/add-disk-adoption-requests/up02.verify.sql | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 schema/crdb/add-disk-adoption-requests/up02.verify.sql diff --git a/schema/crdb/add-disk-adoption-requests/up02.verify.sql b/schema/crdb/add-disk-adoption-requests/up02.verify.sql new file mode 100644 index 00000000000..9d49f7780b1 --- /dev/null +++ b/schema/crdb/add-disk-adoption-requests/up02.verify.sql @@ -0,0 +1,2 @@ +-- DO NOT EDIT. Generated by test_migration_verification_files. +SELECT CAST(IF((SELECT true WHERE EXISTS (SELECT index_name FROM omicron.crdb_internal.table_indexes WHERE descriptor_name = 'physical_disk_adoption_request' AND index_name = 'physical_disk_adoption_request_by_physical_id')),'true','Schema change verification failed: index physical_disk_adoption_request_by_physical_id on table physical_disk_adoption_request does not exist') AS BOOL); From b4d48febcf0266faa4b08cfe081d224f723b2d59 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Thu, 9 Apr 2026 01:06:31 +0000 Subject: [PATCH 04/26] minor fixes --- nexus/db-queries/src/db/datastore/physical_disk.rs | 3 ++- nexus/src/external_api/http_entrypoints.rs | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index bd852eb521e..15d95e65773 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -56,6 +56,7 @@ impl DataStore { disk: PhysicalDisk, zpool: Zpool, ) -> Result<(), Error> { + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; let conn = &*self.pool_connection_authorized(&opctx).await?; let err = OptionalError::new(); @@ -251,7 +252,7 @@ impl DataStore { id: PhysicalDiskUuid, policy: PhysicalDiskPolicy, ) -> Result<(), Error> { - opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; use nexus_db_schema::schema::physical_disk::dsl; let now = Utc::now(); diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 9c5cadc518f..845ab381f68 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6488,6 +6488,13 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } + // This request isn't currently paginated. The query is somewhat complex + // underneath and doesn't support pagination right now. We would need a way + // to order filtered `InvPhysicalDisk` objects to support pagination, and + // we've decided to punt on this for now. The justification for punting is + // that in single-rack systems there is a limit to the number of disks we'd + // adopt at one time. The maximum is 320 U.2 drives in an entire rack, and + // we won't be replacing them all at once. async fn sled_list_uninitialized( rqctx: RequestContext, query: Query>, From 2c79dd0446c30982d2a13d42befca911c76e6da4 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Thu, 9 Apr 2026 19:53:01 +0000 Subject: [PATCH 05/26] A few more review fixes --- nexus/db-model/src/physical_disk.rs | 6 ++- .../src/db/datastore/physical_disk.rs | 36 +++++++-------- nexus/external-api/output/nexus_tags.txt | 4 +- nexus/external-api/src/lib.rs | 18 ++++---- nexus/src/app/sled.rs | 6 +-- nexus/src/external_api/http_entrypoints.rs | 19 ++++---- nexus/tests/integration_tests/endpoints.rs | 10 ++--- nexus/types/versions/src/latest.rs | 4 +- .../src/manual_disk_adoption/physical_disk.rs | 12 ++--- ....json => nexus-2026040200.0.0-a5ad49.json} | 44 +++++++++---------- openapi/nexus/nexus-latest.json | 2 +- 11 files changed, 81 insertions(+), 80 deletions(-) rename openapi/nexus/{nexus-2026040200.0.0-430bef.json => nexus-2026040200.0.0-a5ad49.json} (99%) diff --git a/nexus/db-model/src/physical_disk.rs b/nexus/db-model/src/physical_disk.rs index da014329371..3e09edf990b 100644 --- a/nexus/db-model/src/physical_disk.rs +++ b/nexus/db-model/src/physical_disk.rs @@ -74,8 +74,10 @@ impl PhysicalDisk { } } -impl From<&PhysicalDisk> for physical_disk_types::PhysicalDiskId { - fn from(value: &PhysicalDisk) -> Self { +impl From + for physical_disk_types::PhysicalDiskManufacturerIdentity +{ + fn from(value: PhysicalDisk) -> Self { Self { vendor: value.vendor.clone(), serial: value.serial.clone(), diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index 15d95e65773..0b43dd3bd12 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -30,7 +30,7 @@ use nexus_db_errors::public_error_from_diesel; use nexus_db_lookup::DbConnection; use nexus_db_model::ApplyPhysicalDiskFilterExt; use nexus_types::deployment::{DiskFilter, SledFilter}; -use nexus_types::external_api::physical_disk::PhysicalDiskId; +use nexus_types::external_api::physical_disk::PhysicalDiskManufacturerIdentity; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; @@ -84,7 +84,7 @@ impl DataStore { // this will return an error. Self::physical_disk_adoption_request_delete_on_connection( &conn, - (&disk).into(), + disk.clone().into(), ) .await .map_err(|txn_error| txn_error.into_diesel(&err))?; @@ -122,7 +122,7 @@ impl DataStore { pub async fn physical_disk_adopt( &self, opctx: &OpContext, - disk_id: PhysicalDiskId, + disk_id: PhysicalDiskManufacturerIdentity, ) -> Result<(), Error> { opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; @@ -356,7 +356,7 @@ impl DataStore { /// /// If "inventory_collection_id" is not associated with a collection, this /// function returns an empty list, rather than failing. - pub async fn physical_disk_uninitialized_list( + pub async fn physical_disk_unadopted_list( &self, opctx: &OpContext, inventory_collection_id: CollectionUuid, @@ -534,7 +534,7 @@ impl DataStore { // Delete an adoption request from the database async fn physical_disk_adoption_request_delete_on_connection( conn: &async_bb8_diesel::Connection, - id: PhysicalDiskId, + id: PhysicalDiskManufacturerIdentity, ) -> Result<(), TransactionError> { let now = Utc::now(); use nexus_db_schema::schema::physical_disk_adoption_request::dsl; @@ -1028,7 +1028,7 @@ mod test { // We need an adoption request before we can adopt the disk // Adoption is idempotent datastore - .physical_disk_adopt(&opctx, (&disk).into()) + .physical_disk_adopt(&opctx, disk.into()) .await .expect("adoption succeeds"); datastore @@ -1051,7 +1051,7 @@ mod test { // We don't want it to fail because it doesn't have an adoption request // though datastore - .physical_disk_adopt(&opctx, (&disk).into()) + .physical_disk_adopt(&opctx, disk.into()) .await .expect("adoption succeeds"); let err = datastore @@ -1081,7 +1081,7 @@ mod test { // No inventory -> No uninitialized disks let uninitialized_disks = datastore - .physical_disk_uninitialized_list( + .physical_disk_unadopted_list( &opctx, CollectionUuid::new_v4(), // Collection that does not exist ) @@ -1113,7 +1113,7 @@ mod test { // Now when we list the uninitialized disks, we should see everything in // the inventory. let uninitialized_disks = datastore - .physical_disk_uninitialized_list(&opctx, collection_id) + .physical_disk_unadopted_list(&opctx, collection_id) .await .expect("Failed to list uninitialized disks"); assert_eq!(uninitialized_disks.len(), 6); @@ -1179,7 +1179,7 @@ mod test { .unwrap(); let uninitialized_disks = datastore - .physical_disk_uninitialized_list(&opctx, collection_id) + .physical_disk_unadopted_list(&opctx, collection_id) .await .expect("Failed to list uninitialized disks"); assert_eq!(uninitialized_disks.len(), 3); @@ -1241,7 +1241,7 @@ mod test { .unwrap(); let uninitialized_disks = datastore - .physical_disk_uninitialized_list(&opctx, collection_id) + .physical_disk_unadopted_list(&opctx, collection_id) .await .expect("Failed to list uninitialized disks"); assert_eq!(uninitialized_disks.len(), 0); @@ -1276,7 +1276,7 @@ mod test { // The set of uninitialized disks should include the expunged/deleted // disks let uninitialized_disks = datastore - .physical_disk_uninitialized_list(&opctx, collection_id) + .physical_disk_unadopted_list(&opctx, collection_id) .await .expect("Failed to list uninitialized disks"); assert_eq!(uninitialized_disks.len(), 2); @@ -1336,7 +1336,7 @@ mod test { // All 6 disks should be uninitialized let uninitialized_disks = datastore - .physical_disk_uninitialized_list(&opctx, collection_id) + .physical_disk_unadopted_list(&opctx, collection_id) .await .expect("Failed to list uninitialized disks"); assert_eq!(uninitialized_disks.len(), 6); @@ -1366,7 +1366,7 @@ mod test { // The remaining 4 disks should be uninitialized let uninitialized_disks = datastore - .physical_disk_uninitialized_list(&opctx, collection_id) + .physical_disk_unadopted_list(&opctx, collection_id) .await .expect("Failed to list uninitialized disks"); assert_eq!(uninitialized_disks.len(), 4); @@ -1392,7 +1392,7 @@ mod test { // All 6 disks should be uninitialized again let uninitialized_disks = datastore - .physical_disk_uninitialized_list(&opctx, collection_id) + .physical_disk_unadopted_list(&opctx, collection_id) .await .expect("Failed to list uninitialized disks"); assert_eq!(uninitialized_disks.len(), 6); @@ -1410,7 +1410,7 @@ mod test { // We should now only have 5 unininitialized disks let uninitialized_disks = datastore - .physical_disk_uninitialized_list(&opctx, collection_id) + .physical_disk_unadopted_list(&opctx, collection_id) .await .expect("Failed to list uninitialized disks"); assert_eq!(uninitialized_disks.len(), 5); @@ -1419,7 +1419,7 @@ mod test { // will not make the disk show up as adoptable. We allow the request to // succeed but not the adoption. This means an operator can insert the // disk after the fact. - let mut disk_not_in_inventory: PhysicalDiskId = + let mut disk_not_in_inventory: PhysicalDiskManufacturerIdentity = disks_a[0].identity.clone().into(); disk_not_in_inventory.vendor = "some-other-vendor".to_string(); datastore @@ -1434,7 +1434,7 @@ mod test { // We should still have only 5 unininitialized disks let uninitialized_disks = datastore - .physical_disk_uninitialized_list(&opctx, collection_id) + .physical_disk_unadopted_list(&opctx, collection_id) .await .expect("Failed to list uninitialized disks"); assert_eq!(uninitialized_disks.len(), 5); diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index f4b528309f6..5635f85b8d3 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -217,10 +217,10 @@ networking_switch_port_apply_settings POST /v1/system/hardware/switch-por networking_switch_port_clear_settings DELETE /v1/system/hardware/switch-port/{port}/settings networking_switch_port_list GET /v1/system/hardware/switch-port networking_switch_port_status GET /v1/system/hardware/switch-port/{port}/status -physical_disk_adopt PUT /v1/system/hardware/disks +physical_disk_enable_adoption PUT /v1/system/hardware/disks physical_disk_list GET /v1/system/hardware/disks physical_disk_list_adoption_requests GET /v1/system/hardware/disk-adoption-requests -physical_disk_list_uninitialized GET /v1/system/hardware/disks-uninitialized +physical_disk_list_unadopted GET /v1/system/hardware/disks-unadopted physical_disk_view GET /v1/system/hardware/disks/{disk_id} rack_list GET /v1/system/hardware/racks rack_view GET /v1/system/hardware/racks/{rack_id} diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 5f8841eaf6d..805b8efa219 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -6683,24 +6683,22 @@ pub trait NexusExternalApi { path_params: Path, ) -> Result, HttpError>; - /// List uninitialized physical disks + /// List physical disks that have not yet been adopted for use #[endpoint { method = GET, - path = "/v1/system/hardware/disks-uninitialized", + path = "/v1/system/hardware/disks-unadopted", tags = ["system/hardware"], versions = VERSION_MANUAL_DISK_ADOPTION.. }] - async fn physical_disk_list_uninitialized( + async fn physical_disk_list_unadopted( rqctx: RequestContext, query: Query>, ) -> Result< - HttpResponseOk< - ResultsPage, - >, + HttpResponseOk>, HttpError, >; - /// List physical disks adoption requests + /// List physical disk adoption requests #[endpoint { method = GET, path = "/v1/system/hardware/disk-adoption-requests", @@ -6717,16 +6715,16 @@ pub trait NexusExternalApi { HttpError, >; - /// Adopt a physical disk for usage by the control plane + /// Enable adoption of a physical disk for general use #[endpoint { method = PUT, path = "/v1/system/hardware/disks", tags = ["system/hardware"], versions = VERSION_MANUAL_DISK_ADOPTION.. }] - async fn physical_disk_adopt( + async fn physical_disk_enable_adoption( rqctx: RequestContext, - req: TypedBody, + req: TypedBody, ) -> Result; // Switches diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index e575cc17941..10eb4f5d961 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -283,7 +283,7 @@ impl super::Nexus { .await } - pub(crate) async fn physical_disk_list_uninitialized( + pub(crate) async fn physical_disk_list_unadopted( &self, opctx: &OpContext, ) -> ListResultVec { @@ -293,7 +293,7 @@ impl super::Nexus { return Ok(vec![]); }; self.db_datastore - .physical_disk_uninitialized_list(opctx, collection_id) + .physical_disk_unadopted_list(opctx, collection_id) .await } @@ -310,7 +310,7 @@ impl super::Nexus { pub(crate) async fn physical_disk_adopt( &self, opctx: &OpContext, - disk_id: nexus_types::external_api::physical_disk::PhysicalDiskId, + disk_id: nexus_types::external_api::physical_disk::PhysicalDiskManufacturerIdentity, ) -> Result<(), Error> { self.db_datastore.physical_disk_adopt(opctx, disk_id).await } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 845ab381f68..01f15355228 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -57,8 +57,8 @@ use nexus_types::external_api::image::Image; use nexus_types::external_api::ip_pool::{IpPool, IpPoolRange}; use nexus_types::external_api::metrics::SystemMetricsPathParam; use nexus_types::external_api::physical_disk::{ - PhysicalDisk, PhysicalDiskAdoptionRequest, PhysicalDiskId, - UninitializedPhysicalDisk, + PhysicalDisk, PhysicalDiskAdoptionRequest, + PhysicalDiskManufacturerIdentity, Unadopted, }; use nexus_types::external_api::probe::ProbeInfo; use nexus_types::external_api::project::Project; @@ -6705,11 +6705,10 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } - async fn physical_disk_list_uninitialized( + async fn physical_disk_list_unadopted( rqctx: RequestContext, query: Query>, - ) -> Result>, HttpError> - { + ) -> Result>, HttpError> { let apictx = rqctx.context(); let pag_params = query.into_inner(); if let dropshot::WhichPage::Next(last_seen) = &pag_params.page { @@ -6724,14 +6723,14 @@ impl NexusExternalApi for NexusExternalApiImpl { let opctx = crate::context::op_context_for_external_api(&rqctx).await?; let disks = nexus - .physical_disk_list_uninitialized(&opctx) + .physical_disk_list_unadopted(&opctx) .await? .into_iter() - .map(|d| UninitializedPhysicalDisk { + .map(|d| Unadopted { sled_id: d.sled_id.into(), slot: d.slot as u64, variant: d.variant.into(), - disk_id: PhysicalDiskId { + disk_id: PhysicalDiskManufacturerIdentity { vendor: d.vendor, serial: d.serial, model: d.model, @@ -6782,9 +6781,9 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } - async fn physical_disk_adopt( + async fn physical_disk_enable_adoption( rqctx: RequestContext, - req: TypedBody, + req: TypedBody, ) -> Result { audit_and_time(&rqctx, |opctx, nexus| async move { nexus.physical_disk_adopt(&opctx, req.into_inner()).await?; diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 51ea5732911..19ac42d5001 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -123,8 +123,8 @@ pub static HARDWARE_SWITCH_URL: LazyLock = LazyLock::new(|| format!("/v1/system/hardware/switches/{}", SWITCH_UUID)); pub static DEMO_HARDWARE_PHYSICAL_DISK_ID: LazyLock< - physical_disk::PhysicalDiskId, -> = LazyLock::new(|| physical_disk::PhysicalDiskId { + physical_disk::PhysicalDiskManufacturerIdentity, +> = LazyLock::new(|| physical_disk::PhysicalDiskManufacturerIdentity { vendor: "test".into(), serial: "test".into(), model: "test".into(), @@ -132,8 +132,8 @@ pub static DEMO_HARDWARE_PHYSICAL_DISK_ID: LazyLock< pub static HARDWARE_DISK_ADOPTION_REQUESTS_URL: &'static str = "/v1/system/hardware/disk-adoption-requests"; -pub static HARDWARE_DISKS_UNINITIALIZED_URL: &'static str = - "/v1/system/hardware/disks-uninitialized"; +pub static HARDWARE_DISKS_UNADOPTED_URL: &'static str = + "/v1/system/hardware/disks-unadopted"; pub const HARDWARE_DISKS_URL: &'static str = "/v1/system/hardware/disks"; pub static HARDWARE_DISK_URL: LazyLock = LazyLock::new(|| { @@ -3015,7 +3015,7 @@ pub static VERIFY_ENDPOINTS: LazyLock> = LazyLock::new( allowed_methods: vec![AllowedMethod::Get], }, VerifyEndpoint { - url: &HARDWARE_DISKS_UNINITIALIZED_URL, + url: &HARDWARE_DISKS_UNADOPTED_URL, visibility: Visibility::Public, unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![AllowedMethod::Get], diff --git a/nexus/types/versions/src/latest.rs b/nexus/types/versions/src/latest.rs index 580c2d59564..5b8266ca66f 100644 --- a/nexus/types/versions/src/latest.rs +++ b/nexus/types/versions/src/latest.rs @@ -450,8 +450,8 @@ pub mod physical_disk { // Types from MANUAL_DISK_ADOPTION. pub use crate::v2026_04_02_00::physical_disk::PhysicalDiskAdoptionRequest; - pub use crate::v2026_04_02_00::physical_disk::PhysicalDiskId; - pub use crate::v2026_04_02_00::physical_disk::UninitializedPhysicalDisk; + pub use crate::v2026_04_02_00::physical_disk::PhysicalDiskManufacturerIdentity; + pub use crate::v2026_04_02_00::physical_disk::Unadopted; } pub mod rack { diff --git a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs index 061163dbadb..ed067ad4b79 100644 --- a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs +++ b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs @@ -12,22 +12,24 @@ use crate::v2025_11_20_00::physical_disk::PhysicalDiskKind; /// A physical disk that has not yet been adopted by the control plane #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] -pub struct UninitializedPhysicalDisk { +pub struct Unadopted { pub sled_id: SledUuid, pub slot: u64, pub variant: PhysicalDiskKind, - pub disk_id: PhysicalDiskId, + pub disk_id: PhysicalDiskManufacturerIdentity, } -/// The unique identity of a physical disk +/// The unique identity of a physical disk provided by the manufacturer #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] -pub struct PhysicalDiskId { +pub struct PhysicalDiskManufacturerIdentity { pub vendor: String, pub serial: String, pub model: String, } -impl From for PhysicalDiskId { +impl From + for PhysicalDiskManufacturerIdentity +{ fn from(value: omicron_common::disk::DiskIdentity) -> Self { Self { vendor: value.vendor, serial: value.serial, model: value.model } } diff --git a/openapi/nexus/nexus-2026040200.0.0-430bef.json b/openapi/nexus/nexus-2026040200.0.0-a5ad49.json similarity index 99% rename from openapi/nexus/nexus-2026040200.0.0-430bef.json rename to openapi/nexus/nexus-2026040200.0.0-a5ad49.json index 2c44a8cbe98..8e4489ef5aa 100644 --- a/openapi/nexus/nexus-2026040200.0.0-430bef.json +++ b/openapi/nexus/nexus-2026040200.0.0-a5ad49.json @@ -7700,7 +7700,7 @@ "tags": [ "system/hardware" ], - "summary": "List physical disks adoption requests", + "summary": "List physical disk adoption requests", "operationId": "physical_disk_list_adoption_requests", "parameters": [ { @@ -7816,13 +7816,13 @@ "tags": [ "system/hardware" ], - "summary": "Adopt a physical disk for usage by the control plane", - "operationId": "physical_disk_adopt", + "summary": "Enable adoption of a physical disk for general use", + "operationId": "physical_disk_enable_adoption", "requestBody": { "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/PhysicalDiskId" + "$ref": "#/components/schemas/PhysicalDiskManufacturerIdentity" } } }, @@ -7880,13 +7880,13 @@ } } }, - "/v1/system/hardware/disks-uninitialized": { + "/v1/system/hardware/disks-unadopted": { "get": { "tags": [ "system/hardware" ], - "summary": "List uninitialized physical disks", - "operationId": "physical_disk_list_uninitialized", + "summary": "List physical disks that have not yet been adopted for use", + "operationId": "physical_disk_list_unadopted", "parameters": [ { "in": "query", @@ -7915,7 +7915,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/UninitializedPhysicalDiskResultsPage" + "$ref": "#/components/schemas/UnadoptedResultsPage" } } } @@ -25377,8 +25377,16 @@ "items" ] }, - "PhysicalDiskId": { - "description": "The unique identity of a physical disk", + "PhysicalDiskKind": { + "description": "Describes the form factor of physical disks.", + "type": "string", + "enum": [ + "m2", + "u2" + ] + }, + "PhysicalDiskManufacturerIdentity": { + "description": "The unique identity of a physical disk provided by the manufacturer", "type": "object", "properties": { "model": { @@ -25397,14 +25405,6 @@ "vendor" ] }, - "PhysicalDiskKind": { - "description": "Describes the form factor of physical disks.", - "type": "string", - "enum": [ - "m2", - "u2" - ] - }, "PhysicalDiskPolicy": { "description": "The operator-defined policy of a physical disk.", "oneOf": [ @@ -29885,12 +29885,12 @@ } } }, - "UninitializedPhysicalDisk": { + "Unadopted": { "description": "A physical disk that has not yet been adopted by the control plane", "type": "object", "properties": { "disk_id": { - "$ref": "#/components/schemas/PhysicalDiskId" + "$ref": "#/components/schemas/PhysicalDiskManufacturerIdentity" }, "sled_id": { "$ref": "#/components/schemas/SledUuid" @@ -29911,7 +29911,7 @@ "variant" ] }, - "UninitializedPhysicalDiskResultsPage": { + "UnadoptedResultsPage": { "description": "A single page of results", "type": "object", "properties": { @@ -29919,7 +29919,7 @@ "description": "list of items on this page of results", "type": "array", "items": { - "$ref": "#/components/schemas/UninitializedPhysicalDisk" + "$ref": "#/components/schemas/Unadopted" } }, "next_page": { diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index 4aacdb7d880..ed3a6c84b07 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026040200.0.0-430bef.json \ No newline at end of file +nexus-2026040200.0.0-a5ad49.json \ No newline at end of file From af22bb6e0927bac1aed126e3e53432e4bf524bde Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Thu, 9 Apr 2026 20:30:17 +0000 Subject: [PATCH 06/26] more small fixes --- nexus/db-model/src/physical_disk.rs | 1 - .../db-queries/src/db/datastore/physical_disk.rs | 16 ++++++++-------- nexus/src/external_api/http_entrypoints.rs | 2 +- .../src/manual_disk_adoption/physical_disk.rs | 3 +-- schema/crdb/dbinit.sql | 10 +++++----- 5 files changed, 15 insertions(+), 17 deletions(-) diff --git a/nexus/db-model/src/physical_disk.rs b/nexus/db-model/src/physical_disk.rs index 3e09edf990b..1e9afbd0d2a 100644 --- a/nexus/db-model/src/physical_disk.rs +++ b/nexus/db-model/src/physical_disk.rs @@ -123,7 +123,6 @@ impl From serial: req.serial, model: req.model, time_created: req.time_created, - time_deleted: req.time_deleted, } } } diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index 0b43dd3bd12..e11d798efbf 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -1028,7 +1028,7 @@ mod test { // We need an adoption request before we can adopt the disk // Adoption is idempotent datastore - .physical_disk_adopt(&opctx, disk.into()) + .physical_disk_adopt(&opctx, disk.clone().into()) .await .expect("adoption succeeds"); datastore @@ -1051,7 +1051,7 @@ mod test { // We don't want it to fail because it doesn't have an adoption request // though datastore - .physical_disk_adopt(&opctx, disk.into()) + .physical_disk_adopt(&opctx, disk.clone().into()) .await .expect("adoption succeeds"); let err = datastore @@ -1150,7 +1150,7 @@ mod test { let (disk_001, zpool) = create_disk_zpool_combo(sled_a.id(), &disks_a[0]); datastore - .physical_disk_adopt(&opctx, (&disk_001).into()) + .physical_disk_adopt(&opctx, disk_001.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1160,7 +1160,7 @@ mod test { let (disk_002, zpool) = create_disk_zpool_combo(sled_a.id(), &disks_a[1]); datastore - .physical_disk_adopt(&opctx, (&disk_002).into()) + .physical_disk_adopt(&opctx, disk_002.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1170,7 +1170,7 @@ mod test { let (disk_101, zpool) = create_disk_zpool_combo(sled_b.id(), &disks_b[0]); datastore - .physical_disk_adopt(&opctx, (&disk_101).into()) + .physical_disk_adopt(&opctx, disk_101.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1212,7 +1212,7 @@ mod test { let (disk_003, zpool) = create_disk_zpool_combo(sled_a.id(), &disks_a[2]); datastore - .physical_disk_adopt(&opctx, (&disk_003).into()) + .physical_disk_adopt(&opctx, disk_003.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1222,7 +1222,7 @@ mod test { let (disk_102, zpool) = create_disk_zpool_combo(sled_b.id(), &disks_b[1]); datastore - .physical_disk_adopt(&opctx, (&disk_102).into()) + .physical_disk_adopt(&opctx, disk_102.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1232,7 +1232,7 @@ mod test { let (disk_103, zpool) = create_disk_zpool_combo(sled_b.id(), &disks_b[2]); datastore - .physical_disk_adopt(&opctx, (&disk_103).into()) + .physical_disk_adopt(&opctx, disk_103.clone().into()) .await .expect("adoption request succeeds"); datastore diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 01f15355228..04c6efc732a 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6728,7 +6728,7 @@ impl NexusExternalApi for NexusExternalApiImpl { .into_iter() .map(|d| Unadopted { sled_id: d.sled_id.into(), - slot: d.slot as u64, + slot: d.slot, variant: d.variant.into(), disk_id: PhysicalDiskManufacturerIdentity { vendor: d.vendor, diff --git a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs index ed067ad4b79..80652d23926 100644 --- a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs +++ b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs @@ -14,7 +14,7 @@ use crate::v2025_11_20_00::physical_disk::PhysicalDiskKind; #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] pub struct Unadopted { pub sled_id: SledUuid, - pub slot: u64, + pub slot: i64, pub variant: PhysicalDiskKind, pub disk_id: PhysicalDiskManufacturerIdentity, } @@ -43,5 +43,4 @@ pub struct PhysicalDiskAdoptionRequest { pub serial: String, pub model: String, pub time_created: chrono::DateTime, - pub time_deleted: Option>, } diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 670eb6ece57..803f4099712 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -504,11 +504,11 @@ CREATE UNIQUE INDEX IF NOT EXISTS lookup_physical_disk_by_sled ON omicron.public id ); --- Any disk present in this table with a NON-NULL time_deleted column +-- Any disk present in this table with a NULL time_deleted column -- is eligible for adoption. CREATE TABLE IF NOT EXISTS physical_disk_adoption_request ( id UUID PRIMARY KEY, - vendor STRING(63) NOT NULL, + vendor STRING(63) NOT NULL, model STRING(63) NOT NULL, serial STRING(63) NOT NULL, time_created TIMESTAMPTZ NOT NULL, @@ -518,9 +518,9 @@ CREATE TABLE IF NOT EXISTS physical_disk_adoption_request ( -- Only one eligible disk with the same physical id may be present at a time CREATE UNIQUE INDEX IF NOT EXISTS physical_disk_adoption_request_by_physical_id ON physical_disk_adoption_request ( - vendor, - model, - serial + vendor, + model, + serial ) WHERE time_deleted IS NULL; -- x509 certificates which may be used by services From b246e49ff319848bea41fd5de43e010997d25c41 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Thu, 9 Apr 2026 21:17:03 +0000 Subject: [PATCH 07/26] Do not error bg task for not found --- nexus/db-queries/src/db/datastore/physical_disk.rs | 14 +++++++------- .../app/background/tasks/physical_disk_adoption.rs | 13 +++++++++++++ ...5ad49.json => nexus-2026040200.0.0-c4bc2e.json} | 8 +------- openapi/nexus/nexus-latest.json | 2 +- 4 files changed, 22 insertions(+), 15 deletions(-) rename openapi/nexus/{nexus-2026040200.0.0-a5ad49.json => nexus-2026040200.0.0-c4bc2e.json} (99%) diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index e11d798efbf..4d22a911627 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -547,13 +547,13 @@ impl DataStore { .execute_async(conn) .await?; - bail_unless!( - rows_modified == 1, - "No adoption request for physical disk: {}:{}:{}", - id.vendor, - id.serial, - id.model - ); + if rows_modified != 1 { + return Err(Error::non_resourcetype_not_found(format!( + "No adoption request found for physical disk: {}:{}:{}", + id.vendor, id.serial, id.model + )) + .into()); + } Ok(()) } diff --git a/nexus/src/app/background/tasks/physical_disk_adoption.rs b/nexus/src/app/background/tasks/physical_disk_adoption.rs index ddae786425a..9f9359eaaa2 100644 --- a/nexus/src/app/background/tasks/physical_disk_adoption.rs +++ b/nexus/src/app/background/tasks/physical_disk_adoption.rs @@ -21,6 +21,7 @@ use nexus_db_queries::context::OpContext; use nexus_db_queries::db::DataStore; use nexus_types::identity::Asset; use nexus_types::inventory::Collection; +use omicron_common::api::external; use omicron_common::api::external::DataPageParams; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::ZpoolUuid; @@ -165,6 +166,18 @@ impl BackgroundTask for PhysicalDiskAdoption { .await; if let Err(err) = result { + // Skip reporting the error if we get back a `NotFound`. + // This means that another nexus concurrently added the + // disk or that the adoptable request was deleted. We + // don't want to report mistakenly one way or another and + // so we just return an empty result here. The nexus that + // actually added the disk or cancelled the request will + // log the result and report the adoption to the task output + // if necessary. + if let external::Error::NotFound { .. } = err { + return json!({}); + } + let err = InlineErrorChain::new(&err); warn!( &opctx.log, diff --git a/openapi/nexus/nexus-2026040200.0.0-a5ad49.json b/openapi/nexus/nexus-2026040200.0.0-c4bc2e.json similarity index 99% rename from openapi/nexus/nexus-2026040200.0.0-a5ad49.json rename to openapi/nexus/nexus-2026040200.0.0-c4bc2e.json index 8e4489ef5aa..4c244e482fd 100644 --- a/openapi/nexus/nexus-2026040200.0.0-a5ad49.json +++ b/openapi/nexus/nexus-2026040200.0.0-c4bc2e.json @@ -25339,11 +25339,6 @@ "type": "string", "format": "date-time" }, - "time_deleted": { - "nullable": true, - "type": "string", - "format": "date-time" - }, "vendor": { "type": "string" } @@ -29897,8 +29892,7 @@ }, "slot": { "type": "integer", - "format": "uint64", - "minimum": 0 + "format": "int64" }, "variant": { "$ref": "#/components/schemas/PhysicalDiskKind" diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index ed3a6c84b07..1b312141ff9 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026040200.0.0-a5ad49.json \ No newline at end of file +nexus-2026040200.0.0-c4bc2e.json \ No newline at end of file From df9afa60a5e6d338da85bcfb40074bca54ede5ea Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Thu, 9 Apr 2026 21:19:47 +0000 Subject: [PATCH 08/26] Fix my fix --- nexus/src/app/background/tasks/physical_disk_adoption.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/nexus/src/app/background/tasks/physical_disk_adoption.rs b/nexus/src/app/background/tasks/physical_disk_adoption.rs index 9f9359eaaa2..adacff2e6bd 100644 --- a/nexus/src/app/background/tasks/physical_disk_adoption.rs +++ b/nexus/src/app/background/tasks/physical_disk_adoption.rs @@ -170,12 +170,9 @@ impl BackgroundTask for PhysicalDiskAdoption { // This means that another nexus concurrently added the // disk or that the adoptable request was deleted. We // don't want to report mistakenly one way or another and - // so we just return an empty result here. The nexus that - // actually added the disk or cancelled the request will - // log the result and report the adoption to the task output - // if necessary. + // so we just continue here. if let external::Error::NotFound { .. } = err { - return json!({}); + continue; } let err = InlineErrorChain::new(&err); From 1fe99e068773cff6087f4488e7724abc2fcf5d4c Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Thu, 9 Apr 2026 22:20:24 +0000 Subject: [PATCH 09/26] Try to fix some queries --- .../src/db/datastore/physical_disk.rs | 165 ++++++++++-------- 1 file changed, 90 insertions(+), 75 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index 4d22a911627..00f8273666f 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -23,6 +23,9 @@ use crate::db::pagination::paginated; use async_bb8_diesel::AsyncRunQueryDsl; use chrono::Utc; use diesel::prelude::*; +use diesel::result::{ + DatabaseErrorKind as DieselErrorKind, Error as DieselError, +}; use nexus_db_errors::ErrorHandler; use nexus_db_errors::OptionalError; use nexus_db_errors::TransactionError; @@ -38,7 +41,6 @@ use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupType; use omicron_common::api::external::ResourceType; -use omicron_common::bail_unless; use omicron_uuid_kinds::CollectionUuid; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::PhysicalDiskUuid; @@ -117,8 +119,8 @@ impl DataStore { /// as long as a non-expunged disk isn't present in the `physical_disk` /// table for the same disk_id. /// - /// This request is idempotent. If the disk is already adopted, or an - /// adoption request exists, success is returned. + /// This request is idempotent. If an existing adoption request exists, + /// success is returned. pub async fn physical_disk_adopt( &self, opctx: &OpContext, @@ -130,11 +132,13 @@ impl DataStore { use nexus_db_schema::schema::physical_disk_adoption_request::dsl as adoption_dsl; let conn = &*self.pool_connection_authorized(opctx).await?; + let err = OptionalError::>::new(); self.transaction_retry_wrapper("physical_disk_adopt") .transaction(&conn, |conn| { let vendor = disk_id.vendor.clone(); let serial = disk_id.serial.clone(); let model = disk_id.model.clone(); + let err = err.clone(); async move { // Check for an active (non-expunged) physical disk with // the same vendor/serial/model. @@ -153,28 +157,17 @@ impl DataStore { .optional()?; if active_disk_exists.is_some() { - return Ok(()); - } - - // Check for an active adoption request with the same - // vendor/serial/model. - let active_request_exists = - adoption_dsl::physical_disk_adoption_request - .filter(adoption_dsl::vendor.eq(vendor.clone())) - .filter(adoption_dsl::serial.eq(serial.clone())) - .filter(adoption_dsl::model.eq(model.clone())) - .filter(adoption_dsl::time_deleted.is_null()) - .select(adoption_dsl::id) - .first_async::(&conn) - .await - .optional()?; - - if active_request_exists.is_some() { - return Ok(()); + return Err(err.bail( + Error::conflict(format!( + "physical disk already adopted: {}:{}:{}", + vendor, model, serial + )) + .into(), + )); } // Insert the adoption request. - diesel::insert_into( + if let Err(err) = diesel::insert_into( adoption_dsl::physical_disk_adoption_request, ) .values(( @@ -185,13 +178,35 @@ impl DataStore { adoption_dsl::time_created.eq(Utc::now()), )) .execute_async(&conn) - .await?; + .await + { + // Check for a unique index violation for an active + // adoption request with the same vendor/serial/model. + // + // Return Ok(()) in this case as the request is idempotent. + match err { + DieselError::DatabaseError( + DieselErrorKind::UniqueViolation, + _, + ) => { + return Ok(()); + } + _ => Err(err)?, + } + } Ok(()) } }) .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + .map_err(|e| { + match err.take() { + // A called function performed its own error propagation. + Some(txn_error) => txn_error.into_public_ignore_retries(), + // The transaction setup/teardown itself encountered a diesel error. + None => public_error_from_diesel(e, ErrorHandler::Server), + } + })?; Ok(()) } @@ -1071,23 +1086,23 @@ mod test { } #[tokio::test] - async fn physical_disk_uninitialized_list() { - let logctx = dev::test_setup_log("physical_disk_uninitialized_list"); + async fn physical_disk_unadopted_list() { + let logctx = dev::test_setup_log("physical_disk_unadopted_list"); let db = TestDatabase::new_with_datastore(&logctx.log).await; let (opctx, datastore) = (db.opctx(), db.datastore()); let sled_a = create_test_sled(&datastore).await; let sled_b = create_test_sled(&datastore).await; - // No inventory -> No uninitialized disks - let uninitialized_disks = datastore + // No inventory -> No unadopted disks + let unadopted_disks = datastore .physical_disk_unadopted_list( &opctx, CollectionUuid::new_v4(), // Collection that does not exist ) .await - .expect("Failed to look up uninitialized disks"); - assert!(uninitialized_disks.is_empty()); + .expect("Failed to look up unadopted disks"); + assert!(unadopted_disks.is_empty()); // Create inventory disks for both sleds let mut builder = nexus_inventory::CollectionBuilder::new("test"); @@ -1110,22 +1125,22 @@ mod test { .await .expect("failed to insert collection"); - // Now when we list the uninitialized disks, we should see everything in + // Now when we list the unadopted disks, we should see everything in // the inventory. - let uninitialized_disks = datastore + let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await - .expect("Failed to list uninitialized disks"); - assert_eq!(uninitialized_disks.len(), 6); + .expect("Failed to list unadopted disks"); + assert_eq!(unadopted_disks.len(), 6); // Normalize the data a bit -- convert to nexus types, and sort vecs for // stability in the comparison. - let mut uninitialized_disks: Vec = - uninitialized_disks + let mut unadopted_disks: Vec = + unadopted_disks .into_iter() .map(|d| inv_phys_disk_to_nexus_phys_disk(&d)) .collect(); - uninitialized_disks + unadopted_disks .sort_by(|a, b| a.identity.partial_cmp(&b.identity).unwrap()); let mut expected_disks: Vec = disks_a @@ -1135,14 +1150,14 @@ mod test { .collect(); expected_disks .sort_by(|a, b| a.identity.partial_cmp(&b.identity).unwrap()); - assert_eq!(uninitialized_disks, expected_disks); + assert_eq!(unadopted_disks, expected_disks); // Let's create control plane objects for some of these disks. // - // They should no longer show up when we list uninitialized devices. + // They should no longer show up when we list unadopted devices. // // This creates disks for: 001, 002, and 101. - // It leaves the following uninitialized: 003, 102, 103 + // It leaves the following unadopted: 003, 102, 103 // // We make sure we submit an adoption request before we attempt to // create the disk and zpools as we don't want automatic adoption in the @@ -1178,23 +1193,23 @@ mod test { .await .unwrap(); - let uninitialized_disks = datastore + let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await - .expect("Failed to list uninitialized disks"); - assert_eq!(uninitialized_disks.len(), 3); + .expect("Failed to list unadopted disks"); + assert_eq!(unadopted_disks.len(), 3); // Pay careful attention to our indexing below. // // We're grabbing the last disk of "disks_a" (which still is // uninitailized) and the last two disks of "disks_b" (of which both are - // still uninitialized). - let mut uninitialized_disks: Vec = - uninitialized_disks + // still unadopted). + let mut unadopted_disks: Vec = + unadopted_disks .into_iter() .map(|d| inv_phys_disk_to_nexus_phys_disk(&d)) .collect(); - uninitialized_disks + unadopted_disks .sort_by(|a, b| a.identity.partial_cmp(&b.identity).unwrap()); let mut expected_disks: Vec = disks_a[2..3] @@ -1204,11 +1219,11 @@ mod test { .collect(); expected_disks .sort_by(|a, b| a.identity.partial_cmp(&b.identity).unwrap()); - assert_eq!(uninitialized_disks, expected_disks); + assert_eq!(unadopted_disks, expected_disks); // Create physical disks for all remaining devices. // - // Observe no remaining uninitialized disks. + // Observe no remaining unadopted disks. let (disk_003, zpool) = create_disk_zpool_combo(sled_a.id(), &disks_a[2]); datastore @@ -1240,13 +1255,13 @@ mod test { .await .unwrap(); - let uninitialized_disks = datastore + let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await - .expect("Failed to list uninitialized disks"); - assert_eq!(uninitialized_disks.len(), 0); + .expect("Failed to list unadopted disks"); + assert_eq!(unadopted_disks.len(), 0); - // Expunge some disks, observe that they are now uninitialized again. This allows + // Expunge some disks, observe that they are now unadopted again. This allows // re-adding them to the control plane. use nexus_db_schema::schema::physical_disk::dsl; @@ -1273,13 +1288,13 @@ mod test { .await .unwrap(); - // The set of uninitialized disks should include the expunged/deleted + // The set of unadopted disks should include the expunged/deleted // disks - let uninitialized_disks = datastore + let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await - .expect("Failed to list uninitialized disks"); - assert_eq!(uninitialized_disks.len(), 2); + .expect("Failed to list unadopted disks"); + assert_eq!(unadopted_disks.len(), 2); db.terminate().await; logctx.cleanup_successful(); @@ -1334,12 +1349,12 @@ mod test { .expect("Failed to look up adoptable disks"); assert!(adoptable_disks.is_empty()); - // All 6 disks should be uninitialized - let uninitialized_disks = datastore + // All 6 disks should be unadopted + let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await - .expect("Failed to list uninitialized disks"); - assert_eq!(uninitialized_disks.len(), 6); + .expect("Failed to list unadopted disks"); + assert_eq!(unadopted_disks.len(), 6); // Make two disks adoptable datastore @@ -1364,12 +1379,12 @@ mod test { .expect("Failed to look up adoptable disks"); assert_eq!(adoptable_disks.len(), 2); - // The remaining 4 disks should be uninitialized - let uninitialized_disks = datastore + // The remaining 4 disks should be unadopted + let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await - .expect("Failed to list uninitialized disks"); - assert_eq!(uninitialized_disks.len(), 4); + .expect("Failed to list unadopted disks"); + assert_eq!(unadopted_disks.len(), 4); // soft-deleting both adoptable disks makes them unadoptable again DataStore::physical_disk_adoption_request_delete_on_connection( @@ -1390,12 +1405,12 @@ mod test { .expect("Failed to look up adoptable disks"); assert!(adoptable_disks.is_empty()); - // All 6 disks should be uninitialized again - let uninitialized_disks = datastore + // All 6 disks should be unadopted again + let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await - .expect("Failed to list uninitialized disks"); - assert_eq!(uninitialized_disks.len(), 6); + .expect("Failed to list unadopted disks"); + assert_eq!(unadopted_disks.len(), 6); // Adding one of the same disks back again should make it adoptable datastore @@ -1409,11 +1424,11 @@ mod test { assert_eq!(adoptable_disks.len(), 1); // We should now only have 5 unininitialized disks - let uninitialized_disks = datastore + let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await - .expect("Failed to list uninitialized disks"); - assert_eq!(uninitialized_disks.len(), 5); + .expect("Failed to list unadopted disks"); + assert_eq!(unadopted_disks.len(), 5); // Making a disk adoption reqeust for a disk that is not in inventory // will not make the disk show up as adoptable. We allow the request to @@ -1433,11 +1448,11 @@ mod test { assert_eq!(adoptable_disks.len(), 1); // We should still have only 5 unininitialized disks - let uninitialized_disks = datastore + let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await - .expect("Failed to list uninitialized disks"); - assert_eq!(uninitialized_disks.len(), 5); + .expect("Failed to list unadopted disks"); + assert_eq!(unadopted_disks.len(), 5); db.terminate().await; logctx.cleanup_successful(); From b448340caf609333b0e5b78bb75b941d53d220cc Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Thu, 9 Apr 2026 23:06:45 +0000 Subject: [PATCH 10/26] fix unique constraint handling --- .../src/db/datastore/physical_disk.rs | 58 ++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index 00f8273666f..3eb33f73e95 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -133,7 +133,8 @@ impl DataStore { let conn = &*self.pool_connection_authorized(opctx).await?; let err = OptionalError::>::new(); - self.transaction_retry_wrapper("physical_disk_adopt") + let txn_res = self + .transaction_retry_wrapper("physical_disk_adopt") .transaction(&conn, |conn| { let vendor = disk_id.vendor.clone(); let serial = disk_id.serial.clone(); @@ -167,7 +168,7 @@ impl DataStore { } // Insert the adoption request. - if let Err(err) = diesel::insert_into( + diesel::insert_into( adoption_dsl::physical_disk_adoption_request, ) .values(( @@ -178,35 +179,40 @@ impl DataStore { adoption_dsl::time_created.eq(Utc::now()), )) .execute_async(&conn) - .await - { - // Check for a unique index violation for an active - // adoption request with the same vendor/serial/model. - // - // Return Ok(()) in this case as the request is idempotent. - match err { - DieselError::DatabaseError( - DieselErrorKind::UniqueViolation, - _, - ) => { - return Ok(()); - } - _ => Err(err)?, - } - } + .await?; Ok(()) } }) - .await - .map_err(|e| { - match err.take() { - // A called function performed its own error propagation. - Some(txn_error) => txn_error.into_public_ignore_retries(), - // The transaction setup/teardown itself encountered a diesel error. - None => public_error_from_diesel(e, ErrorHandler::Server), + .await; + + // Check for a unique index violation for an active + // adoption request with the same vendor/serial/model. + // + // Return Ok(()) in this case as the request is idempotent. + if let Err(e) = txn_res { + match err.take() { + // A called function performed its own error propagation. + Some(txn_error) => { + return Err(txn_error.into_public_ignore_retries()); } - })?; + // The transaction setup/teardown itself encountered a diesel error. + None => match e { + DieselError::DatabaseError( + DieselErrorKind::UniqueViolation, + _, + ) => { + return Ok(()); + } + _ => { + return Err(public_error_from_diesel( + e, + ErrorHandler::Server, + )); + } + }, + } + } Ok(()) } From daa5057386bbab9d9d129763ed6633e1d63291ea Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Fri, 10 Apr 2026 15:29:22 +0000 Subject: [PATCH 11/26] Use typed uuid --- nexus/db-model/src/physical_disk.rs | 5 +++-- nexus/src/external_api/http_entrypoints.rs | 4 +++- .../src/manual_disk_adoption/physical_disk.rs | 4 ++-- ...-c4bc2e.json => nexus-2026040200.0.0-4e9601.json} | 12 ++++++++++-- openapi/nexus/nexus-latest.json | 2 +- uuid-kinds/src/lib.rs | 1 + 6 files changed, 20 insertions(+), 8 deletions(-) rename openapi/nexus/{nexus-2026040200.0.0-c4bc2e.json => nexus-2026040200.0.0-4e9601.json} (99%) diff --git a/nexus/db-model/src/physical_disk.rs b/nexus/db-model/src/physical_disk.rs index 1e9afbd0d2a..598cf02d49f 100644 --- a/nexus/db-model/src/physical_disk.rs +++ b/nexus/db-model/src/physical_disk.rs @@ -12,6 +12,7 @@ use db_macros::Asset; use nexus_db_schema::schema::{physical_disk, zpool}; use nexus_types::external_api::physical_disk as physical_disk_types; use nexus_types::identity::Asset; +use omicron_uuid_kinds::PhysicalDiskAdoptionRequestKind; use omicron_uuid_kinds::PhysicalDiskKind as PhysicalDiskUuidKind; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::SledKind; @@ -105,7 +106,7 @@ impl From for physical_disk_types::PhysicalDisk { #[derive(Queryable, Insertable, Debug, Clone, Selectable)] #[diesel(table_name = nexus_db_schema::schema::physical_disk_adoption_request)] pub struct PhysicalDiskAdoptionRequest { - pub id: uuid::Uuid, + pub id: DbTypedUuid, pub vendor: String, pub serial: String, pub model: String, @@ -118,7 +119,7 @@ impl From { fn from(req: PhysicalDiskAdoptionRequest) -> Self { Self { - id: req.id, + id: req.id.into(), vendor: req.vendor, serial: req.serial, model: req.model, diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 04c6efc732a..eab59a32f58 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6771,7 +6771,9 @@ impl NexusExternalApi for NexusExternalApiImpl { Ok(HttpResponseOk(ScanById::results_page( &query, requests, - &|_, req: &PhysicalDiskAdoptionRequest| req.id, + &|_, req: &PhysicalDiskAdoptionRequest| { + req.id.into_untyped_uuid() + }, )?)) }; apictx diff --git a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs index 80652d23926..c85c110cc77 100644 --- a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs +++ b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs @@ -4,7 +4,7 @@ //! Physical disk types for version MANUAL_DISK_ADOPTION. -use omicron_uuid_kinds::SledUuid; +use omicron_uuid_kinds::{PhysicalDiskAdoptionRequestUuid, SledUuid}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -38,7 +38,7 @@ impl From /// A request to adopt a physical disk into the control plane #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] pub struct PhysicalDiskAdoptionRequest { - pub id: uuid::Uuid, + pub id: PhysicalDiskAdoptionRequestUuid, pub vendor: String, pub serial: String, pub model: String, diff --git a/openapi/nexus/nexus-2026040200.0.0-c4bc2e.json b/openapi/nexus/nexus-2026040200.0.0-4e9601.json similarity index 99% rename from openapi/nexus/nexus-2026040200.0.0-c4bc2e.json rename to openapi/nexus/nexus-2026040200.0.0-4e9601.json index 4c244e482fd..3d8d5fa179b 100644 --- a/openapi/nexus/nexus-2026040200.0.0-c4bc2e.json +++ b/openapi/nexus/nexus-2026040200.0.0-4e9601.json @@ -25326,8 +25326,7 @@ "type": "object", "properties": { "id": { - "type": "string", - "format": "uuid" + "$ref": "#/components/schemas/PhysicalDiskAdoptionRequestUuid" }, "model": { "type": "string" @@ -25372,6 +25371,15 @@ "items" ] }, + "PhysicalDiskAdoptionRequestUuid": { + "x-rust-type": { + "crate": "omicron-uuid-kinds", + "path": "omicron_uuid_kinds::PhysicalDiskAdoptionRequestUuid", + "version": "*" + }, + "type": "string", + "format": "uuid" + }, "PhysicalDiskKind": { "description": "Describes the form factor of physical disks.", "type": "string", diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index 1b312141ff9..db1f3d87eb2 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026040200.0.0-c4bc2e.json \ No newline at end of file +nexus-2026040200.0.0-4e9601.json \ No newline at end of file diff --git a/uuid-kinds/src/lib.rs b/uuid-kinds/src/lib.rs index 6df4b15ae70..167768e04f1 100644 --- a/uuid-kinds/src/lib.rs +++ b/uuid-kinds/src/lib.rs @@ -71,6 +71,7 @@ impl_typed_uuid_kinds! { OmicronSledConfig = {}, OmicronZone = {}, PhysicalDisk = {}, + PhysicalDiskAdoptionRequest = {}, Probe = {}, Propolis = {}, Rack = {}, From dcd1a066c6768daec57b06c6665fc8ad7cbd67a6 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Fri, 10 Apr 2026 15:43:55 +0000 Subject: [PATCH 12/26] add test and fix fn name --- .../src/db/datastore/physical_disk.rs | 90 ++++++++++++++++--- nexus/src/app/sled.rs | 2 +- 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index 3eb33f73e95..25f6c067395 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -121,7 +121,7 @@ impl DataStore { /// /// This request is idempotent. If an existing adoption request exists, /// success is returned. - pub async fn physical_disk_adopt( + pub async fn physical_disk_enable_adoption( &self, opctx: &OpContext, disk_id: PhysicalDiskManufacturerIdentity, @@ -1049,7 +1049,7 @@ mod test { // We need an adoption request before we can adopt the disk // Adoption is idempotent datastore - .physical_disk_adopt(&opctx, disk.clone().into()) + .physical_disk_enable_adoption(&opctx, disk.clone().into()) .await .expect("adoption succeeds"); datastore @@ -1072,7 +1072,7 @@ mod test { // We don't want it to fail because it doesn't have an adoption request // though datastore - .physical_disk_adopt(&opctx, disk.clone().into()) + .physical_disk_enable_adoption(&opctx, disk.clone().into()) .await .expect("adoption succeeds"); let err = datastore @@ -1171,7 +1171,7 @@ mod test { let (disk_001, zpool) = create_disk_zpool_combo(sled_a.id(), &disks_a[0]); datastore - .physical_disk_adopt(&opctx, disk_001.clone().into()) + .physical_disk_enable_adoption(&opctx, disk_001.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1181,7 +1181,7 @@ mod test { let (disk_002, zpool) = create_disk_zpool_combo(sled_a.id(), &disks_a[1]); datastore - .physical_disk_adopt(&opctx, disk_002.clone().into()) + .physical_disk_enable_adoption(&opctx, disk_002.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1191,7 +1191,7 @@ mod test { let (disk_101, zpool) = create_disk_zpool_combo(sled_b.id(), &disks_b[0]); datastore - .physical_disk_adopt(&opctx, disk_101.clone().into()) + .physical_disk_enable_adoption(&opctx, disk_101.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1233,7 +1233,7 @@ mod test { let (disk_003, zpool) = create_disk_zpool_combo(sled_a.id(), &disks_a[2]); datastore - .physical_disk_adopt(&opctx, disk_003.clone().into()) + .physical_disk_enable_adoption(&opctx, disk_003.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1243,7 +1243,7 @@ mod test { let (disk_102, zpool) = create_disk_zpool_combo(sled_b.id(), &disks_b[1]); datastore - .physical_disk_adopt(&opctx, disk_102.clone().into()) + .physical_disk_enable_adoption(&opctx, disk_102.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1253,7 +1253,7 @@ mod test { let (disk_103, zpool) = create_disk_zpool_combo(sled_b.id(), &disks_b[2]); datastore - .physical_disk_adopt(&opctx, disk_103.clone().into()) + .physical_disk_enable_adoption(&opctx, disk_103.clone().into()) .await .expect("adoption request succeeds"); datastore @@ -1364,17 +1364,26 @@ mod test { // Make two disks adoptable datastore - .physical_disk_adopt(&opctx, disks_a[0].identity.clone().into()) + .physical_disk_enable_adoption( + &opctx, + disks_a[0].identity.clone().into(), + ) .await .expect("adoption request succeeds"); datastore - .physical_disk_adopt(&opctx, disks_b[0].identity.clone().into()) + .physical_disk_enable_adoption( + &opctx, + disks_b[0].identity.clone().into(), + ) .await .expect("adoption request succeeds"); // Adoption is idempotent datastore - .physical_disk_adopt(&opctx, disks_b[0].identity.clone().into()) + .physical_disk_enable_adoption( + &opctx, + disks_b[0].identity.clone().into(), + ) .await .expect("adoption request succeeds"); @@ -1420,7 +1429,10 @@ mod test { // Adding one of the same disks back again should make it adoptable datastore - .physical_disk_adopt(&opctx, disks_a[0].identity.clone().into()) + .physical_disk_enable_adoption( + &opctx, + disks_a[0].identity.clone().into(), + ) .await .expect("adoption request succeeds"); let adoptable_disks = datastore @@ -1444,7 +1456,7 @@ mod test { disks_a[0].identity.clone().into(); disk_not_in_inventory.vendor = "some-other-vendor".to_string(); datastore - .physical_disk_adopt(&opctx, disk_not_in_inventory) + .physical_disk_enable_adoption(&opctx, disk_not_in_inventory) .await .expect("adoption request succeeds"); let adoptable_disks = datastore @@ -1463,4 +1475,54 @@ mod test { db.terminate().await; logctx.cleanup_successful(); } + + #[tokio::test] + async fn physical_disk_insert_disk_and_zpool_fails_without_adoption() { + let logctx = dev::test_setup_log( + "physical_disk_insert_disk_and_zpool_fails_without_adoption", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + let sled_a = create_test_sled(&datastore).await; + + // No inventory -> No unadopted disks + let unadopted_disks = datastore + .physical_disk_unadopted_list( + &opctx, + CollectionUuid::new_v4(), // Collection that does not exist + ) + .await + .expect("Failed to look up unadopted disks"); + assert!(unadopted_disks.is_empty()); + + // Create an inventory disk + let mut builder = nexus_inventory::CollectionBuilder::new("test"); + let inv_disk = create_inv_disk("serial-001".to_string(), 1); + add_sled_to_inventory(&mut builder, &sled_a, vec![inv_disk.clone()]); + let collection = builder.build(); + let collection_id = collection.id; + datastore + .inventory_insert_collection(&opctx, &collection) + .await + .expect("failed to insert collection"); + + // Now when we list the unadopted disks, we should see our disk. + let unadopted_disks = datastore + .physical_disk_unadopted_list(&opctx, collection_id) + .await + .expect("Failed to list unadopted disks"); + assert_eq!(unadopted_disks.len(), 1); + + // Fail to create a control plane object for our disk because we have + // not called `datastore.physical_disk_enable_adoption`. + let (disk, zpool) = create_disk_zpool_combo(sled_a.id(), &inv_disk); + datastore + .physical_disk_and_zpool_insert(&opctx, disk, zpool) + .await + .expect_err("insertion fails without an adoption request"); + + db.terminate().await; + logctx.cleanup_successful(); + } } diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index 10eb4f5d961..1206fa23c6a 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -312,7 +312,7 @@ impl super::Nexus { opctx: &OpContext, disk_id: nexus_types::external_api::physical_disk::PhysicalDiskManufacturerIdentity, ) -> Result<(), Error> { - self.db_datastore.physical_disk_adopt(opctx, disk_id).await + self.db_datastore.physical_disk_enable_adoption(opctx, disk_id).await } /// Inserts a physical disk into the database unless it already exists. From 8bce6a0ff46f8a6308b54977994ddec83ab1d259 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Fri, 10 Apr 2026 17:49:30 +0000 Subject: [PATCH 13/26] Add delete endpoint and fix a bunch of stuff --- .../src/db/datastore/physical_disk.rs | 20 +++++ nexus/external-api/output/nexus_tags.txt | 3 +- nexus/external-api/src/lib.rs | 16 +++- nexus/src/app/sled.rs | 13 ++- nexus/src/external_api/http_entrypoints.rs | 22 ++++- nexus/test-utils/src/lib.rs | 2 + nexus/tests/integration_tests/endpoints.rs | 32 +++++-- nexus/types/versions/src/impls/mod.rs | 30 +++++++ .../types/versions/src/initial/path_params.rs | 28 +----- nexus/types/versions/src/latest.rs | 1 + .../src/manual_disk_adoption/physical_disk.rs | 9 ++ ....json => nexus-2026040200.0.0-79a772.json} | 90 +++++++++++++------ openapi/nexus/nexus-latest.json | 2 +- 13 files changed, 200 insertions(+), 68 deletions(-) rename openapi/nexus/{nexus-2026040200.0.0-4e9601.json => nexus-2026040200.0.0-79a772.json} (99%) diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index 25f6c067395..46345979438 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -43,6 +43,7 @@ use omicron_common::api::external::LookupType; use omicron_common::api::external::ResourceType; use omicron_uuid_kinds::CollectionUuid; use omicron_uuid_kinds::GenericUuid; +use omicron_uuid_kinds::PhysicalDiskAdoptionRequestUuid; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::SledUuid; use uuid::Uuid; @@ -553,6 +554,25 @@ impl DataStore { } // Delete an adoption request from the database + pub async fn physical_disk_adoption_request_delete( + &self, + opctx: &OpContext, + id: PhysicalDiskAdoptionRequestUuid, + ) -> DeleteResult { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + let now = Utc::now(); + use nexus_db_schema::schema::physical_disk_adoption_request::dsl; + diesel::update(dsl::physical_disk_adoption_request) + .filter(dsl::id.eq(to_db_typed_uuid(id))) + .filter(dsl::time_deleted.is_null()) + .set(dsl::time_deleted.eq(now)) + .execute_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map(|_rows_modified| ()) + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + + // Delete an adoption request from the database when given a connection async fn physical_disk_adoption_request_delete_on_connection( conn: &async_bb8_diesel::Connection, id: PhysicalDiskManufacturerIdentity, diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index 5635f85b8d3..65889dcbe16 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -217,7 +217,8 @@ networking_switch_port_apply_settings POST /v1/system/hardware/switch-por networking_switch_port_clear_settings DELETE /v1/system/hardware/switch-port/{port}/settings networking_switch_port_list GET /v1/system/hardware/switch-port networking_switch_port_status GET /v1/system/hardware/switch-port/{port}/status -physical_disk_enable_adoption PUT /v1/system/hardware/disks +physical_disk_disable_adoption DELETE /v1/system/hardware/disk-adoption-request/{physical_disk_adoption_req_id} +physical_disk_enable_adoption PUT /v1/system/hardware/disks-adoption-request physical_disk_list GET /v1/system/hardware/disks physical_disk_list_adoption_requests GET /v1/system/hardware/disk-adoption-requests physical_disk_list_unadopted GET /v1/system/hardware/disks-unadopted diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 805b8efa219..c98dc19bf31 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -6718,7 +6718,7 @@ pub trait NexusExternalApi { /// Enable adoption of a physical disk for general use #[endpoint { method = PUT, - path = "/v1/system/hardware/disks", + path = "/v1/system/hardware/disk-adoption-request", tags = ["system/hardware"], versions = VERSION_MANUAL_DISK_ADOPTION.. }] @@ -6727,6 +6727,20 @@ pub trait NexusExternalApi { req: TypedBody, ) -> Result; + /// Disable adoption of a physical disk for general use + #[endpoint { + method = DELETE, + path = "/v1/system/hardware/disk-adoption-request/{physical_disk_adoption_req_id}", + tags = ["system/hardware"], + versions = VERSION_MANUAL_DISK_ADOPTION.. + }] + async fn physical_disk_disable_adoption( + rqctx: RequestContext, + path_params: Path< + latest::physical_disk::PhysicalDiskAdoptionRequestPath, + >, + ) -> Result; + // Switches /// List switches diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index 1206fa23c6a..c377cbf1af1 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -25,6 +25,7 @@ use omicron_common::api::external::LookupResult; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::InstanceUuid; +use omicron_uuid_kinds::PhysicalDiskAdoptionRequestUuid; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::PropolisUuid; use omicron_uuid_kinds::RackUuid; @@ -307,7 +308,7 @@ impl super::Nexus { .await } - pub(crate) async fn physical_disk_adopt( + pub(crate) async fn physical_disk_enable_adoption( &self, opctx: &OpContext, disk_id: nexus_types::external_api::physical_disk::PhysicalDiskManufacturerIdentity, @@ -315,6 +316,16 @@ impl super::Nexus { self.db_datastore.physical_disk_enable_adoption(opctx, disk_id).await } + pub(crate) async fn physical_disk_disable_adoption( + &self, + opctx: &OpContext, + req_uuid: PhysicalDiskAdoptionRequestUuid, + ) -> Result<(), Error> { + self.db_datastore + .physical_disk_adoption_request_delete(opctx, req_uuid) + .await + } + /// Inserts a physical disk into the database unless it already exists. /// /// NOTE: I'd like to re-work this to avoid the upsert-like behavior - can diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index eab59a32f58..3e79d50d9a6 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -57,7 +57,7 @@ use nexus_types::external_api::image::Image; use nexus_types::external_api::ip_pool::{IpPool, IpPoolRange}; use nexus_types::external_api::metrics::SystemMetricsPathParam; use nexus_types::external_api::physical_disk::{ - PhysicalDisk, PhysicalDiskAdoptionRequest, + PhysicalDisk, PhysicalDiskAdoptionRequest, PhysicalDiskAdoptionRequestPath, PhysicalDiskManufacturerIdentity, Unadopted, }; use nexus_types::external_api::probe::ProbeInfo; @@ -6788,12 +6788,30 @@ impl NexusExternalApi for NexusExternalApiImpl { req: TypedBody, ) -> Result { audit_and_time(&rqctx, |opctx, nexus| async move { - nexus.physical_disk_adopt(&opctx, req.into_inner()).await?; + nexus + .physical_disk_enable_adoption(&opctx, req.into_inner()) + .await?; Ok(HttpResponseUpdatedNoContent()) }) .await } + async fn physical_disk_disable_adoption( + rqctx: RequestContext, + path_params: Path, + ) -> Result { + audit_and_time(&rqctx, |opctx, nexus| async move { + nexus + .physical_disk_disable_adoption( + &opctx, + path_params.into_inner().physical_disk_adoption_req_id, + ) + .await?; + Ok(HttpResponseDeleted()) + }) + .await + } + // Switches async fn switch_list( diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index 1d0925527a2..e57f667fe8e 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -46,6 +46,8 @@ pub const PHYSICAL_DISK_UUID: &str = "fbf4e1f1-410e-4314-bff1-fec0504be07e"; pub const OXIMETER_UUID: &str = "39e6175b-4df2-4730-b11d-cbc1e60a2e78"; pub const PRODUCER_UUID: &str = "a6458b7d-87c3-4483-be96-854d814c20de"; pub const RACK_SUBNET: &str = "fd00:1122:3344:0100::/56"; +pub const PHYSICAL_DISK_ADOPTION_REQ_UUID: &str = + "f6944257-9799-4e3e-89b1-c2297b67c16f"; /// Password for the user created by the test suite /// diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 19ac42d5001..9a324be5d90 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -14,6 +14,7 @@ use internal_dns_types::names::DNS_ZONE_EXTERNAL_TESTING; use nexus_db_queries::authn; use nexus_db_queries::db::fixed_data::silo::DEFAULT_SILO; use nexus_db_queries::db::identity::Resource; +use nexus_test_utils::PHYSICAL_DISK_ADOPTION_REQ_UUID; use nexus_test_utils::PHYSICAL_DISK_UUID; use nexus_test_utils::RACK_UUID; use nexus_test_utils::SLED_AGENT_UUID; @@ -132,6 +133,14 @@ pub static DEMO_HARDWARE_PHYSICAL_DISK_ID: LazyLock< pub static HARDWARE_DISK_ADOPTION_REQUESTS_URL: &'static str = "/v1/system/hardware/disk-adoption-requests"; +pub static HARDWARE_DISK_ADOPTION_REQUEST_URL: &'static str = + "/v1/system/hardware/disk-adoption-request"; +pub static HARDWARE_DISK_ADOPTION_REQUEST_DELETE_URL: LazyLock = + LazyLock::new(|| { + format!( + "/v1/system/hardware/disk-adoption-request/{PHYSICAL_DISK_ADOPTION_REQ_UUID}" + ) + }); pub static HARDWARE_DISKS_UNADOPTED_URL: &'static str = "/v1/system/hardware/disks-unadopted"; @@ -3000,13 +3009,7 @@ pub static VERIFY_ENDPOINTS: LazyLock> = LazyLock::new( url: &HARDWARE_DISKS_URL, visibility: Visibility::Public, unprivileged_access: UnprivilegedAccess::None, - allowed_methods: vec![ - AllowedMethod::Get, - AllowedMethod::Put( - serde_json::to_value(&*DEMO_HARDWARE_PHYSICAL_DISK_ID) - .unwrap(), - ), - ], + allowed_methods: vec![AllowedMethod::Get], }, VerifyEndpoint { url: &HARDWARE_DISK_URL, @@ -3020,6 +3023,21 @@ pub static VERIFY_ENDPOINTS: LazyLock> = LazyLock::new( unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![AllowedMethod::Get], }, + VerifyEndpoint { + url: &HARDWARE_DISK_ADOPTION_REQUEST_URL, + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Put( + serde_json::to_value(&*DEMO_HARDWARE_PHYSICAL_DISK_ID) + .unwrap(), + )], + }, + VerifyEndpoint { + url: &HARDWARE_DISK_ADOPTION_REQUEST_DELETE_URL, + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Delete], + }, VerifyEndpoint { url: &HARDWARE_DISK_ADOPTION_REQUESTS_URL, visibility: Visibility::Public, diff --git a/nexus/types/versions/src/impls/mod.rs b/nexus/types/versions/src/impls/mod.rs index 56a1b3b8f2f..a71703a2e14 100644 --- a/nexus/types/versions/src/impls/mod.rs +++ b/nexus/types/versions/src/impls/mod.rs @@ -11,6 +11,36 @@ //! //! Within this module, types are referred to using `latest::` identifiers. +macro_rules! path_param { + ($struct:ident, $param:ident, $name:tt) => { + #[derive(Serialize, Deserialize, JsonSchema)] + pub struct $struct { + #[doc = "Name or ID of the "] + #[doc = $name] + pub $param: NameOrId, + } + }; +} + +macro_rules! id_path_param { + ($struct:ident, $param:ident, $name:tt) => { + id_path_param!($struct, $param, $name, Uuid); + }; + + ($struct:ident, $param:ident, $name:tt, $uuid_type:ident) => { + #[derive(Serialize, Deserialize, JsonSchema)] + pub struct $struct { + #[doc = "ID of the "] + #[doc = $name] + #[schemars(with = "Uuid")] + pub $param: $uuid_type, + } + }; +} + +pub(crate) use id_path_param; +pub(crate) use path_param; + mod alert; mod disk; mod hardware; diff --git a/nexus/types/versions/src/initial/path_params.rs b/nexus/types/versions/src/initial/path_params.rs index 34f8ec5042d..d034f0e6920 100644 --- a/nexus/types/versions/src/initial/path_params.rs +++ b/nexus/types/versions/src/initial/path_params.rs @@ -4,39 +4,13 @@ //! Path parameter types for version INITIAL. +use crate::impls::{id_path_param, path_param}; use omicron_common::api::external::NameOrId; use omicron_uuid_kinds::*; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use uuid::Uuid; -macro_rules! path_param { - ($struct:ident, $param:ident, $name:tt) => { - #[derive(Serialize, Deserialize, JsonSchema)] - pub struct $struct { - #[doc = "Name or ID of the "] - #[doc = $name] - pub $param: NameOrId, - } - }; -} - -macro_rules! id_path_param { - ($struct:ident, $param:ident, $name:tt) => { - id_path_param!($struct, $param, $name, Uuid); - }; - - ($struct:ident, $param:ident, $name:tt, $uuid_type:ident) => { - #[derive(Serialize, Deserialize, JsonSchema)] - pub struct $struct { - #[doc = "ID of the "] - #[doc = $name] - #[schemars(with = "Uuid")] - pub $param: $uuid_type, - } - }; -} - path_param!(AffinityGroupPath, affinity_group, "affinity group"); path_param!(AntiAffinityGroupPath, anti_affinity_group, "anti affinity group"); path_param!(MulticastGroupPath, multicast_group, "multicast group"); diff --git a/nexus/types/versions/src/latest.rs b/nexus/types/versions/src/latest.rs index 5b8266ca66f..46741e542ea 100644 --- a/nexus/types/versions/src/latest.rs +++ b/nexus/types/versions/src/latest.rs @@ -450,6 +450,7 @@ pub mod physical_disk { // Types from MANUAL_DISK_ADOPTION. pub use crate::v2026_04_02_00::physical_disk::PhysicalDiskAdoptionRequest; + pub use crate::v2026_04_02_00::physical_disk::PhysicalDiskAdoptionRequestPath; pub use crate::v2026_04_02_00::physical_disk::PhysicalDiskManufacturerIdentity; pub use crate::v2026_04_02_00::physical_disk::Unadopted; } diff --git a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs index c85c110cc77..7dd193a71ac 100644 --- a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs +++ b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs @@ -4,9 +4,11 @@ //! Physical disk types for version MANUAL_DISK_ADOPTION. +use crate::impls::id_path_param; use omicron_uuid_kinds::{PhysicalDiskAdoptionRequestUuid, SledUuid}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use uuid::Uuid; use crate::v2025_11_20_00::physical_disk::PhysicalDiskKind; @@ -44,3 +46,10 @@ pub struct PhysicalDiskAdoptionRequest { pub model: String, pub time_created: chrono::DateTime, } + +id_path_param!( + PhysicalDiskAdoptionRequestPath, + physical_disk_adoption_req_id, + "physical disk adoption request", + PhysicalDiskAdoptionRequestUuid +); diff --git a/openapi/nexus/nexus-2026040200.0.0-4e9601.json b/openapi/nexus/nexus-2026040200.0.0-79a772.json similarity index 99% rename from openapi/nexus/nexus-2026040200.0.0-4e9601.json rename to openapi/nexus/nexus-2026040200.0.0-79a772.json index 3d8d5fa179b..4b0812e30b8 100644 --- a/openapi/nexus/nexus-2026040200.0.0-4e9601.json +++ b/openapi/nexus/nexus-2026040200.0.0-79a772.json @@ -7695,6 +7695,38 @@ } } }, + "/v1/system/hardware/disk-adoption-request/{physical_disk_adoption_req_id}": { + "delete": { + "tags": [ + "system/hardware" + ], + "summary": "Disable adoption of a physical disk for general use", + "operationId": "physical_disk_disable_adoption", + "parameters": [ + { + "in": "path", + "name": "physical_disk_adoption_req_id", + "description": "ID of the physical disk adoption request", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "204": { + "description": "successful deletion" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/system/hardware/disk-adoption-requests": { "get": { "tags": [ @@ -7811,34 +7843,6 @@ "x-dropshot-pagination": { "required": [] } - }, - "put": { - "tags": [ - "system/hardware" - ], - "summary": "Enable adoption of a physical disk for general use", - "operationId": "physical_disk_enable_adoption", - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/PhysicalDiskManufacturerIdentity" - } - } - }, - "required": true - }, - "responses": { - "204": { - "description": "resource updated" - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } } }, "/v1/system/hardware/disks/{disk_id}": { @@ -7880,6 +7884,36 @@ } } }, + "/v1/system/hardware/disks-adoption-request": { + "put": { + "tags": [ + "system/hardware" + ], + "summary": "Enable adoption of a physical disk for general use", + "operationId": "physical_disk_enable_adoption", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PhysicalDiskManufacturerIdentity" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "resource updated" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/system/hardware/disks-unadopted": { "get": { "tags": [ diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index db1f3d87eb2..cb30223b57d 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026040200.0.0-4e9601.json \ No newline at end of file +nexus-2026040200.0.0-79a772.json \ No newline at end of file From 87bc5a0d7ce706398b76c53d22f8ed099fe5ad1d Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Fri, 10 Apr 2026 19:02:06 +0000 Subject: [PATCH 14/26] openapi --- nexus/external-api/output/nexus_tags.txt | 2 +- ....json => nexus-2026040200.0.0-3a8fb6.json} | 60 +++++++++---------- openapi/nexus/nexus-latest.json | 2 +- 3 files changed, 32 insertions(+), 32 deletions(-) rename openapi/nexus/{nexus-2026040200.0.0-79a772.json => nexus-2026040200.0.0-3a8fb6.json} (99%) diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index 65889dcbe16..8035742b535 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -218,7 +218,7 @@ networking_switch_port_clear_settings DELETE /v1/system/hardware/switch-por networking_switch_port_list GET /v1/system/hardware/switch-port networking_switch_port_status GET /v1/system/hardware/switch-port/{port}/status physical_disk_disable_adoption DELETE /v1/system/hardware/disk-adoption-request/{physical_disk_adoption_req_id} -physical_disk_enable_adoption PUT /v1/system/hardware/disks-adoption-request +physical_disk_enable_adoption PUT /v1/system/hardware/disk-adoption-request physical_disk_list GET /v1/system/hardware/disks physical_disk_list_adoption_requests GET /v1/system/hardware/disk-adoption-requests physical_disk_list_unadopted GET /v1/system/hardware/disks-unadopted diff --git a/openapi/nexus/nexus-2026040200.0.0-79a772.json b/openapi/nexus/nexus-2026040200.0.0-3a8fb6.json similarity index 99% rename from openapi/nexus/nexus-2026040200.0.0-79a772.json rename to openapi/nexus/nexus-2026040200.0.0-3a8fb6.json index 4b0812e30b8..24a4f01b7e2 100644 --- a/openapi/nexus/nexus-2026040200.0.0-79a772.json +++ b/openapi/nexus/nexus-2026040200.0.0-3a8fb6.json @@ -7695,6 +7695,36 @@ } } }, + "/v1/system/hardware/disk-adoption-request": { + "put": { + "tags": [ + "system/hardware" + ], + "summary": "Enable adoption of a physical disk for general use", + "operationId": "physical_disk_enable_adoption", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PhysicalDiskManufacturerIdentity" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "resource updated" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/system/hardware/disk-adoption-request/{physical_disk_adoption_req_id}": { "delete": { "tags": [ @@ -7884,36 +7914,6 @@ } } }, - "/v1/system/hardware/disks-adoption-request": { - "put": { - "tags": [ - "system/hardware" - ], - "summary": "Enable adoption of a physical disk for general use", - "operationId": "physical_disk_enable_adoption", - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/PhysicalDiskManufacturerIdentity" - } - } - }, - "required": true - }, - "responses": { - "204": { - "description": "resource updated" - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } - } - }, "/v1/system/hardware/disks-unadopted": { "get": { "tags": [ diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index cb30223b57d..e88c1c40e33 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026040200.0.0-79a772.json \ No newline at end of file +nexus-2026040200.0.0-3a8fb6.json \ No newline at end of file From c9618fcf8ec82f8d6674e29ba5241c2c182cdca4 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Mon, 20 Apr 2026 18:59:41 +0000 Subject: [PATCH 15/26] Auto adopt newly seen physical disks --- .../src/db/datastore/physical_disk.rs | 255 +++++++++++++++++- .../tasks/physical_disk_adoption.rs | 178 +++++++----- 2 files changed, 358 insertions(+), 75 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index 46345979438..6d46b989850 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -135,7 +135,7 @@ impl DataStore { let conn = &*self.pool_connection_authorized(opctx).await?; let err = OptionalError::>::new(); let txn_res = self - .transaction_retry_wrapper("physical_disk_adopt") + .transaction_retry_wrapper("physical_disk_enable_adoption") .transaction(&conn, |conn| { let vendor = disk_id.vendor.clone(); let serial = disk_id.serial.clone(); @@ -310,6 +310,155 @@ impl DataStore { Ok(()) } + /// Enable adoption for any physical disks whose vendor/model/serial has not + /// been seen before. + /// + /// We only want to mandate that expunged disks get manually re-adopted by + /// operators. For newly discovered disks we want to auto-adopt them. We + /// are doing this to maintain our current operational simplicity, such that + /// when a user adds a new sled to a rack with new disks they don't have to + /// manually add individual disks. This allows us to safely re-add expunged + /// disks without changing unrelated operator behavior at the same time. + /// + /// This still leaves a security hole such that arbitrary disks can be + /// inserted and automatically adopted. Eventually we want to make it so + /// that all new physical disks are manually adopted, but this is a more + /// complicated change. + pub async fn physical_disk_enable_adoption_for_all_new_disks_in_inventory( + &self, + opctx: &OpContext, + inventory_collection_id: CollectionUuid, + ) -> Result<(), Error> { + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; + + let conn = &*self.pool_connection_authorized(opctx).await?; + let err = OptionalError::>::new(); + + use nexus_db_schema::schema::physical_disk_adoption_request::dsl as adoption_dsl; + + self.transaction_retry_wrapper( + "physical_disk_enable_adoption_all_new_disks", + ) + .transaction(&conn, |conn| { + let err = err.clone(); + + async move { + // Find all new disks + let new_inv_disks = self + .physical_disk_list_new_inventory_on_collection( + &conn, + inventory_collection_id, + ) + .await + .map_err(|txn_error| txn_error.into_diesel(&err))?; + + // Enable adoption for each disk. + for disk in new_inv_disks { + // Insert the adoption request. + diesel::insert_into( + adoption_dsl::physical_disk_adoption_request, + ) + .values(( + adoption_dsl::id.eq(Uuid::new_v4()), + adoption_dsl::vendor.eq(disk.vendor), + adoption_dsl::serial.eq(disk.serial), + adoption_dsl::model.eq(disk.model), + adoption_dsl::time_created.eq(Utc::now()), + )) + .execute_async(&conn) + .await?; + } + Ok(()) + } + }) + .await + .map_err(|e| { + match err.take() { + // A called function performed its own error propagation. + Some(txn_error) => txn_error.into_public_ignore_retries(), + // The transaction setup/teardown itself encountered a diesel error. + None => public_error_from_diesel(e, ErrorHandler::Server), + } + })?; + Ok(()) + } + + /// Returns all physical disks in inventory that are inserted in + /// active sleds and do not yet have a record in `physical_disk` or + /// `physical_disk_adoption_request` tables. + pub async fn physical_disk_list_new_inventory_on_collection( + &self, + conn: &async_bb8_diesel::Connection, + inventory_collection_id: CollectionUuid, + ) -> Result, TransactionError> { + use nexus_db_schema::schema::inv_physical_disk::dsl as inv_physical_disk_dsl; + use nexus_db_schema::schema::physical_disk::dsl as physical_disk_dsl; + use nexus_db_schema::schema::physical_disk_adoption_request::dsl as adoption_request_dsl; + use nexus_db_schema::schema::sled::dsl as sled_dsl; + + let disks = sled_dsl::sled + // If the sled is not in-service, drop the list immediately. + .filter(sled_dsl::time_deleted.is_null()) + .sled_filter(SledFilter::InService) + // Look up all inventory physical disks that could match this sled + .inner_join( + inv_physical_disk_dsl::inv_physical_disk.on( + inv_physical_disk_dsl::inv_collection_id + .eq(inventory_collection_id.into_untyped_uuid()) + .and(inv_physical_disk_dsl::sled_id.eq(sled_dsl::id)) + .and( + inv_physical_disk_dsl::variant + .eq(PhysicalDiskKind::U2), + ), + ), + ) + // Filter out any disks in the inventory for which we have ever had + // a control plane disk. + .filter(diesel::dsl::not(diesel::dsl::exists( + physical_disk_dsl::physical_disk + .select(0.into_sql::()) + .filter(physical_disk_dsl::variant.eq(PhysicalDiskKind::U2)) + .filter(physical_disk_dsl::sled_id.eq(sled_dsl::id)) + .filter( + physical_disk_dsl::vendor + .eq(inv_physical_disk_dsl::vendor), + ) + .filter( + physical_disk_dsl::model + .eq(inv_physical_disk_dsl::model), + ) + .filter( + physical_disk_dsl::serial + .eq(inv_physical_disk_dsl::serial), + ), + ))) + // Filter out physical disks that exist in + // `physical_disk_adoption_request`. This means they are in the + // process of being adopted already. + .filter(diesel::dsl::not(diesel::dsl::exists( + adoption_request_dsl::physical_disk_adoption_request + .select(0.into_sql::()) + .filter(adoption_request_dsl::time_deleted.is_null()) + .filter( + adoption_request_dsl::vendor + .eq(inv_physical_disk_dsl::vendor), + ) + .filter( + adoption_request_dsl::model + .eq(inv_physical_disk_dsl::model), + ) + .filter( + adoption_request_dsl::serial + .eq(inv_physical_disk_dsl::serial), + ), + ))) + .select(InvPhysicalDisk::as_select()) + .get_results_async(conn) + .await?; + + Ok(disks) + } + /// Returns all physical disks which are eligible for adoption. /// /// These disks: @@ -1545,4 +1694,108 @@ mod test { db.terminate().await; logctx.cleanup_successful(); } + + #[tokio::test] + async fn physical_disk_list_new_inventory_on_connection() { + let logctx = dev::test_setup_log( + "physical_disk_list_new_inventory_on_connection", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + let conn = datastore.pool_connection_for_tests().await.unwrap(); + let sled = create_test_sled(&datastore).await; + + // No inventory -> No new disks + let new = datastore + .physical_disk_list_new_inventory_on_collection( + &conn, + CollectionUuid::new_v4(), + ) + .await + .expect("failed to list new disks"); + + assert!(new.is_empty()); + + // Create an inventory disk + let mut builder = nexus_inventory::CollectionBuilder::new("test"); + let inv_disk = create_inv_disk("serial-001".to_string(), 1); + add_sled_to_inventory(&mut builder, &sled, vec![inv_disk.clone()]); + let collection = builder.build(); + let collection_id = collection.id; + + datastore + .inventory_insert_collection(&opctx, &collection) + .await + .expect("failed to insert collection"); + + // We should get this disk returned as brand new + let new = datastore + .physical_disk_list_new_inventory_on_collection( + &conn, + collection_id, + ) + .await + .expect("failed to list new disks"); + + assert_eq!(new.len(), 1); + + // Enabling adoption for all new disks should ensure this disk is + // adopted + datastore + .physical_disk_enable_adoption_for_all_new_disks_in_inventory( + &opctx, + collection_id, + ) + .await + .expect("enabling adoption should succeed"); + + // We should no longer treat this disk as new since it has an + // adoption request. + let new = datastore + .physical_disk_list_new_inventory_on_collection( + &conn, + CollectionUuid::new_v4(), + ) + .await + .expect("failed to list new disks"); + assert!(new.is_empty()); + + // Adopting the disk should also continue to not return it as an new disk + let (disk, zpool) = create_disk_zpool_combo(sled.id(), &inv_disk); + datastore + .physical_disk_and_zpool_insert(&opctx, disk.clone(), zpool) + .await + .unwrap(); + let new = datastore + .physical_disk_list_new_inventory_on_collection( + &conn, + CollectionUuid::new_v4(), + ) + .await + .expect("failed to list new disks"); + assert!(new.is_empty()); + + // Expunging the disk should also continue to not return it as an new disk + use nexus_db_schema::schema::physical_disk::dsl; + diesel::update(dsl::physical_disk) + .filter(dsl::id.eq(to_db_typed_uuid(disk.id()))) + .filter(dsl::time_deleted.is_null()) + .set(dsl::disk_policy.eq(PhysicalDiskPolicy::Expunged)) + .execute_async( + &*datastore.pool_connection_authorized(&opctx).await.unwrap(), + ) + .await + .unwrap(); + let new = datastore + .physical_disk_list_new_inventory_on_collection( + &conn, + CollectionUuid::new_v4(), + ) + .await + .expect("failed to list new disks"); + assert!(new.is_empty()); + + db.terminate().await; + logctx.cleanup_successful(); + } } diff --git a/nexus/src/app/background/tasks/physical_disk_adoption.rs b/nexus/src/app/background/tasks/physical_disk_adoption.rs index adacff2e6bd..3c74b9d0ecd 100644 --- a/nexus/src/app/background/tasks/physical_disk_adoption.rs +++ b/nexus/src/app/background/tasks/physical_disk_adoption.rs @@ -23,6 +23,7 @@ use nexus_types::identity::Asset; use nexus_types::inventory::Collection; use omicron_common::api::external; use omicron_common::api::external::DataPageParams; +use omicron_uuid_kinds::CollectionUuid; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::ZpoolUuid; use serde_json::json; @@ -52,6 +53,94 @@ impl PhysicalDiskAdoption { rx_inventory_collection, } } + + /// Insert disks and zpools for any adoptable physical disks + async fn physical_disk_adopt( + &self, + opctx: &OpContext, + collection_id: CollectionUuid, + ) -> serde_json::Value { + let mut disks_added = 0; + + let result = self + .datastore + .physical_disk_adoptable_list(opctx, collection_id) + .await; + + let adoptable = match result { + Ok(adoptable) => adoptable, + Err(err) => { + let err = InlineErrorChain::new(&err); + warn!( + &opctx.log, + "Physical Disk Adoption: \ + failed to query for insertable disks"; + &err, + ); + let err = format!("failed to query database: {err}"); + return json!({ "error": err }); + } + }; + + for inv_disk in adoptable { + let disk = PhysicalDisk::new( + PhysicalDiskUuid::new_v4(), + inv_disk.vendor, + inv_disk.serial, + inv_disk.model, + inv_disk.variant, + inv_disk.sled_id.into(), + ); + + let zpool = Zpool::new( + ZpoolUuid::new_v4(), + inv_disk.sled_id.into(), + disk.id(), + CONTROL_PLANE_STORAGE_BUFFER.into(), + ); + + let result = self + .datastore + .physical_disk_and_zpool_insert(opctx, disk.clone(), zpool) + .await; + + if let Err(err) = result { + // Skip reporting the error if we get back a `NotFound`. + // This means that another nexus concurrently added the + // disk or that the adoptable request was deleted. We + // don't want to report mistakenly one way or another and + // so we just continue here. + if let external::Error::NotFound { .. } = err { + continue; + } + + let err = InlineErrorChain::new(&err); + warn!( + &opctx.log, + "Physical Disk Adoption: \ + failed to insert new disk and zpool"; + &err, + ); + let msg = format!( + "failed to insert disk/zpool: {err}; disk = {disk:#?}", + ); + return json!({ "error": msg }); + } + + disks_added += 1; + + info!( + &opctx.log, + "Physical Disk Adoption: \ + Successfully added a new disk and zpool"; + "disk" => #?disk + ); + } + + json!({ + "physical_disks_added": disks_added, + }) + } } impl BackgroundTask for PhysicalDiskAdoption { @@ -105,8 +194,6 @@ impl BackgroundTask for PhysicalDiskAdoption { } } - let mut disks_added = 0; - // Grab the most-recently-loaded collection ID. The ID is `Copy`, so // we can grab it and then unlock the watch channel. let Some(collection_id) = self @@ -123,84 +210,27 @@ impl BackgroundTask for PhysicalDiskAdoption { return json!({ "error": "no inventory" }); }; - let result = self + if let Err(err) = self .datastore - .physical_disk_adoptable_list(opctx, collection_id) - .await; - - let adoptable = match result { - Ok(adoptable) => adoptable, - Err(err) => { - let err = InlineErrorChain::new(&err); - warn!( - &opctx.log, - "Physical Disk Adoption: \ - failed to query for insertable disks"; - &err, - ); - let err = format!("failed to query database: {err}"); - return json!({ "error": err }); - } - }; - - for inv_disk in adoptable { - let disk = PhysicalDisk::new( - PhysicalDiskUuid::new_v4(), - inv_disk.vendor, - inv_disk.serial, - inv_disk.model, - inv_disk.variant, - inv_disk.sled_id.into(), - ); - - let zpool = Zpool::new( - ZpoolUuid::new_v4(), - inv_disk.sled_id.into(), - disk.id(), - CONTROL_PLANE_STORAGE_BUFFER.into(), - ); - - let result = self - .datastore - .physical_disk_and_zpool_insert(opctx, disk.clone(), zpool) - .await; - - if let Err(err) = result { - // Skip reporting the error if we get back a `NotFound`. - // This means that another nexus concurrently added the - // disk or that the adoptable request was deleted. We - // don't want to report mistakenly one way or another and - // so we just continue here. - if let external::Error::NotFound { .. } = err { - continue; - } - - let err = InlineErrorChain::new(&err); - warn!( - &opctx.log, - "Physical Disk Adoption: \ - failed to insert new disk and zpool"; - &err, - ); - let msg = format!( - "failed to insert disk/zpool: {err}; disk = {disk:#?}", - ); - return json!({ "error": msg }); - } - - disks_added += 1; - - info!( + .physical_disk_enable_adoption_for_all_new_disks_in_inventory( + opctx, + collection_id, + ) + .await + { + let err = InlineErrorChain::new(&err); + warn!( &opctx.log, "Physical Disk Adoption: \ - Successfully added a new disk and zpool"; - "disk" => #?disk + failed to enable adoption for new disks"; + &err, ); + + // We still want to adopt disks that already are enabled for + // adoption below so we fall through. } - json!({ - "physical_disks_added": disks_added, - }) + self.physical_disk_adopt(opctx, collection_id).await } .boxed() } From a2ae344e8c29785e2827021d6e424e523080a403 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 28 Apr 2026 19:25:34 +0000 Subject: [PATCH 16/26] fixes from review by sean --- nexus/db-model/src/physical_disk.rs | 6 +-- .../src/db/datastore/physical_disk.rs | 39 +++++++++---------- nexus/db-queries/src/db/datastore/rack.rs | 2 +- nexus/src/external_api/http_entrypoints.rs | 2 +- 4 files changed, 21 insertions(+), 28 deletions(-) diff --git a/nexus/db-model/src/physical_disk.rs b/nexus/db-model/src/physical_disk.rs index 598cf02d49f..00f372455ac 100644 --- a/nexus/db-model/src/physical_disk.rs +++ b/nexus/db-model/src/physical_disk.rs @@ -79,11 +79,7 @@ impl From for physical_disk_types::PhysicalDiskManufacturerIdentity { fn from(value: PhysicalDisk) -> Self { - Self { - vendor: value.vendor.clone(), - serial: value.serial.clone(), - model: value.model.clone(), - } + Self { vendor: value.vendor, serial: value.serial, model: value.model } } } diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index 6d46b989850..ca6b22815c3 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -92,11 +92,9 @@ impl DataStore { .await .map_err(|txn_error| txn_error.into_diesel(&err))?; - Self::physical_disk_insert_on_connection( - &conn, opctx, disk, - ) - .await - .map_err(|txn_error| txn_error.into_diesel(&err))?; + Self::physical_disk_insert_on_connection(&conn, disk) + .await + .map_err(|txn_error| txn_error.into_diesel(&err))?; Self::zpool_insert_on_connection(&conn, opctx, zpool) .await @@ -229,8 +227,9 @@ impl DataStore { opctx: &OpContext, disk: PhysicalDisk, ) -> CreateResult { + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; let conn = &*self.pool_connection_authorized(&opctx).await?; - let disk = Self::physical_disk_insert_on_connection(&conn, opctx, disk) + let disk = Self::physical_disk_insert_on_connection(&conn, disk) .await .map_err(|err| err.into_public_ignore_retries())?; Ok(disk) @@ -238,10 +237,8 @@ impl DataStore { pub async fn physical_disk_insert_on_connection( conn: &async_bb8_diesel::Connection, - opctx: &OpContext, disk: PhysicalDisk, ) -> Result> { - opctx.authorize(authz::Action::Read, &authz::FLEET).await?; use nexus_db_schema::schema::physical_disk::dsl; let sled_id = disk.sled_id(); @@ -274,7 +271,7 @@ impl DataStore { id: PhysicalDiskUuid, policy: PhysicalDiskPolicy, ) -> Result<(), Error> { - opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; use nexus_db_schema::schema::physical_disk::dsl; let now = Utc::now(); @@ -295,7 +292,7 @@ impl DataStore { id: PhysicalDiskUuid, state: PhysicalDiskState, ) -> Result<(), Error> { - opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; use nexus_db_schema::schema::physical_disk::dsl; let now = Utc::now(); @@ -345,7 +342,7 @@ impl DataStore { async move { // Find all new disks let new_inv_disks = self - .physical_disk_list_new_inventory_on_collection( + .physical_disk_list_new_inventory_on_connection( &conn, inventory_collection_id, ) @@ -386,7 +383,7 @@ impl DataStore { /// Returns all physical disks in inventory that are inserted in /// active sleds and do not yet have a record in `physical_disk` or /// `physical_disk_adoption_request` tables. - pub async fn physical_disk_list_new_inventory_on_collection( + pub async fn physical_disk_list_new_inventory_on_connection( &self, conn: &async_bb8_diesel::Connection, inventory_collection_id: CollectionUuid, @@ -708,7 +705,7 @@ impl DataStore { opctx: &OpContext, id: PhysicalDiskAdoptionRequestUuid, ) -> DeleteResult { - opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + opctx.authorize(authz::Action::Delete, &authz::FLEET).await?; let now = Utc::now(); use nexus_db_schema::schema::physical_disk_adoption_request::dsl; diesel::update(dsl::physical_disk_adoption_request) @@ -1377,7 +1374,7 @@ mod test { // Pay careful attention to our indexing below. // // We're grabbing the last disk of "disks_a" (which still is - // uninitailized) and the last two disks of "disks_b" (of which both are + // uninitialized) and the last two disks of "disks_b" (of which both are // still unadopted). let mut unadopted_disks: Vec = unadopted_disks @@ -1610,7 +1607,7 @@ mod test { .expect("Failed to look up adoptable disks"); assert_eq!(adoptable_disks.len(), 1); - // We should now only have 5 unininitialized disks + // We should now only have 5 uninitialized disks let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await @@ -1634,7 +1631,7 @@ mod test { .expect("Failed to look up adoptable disks"); assert_eq!(adoptable_disks.len(), 1); - // We should still have only 5 unininitialized disks + // We should still have only 5 uninitialized disks let unadopted_disks = datastore .physical_disk_unadopted_list(&opctx, collection_id) .await @@ -1707,7 +1704,7 @@ mod test { // No inventory -> No new disks let new = datastore - .physical_disk_list_new_inventory_on_collection( + .physical_disk_list_new_inventory_on_connection( &conn, CollectionUuid::new_v4(), ) @@ -1730,7 +1727,7 @@ mod test { // We should get this disk returned as brand new let new = datastore - .physical_disk_list_new_inventory_on_collection( + .physical_disk_list_new_inventory_on_connection( &conn, collection_id, ) @@ -1752,7 +1749,7 @@ mod test { // We should no longer treat this disk as new since it has an // adoption request. let new = datastore - .physical_disk_list_new_inventory_on_collection( + .physical_disk_list_new_inventory_on_connection( &conn, CollectionUuid::new_v4(), ) @@ -1767,7 +1764,7 @@ mod test { .await .unwrap(); let new = datastore - .physical_disk_list_new_inventory_on_collection( + .physical_disk_list_new_inventory_on_connection( &conn, CollectionUuid::new_v4(), ) @@ -1787,7 +1784,7 @@ mod test { .await .unwrap(); let new = datastore - .physical_disk_list_new_inventory_on_collection( + .physical_disk_list_new_inventory_on_connection( &conn, CollectionUuid::new_v4(), ) diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index 75c0d89ca5e..a52f822a6f4 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -876,7 +876,7 @@ impl DataStore { for physical_disk in physical_disks { info!(log, "physical disk upsert in handoff: {physical_disk:#?}"); - if let Err(e) = Self::physical_disk_insert_on_connection(&conn, &opctx, physical_disk) + if let Err(e) = Self::physical_disk_insert_on_connection(&conn, physical_disk) .await { if !matches!(e, TransactionError::CustomError(Error::ObjectAlreadyExists { .. })) { error!(log, "Failed to upsert physical disk"; "err" => #%e); diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 3e79d50d9a6..6325d5b4026 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6714,7 +6714,7 @@ impl NexusExternalApi for NexusExternalApiImpl { if let dropshot::WhichPage::Next(last_seen) = &pag_params.page { return Err(Error::invalid_value( last_seen.clone(), - "bad page token", + "pagination currently unsupported", ) .into()); } From 73c5c4627ae59e410e20691d4c62aac7084adfd2 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 28 Apr 2026 19:45:37 +0000 Subject: [PATCH 17/26] Make constructing a `PhysicalDisk` safer When we have an inventory disk we pass that in to `new` and set the fields inside that method. This prevents passing strings in the wrong order. However, most callse to `PhysicalDisk::new` are in tests. Rather than forcing the creation of an intermediate type we add a new `from_parts` method to allow more verbose construction. --- nexus/db-model/src/physical_disk.rs | 17 +++++++++++++++-- nexus/db-queries/src/db/datastore/mod.rs | 2 +- .../src/db/datastore/physical_disk.rs | 18 +++++++++--------- nexus/db-queries/src/db/datastore/sled.rs | 6 +++--- .../src/db/queries/region_allocation.rs | 2 +- .../execution/src/omicron_physical_disks.rs | 2 +- .../tasks/decommissioned_disk_cleaner.rs | 2 +- .../background/tasks/physical_disk_adoption.rs | 13 +++---------- .../tasks/support_bundle_collector.rs | 2 +- nexus/src/app/rack.rs | 2 +- nexus/src/app/sled.rs | 2 +- nexus/tests/integration_tests/sleds.rs | 2 +- 12 files changed, 38 insertions(+), 32 deletions(-) diff --git a/nexus/db-model/src/physical_disk.rs b/nexus/db-model/src/physical_disk.rs index 00f372455ac..fbcee895878 100644 --- a/nexus/db-model/src/physical_disk.rs +++ b/nexus/db-model/src/physical_disk.rs @@ -6,6 +6,7 @@ use super::{ Generation, PhysicalDiskKind, PhysicalDiskPolicy, PhysicalDiskState, }; use crate::DbTypedUuid; +use crate::InvPhysicalDisk; use crate::collection::DatastoreCollectionConfig; use chrono::{DateTime, Utc}; use db_macros::Asset; @@ -39,8 +40,20 @@ pub struct PhysicalDisk { } impl PhysicalDisk { - /// Creates a new in-service, active disk - pub fn new( + /// Creates a new in-service, active disk from an inventory disk + pub fn new(inv_disk: InvPhysicalDisk) -> Self { + Self::from_parts( + PhysicalDiskUuid::new_v4(), + inv_disk.vendor, + inv_disk.serial, + inv_disk.model, + inv_disk.variant, + inv_disk.sled_id.into(), + ) + } + + /// Creates a new in-service, active disk from individual fields + pub fn from_parts( id: PhysicalDiskUuid, vendor: String, serial: String, diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index bcf16ba016b..6b6dc920a5e 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -1013,7 +1013,7 @@ mod test { kind: PhysicalDiskKind, serial: String, ) -> PhysicalDiskUuid { - let physical_disk = PhysicalDisk::new( + let physical_disk = PhysicalDisk::from_parts( PhysicalDiskUuid::new_v4(), TEST_VENDOR.into(), serial, diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index ca6b22815c3..f5f5445e6d9 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -795,7 +795,7 @@ mod test { let sled_id = sled.id(); // Insert a disk - let disk = PhysicalDisk::new( + let disk = PhysicalDisk::from_parts( PhysicalDiskUuid::new_v4(), String::from("Oxide"), String::from("123"), @@ -836,7 +836,7 @@ mod test { let sled_id = sled.id(); // Insert a disk - let disk = PhysicalDisk::new( + let disk = PhysicalDisk::from_parts( PhysicalDiskUuid::new_v4(), String::from("Oxide"), String::from("123"), @@ -850,7 +850,7 @@ mod test { .expect("Failed first attempt at upserting disk"); // Insert a second disk - let disk = PhysicalDisk::new( + let disk = PhysicalDisk::from_parts( PhysicalDiskUuid::new_v4(), String::from("Noxide"), String::from("456"), @@ -884,7 +884,7 @@ mod test { // Insert a disk let disk_id = PhysicalDiskUuid::new_v4(); - let disk = PhysicalDisk::new( + let disk = PhysicalDisk::from_parts( disk_id, String::from("Oxide"), String::from("123"), @@ -942,7 +942,7 @@ mod test { // Insert a disk let disk_id = PhysicalDiskUuid::new_v4(); - let disk = PhysicalDisk::new( + let disk = PhysicalDisk::from_parts( disk_id, String::from("Oxide"), String::from("123"), @@ -984,7 +984,7 @@ mod test { // Attach the disk to the second sled let disk_id = PhysicalDiskUuid::new_v4(); - let disk = PhysicalDisk::new( + let disk = PhysicalDisk::from_parts( disk_id, String::from("Oxide"), String::from("123"), @@ -1031,7 +1031,7 @@ mod test { // Insert a disk let disk_id = PhysicalDiskUuid::new_v4(); - let disk = PhysicalDisk::new( + let disk = PhysicalDisk::from_parts( disk_id, String::from("Oxide"), String::from("123"), @@ -1062,7 +1062,7 @@ mod test { .expect("Failed to delete disk"); // "Report the disk" from the second sled - let disk = PhysicalDisk::new( + let disk = PhysicalDisk::from_parts( PhysicalDiskUuid::new_v4(), String::from("Oxide"), String::from("123"), @@ -1181,7 +1181,7 @@ mod test { sled_id: SledUuid, inv_disk: &InventoryDisk, ) -> (PhysicalDisk, Zpool) { - let disk = PhysicalDisk::new( + let disk = PhysicalDisk::from_parts( PhysicalDiskUuid::new_v4(), inv_disk.identity.vendor.clone(), inv_disk.identity.serial.clone(), diff --git a/nexus/db-queries/src/db/datastore/sled.rs b/nexus/db-queries/src/db/datastore/sled.rs index e260c87312f..1e31d1930f1 100644 --- a/nexus/db-queries/src/db/datastore/sled.rs +++ b/nexus/db-queries/src/db/datastore/sled.rs @@ -3441,7 +3441,7 @@ pub(in crate::db::datastore) mod test { // (Note: This isn't really enough DB fakery to actually provision e.g. // Crucible regions, but it creates enough of a control plane object to // be associated with the Sled by UUID) - let disk1 = PhysicalDisk::new( + let disk1 = PhysicalDisk::from_parts( PhysicalDiskUuid::new_v4(), "vendor1".to_string(), "serial1".to_string(), @@ -3449,7 +3449,7 @@ pub(in crate::db::datastore) mod test { PhysicalDiskKind::U2, sled_id, ); - let disk2 = PhysicalDisk::new( + let disk2 = PhysicalDisk::from_parts( PhysicalDiskUuid::new_v4(), "vendor2".to_string(), "serial2".to_string(), @@ -3891,7 +3891,7 @@ pub(in crate::db::datastore) mod test { datastore.sled_upsert(sled).await.expect("failed to upsert sled"); for u2 in &sled_config.u2s { - let physical_disk = db::model::PhysicalDisk::new( + let physical_disk = db::model::PhysicalDisk::from_parts( u2.physical_disk_id, String::from("vendor"), u2.physical_disk_serial.clone(), diff --git a/nexus/db-queries/src/db/queries/region_allocation.rs b/nexus/db-queries/src/db/queries/region_allocation.rs index 4765197d7ec..74893455d36 100644 --- a/nexus/db-queries/src/db/queries/region_allocation.rs +++ b/nexus/db-queries/src/db/queries/region_allocation.rs @@ -1092,7 +1092,7 @@ mod test { datastore.sled_upsert(sled).await.expect("failed to upsert sled"); for u2 in &sled_config.u2s { - let physical_disk = db::model::PhysicalDisk::new( + let physical_disk = db::model::PhysicalDisk::from_parts( u2.physical_disk_id, String::from("vendor"), u2.physical_disk_serial.clone(), diff --git a/nexus/reconfigurator/execution/src/omicron_physical_disks.rs b/nexus/reconfigurator/execution/src/omicron_physical_disks.rs index 4be7bd3efbb..dbd418f52ff 100644 --- a/nexus/reconfigurator/execution/src/omicron_physical_disks.rs +++ b/nexus/reconfigurator/execution/src/omicron_physical_disks.rs @@ -112,7 +112,7 @@ mod test { sled_id: SledUuid, ) -> PhysicalDiskUuid { let id = PhysicalDiskUuid::new_v4(); - let physical_disk = PhysicalDisk::new( + let physical_disk = PhysicalDisk::from_parts( id, "v".into(), format!("s-{i})"), diff --git a/nexus/src/app/background/tasks/decommissioned_disk_cleaner.rs b/nexus/src/app/background/tasks/decommissioned_disk_cleaner.rs index ff7960f20db..68cfd1c91fc 100644 --- a/nexus/src/app/background/tasks/decommissioned_disk_cleaner.rs +++ b/nexus/src/app/background/tasks/decommissioned_disk_cleaner.rs @@ -199,7 +199,7 @@ mod tests { sled_id: SledUuid, ) -> PhysicalDiskUuid { let id = PhysicalDiskUuid::new_v4(); - let physical_disk = PhysicalDisk::new( + let physical_disk = PhysicalDisk::from_parts( id, "v".into(), format!("s-{i})"), diff --git a/nexus/src/app/background/tasks/physical_disk_adoption.rs b/nexus/src/app/background/tasks/physical_disk_adoption.rs index 3c74b9d0ecd..7962616a905 100644 --- a/nexus/src/app/background/tasks/physical_disk_adoption.rs +++ b/nexus/src/app/background/tasks/physical_disk_adoption.rs @@ -24,7 +24,6 @@ use nexus_types::inventory::Collection; use omicron_common::api::external; use omicron_common::api::external::DataPageParams; use omicron_uuid_kinds::CollectionUuid; -use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::ZpoolUuid; use serde_json::json; use slog_error_chain::InlineErrorChain; @@ -83,18 +82,12 @@ impl PhysicalDiskAdoption { }; for inv_disk in adoptable { - let disk = PhysicalDisk::new( - PhysicalDiskUuid::new_v4(), - inv_disk.vendor, - inv_disk.serial, - inv_disk.model, - inv_disk.variant, - inv_disk.sled_id.into(), - ); + let sled_id = inv_disk.sled_id.into(); + let disk = PhysicalDisk::new(inv_disk); let zpool = Zpool::new( ZpoolUuid::new_v4(), - inv_disk.sled_id.into(), + sled_id, disk.id(), CONTROL_PLANE_STORAGE_BUFFER.into(), ); diff --git a/nexus/src/app/background/tasks/support_bundle_collector.rs b/nexus/src/app/background/tasks/support_bundle_collector.rs index d9091f1cc98..4c18eee7264 100644 --- a/nexus/src/app/background/tasks/support_bundle_collector.rs +++ b/nexus/src/app/background/tasks/support_bundle_collector.rs @@ -588,7 +588,7 @@ mod test { sled_id: SledUuid, ) -> PhysicalDiskUuid { let id = PhysicalDiskUuid::new_v4(); - let physical_disk = PhysicalDisk::new( + let physical_disk = PhysicalDisk::from_parts( id, "v".into(), format!("s-{i})"), diff --git a/nexus/src/app/rack.rs b/nexus/src/app/rack.rs index 7a2b2c8b72f..23aa2233411 100644 --- a/nexus/src/app/rack.rs +++ b/nexus/src/app/rack.rs @@ -95,7 +95,7 @@ impl super::Nexus { .physical_disks .into_iter() .map(|disk| { - db::model::PhysicalDisk::new( + db::model::PhysicalDisk::from_parts( disk.id, disk.vendor, disk.serial, diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index c377cbf1af1..014baba318d 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -365,7 +365,7 @@ impl super::Nexus { Err(err) => return Err(err), } - let disk = db::model::PhysicalDisk::new( + let disk = db::model::PhysicalDisk::from_parts( request.id, request.vendor, request.serial, diff --git a/nexus/tests/integration_tests/sleds.rs b/nexus/tests/integration_tests/sleds.rs index 4252be1d6dd..9e2632a2ee7 100644 --- a/nexus/tests/integration_tests/sleds.rs +++ b/nexus/tests/integration_tests/sleds.rs @@ -121,7 +121,7 @@ async fn test_physical_disk_create_list_delete( let nexus = &cptestctx.server.server_context().nexus; let datastore = nexus.datastore(); let sled_id = SledUuid::from_str(&SLED_AGENT_UUID).unwrap(); - let physical_disk = DbPhysicalDisk::new( + let physical_disk = DbPhysicalDisk::from_parts( PhysicalDiskUuid::new_v4(), "v".into(), "s".into(), From 85ef45cb2583bb7e5bf60a92cb25637dec01f41a Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 28 Apr 2026 20:23:53 +0000 Subject: [PATCH 18/26] Some fixes from johns review --- nexus/db-model/src/physical_disk.rs | 9 ++++++--- .../src/db/datastore/physical_disk.rs | 20 +++++++++++++------ nexus/external-api/src/lib.rs | 2 +- .../tasks/physical_disk_adoption.rs | 2 +- nexus/src/external_api/http_entrypoints.rs | 7 ++++--- nexus/types/versions/src/latest.rs | 2 +- .../src/manual_disk_adoption/physical_disk.rs | 6 ++---- 7 files changed, 29 insertions(+), 19 deletions(-) diff --git a/nexus/db-model/src/physical_disk.rs b/nexus/db-model/src/physical_disk.rs index fbcee895878..77df707900b 100644 --- a/nexus/db-model/src/physical_disk.rs +++ b/nexus/db-model/src/physical_disk.rs @@ -12,6 +12,7 @@ use chrono::{DateTime, Utc}; use db_macros::Asset; use nexus_db_schema::schema::{physical_disk, zpool}; use nexus_types::external_api::physical_disk as physical_disk_types; +use nexus_types::external_api::physical_disk::PhysicalDiskManufacturerIdentity; use nexus_types::identity::Asset; use omicron_uuid_kinds::PhysicalDiskAdoptionRequestKind; use omicron_uuid_kinds::PhysicalDiskKind as PhysicalDiskUuidKind; @@ -129,9 +130,11 @@ impl From fn from(req: PhysicalDiskAdoptionRequest) -> Self { Self { id: req.id.into(), - vendor: req.vendor, - serial: req.serial, - model: req.model, + disk_id: PhysicalDiskManufacturerIdentity { + vendor: req.vendor, + serial: req.serial, + model: req.model, + }, time_created: req.time_created, } } diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index f5f5445e6d9..1fa08671d92 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -708,14 +708,22 @@ impl DataStore { opctx.authorize(authz::Action::Delete, &authz::FLEET).await?; let now = Utc::now(); use nexus_db_schema::schema::physical_disk_adoption_request::dsl; - diesel::update(dsl::physical_disk_adoption_request) + let rows_modified = diesel::update(dsl::physical_disk_adoption_request) .filter(dsl::id.eq(to_db_typed_uuid(id))) .filter(dsl::time_deleted.is_null()) .set(dsl::time_deleted.eq(now)) .execute_async(&*self.pool_connection_authorized(opctx).await?) .await - .map(|_rows_modified| ()) - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + if rows_modified != 1 { + return Err(Error::non_resourcetype_not_found(format!( + "No active adoption request found for id: {id}" + )) + .into()); + } + + Ok(()) } // Delete an adoption request from the database when given a connection @@ -1751,7 +1759,7 @@ mod test { let new = datastore .physical_disk_list_new_inventory_on_connection( &conn, - CollectionUuid::new_v4(), + collection_id, ) .await .expect("failed to list new disks"); @@ -1766,7 +1774,7 @@ mod test { let new = datastore .physical_disk_list_new_inventory_on_connection( &conn, - CollectionUuid::new_v4(), + collection_id, ) .await .expect("failed to list new disks"); @@ -1786,7 +1794,7 @@ mod test { let new = datastore .physical_disk_list_new_inventory_on_connection( &conn, - CollectionUuid::new_v4(), + collection_id, ) .await .expect("failed to list new disks"); diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index c86889ffddf..fe7374dc258 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -6749,7 +6749,7 @@ pub trait NexusExternalApi { rqctx: RequestContext, query: Query>, ) -> Result< - HttpResponseOk>, + HttpResponseOk>, HttpError, >; diff --git a/nexus/src/app/background/tasks/physical_disk_adoption.rs b/nexus/src/app/background/tasks/physical_disk_adoption.rs index 7962616a905..0a10ee4fe59 100644 --- a/nexus/src/app/background/tasks/physical_disk_adoption.rs +++ b/nexus/src/app/background/tasks/physical_disk_adoption.rs @@ -100,7 +100,7 @@ impl PhysicalDiskAdoption { if let Err(err) = result { // Skip reporting the error if we get back a `NotFound`. // This means that another nexus concurrently added the - // disk or that the adoptable request was deleted. We + // disk or that the adoption request was deleted. We // don't want to report mistakenly one way or another and // so we just continue here. if let external::Error::NotFound { .. } = err { diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 6325d5b4026..49e86724af8 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -58,7 +58,7 @@ use nexus_types::external_api::ip_pool::{IpPool, IpPoolRange}; use nexus_types::external_api::metrics::SystemMetricsPathParam; use nexus_types::external_api::physical_disk::{ PhysicalDisk, PhysicalDiskAdoptionRequest, PhysicalDiskAdoptionRequestPath, - PhysicalDiskManufacturerIdentity, Unadopted, + PhysicalDiskManufacturerIdentity, UnadoptedPhysicalDisk, }; use nexus_types::external_api::probe::ProbeInfo; use nexus_types::external_api::project::Project; @@ -6708,7 +6708,8 @@ impl NexusExternalApi for NexusExternalApiImpl { async fn physical_disk_list_unadopted( rqctx: RequestContext, query: Query>, - ) -> Result>, HttpError> { + ) -> Result>, HttpError> + { let apictx = rqctx.context(); let pag_params = query.into_inner(); if let dropshot::WhichPage::Next(last_seen) = &pag_params.page { @@ -6726,7 +6727,7 @@ impl NexusExternalApi for NexusExternalApiImpl { .physical_disk_list_unadopted(&opctx) .await? .into_iter() - .map(|d| Unadopted { + .map(|d| UnadoptedPhysicalDisk { sled_id: d.sled_id.into(), slot: d.slot, variant: d.variant.into(), diff --git a/nexus/types/versions/src/latest.rs b/nexus/types/versions/src/latest.rs index e16b09d0783..e22eb91330e 100644 --- a/nexus/types/versions/src/latest.rs +++ b/nexus/types/versions/src/latest.rs @@ -455,7 +455,7 @@ pub mod physical_disk { pub use crate::v2026_04_21_00::physical_disk::PhysicalDiskAdoptionRequest; pub use crate::v2026_04_21_00::physical_disk::PhysicalDiskAdoptionRequestPath; pub use crate::v2026_04_21_00::physical_disk::PhysicalDiskManufacturerIdentity; - pub use crate::v2026_04_21_00::physical_disk::Unadopted; + pub use crate::v2026_04_21_00::physical_disk::UnadoptedPhysicalDisk; } pub mod rack { diff --git a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs index 7dd193a71ac..e20a8a71124 100644 --- a/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs +++ b/nexus/types/versions/src/manual_disk_adoption/physical_disk.rs @@ -14,7 +14,7 @@ use crate::v2025_11_20_00::physical_disk::PhysicalDiskKind; /// A physical disk that has not yet been adopted by the control plane #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] -pub struct Unadopted { +pub struct UnadoptedPhysicalDisk { pub sled_id: SledUuid, pub slot: i64, pub variant: PhysicalDiskKind, @@ -41,9 +41,7 @@ impl From #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] pub struct PhysicalDiskAdoptionRequest { pub id: PhysicalDiskAdoptionRequestUuid, - pub vendor: String, - pub serial: String, - pub model: String, + pub disk_id: PhysicalDiskManufacturerIdentity, pub time_created: chrono::DateTime, } From f42bb3503c34ff0597ad99788bd4ff4bb7e69237 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 28 Apr 2026 20:47:35 +0000 Subject: [PATCH 19/26] more review fixes for john --- .../src/db/datastore/physical_disk.rs | 41 +++++++++++++------ nexus/external-api/src/lib.rs | 9 +++- .../tasks/physical_disk_adoption.rs | 11 +++++ nexus/src/app/sled.rs | 11 ++++- nexus/src/external_api/http_entrypoints.rs | 7 ++-- ....json => nexus-2026042100.0.0-727d17.json} | 37 ++++++++--------- openapi/nexus/nexus-latest.json | 2 +- 7 files changed, 78 insertions(+), 40 deletions(-) rename openapi/nexus/{nexus-2026042100.0.0-e56b51.json => nexus-2026042100.0.0-727d17.json} (99%) diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index 1fa08671d92..3794250f51a 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -124,7 +124,7 @@ impl DataStore { &self, opctx: &OpContext, disk_id: PhysicalDiskManufacturerIdentity, - ) -> Result<(), Error> { + ) -> Result { opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; use nexus_db_schema::schema::physical_disk::dsl as physical_disk_dsl; @@ -167,7 +167,7 @@ impl DataStore { } // Insert the adoption request. - diesel::insert_into( + let request = diesel::insert_into( adoption_dsl::physical_disk_adoption_request, ) .values(( @@ -177,31 +177,34 @@ impl DataStore { adoption_dsl::model.eq(model), adoption_dsl::time_created.eq(Utc::now()), )) - .execute_async(&conn) + .returning(PhysicalDiskAdoptionRequest::as_returning()) + .get_result_async(&conn) .await?; - Ok(()) + Ok(request) } }) .await; - // Check for a unique index violation for an active - // adoption request with the same vendor/serial/model. - // - // Return Ok(()) in this case as the request is idempotent. - if let Err(e) = txn_res { - match err.take() { + match txn_res { + Ok(request) => return Ok(request), + Err(e) => match err.take() { // A called function performed its own error propagation. Some(txn_error) => { return Err(txn_error.into_public_ignore_retries()); } // The transaction setup/teardown itself encountered a diesel error. None => match e { + // Check for a unique index violation for an active + // adoption request with the same vendor/serial/model. + // + // Return the existing request in this case as the + // request is idempotent. DieselError::DatabaseError( DieselErrorKind::UniqueViolation, _, ) => { - return Ok(()); + // Fall through to query the existing request below. } _ => { return Err(public_error_from_diesel( @@ -210,10 +213,22 @@ impl DataStore { )); } }, - } + }, } - Ok(()) + // Idempotent case: an adoption request already exists for this disk. + // Query it back and return it. + let conn = &*self.pool_connection_authorized(opctx).await?; + let request = adoption_dsl::physical_disk_adoption_request + .filter(adoption_dsl::vendor.eq(disk_id.vendor)) + .filter(adoption_dsl::serial.eq(disk_id.serial)) + .filter(adoption_dsl::model.eq(disk_id.model)) + .filter(adoption_dsl::time_deleted.is_null()) + .select(PhysicalDiskAdoptionRequest::as_select()) + .first_async(conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + Ok(request) } /// Stores a new physical disk in the database. diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index fe7374dc258..0e75700a08c 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -6749,7 +6749,9 @@ pub trait NexusExternalApi { rqctx: RequestContext, query: Query>, ) -> Result< - HttpResponseOk>, + HttpResponseOk< + ResultsPage, + >, HttpError, >; @@ -6780,7 +6782,10 @@ pub trait NexusExternalApi { async fn physical_disk_enable_adoption( rqctx: RequestContext, req: TypedBody, - ) -> Result; + ) -> Result< + HttpResponseCreated, + HttpError, + >; /// Disable adoption of a physical disk for general use #[endpoint { diff --git a/nexus/src/app/background/tasks/physical_disk_adoption.rs b/nexus/src/app/background/tasks/physical_disk_adoption.rs index 0a10ee4fe59..82c60cf7c90 100644 --- a/nexus/src/app/background/tasks/physical_disk_adoption.rs +++ b/nexus/src/app/background/tasks/physical_disk_adoption.rs @@ -203,6 +203,17 @@ impl BackgroundTask for PhysicalDiskAdoption { return json!({ "error": "no inventory" }); }; + // We only force manual re-adoption of expunged disks currently. + // In the future we wish to enable manual adoption of all disks + // for security reasons. Doing this now would be a somewhat large + // undertaking and unergonomic if done naively. The naive approach + // would require users to have to issue adoption requests for up to + // 10 disks when they add a new sled to a rack. + // + // However, we also don't want to bifurcate the code paths that + // check to see if adoption is enabled. Therefore in the case of + // new disks we automatically fill in an adoption request as if an + // operator had done so manually via the external API. if let Err(err) = self .datastore .physical_disk_enable_adoption_for_all_new_disks_in_inventory( diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index 014baba318d..d2ee2f40c44 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -312,8 +312,15 @@ impl super::Nexus { &self, opctx: &OpContext, disk_id: nexus_types::external_api::physical_disk::PhysicalDiskManufacturerIdentity, - ) -> Result<(), Error> { - self.db_datastore.physical_disk_enable_adoption(opctx, disk_id).await + ) -> Result< + nexus_types::external_api::physical_disk::PhysicalDiskAdoptionRequest, + Error, + > { + let request = self + .db_datastore + .physical_disk_enable_adoption(opctx, disk_id) + .await?; + Ok(request.into()) } pub(crate) async fn physical_disk_disable_adoption( diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 49e86724af8..c895402eb1b 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6787,12 +6787,13 @@ impl NexusExternalApi for NexusExternalApiImpl { async fn physical_disk_enable_adoption( rqctx: RequestContext, req: TypedBody, - ) -> Result { + ) -> Result, HttpError> + { audit_and_time(&rqctx, |opctx, nexus| async move { - nexus + let request = nexus .physical_disk_enable_adoption(&opctx, req.into_inner()) .await?; - Ok(HttpResponseUpdatedNoContent()) + Ok(HttpResponseCreated(request)) }) .await } diff --git a/openapi/nexus/nexus-2026042100.0.0-e56b51.json b/openapi/nexus/nexus-2026042100.0.0-727d17.json similarity index 99% rename from openapi/nexus/nexus-2026042100.0.0-e56b51.json rename to openapi/nexus/nexus-2026042100.0.0-727d17.json index c7f0cfacf4f..f51bf1c6099 100644 --- a/openapi/nexus/nexus-2026042100.0.0-e56b51.json +++ b/openapi/nexus/nexus-2026042100.0.0-727d17.json @@ -7713,8 +7713,15 @@ "required": true }, "responses": { - "204": { - "description": "resource updated" + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PhysicalDiskAdoptionRequest" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -7949,7 +7956,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/UnadoptedResultsPage" + "$ref": "#/components/schemas/UnadoptedPhysicalDiskResultsPage" } } } @@ -25346,29 +25353,21 @@ "description": "A request to adopt a physical disk into the control plane", "type": "object", "properties": { + "disk_id": { + "$ref": "#/components/schemas/PhysicalDiskManufacturerIdentity" + }, "id": { "$ref": "#/components/schemas/PhysicalDiskAdoptionRequestUuid" }, - "model": { - "type": "string" - }, - "serial": { - "type": "string" - }, "time_created": { "type": "string", "format": "date-time" - }, - "vendor": { - "type": "string" } }, "required": [ + "disk_id", "id", - "model", - "serial", - "time_created", - "vendor" + "time_created" ] }, "PhysicalDiskAdoptionRequestResultsPage": { @@ -29963,7 +29962,7 @@ } } }, - "Unadopted": { + "UnadoptedPhysicalDisk": { "description": "A physical disk that has not yet been adopted by the control plane", "type": "object", "properties": { @@ -29988,7 +29987,7 @@ "variant" ] }, - "UnadoptedResultsPage": { + "UnadoptedPhysicalDiskResultsPage": { "description": "A single page of results", "type": "object", "properties": { @@ -29996,7 +29995,7 @@ "description": "list of items on this page of results", "type": "array", "items": { - "$ref": "#/components/schemas/Unadopted" + "$ref": "#/components/schemas/UnadoptedPhysicalDisk" } }, "next_page": { diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index c0843afb5b4..db00e4da880 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026042100.0.0-e56b51.json \ No newline at end of file +nexus-2026042100.0.0-727d17.json \ No newline at end of file From 67235fa7b498e5f970e08cd335b32ab94b5f2792 Mon Sep 17 00:00:00 2001 From: Andy Fiddaman Date: Fri, 13 Mar 2026 16:48:11 +0000 Subject: [PATCH 20/26] Move to Helios v3 --- .github/buildomat/jobs/a4x2-prepare.sh | 2 +- .../buildomat/jobs/build-and-test-helios.sh | 2 +- .github/buildomat/jobs/check-features.sh | 2 +- .github/buildomat/jobs/clippy.sh | 2 +- .github/buildomat/jobs/deploy.sh | 2 +- .github/buildomat/jobs/omicron-common.sh | 2 +- .github/buildomat/jobs/package.sh | 2 +- .github/buildomat/jobs/tuf-repo.sh | 2 +- dev-tools/releng/src/helios.rs | 20 +++++++++++++++++-- dev-tools/releng/src/main.rs | 6 +++--- tools/install_opte.sh | 2 +- tools/scrimlet/create-softnpu-zone.sh | 2 +- tools/uninstall_opte.sh | 10 +++++----- 13 files changed, 36 insertions(+), 20 deletions(-) diff --git a/.github/buildomat/jobs/a4x2-prepare.sh b/.github/buildomat/jobs/a4x2-prepare.sh index ae10da2ecb3..4e0647bff41 100755 --- a/.github/buildomat/jobs/a4x2-prepare.sh +++ b/.github/buildomat/jobs/a4x2-prepare.sh @@ -2,7 +2,7 @@ #: #: name = "a4x2-prepare" #: variety = "basic" -#: target = "helios-2.0" +#: target = "helios-3.0" #: rust_toolchain = true #: output_rules = [ #: "=/out/cargo-bay-ce.tgz", diff --git a/.github/buildomat/jobs/build-and-test-helios.sh b/.github/buildomat/jobs/build-and-test-helios.sh index dc665ee5a87..94448bc651a 100755 --- a/.github/buildomat/jobs/build-and-test-helios.sh +++ b/.github/buildomat/jobs/build-and-test-helios.sh @@ -2,7 +2,7 @@ #: #: name = "build-and-test (helios)" #: variety = "basic" -#: target = "helios-2.0-32c256gb" +#: target = "helios-3.0-32c256gb" #: rust_toolchain = true #: output_rules = [ #: "%/work/*", diff --git a/.github/buildomat/jobs/check-features.sh b/.github/buildomat/jobs/check-features.sh index 03dbee4cfa2..0e3e982ccd4 100644 --- a/.github/buildomat/jobs/check-features.sh +++ b/.github/buildomat/jobs/check-features.sh @@ -2,7 +2,7 @@ #: #: name = "check-features (helios)" #: variety = "basic" -#: target = "helios-2.0" +#: target = "helios-3.0" #: rust_toolchain = true #: output_rules = [ #: "/out/*", diff --git a/.github/buildomat/jobs/clippy.sh b/.github/buildomat/jobs/clippy.sh index 66f5208088d..179c21d737b 100755 --- a/.github/buildomat/jobs/clippy.sh +++ b/.github/buildomat/jobs/clippy.sh @@ -2,7 +2,7 @@ #: #: name = "clippy (helios)" #: variety = "basic" -#: target = "helios-2.0" +#: target = "helios-3.0" #: rust_toolchain = true #: output_rules = [] diff --git a/.github/buildomat/jobs/deploy.sh b/.github/buildomat/jobs/deploy.sh index 7a3ac17b054..c32c1d48d59 100755 --- a/.github/buildomat/jobs/deploy.sh +++ b/.github/buildomat/jobs/deploy.sh @@ -2,7 +2,7 @@ #: #: name = "helios / deploy" #: variety = "basic" -#: target = "lab-2.0-opte-0.40" +#: target = "lab-3.0-opte-0.40" #: output_rules = [ #: "%/var/svc/log/oxide-*.log*", #: "%/zone/oxz_*/root/var/svc/log/oxide-*.log*", diff --git a/.github/buildomat/jobs/omicron-common.sh b/.github/buildomat/jobs/omicron-common.sh index 676c18f52ad..fe75a1249d3 100755 --- a/.github/buildomat/jobs/omicron-common.sh +++ b/.github/buildomat/jobs/omicron-common.sh @@ -2,7 +2,7 @@ #: #: name = "omicron-common (helios)" #: variety = "basic" -#: target = "helios-2.0" +#: target = "helios-3.0" #: rust_toolchain = true #: output_rules = [] diff --git a/.github/buildomat/jobs/package.sh b/.github/buildomat/jobs/package.sh index b43b91e9ec4..cccf2edf8d6 100755 --- a/.github/buildomat/jobs/package.sh +++ b/.github/buildomat/jobs/package.sh @@ -2,7 +2,7 @@ #: #: name = "helios / package" #: variety = "basic" -#: target = "helios-2.0-16c64gb" +#: target = "helios-3.0-16c64gb" #: rust_toolchain = true #: output_rules = [ #: "=/work/package.tar.gz", diff --git a/.github/buildomat/jobs/tuf-repo.sh b/.github/buildomat/jobs/tuf-repo.sh index f139dbfb08e..93ac977971c 100755 --- a/.github/buildomat/jobs/tuf-repo.sh +++ b/.github/buildomat/jobs/tuf-repo.sh @@ -2,7 +2,7 @@ #: #: name = "helios / build TUF repo" #: variety = "basic" -#: target = "helios-2.0-16c128gb" +#: target = "helios-3.0-16c128gb" #: rust_toolchain = true #: output_rules = [ #: "=/work/manifest.toml", diff --git a/dev-tools/releng/src/helios.rs b/dev-tools/releng/src/helios.rs index 5474450cfc9..e3353771cd7 100644 --- a/dev-tools/releng/src/helios.rs +++ b/dev-tools/releng/src/helios.rs @@ -23,7 +23,7 @@ const MANIFEST_PATH: &str = "incorporation.p5m"; const REPO_PATH: &str = "incorporation"; pub const ARCHIVE_PATH: &str = "incorporation.p5p"; -pub const PUBLISHER: &str = "helios-dev"; +pub const PUBLISHER: &str = "helios"; pub(crate) enum Action { Generate { version: String }, @@ -119,9 +119,25 @@ async fn generate_incorporation_manifest( fmri: String, } + let stdout = Command::new("pkg") + .args([ + "list", + "-H", + "-o", + "branch", + "-n", + "-g", + HELIOS_PKGREPO, + "release/name", + ]) + .ensure_stdout(&logger) + .await?; + + let branch = stdout.trim(); + let mut manifest = BufWriter::new(File::create(path).await?); let preamble = format!( - r#"set name=pkg.fmri value=pkg://{PUBLISHER}/{INCORP_NAME}@{version},5.11 + r#"set name=pkg.fmri value=pkg://{PUBLISHER}/{INCORP_NAME}@{version},5.11-{branch} set name=pkg.summary value="Incorporation to constrain software delivered in Omicron Release V{version} images" set name=info.classification value="org.opensolaris.category.2008:Meta Packages/Incorporations" set name=variant.opensolaris.zone value=global value=nonglobal diff --git a/dev-tools/releng/src/main.rs b/dev-tools/releng/src/main.rs index 874f25e3fe7..ab9c7946f1c 100644 --- a/dev-tools/releng/src/main.rs +++ b/dev-tools/releng/src/main.rs @@ -91,7 +91,7 @@ const TUF_PACKAGES: [&PackageName; 12] = [ &PackageName::new_const("probe"), ]; -const HELIOS_PKGREPO: &str = "https://pkg.oxide.computer/helios/2/dev/"; +const HELIOS_PKGREPO: &str = "https://pkg.oxide.computer/helios/3/dev/"; const HELIOS_REPO: &str = "https://github.com/oxidecomputer/helios.git"; static WORKSPACE_DIR: LazyLock = LazyLock::new(|| { @@ -158,7 +158,7 @@ struct Args { #[clap(long)] extra_manifest: Option, - /// Extra helios-dev origin to be passed along to helios-build + /// Extra helios origin to be passed along to helios-build #[clap(long)] extra_origin: Option, @@ -672,7 +672,7 @@ async fn main() -> Result<()> { if !args.helios_local { image_cmd = image_cmd .arg("-p") // use an external package repository - .arg(format!("helios-dev={HELIOS_PKGREPO}")) + .arg(format!("helios={HELIOS_PKGREPO}")) } // helios-build experiment-image diff --git a/tools/install_opte.sh b/tools/install_opte.sh index d56523764d9..11b7f90aa04 100755 --- a/tools/install_opte.sh +++ b/tools/install_opte.sh @@ -73,7 +73,7 @@ fi # Actually install the xde kernel module and opteadm tool RC=0 -pfexec pkg install -v pkg://helios-dev/driver/network/opte@"$OPTE_VERSION" || RC=$? +pfexec pkg install -v pkg://helios/driver/network/opte@"$OPTE_VERSION" || RC=$? if [[ "$RC" -eq 0 ]]; then echo "xde driver installed successfully" elif [[ "$RC" -eq 4 ]]; then diff --git a/tools/scrimlet/create-softnpu-zone.sh b/tools/scrimlet/create-softnpu-zone.sh index b2136e1a908..c507cc8dd84 100755 --- a/tools/scrimlet/create-softnpu-zone.sh +++ b/tools/scrimlet/create-softnpu-zone.sh @@ -12,7 +12,7 @@ cp out/softnpu/scadm /opt/oxide/softnpu/stuff/ zfs create -p -o mountpoint=/softnpu-zone rpool/softnpu-zone -pkg set-publisher --search-first helios-dev +pkg set-publisher --search-first helios zonecfg -z softnpu -f tools/scrimlet/softnpu-zone.txt zoneadm -z softnpu install diff --git a/tools/uninstall_opte.sh b/tools/uninstall_opte.sh index da625047e5d..efb2ec44820 100755 --- a/tools/uninstall_opte.sh +++ b/tools/uninstall_opte.sh @@ -28,7 +28,7 @@ fi PUBLISHERS=("on-nightly" "helios-netdev") # The stock Helios publisher and incorporation base information to revert to -HELIOS_PUBLISHER="helios-dev" +HELIOS_PUBLISHER="helios" STOCK_CONSOLIDATION="pkg://$HELIOS_PUBLISHER/consolidation/osnet/osnet-incorporation" # Saved list of publisher origins we've removed if we need to restore them @@ -71,7 +71,7 @@ function restore_removed_publishers { done } -# Install stock incorporation from helios-dev, pushing the publisher to the top +# Install stock incorporation from helios, pushing the publisher to the top function to_stock_helios { local CONSOLIDATION="$1" echo "Installing stock Helios kernel and networking bits" @@ -125,7 +125,7 @@ function to_stock_helios { pkg install --no-refresh -v "${REJECT_ARGS[@]}" "$CONSOLIDATION" } -# If helios-dev exists, echo the full osnet-incorporation package we'll be +# If helios exists, echo the full osnet-incorporation package we'll be # installing to. If it does _not_ exist, fail. function ensure_helios_publisher_exists { pkg publisher "$HELIOS_PUBLISHER" > /dev/null || \ @@ -135,8 +135,8 @@ function ensure_helios_publisher_exists { echo "No osnet-incorporation package exists on this system," echo "so we cannot determine the exact package to install" echo "to revert to stock Helios. You may need to update your" - echo "helios-dev publisher to refresh that publishers list of" - echo "available packages, with \"pkg refresh helios-dev\"" + echo "helios publisher to refresh that publishers list of" + echo "available packages, with \"pkg refresh helios\"" exit 1 fi echo "$CONSOLIDATION" | tr -s ' ' | awk '{ print $1 "@" $(NF-1); }' From 698b3b8040a08b001f775cb688564553926ccd37 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Wed, 6 May 2026 18:14:40 +0000 Subject: [PATCH 21/26] Better handling of bg task status --- dev-tools/omdb/src/bin/omdb/nexus.rs | 23 ++++++++++ .../tasks/physical_disk_adoption.rs | 46 ++++++++++--------- nexus/types/src/internal_api/background.rs | 10 ++++ 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 2bec4896d33..f11405a3667 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -65,6 +65,7 @@ use nexus_types::internal_api::background::InstanceReincarnationStatus; use nexus_types::internal_api::background::InstanceUpdaterStatus; use nexus_types::internal_api::background::InventoryLoadStatus; use nexus_types::internal_api::background::LookupRegionPortStatus; +use nexus_types::internal_api::background::PhysicalDiskAdoptionStatus; use nexus_types::internal_api::background::ProbeDistributorStatus; use nexus_types::internal_api::background::ReadOnlyRegionReplacementStartStatus; use nexus_types::internal_api::background::RegionReplacementDriverStatus; @@ -1300,6 +1301,9 @@ fn print_task_details(bgtask: &BackgroundTask, details: &serde_json::Value) { "phantom_disks" => { print_task_phantom_disks(details); } + "physical_disk_adoption" => { + print_task_physical_disk_adoption(details); + } "probe_distributor" => { print_task_probe_distributor(details); } @@ -3933,6 +3937,25 @@ fn print_task_trust_quorum_manager(details: &serde_json::Value) { } } +fn print_task_physical_disk_adoption(details: &serde_json::Value) { + let status = match serde_json::from_value::( + details.clone(), + ) { + Ok(status) => status, + Err(error) => { + eprintln!( + "warning: failed to interpret task details: {:?}: {:#?}", + error, details + ); + return; + } + }; + println!("physical disks added: {}", status.disks_added); + for error in status.errors { + println!("{ERRICON} {error}"); + } +} + const ERRICON: &str = "/!\\"; fn warn_if_nonzero(n: usize) -> &'static str { diff --git a/nexus/src/app/background/tasks/physical_disk_adoption.rs b/nexus/src/app/background/tasks/physical_disk_adoption.rs index 82c60cf7c90..510d140488b 100644 --- a/nexus/src/app/background/tasks/physical_disk_adoption.rs +++ b/nexus/src/app/background/tasks/physical_disk_adoption.rs @@ -20,6 +20,7 @@ use nexus_db_model::Zpool; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::DataStore; use nexus_types::identity::Asset; +use nexus_types::internal_api::background::PhysicalDiskAdoptionStatus; use nexus_types::inventory::Collection; use omicron_common::api::external; use omicron_common::api::external::DataPageParams; @@ -58,9 +59,8 @@ impl PhysicalDiskAdoption { &self, opctx: &OpContext, collection_id: CollectionUuid, + status: &mut PhysicalDiskAdoptionStatus, ) -> serde_json::Value { - let mut disks_added = 0; - let result = self .datastore .physical_disk_adoptable_list(opctx, collection_id) @@ -77,7 +77,8 @@ impl PhysicalDiskAdoption { &err, ); let err = format!("failed to query database: {err}"); - return json!({ "error": err }); + status.errors.push(err); + return json!(status); } }; @@ -117,22 +118,20 @@ impl PhysicalDiskAdoption { let msg = format!( "failed to insert disk/zpool: {err}; disk = {disk:#?}", ); - return json!({ "error": msg }); - } + status.errors.push(msg); + } else { + status.disks_added += 1; - disks_added += 1; - - info!( - &opctx.log, - "Physical Disk Adoption: \ - Successfully added a new disk and zpool"; - "disk" => #?disk - ); + info!( + &opctx.log, + "Physical Disk Adoption: \ + Successfully added a new disk and zpool"; + "disk" => #?disk + ); + } } - json!({ - "physical_disks_added": disks_added, - }) + json!(status) } } @@ -142,8 +141,10 @@ impl BackgroundTask for PhysicalDiskAdoption { opctx: &'a OpContext, ) -> BoxFuture<'a, serde_json::Value> { async { + let mut status = PhysicalDiskAdoptionStatus::default(); if self.disable { - return json!({ "error": "task disabled" }); + status.errors.push("task disabled".to_owned()); + return json!(status); } // Only adopt physical disks after rack handoff has completed. @@ -171,7 +172,8 @@ impl BackgroundTask for PhysicalDiskAdoption { "rack not yet initialized: {}", self.rack_id ); - return json!({"error": msg}); + status.errors.push(msg); + return json!(status); } } Err(err) => { @@ -183,7 +185,8 @@ impl BackgroundTask for PhysicalDiskAdoption { &err, ); let err = format!("failed to query database: {err}"); - return json!({ "error": err }); + status.errors.push(err); + return json!(status); } } @@ -200,7 +203,8 @@ impl BackgroundTask for PhysicalDiskAdoption { "Physical Disk Adoption: skipped"; "reason" => "no inventory" ); - return json!({ "error": "no inventory" }); + status.errors.push("no inventory".to_owned()); + return json!(status); }; // We only force manual re-adoption of expunged disks currently. @@ -234,7 +238,7 @@ impl BackgroundTask for PhysicalDiskAdoption { // adoption below so we fall through. } - self.physical_disk_adopt(opctx, collection_id).await + self.physical_disk_adopt(opctx, collection_id, &mut status).await } .boxed() } diff --git a/nexus/types/src/internal_api/background.rs b/nexus/types/src/internal_api/background.rs index cbf09135bde..59ecb3a0868 100644 --- a/nexus/types/src/internal_api/background.rs +++ b/nexus/types/src/internal_api/background.rs @@ -1166,6 +1166,16 @@ pub struct ServiceFirewallRuleStatus { pub sled_push_errors: Option>, } +/// Status of the `PhysicalDiskAdoption` background task +#[derive(Debug, Default, Deserialize, Serialize)] +pub struct PhysicalDiskAdoptionStatus { + /// The number of physical disks added during this activation + pub disks_added: usize, + + /// Errors encountered during this activation + pub errors: Vec, +} + #[cfg(test)] mod test { use super::TufRepoInfo; From 7ceb9a87c67f9e65f119c6a13d5e27538de1d9f6 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Wed, 6 May 2026 18:52:53 +0000 Subject: [PATCH 22/26] clippy --- nexus/db-queries/src/db/datastore/physical_disk.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index 0357d56ed0d..f503e034385 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -744,8 +744,7 @@ impl DataStore { if rows_modified != 1 { return Err(Error::non_resourcetype_not_found(format!( "No active adoption request found for id: {id}" - )) - .into()); + ))); } Ok(()) From 6a31229e0b03009a3e81674da851ebe67a5d66ab Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Thu, 7 May 2026 17:18:27 +0000 Subject: [PATCH 23/26] fix schema version --- schema/crdb/dbinit.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index e483c33cecd..5055740c4e1 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -8576,7 +8576,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '256.0.0', NULL) + (TRUE, NOW(), NOW(), '257.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; From ea0dcac1068aa1b2dea4e96ad13085639358ab9c Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Thu, 7 May 2026 18:29:39 +0000 Subject: [PATCH 24/26] expectorate --- dev-tools/omdb/tests/successes.out | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dev-tools/omdb/tests/successes.out b/dev-tools/omdb/tests/successes.out index 8662889facf..17981175cf0 100644 --- a/dev-tools/omdb/tests/successes.out +++ b/dev-tools/omdb/tests/successes.out @@ -852,7 +852,8 @@ task: "physical_disk_adoption" configured period: every s last completed activation: , triggered by started at (s ago) and ran for ms - last completion reported error: task disabled +physical disks added: 0 +/!\ task disabled task: "probe_distributor" configured period: every m @@ -1533,7 +1534,8 @@ task: "physical_disk_adoption" configured period: every s last completed activation: , triggered by started at (s ago) and ran for ms - last completion reported error: task disabled +physical disks added: 0 +/!\ task disabled task: "probe_distributor" configured period: every m From 875e4ce5fe964e6780a6c085458e4a4caad073c7 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Fri, 8 May 2026 19:35:23 +0000 Subject: [PATCH 25/26] huzzah --- .../src/db/datastore/physical_disk.rs | 68 +++++++------------ 1 file changed, 24 insertions(+), 44 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/physical_disk.rs b/nexus/db-queries/src/db/datastore/physical_disk.rs index f503e034385..52911703e06 100644 --- a/nexus/db-queries/src/db/datastore/physical_disk.rs +++ b/nexus/db-queries/src/db/datastore/physical_disk.rs @@ -23,9 +23,6 @@ use crate::db::pagination::paginated; use async_bb8_diesel::AsyncRunQueryDsl; use chrono::Utc; use diesel::prelude::*; -use diesel::result::{ - DatabaseErrorKind as DieselErrorKind, Error as DieselError, -}; use nexus_db_errors::ErrorHandler; use nexus_db_errors::OptionalError; use nexus_db_errors::TransactionError; @@ -175,9 +172,28 @@ impl DataStore { .into(), )); } + // Idempotent case: if an adoption request already exists + // for this disk then query it back and return it. + // + // Note: We are only doing this before the insert + // due to diesel limitations with regards to + // on_conflict/on_constraint for partial indexes. + let req = adoption_dsl::physical_disk_adoption_request + .filter(adoption_dsl::vendor.eq(vendor.clone())) + .filter(adoption_dsl::serial.eq(serial.clone())) + .filter(adoption_dsl::model.eq(model.clone())) + .filter(adoption_dsl::time_deleted.is_null()) + .select(PhysicalDiskAdoptionRequest::as_select()) + .first_async(&conn) + .await + .optional()?; + + if let Some(req) = req { + return Ok(req); + } // Insert the adoption request. - let request = diesel::insert_into( + diesel::insert_into( adoption_dsl::physical_disk_adoption_request, ) .values(( @@ -189,56 +205,20 @@ impl DataStore { )) .returning(PhysicalDiskAdoptionRequest::as_returning()) .get_result_async(&conn) - .await?; - - Ok(request) + .await } }) .await; match txn_res { - Ok(request) => return Ok(request), + Ok(request) => Ok(request), Err(e) => match err.take() { // A called function performed its own error propagation. - Some(txn_error) => { - return Err(txn_error.into_public_ignore_retries()); - } + Some(txn_error) => Err(txn_error.into_public_ignore_retries()), // The transaction setup/teardown itself encountered a diesel error. - None => match e { - // Check for a unique index violation for an active - // adoption request with the same vendor/serial/model. - // - // Return the existing request in this case as the - // request is idempotent. - DieselError::DatabaseError( - DieselErrorKind::UniqueViolation, - _, - ) => { - // Fall through to query the existing request below. - } - _ => { - return Err(public_error_from_diesel( - e, - ErrorHandler::Server, - )); - } - }, + None => Err(public_error_from_diesel(e, ErrorHandler::Server)), }, } - - // Idempotent case: an adoption request already exists for this disk. - // Query it back and return it. - let conn = &*self.pool_connection_authorized(opctx).await?; - let request = adoption_dsl::physical_disk_adoption_request - .filter(adoption_dsl::vendor.eq(disk_id.vendor)) - .filter(adoption_dsl::serial.eq(disk_id.serial)) - .filter(adoption_dsl::model.eq(disk_id.model)) - .filter(adoption_dsl::time_deleted.is_null()) - .select(PhysicalDiskAdoptionRequest::as_select()) - .first_async(conn) - .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; - Ok(request) } /// Stores a new physical disk in the database. From 156f06e961a1303a023d07d565773436aa5add46 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Fri, 8 May 2026 22:05:04 +0000 Subject: [PATCH 26/26] NAMESPACE!!!!!!!!!!!!! --- schema/crdb/add-disk-adoption-requests/up01.sql | 2 +- schema/crdb/dbinit.sql | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/schema/crdb/add-disk-adoption-requests/up01.sql b/schema/crdb/add-disk-adoption-requests/up01.sql index 8d23de3c40f..5920c98e462 100644 --- a/schema/crdb/add-disk-adoption-requests/up01.sql +++ b/schema/crdb/add-disk-adoption-requests/up01.sql @@ -1,4 +1,4 @@ -CREATE TABLE IF NOT EXISTS physical_disk_adoption_request ( +CREATE TABLE IF NOT EXISTS omicron.public.physical_disk_adoption_request ( id UUID PRIMARY KEY, vendor STRING(63) NOT NULL, model STRING(63) NOT NULL, diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 5055740c4e1..9be0e8b1693 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -506,7 +506,7 @@ CREATE UNIQUE INDEX IF NOT EXISTS lookup_physical_disk_by_sled ON omicron.public -- Any disk present in this table with a NULL time_deleted column -- is eligible for adoption. -CREATE TABLE IF NOT EXISTS physical_disk_adoption_request ( +CREATE TABLE IF NOT EXISTS omicron.public.physical_disk_adoption_request ( id UUID PRIMARY KEY, vendor STRING(63) NOT NULL, model STRING(63) NOT NULL, @@ -517,7 +517,7 @@ CREATE TABLE IF NOT EXISTS physical_disk_adoption_request ( -- Only one eligible disk with the same physical id may be present at a time CREATE UNIQUE INDEX IF NOT EXISTS physical_disk_adoption_request_by_physical_id -ON physical_disk_adoption_request ( +ON omicron.public.physical_disk_adoption_request ( vendor, model, serial