Skip to content

Commit

Permalink
safer API for Dataset::new
Browse files Browse the repository at this point in the history
  • Loading branch information
smklein committed Aug 8, 2024
1 parent 994757e commit 7156ba0
Show file tree
Hide file tree
Showing 10 changed files with 31 additions and 38 deletions.
13 changes: 8 additions & 5 deletions nexus/db-model/src/dataset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -49,13 +50,15 @@ impl Dataset {
id: Uuid,
pool_id: Uuid,
addr: Option<SocketAddrV6>,
kind: DatasetKind,
zone_name: Option<String>,
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,
Expand Down
8 changes: 6 additions & 2 deletions nexus/db-model/src/dataset_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ impl_enum_type!(
Debug => b"debug"
);

impl From<internal::shared::DatasetKind> 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,
Expand All @@ -45,6 +45,10 @@ impl From<internal::shared::DatasetKind> 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,
}
Expand Down
13 changes: 5 additions & 8 deletions nexus/db-queries/src/db/datastore/dataset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand Down
13 changes: 5 additions & 8 deletions nexus/db-queries/src/db/datastore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -908,7 +909,6 @@ mod test {
zpool.pool_id,
bogus_addr,
DatasetKind::Crucible,
None,
);

let datastore = datastore.clone();
Expand Down Expand Up @@ -1281,7 +1281,6 @@ mod test {
zpool_id,
bogus_addr,
DatasetKind::Crucible,
None,
);
let datastore = datastore.clone();
async move {
Expand Down Expand Up @@ -1382,7 +1381,6 @@ mod test {
zpool_id,
bogus_addr,
DatasetKind::Crucible,
None,
);
let datastore = datastore.clone();
async move {
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions nexus/reconfigurator/execution/src/datasets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions nexus/reconfigurator/execution/src/omicron_physical_disks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -435,7 +435,6 @@ mod test {
0,
)),
DatasetKind::Crucible,
None,
))
.await
.unwrap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -246,7 +246,6 @@ mod tests {
0,
)),
DatasetKind::Crucible,
None,
))
.await
.unwrap();
Expand Down
3 changes: 1 addition & 2 deletions nexus/src/app/rack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 1 addition & 5 deletions nexus/src/app/sagas/region_replacement_start.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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,
),
];

Expand Down
4 changes: 2 additions & 2 deletions nexus/src/app/sled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(())
}
Expand Down

0 comments on commit 7156ba0

Please sign in to comment.