Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add disposition to blueprint disks #7169

Merged
merged 5 commits into from
Nov 26, 2024
Merged

Add disposition to blueprint disks #7169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
83f1b9f
Add disposition to blueprint disks
andrewjstone Nov 25, 2024
81f970d
schema test fixes
andrewjstone Nov 26, 2024
3f52153
do not nest OmicronPhysicalDiskConfig
andrewjstone Nov 26, 2024
4347ad5
expectorate
andrewjstone Nov 26, 2024
068a769
openapi
andrewjstone Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions clients/nexus-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ progenitor::generate_api!(
// as "blueprint" this way, but we have really useful functionality
// (e.g., diff'ing) that's implemented on our local type.
Blueprint = nexus_types::deployment::Blueprint,
BlueprintPhysicalDiskConfig = nexus_types::deployment::BlueprintPhysicalDiskConfig,
BlueprintPhysicalDisksConfig = nexus_types::deployment::BlueprintPhysicalDisksConfig,
BlueprintPhysicalDiskDisposition = nexus_types::deployment::BlueprintPhysicalDiskDisposition,
Certificate = omicron_common::api::internal::nexus::Certificate,
ClickhouseMode = nexus_types::deployment::ClickhouseMode,
ClickhousePolicy = nexus_types::deployment::ClickhousePolicy,
Expand All @@ -43,8 +46,8 @@ progenitor::generate_api!(
NetworkInterface = omicron_common::api::internal::shared::NetworkInterface,
NetworkInterfaceKind = omicron_common::api::internal::shared::NetworkInterfaceKind,
NewPasswordHash = omicron_passwords::NewPasswordHash,
OmicronPhysicalDiskConfig = nexus_types::disk::OmicronPhysicalDiskConfig,
OmicronPhysicalDisksConfig = nexus_types::disk::OmicronPhysicalDisksConfig,
OmicronPhysicalDiskConfig = omicron_common::disk::OmicronPhysicalDiskConfig,
OmicronPhysicalDisksConfig = omicron_common::disk::OmicronPhysicalDisksConfig,
RecoverySiloConfig = nexus_sled_agent_shared::recovery_silo::RecoverySiloConfig,
Srv = nexus_types::internal_api::params::Srv,
TypedUuidForCollectionKind = omicron_uuid_kinds::CollectionUuid,
Expand Down
3 changes: 3 additions & 0 deletions clients/sled-agent-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ progenitor::generate_api!(
DatasetsConfig = omicron_common::disk::DatasetsConfig,
DatasetKind = omicron_common::api::internal::shared::DatasetKind,
DiskIdentity = omicron_common::disk::DiskIdentity,
DisksManagementResult = omicron_common::disk::DisksManagementResult,
DiskManagementStatus = omicron_common::disk::DiskManagementStatus,
DiskManagementError = omicron_common::disk::DiskManagementError,
DiskVariant = omicron_common::disk::DiskVariant,
ExternalIpGatewayMap = omicron_common::api::internal::shared::ExternalIpGatewayMap,
Generation = omicron_common::api::external::Generation,
Expand Down
55 changes: 55 additions & 0 deletions nexus/db-model/src/deployment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use nexus_types::deployment::BlueprintDatasetConfig;
use nexus_types::deployment::BlueprintDatasetDisposition;
use nexus_types::deployment::BlueprintDatasetsConfig;
use nexus_types::deployment::BlueprintPhysicalDiskConfig;
use nexus_types::deployment::BlueprintPhysicalDiskDisposition;
use nexus_types::deployment::BlueprintPhysicalDisksConfig;
use nexus_types::deployment::BlueprintTarget;
use nexus_types::deployment::BlueprintZoneConfig;
Expand Down Expand Up @@ -146,6 +147,54 @@ pub struct BpSledState {
pub sled_state: SledState,
}

impl_enum_type!(
#[derive(Clone, SqlType, Debug, QueryId)]
#[diesel(postgres_type(name = "bp_physical_disk_disposition", schema = "public"))]
pub struct DbBpPhysicalDiskDispositionEnum;

/// This type is not actually public, because [`BlueprintPhysicalDiskDisposition`]
/// interacts with external logic.
///
/// However, it must be marked `pub` to avoid errors like `crate-private
/// type `BpPhysicalDiskDispositionEnum` in public interface`. Marking this type `pub`,
/// without actually making it public, tricks rustc in a desirable way.
#[derive(Clone, Copy, Debug, AsExpression, FromSqlRow, PartialEq)]
#[diesel(sql_type = DbBpPhysicalDiskDispositionEnum)]
pub enum DbBpPhysicalDiskDisposition;

// Enum values
InService => b"in_service"
Expunged => b"expunged"
);

/// Converts a [`BlueprintPhysicalDiskDisposition`] to a version that can be inserted
/// into a database.
pub fn to_db_bp_physical_disk_disposition(
disposition: BlueprintPhysicalDiskDisposition,
) -> DbBpPhysicalDiskDisposition {
match disposition {
BlueprintPhysicalDiskDisposition::InService => {
DbBpPhysicalDiskDisposition::InService
}
BlueprintPhysicalDiskDisposition::Expunged => {
DbBpPhysicalDiskDisposition::Expunged
}
}
}

impl From<DbBpPhysicalDiskDisposition> for BlueprintPhysicalDiskDisposition {
fn from(disposition: DbBpPhysicalDiskDisposition) -> Self {
match disposition {
DbBpPhysicalDiskDisposition::InService => {
BlueprintPhysicalDiskDisposition::InService
}
DbBpPhysicalDiskDisposition::Expunged => {
BlueprintPhysicalDiskDisposition::Expunged
}
}
}
}

/// See [`nexus_types::deployment::BlueprintPhysicalDisksConfig`].
#[derive(Queryable, Clone, Debug, Selectable, Insertable)]
#[diesel(table_name = bp_sled_omicron_physical_disks)]
Expand Down Expand Up @@ -182,6 +231,8 @@ pub struct BpOmicronPhysicalDisk {

pub id: DbTypedUuid<PhysicalDiskKind>,
pub pool_id: Uuid,

pub disposition: DbBpPhysicalDiskDisposition,
}

impl BpOmicronPhysicalDisk {
Expand All @@ -198,13 +249,17 @@ impl BpOmicronPhysicalDisk {
model: disk_config.identity.model.clone(),
id: disk_config.id.into(),
pool_id: disk_config.pool_id.into_untyped_uuid(),
disposition: to_db_bp_physical_disk_disposition(
disk_config.disposition,
),
}
}
}

impl From<BpOmicronPhysicalDisk> for BlueprintPhysicalDiskConfig {
fn from(disk: BpOmicronPhysicalDisk) -> Self {
Self {
disposition: disk.disposition.into(),
identity: DiskIdentity {
vendor: disk.vendor,
serial: disk.serial,
Expand Down
2 changes: 2 additions & 0 deletions nexus/db-model/src/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1665,6 +1665,8 @@ table! {

id -> Uuid,
pool_id -> Uuid,

disposition -> crate::DbBpPhysicalDiskDispositionEnum,
}
}

Expand Down
3 changes: 2 additions & 1 deletion nexus/db-model/src/schema_versions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::collections::BTreeMap;
///
/// 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: SemverVersion = SemverVersion::new(115, 0, 0);
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(116, 0, 0);

/// List of all past database schema versions, in *reverse* order
///
Expand All @@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::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(116, "bp-physical-disk-disposition"),
KnownVersion::new(115, "inv-omicron-physical-disks-generation"),
KnownVersion::new(114, "crucible-ref-count-records"),
KnownVersion::new(113, "add-tx-eq"),
Expand Down
74 changes: 36 additions & 38 deletions nexus/reconfigurator/execution/src/omicron_physical_disks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ pub(crate) async fn deploy_disks(
db_sled.sled_agent_address(),
&log,
);
let result =
client.omicron_physical_disks_put(&config).await.with_context(
|| format!("Failed to put {config:#?} to sled {sled_id}"),
);
let result = client
.omicron_physical_disks_put(&config.clone().into())
.await
.with_context(|| {
format!("Failed to put {config:#?} to sled {sled_id}")
});
match result {
Err(error) => {
warn!(log, "{error:#}");
Expand Down Expand Up @@ -146,6 +148,7 @@ mod test {
use nexus_sled_agent_shared::inventory::SledRole;
use nexus_test_utils::SLED_AGENT_UUID;
use nexus_test_utils_macros::nexus_test;
use nexus_types::deployment::BlueprintPhysicalDiskDisposition;
use nexus_types::deployment::{
Blueprint, BlueprintPhysicalDiskConfig, BlueprintPhysicalDisksConfig,
BlueprintTarget, CockroachDbPreserveDowngrade, DiskFilter,
Expand All @@ -155,6 +158,10 @@ mod test {
use omicron_common::api::external::Generation;
use omicron_common::api::internal::shared::DatasetKind;
use omicron_common::disk::DiskIdentity;
use omicron_common::disk::DiskManagementError;
use omicron_common::disk::DiskManagementStatus;
use omicron_common::disk::DisksManagementResult;
use omicron_common::disk::OmicronPhysicalDisksConfig;
use omicron_uuid_kinds::DatasetUuid;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::PhysicalDiskUuid;
Expand Down Expand Up @@ -241,8 +248,9 @@ mod test {
// See `rack_setup::service::ServiceInner::run` for more details.
fn make_disks() -> BlueprintPhysicalDisksConfig {
BlueprintPhysicalDisksConfig {
generation: Generation::new(),
generation: Generation::new().next(),
disks: vec![BlueprintPhysicalDiskConfig {
disposition: BlueprintPhysicalDiskDisposition::InService,
identity: DiskIdentity {
vendor: "test-vendor".to_string(),
serial: "test-serial".to_string(),
Expand Down Expand Up @@ -270,18 +278,16 @@ mod test {
s.expect(
Expectation::matching(all_of![
request::method_path("PUT", "/omicron-physical-disks",),
// Our generation number should be 1 and there should
// Our generation number should be 2 and there should
// be only a single disk.
request::body(json_decoded(
|c: &BlueprintPhysicalDisksConfig| {
c.generation == 1u32.into() && c.disks.len() == 1
|c: &OmicronPhysicalDisksConfig| {
c.generation == 2u32.into() && c.disks.len() == 1
}
))
])
.respond_with(json_encoded(
sled_agent_client::types::DisksManagementResult {
status: vec![],
},
DisksManagementResult { status: vec![] },
)),
);
}
Expand All @@ -303,9 +309,7 @@ mod test {
"/omicron-physical-disks",
))
.respond_with(json_encoded(
sled_agent_client::types::DisksManagementResult {
status: vec![],
},
DisksManagementResult { status: vec![] },
)),
);
}
Expand All @@ -323,11 +327,9 @@ mod test {
"PUT",
"/omicron-physical-disks",
))
.respond_with(json_encoded(
sled_agent_client::types::DisksManagementResult {
status: vec![],
},
)),
.respond_with(json_encoded(DisksManagementResult {
status: vec![],
})),
);
s2.expect(
Expectation::matching(request::method_path(
Expand All @@ -346,7 +348,7 @@ mod test {
assert_eq!(errors.len(), 1);
assert!(errors[0]
.to_string()
.starts_with("Failed to put OmicronPhysicalDisksConfig"));
.starts_with("Failed to put BlueprintPhysicalDisksConfig"));
s1.verify_and_clear();
s2.verify_and_clear();

Expand All @@ -358,30 +360,26 @@ mod test {
"PUT",
"/omicron-physical-disks",
))
.respond_with(json_encoded(
sled_agent_client::types::DisksManagementResult {
status: vec![],
},
)),
.respond_with(json_encoded(DisksManagementResult {
status: vec![],
})),
);
s2.expect(
Expectation::matching(request::method_path(
"PUT",
"/omicron-physical-disks",
))
.respond_with(json_encoded(sled_agent_client::types::DisksManagementResult {
status: vec![
sled_agent_client::types::DiskManagementStatus {
identity: omicron_common::disk::DiskIdentity {
vendor: "v".to_string(),
serial: "s".to_string(),
model: "m".to_string(),
},

// This error could occur if a disk is removed
err: Some(sled_agent_client::types::DiskManagementError::NotFound),
}
]
.respond_with(json_encoded(DisksManagementResult {
status: vec![DiskManagementStatus {
identity: omicron_common::disk::DiskIdentity {
vendor: "v".to_string(),
serial: "s".to_string(),
model: "m".to_string(),
},

// This error could occur if a disk is removed
err: Some(DiskManagementError::NotFound),
}],
})),
);

Expand Down
2 changes: 2 additions & 0 deletions nexus/reconfigurator/planning/src/blueprint_builder/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use nexus_sled_agent_shared::inventory::ZoneKind;
use nexus_types::deployment::blueprint_zone_type;
use nexus_types::deployment::Blueprint;
use nexus_types::deployment::BlueprintPhysicalDiskConfig;
use nexus_types::deployment::BlueprintPhysicalDiskDisposition;
use nexus_types::deployment::BlueprintZoneConfig;
use nexus_types::deployment::BlueprintZoneDisposition;
use nexus_types::deployment::BlueprintZoneFilter;
Expand Down Expand Up @@ -887,6 +888,7 @@ impl<'a> BlueprintBuilder<'a> {
for (disk_id, (zpool, disk)) in database_disks {
database_disk_ids.insert(disk_id);
sled_storage.ensure_disk(BlueprintPhysicalDiskConfig {
disposition: BlueprintPhysicalDiskDisposition::InService,
identity: disk.disk_identity.clone(),
id: disk_id,
pool_id: *zpool,
Expand Down
11 changes: 8 additions & 3 deletions nexus/test-utils/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ use nexus_types::deployment::Blueprint;
use nexus_types::deployment::BlueprintDatasetConfig;
use nexus_types::deployment::BlueprintDatasetDisposition;
use nexus_types::deployment::BlueprintDatasetsConfig;
use nexus_types::deployment::BlueprintPhysicalDiskConfig;
use nexus_types::deployment::BlueprintPhysicalDiskDisposition;
use nexus_types::deployment::BlueprintPhysicalDisksConfig;
use nexus_types::deployment::BlueprintZoneConfig;
use nexus_types::deployment::BlueprintZoneDisposition;
Expand Down Expand Up @@ -68,7 +70,6 @@ use omicron_common::api::internal::shared::NetworkInterface;
use omicron_common::api::internal::shared::NetworkInterfaceKind;
use omicron_common::api::internal::shared::SwitchLocation;
use omicron_common::disk::CompressionAlgorithm;
use omicron_common::disk::OmicronPhysicalDiskConfig;
use omicron_common::zpool_name::ZpoolName;
use omicron_sled_agent::sim;
use omicron_test_utils::dev;
Expand Down Expand Up @@ -831,7 +832,9 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
let mut datasets = BTreeMap::new();
for zone in zones {
if let Some(zpool) = &zone.filesystem_pool {
disks.push(OmicronPhysicalDiskConfig {
disks.push(BlueprintPhysicalDiskConfig {
disposition:
BlueprintPhysicalDiskDisposition::InService,
identity: omicron_common::disk::DiskIdentity {
vendor: "nexus-tests".to_string(),
model: "nexus-test-model".to_string(),
Expand Down Expand Up @@ -868,7 +871,9 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
// Populate extra fake disks, giving each sled 10 total.
if disks.len() < 10 {
for _ in disks.len()..10 {
disks.push(OmicronPhysicalDiskConfig {
disks.push(BlueprintPhysicalDiskConfig {
disposition:
BlueprintPhysicalDiskDisposition::InService,
identity: omicron_common::disk::DiskIdentity {
vendor: "nexus-tests".to_string(),
model: "nexus-test-model".to_string(),
Expand Down
Loading
Loading