From 220bb3243ed1daeb5625d4db3c6cf062345ce2f3 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 6 Jan 2025 11:03:01 -0800 Subject: [PATCH 1/2] [reconfigurator] Avoid deploying expunged datasets --- .../reconfigurator/execution/src/datasets.rs | 93 +++++++++++++++++++ nexus/types/src/deployment.rs | 9 +- 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/nexus/reconfigurator/execution/src/datasets.rs b/nexus/reconfigurator/execution/src/datasets.rs index b0aa7410f0a..b73d021fd06 100644 --- a/nexus/reconfigurator/execution/src/datasets.rs +++ b/nexus/reconfigurator/execution/src/datasets.rs @@ -255,16 +255,19 @@ mod tests { use super::*; use nexus_db_model::Zpool; use nexus_reconfigurator_planning::example::ExampleSystemBuilder; + use nexus_sled_agent_shared::inventory::SledRole; use nexus_test_utils_macros::nexus_test; use nexus_types::deployment::Blueprint; use nexus_types::deployment::BlueprintZoneFilter; use omicron_common::api::external::ByteCount; + use omicron_common::api::external::Generation; use omicron_common::api::internal::shared::DatasetKind; use omicron_common::disk::CompressionAlgorithm; use omicron_common::zpool_name::ZpoolName; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::ZpoolUuid; + use std::net::SocketAddr; type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; @@ -290,6 +293,96 @@ mod tests { .collect::>() } + #[nexus_test] + async fn test_deploy_datasets(cptestctx: &ControlPlaneTestContext) { + // Set up. + let nexus = &cptestctx.server.server_context().nexus; + let datastore = nexus.datastore(); + let opctx = OpContext::for_tests( + cptestctx.logctx.log.clone(), + datastore.clone(), + ); + + let sim_sled_agent_addr = + match cptestctx.sled_agent.http_server.local_addr() { + SocketAddr::V6(addr) => addr, + _ => panic!( + "Unexpected address type for sled agent (wanted IPv6)" + ), + }; + let sim_sled_agent = &cptestctx.sled_agent.sled_agent; + + // This is a fully fabricated dataset list for a simulated sled agent. + // + // We're testing the validity of the deployment calls here, not of any + // blueprint. + + let sleds_by_id = BTreeMap::from([( + sim_sled_agent.id, + Sled::new( + sim_sled_agent.id, + sim_sled_agent_addr, + SledRole::Scrimlet, + ), + )]); + + // Create two datasets which look like they came from the blueprint: One + // which is in-service, and one which is expunged. + // + // During deployment, the in-service dataset should be deployed, but the + // expunged dataset should be ignored. + let dataset_id = DatasetUuid::new_v4(); + let expunged_dataset_id = DatasetUuid::new_v4(); + let datasets_config = BlueprintDatasetsConfig { + generation: Generation::new(), + datasets: BTreeMap::from([ + ( + dataset_id, + BlueprintDatasetConfig { + disposition: BlueprintDatasetDisposition::InService, + id: dataset_id, + pool: ZpoolName::new_external(ZpoolUuid::new_v4()), + kind: DatasetKind::Crucible, + address: None, + quota: None, + reservation: None, + compression: CompressionAlgorithm::Off, + }, + ), + ( + expunged_dataset_id, + BlueprintDatasetConfig { + disposition: BlueprintDatasetDisposition::Expunged, + id: expunged_dataset_id, + pool: ZpoolName::new_external(ZpoolUuid::new_v4()), + kind: DatasetKind::Crucible, + address: None, + quota: None, + reservation: None, + compression: CompressionAlgorithm::Off, + }, + ), + ]), + }; + let sled_configs = + BTreeMap::from([(sim_sled_agent.id, datasets_config.clone())]); + + // Give the simulated sled agent a configuration to deploy + deploy_datasets(&opctx, &sleds_by_id, &sled_configs) + .await + .expect("Deploying datasets should have succeeded"); + + // Observe the latest configuration stored on the simulated sled agent, + // and verify that this output matches the "deploy_datasets" input. + let observed_config = + sim_sled_agent.datasets_config_list().await.unwrap(); + assert_eq!(observed_config, datasets_config.into(),); + + // We expect to see the single in-service dataset we supplied as input. + assert_eq!(observed_config.datasets.len(), 1,); + assert!(observed_config.datasets.contains_key(&dataset_id)); + } + #[nexus_test] async fn test_dataset_record_create(cptestctx: &ControlPlaneTestContext) { const TEST_NAME: &str = "test_dataset_record_create"; diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index 8915ac8746a..c4c4ac07f96 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -969,7 +969,14 @@ impl From for DatasetsConfig { datasets: config .datasets .into_iter() - .map(|(id, d)| (id, d.into())) + .filter_map(|(id, d)| { + if d.disposition.matches(BlueprintDatasetFilter::InService) + { + Some((id, d.into())) + } else { + None + } + }) .collect(), } } From 0bca90ffe3023ec28fe8eb4b7d2a8620c2c91d66 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 8 Jan 2025 13:15:27 -0800 Subject: [PATCH 2/2] Explicit naming --- nexus/reconfigurator/execution/src/datasets.rs | 4 ++-- nexus/types/src/deployment.rs | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/nexus/reconfigurator/execution/src/datasets.rs b/nexus/reconfigurator/execution/src/datasets.rs index b73d021fd06..53aa84e7b34 100644 --- a/nexus/reconfigurator/execution/src/datasets.rs +++ b/nexus/reconfigurator/execution/src/datasets.rs @@ -57,7 +57,7 @@ pub(crate) async fn deploy_datasets( &log, ); - let config: DatasetsConfig = config.clone().into(); + let config: DatasetsConfig = config.clone().into_in_service_datasets(); let result = client.datasets_put(&config).await.with_context( || format!("Failed to put {config:#?} to sled {sled_id}"), @@ -376,7 +376,7 @@ mod tests { // and verify that this output matches the "deploy_datasets" input. let observed_config = sim_sled_agent.datasets_config_list().await.unwrap(); - assert_eq!(observed_config, datasets_config.into(),); + assert_eq!(observed_config, datasets_config.into_in_service_datasets()); // We expect to see the single in-service dataset we supplied as input. assert_eq!(observed_config.datasets.len(), 1,); diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index fb6ee0af64d..a1e819263fd 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -957,11 +957,19 @@ pub struct BlueprintDatasetsConfig { pub datasets: BTreeMap, } -impl From for DatasetsConfig { - fn from(config: BlueprintDatasetsConfig) -> Self { - Self { - generation: config.generation, - datasets: config +impl BlueprintDatasetsConfig { + /// Converts [Self] into [DatasetsConfig]. + /// + /// [DatasetsConfig] is a format of the dataset configuration that can be + /// passed to Sled Agents for deployment. + /// + /// This function is effectively a [std::convert::From] implementation, but + /// is named slightly more explicitly, as it filters the blueprint + /// configuration to only consider in-service datasets. + pub fn into_in_service_datasets(self) -> DatasetsConfig { + DatasetsConfig { + generation: self.generation, + datasets: self .datasets .into_iter() .filter_map(|(id, d)| {