From 7156ba06870c10ea01df546f10d9f7b40f07a5a1 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 7 Aug 2024 17:18:40 -0700 Subject: [PATCH] safer API for Dataset::new --- nexus/db-model/src/dataset.rs | 13 ++++++++----- nexus/db-model/src/dataset_kind.rs | 8 ++++++-- nexus/db-queries/src/db/datastore/dataset.rs | 13 +++++-------- nexus/db-queries/src/db/datastore/mod.rs | 13 +++++-------- nexus/reconfigurator/execution/src/datasets.rs | 3 +-- .../execution/src/omicron_physical_disks.rs | 3 +-- .../background/tasks/decommissioned_disk_cleaner.rs | 3 +-- nexus/src/app/rack.rs | 3 +-- nexus/src/app/sagas/region_replacement_start.rs | 6 +----- nexus/src/app/sled.rs | 4 ++-- 10 files changed, 31 insertions(+), 38 deletions(-) diff --git a/nexus/db-model/src/dataset.rs b/nexus/db-model/src/dataset.rs index d525b80241..f896f11c5b 100644 --- a/nexus/db-model/src/dataset.rs +++ b/nexus/db-model/src/dataset.rs @@ -8,6 +8,7 @@ use crate::ipv6; use crate::schema::{dataset, region}; use chrono::{DateTime, Utc}; use db_macros::Asset; +use omicron_common::api::internal::shared::DatasetKind as ApiDatasetKind; use serde::{Deserialize, Serialize}; use std::net::{Ipv6Addr, SocketAddrV6}; use uuid::Uuid; @@ -49,13 +50,15 @@ impl Dataset { id: Uuid, pool_id: Uuid, addr: Option, - kind: DatasetKind, - zone_name: Option, + api_kind: ApiDatasetKind, ) -> Self { - let size_used = match kind { - DatasetKind::Crucible => Some(0), - _ => None, + let kind = DatasetKind::from(&api_kind); + let (size_used, zone_name) = match api_kind { + ApiDatasetKind::Crucible => (Some(0), None), + ApiDatasetKind::Zone { name } => (None, Some(name)), + _ => (None, None), }; + Self { identity: DatasetIdentity::new(id), time_deleted: None, diff --git a/nexus/db-model/src/dataset_kind.rs b/nexus/db-model/src/dataset_kind.rs index 2e71b96a41..856b340a11 100644 --- a/nexus/db-model/src/dataset_kind.rs +++ b/nexus/db-model/src/dataset_kind.rs @@ -27,8 +27,8 @@ impl_enum_type!( Debug => b"debug" ); -impl From for DatasetKind { - fn from(k: internal::shared::DatasetKind) -> Self { +impl From<&internal::shared::DatasetKind> for DatasetKind { + fn from(k: &internal::shared::DatasetKind) -> Self { match k { internal::shared::DatasetKind::Crucible => DatasetKind::Crucible, internal::shared::DatasetKind::Cockroach => DatasetKind::Cockroach, @@ -45,6 +45,10 @@ impl From for DatasetKind { DatasetKind::InternalDns } internal::shared::DatasetKind::ZoneRoot => DatasetKind::ZoneRoot, + // Enums in the database do not have associated data, so this drops + // the "name" of the zone and only considers the type. + // + // The zone name, if it exists, is stored in a separate column. internal::shared::DatasetKind::Zone { .. } => DatasetKind::Zone, internal::shared::DatasetKind::Debug => DatasetKind::Debug, } diff --git a/nexus/db-queries/src/db/datastore/dataset.rs b/nexus/db-queries/src/db/datastore/dataset.rs index 8a814aea80..0fe1c7912e 100644 --- a/nexus/db-queries/src/db/datastore/dataset.rs +++ b/nexus/db-queries/src/db/datastore/dataset.rs @@ -241,6 +241,7 @@ mod test { use nexus_db_model::SledSystemHardware; use nexus_db_model::SledUpdate; use nexus_test_utils::db::test_setup_database; + use omicron_common::api::internal::shared::DatasetKind as ApiDatasetKind; use omicron_test_utils::dev; #[tokio::test] @@ -291,8 +292,7 @@ mod test { Uuid::new_v4(), zpool_id, Some("[::1]:0".parse().unwrap()), - DatasetKind::Crucible, - None, + ApiDatasetKind::Crucible, )) .await .expect("failed to insert dataset") @@ -325,8 +325,7 @@ mod test { dataset1.id(), zpool_id, Some("[::1]:12345".parse().unwrap()), - DatasetKind::Cockroach, - None, + ApiDatasetKind::Cockroach, )) .await .expect("failed to do-nothing insert dataset"); @@ -342,8 +341,7 @@ mod test { Uuid::new_v4(), zpool_id, Some("[::1]:0".parse().unwrap()), - DatasetKind::Cockroach, - None, + ApiDatasetKind::Cockroach, )) .await .expect("failed to upsert dataset"); @@ -375,8 +373,7 @@ mod test { dataset1.id(), zpool_id, Some("[::1]:12345".parse().unwrap()), - DatasetKind::Cockroach, - None, + ApiDatasetKind::Cockroach, )) .await .expect("failed to do-nothing insert dataset"); diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index 10c812de89..881f0d4aa5 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -392,10 +392,10 @@ mod test { use crate::db::identity::Asset; use crate::db::lookup::LookupPath; use crate::db::model::{ - BlockSize, ConsoleSession, Dataset, DatasetKind, ExternalIp, - PhysicalDisk, PhysicalDiskKind, PhysicalDiskPolicy, PhysicalDiskState, - Project, Rack, Region, SiloUser, SledBaseboard, SledSystemHardware, - SledUpdate, SshKey, Zpool, + BlockSize, ConsoleSession, Dataset, ExternalIp, PhysicalDisk, + PhysicalDiskKind, PhysicalDiskPolicy, PhysicalDiskState, Project, Rack, + Region, SiloUser, SledBaseboard, SledSystemHardware, SledUpdate, + SshKey, Zpool, }; use crate::db::queries::vpc_subnet::InsertVpcSubnetQuery; use chrono::{Duration, Utc}; @@ -411,6 +411,7 @@ mod test { use omicron_common::api::external::{ ByteCount, Error, IdentityMetadataCreateParams, LookupType, Name, }; + use omicron_common::api::internal::shared::DatasetKind; use omicron_test_utils::dev; use omicron_uuid_kinds::CollectionUuid; use omicron_uuid_kinds::GenericUuid; @@ -908,7 +909,6 @@ mod test { zpool.pool_id, bogus_addr, DatasetKind::Crucible, - None, ); let datastore = datastore.clone(); @@ -1281,7 +1281,6 @@ mod test { zpool_id, bogus_addr, DatasetKind::Crucible, - None, ); let datastore = datastore.clone(); async move { @@ -1382,7 +1381,6 @@ mod test { zpool_id, bogus_addr, DatasetKind::Crucible, - None, ); let datastore = datastore.clone(); async move { @@ -1458,7 +1456,6 @@ mod test { zpool_id, bogus_addr, DatasetKind::Crucible, - None, ); datastore.dataset_upsert(dataset).await.unwrap(); physical_disk_ids.push(physical_disk_id); diff --git a/nexus/reconfigurator/execution/src/datasets.rs b/nexus/reconfigurator/execution/src/datasets.rs index d20de82cef..2f84378a13 100644 --- a/nexus/reconfigurator/execution/src/datasets.rs +++ b/nexus/reconfigurator/execution/src/datasets.rs @@ -67,8 +67,7 @@ pub(crate) async fn ensure_dataset_records_exist( id.into_untyped_uuid(), pool_id.into_untyped_uuid(), Some(address), - kind.clone().into(), - kind.zone_name().map(String::from), + kind.clone(), ); let maybe_inserted = datastore .dataset_insert_if_not_exists(dataset) diff --git a/nexus/reconfigurator/execution/src/omicron_physical_disks.rs b/nexus/reconfigurator/execution/src/omicron_physical_disks.rs index 9dcaa098d5..380448acf2 100644 --- a/nexus/reconfigurator/execution/src/omicron_physical_disks.rs +++ b/nexus/reconfigurator/execution/src/omicron_physical_disks.rs @@ -124,7 +124,6 @@ mod test { use httptest::responders::status_code; use httptest::Expectation; use nexus_db_model::Dataset; - use nexus_db_model::DatasetKind; use nexus_db_model::PhysicalDisk; use nexus_db_model::PhysicalDiskKind; use nexus_db_model::PhysicalDiskPolicy; @@ -142,6 +141,7 @@ mod test { use nexus_types::identity::Asset; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::Generation; + use omicron_common::api::internal::shared::DatasetKind; use omicron_common::disk::DiskIdentity; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::PhysicalDiskUuid; @@ -435,7 +435,6 @@ mod test { 0, )), DatasetKind::Crucible, - None, )) .await .unwrap(); diff --git a/nexus/src/app/background/tasks/decommissioned_disk_cleaner.rs b/nexus/src/app/background/tasks/decommissioned_disk_cleaner.rs index 0e90ed84ab..6e49ddc7f0 100644 --- a/nexus/src/app/background/tasks/decommissioned_disk_cleaner.rs +++ b/nexus/src/app/background/tasks/decommissioned_disk_cleaner.rs @@ -179,13 +179,13 @@ mod tests { use diesel::ExpressionMethods; use diesel::QueryDsl; use nexus_db_model::Dataset; - use nexus_db_model::DatasetKind; use nexus_db_model::PhysicalDisk; use nexus_db_model::PhysicalDiskKind; use nexus_db_model::PhysicalDiskPolicy; use nexus_db_model::Region; use nexus_test_utils::SLED_AGENT_UUID; use nexus_test_utils_macros::nexus_test; + use omicron_common::api::internal::shared::DatasetKind; use omicron_uuid_kinds::{ DatasetUuid, PhysicalDiskUuid, RegionUuid, SledUuid, }; @@ -246,7 +246,6 @@ mod tests { 0, )), DatasetKind::Crucible, - None, )) .await .unwrap(); diff --git a/nexus/src/app/rack.rs b/nexus/src/app/rack.rs index 432c44bea4..b289e871eb 100644 --- a/nexus/src/app/rack.rs +++ b/nexus/src/app/rack.rs @@ -146,8 +146,7 @@ impl super::Nexus { dataset.dataset_id, dataset.zpool_id, Some(dataset.request.address), - dataset.request.kind.clone().into(), - dataset.request.kind.zone_name().map(String::from), + dataset.request.kind, ) }) .collect(); diff --git a/nexus/src/app/sagas/region_replacement_start.rs b/nexus/src/app/sagas/region_replacement_start.rs index 60c35781f1..c2b886938a 100644 --- a/nexus/src/app/sagas/region_replacement_start.rs +++ b/nexus/src/app/sagas/region_replacement_start.rs @@ -776,7 +776,6 @@ pub(crate) mod test { }; use chrono::Utc; use nexus_db_model::Dataset; - use nexus_db_model::DatasetKind; use nexus_db_model::Region; use nexus_db_model::RegionReplacement; use nexus_db_model::RegionReplacementState; @@ -787,6 +786,7 @@ pub(crate) mod test { use nexus_test_utils::resource_helpers::create_project; use nexus_test_utils_macros::nexus_test; use nexus_types::identity::Asset; + use omicron_common::api::internal::shared::DatasetKind; use sled_agent_client::types::VolumeConstructionRequest; use uuid::Uuid; @@ -903,28 +903,24 @@ pub(crate) mod test { Uuid::new_v4(), Some("[fd00:1122:3344:101::1]:12345".parse().unwrap()), DatasetKind::Crucible, - None, ), Dataset::new( Uuid::new_v4(), Uuid::new_v4(), Some("[fd00:1122:3344:102::1]:12345".parse().unwrap()), DatasetKind::Crucible, - None, ), Dataset::new( Uuid::new_v4(), Uuid::new_v4(), Some("[fd00:1122:3344:103::1]:12345".parse().unwrap()), DatasetKind::Crucible, - None, ), Dataset::new( Uuid::new_v4(), Uuid::new_v4(), Some("[fd00:1122:3344:104::1]:12345".parse().unwrap()), DatasetKind::Crucible, - None, ), ]; diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index a0e1cc3526..9c21ca73a1 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -12,7 +12,6 @@ use nexus_db_queries::authz; use nexus_db_queries::context::OpContext; use nexus_db_queries::db; use nexus_db_queries::db::lookup; -use nexus_db_queries::db::model::DatasetKind; use nexus_sled_agent_shared::inventory::SledRole; use nexus_types::deployment::DiskFilter; use nexus_types::deployment::SledFilter; @@ -23,6 +22,7 @@ use omicron_common::api::external::DataPageParams; use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; +use omicron_common::api::internal::shared::DatasetKind; use omicron_uuid_kinds::{GenericUuid, SledUuid}; use sled_agent_client::Client as SledAgentClient; use std::net::SocketAddrV6; @@ -308,7 +308,7 @@ impl super::Nexus { ); let kind = DatasetKind::Crucible; let dataset = - db::model::Dataset::new(id, zpool_id, Some(address), kind, None); + db::model::Dataset::new(id, zpool_id, Some(address), kind); self.db_datastore.dataset_upsert(dataset).await?; Ok(()) }