From 1016abadfeb9b7876ab8281ab7d394fc4627ef77 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Fri, 7 Feb 2025 11:48:56 -0500 Subject: [PATCH] [reconfigurator] Remove spicy blueprint -> sled-agent From impls --- .../execution/src/clickhouse.rs | 17 +--- .../execution/src/omicron_physical_disks.rs | 4 +- .../execution/src/omicron_zones.rs | 5 +- nexus/reconfigurator/planning/src/example.rs | 7 +- nexus/reconfigurator/planning/src/planner.rs | 5 +- nexus/types/src/deployment.rs | 81 +++++++++++-------- sled-agent/src/rack_setup/service.rs | 2 +- 7 files changed, 60 insertions(+), 61 deletions(-) diff --git a/nexus/reconfigurator/execution/src/clickhouse.rs b/nexus/reconfigurator/execution/src/clickhouse.rs index a5642015c4..7eb6aff155 100644 --- a/nexus/reconfigurator/execution/src/clickhouse.rs +++ b/nexus/reconfigurator/execution/src/clickhouse.rs @@ -22,7 +22,7 @@ use futures::future::Either; use futures::stream::FuturesUnordered; use futures::stream::StreamExt; use nexus_db_queries::context::OpContext; -use nexus_sled_agent_shared::inventory::OmicronZoneType; +use nexus_types::deployment::Blueprint; use nexus_types::deployment::BlueprintZoneFilter; use nexus_types::deployment::BlueprintZonesConfig; use nexus_types::deployment::ClickhouseClusterConfig; @@ -182,18 +182,9 @@ pub(crate) async fn deploy_single_node( opctx: &OpContext, zones: &BTreeMap, ) -> Result<(), anyhow::Error> { - if let Some(zone) = zones - .values() - .flat_map(|zones| { - zones - .to_omicron_zones_config(BlueprintZoneFilter::ShouldBeRunning) - .zones - .into_iter() - .find(|zone| { - matches!(zone.zone_type, OmicronZoneType::Clickhouse { .. }) - }) - }) - .next() + if let Some((_, zone)) = + Blueprint::filtered_zones(zones, BlueprintZoneFilter::ShouldBeRunning) + .find(|(_, zone)| zone.zone_type.is_clickhouse()) { let admin_addr = SocketAddr::V6(SocketAddrV6::new( zone.underlay_ip(), diff --git a/nexus/reconfigurator/execution/src/omicron_physical_disks.rs b/nexus/reconfigurator/execution/src/omicron_physical_disks.rs index d856a33321..6655eeb8b6 100644 --- a/nexus/reconfigurator/execution/src/omicron_physical_disks.rs +++ b/nexus/reconfigurator/execution/src/omicron_physical_disks.rs @@ -48,7 +48,9 @@ pub(crate) async fn deploy_disks( &log, ); let result = client - .omicron_physical_disks_put(&config.clone().into()) + .omicron_physical_disks_put( + &config.clone().into_in_service_disks(), + ) .await .with_context(|| { format!("Failed to put {config:#?} to sled {sled_id}") diff --git a/nexus/reconfigurator/execution/src/omicron_zones.rs b/nexus/reconfigurator/execution/src/omicron_zones.rs index 8f7a5b7d96..73ef7fcaa7 100644 --- a/nexus/reconfigurator/execution/src/omicron_zones.rs +++ b/nexus/reconfigurator/execution/src/omicron_zones.rs @@ -19,7 +19,6 @@ use nexus_db_queries::db::datastore::CollectorReassignment; use nexus_db_queries::db::DataStore; use nexus_types::deployment::BlueprintZoneConfig; use nexus_types::deployment::BlueprintZoneDisposition; -use nexus_types::deployment::BlueprintZoneFilter; use nexus_types::deployment::BlueprintZoneType; use nexus_types::deployment::BlueprintZonesConfig; use omicron_common::address::COCKROACH_ADMIN_PORT; @@ -65,8 +64,8 @@ pub(crate) async fn deploy_zones( db_sled.sled_agent_address(), &opctx.log, ); - let omicron_zones = config - .to_omicron_zones_config(BlueprintZoneFilter::ShouldBeRunning); + let omicron_zones = + config.clone().into_running_omicron_zones_config(); let result = client .omicron_zones_put(&omicron_zones) .await diff --git a/nexus/reconfigurator/planning/src/example.rs b/nexus/reconfigurator/planning/src/example.rs index 0431d356d3..b2dd869c20 100644 --- a/nexus/reconfigurator/planning/src/example.rs +++ b/nexus/reconfigurator/planning/src/example.rs @@ -15,7 +15,6 @@ use crate::system::SledBuilder; use crate::system::SystemDescription; use nexus_inventory::CollectionBuilderRng; use nexus_types::deployment::Blueprint; -use nexus_types::deployment::BlueprintZoneFilter; use nexus_types::deployment::OmicronZoneNic; use nexus_types::deployment::PlanningInput; use nexus_types::deployment::SledFilter; @@ -490,9 +489,7 @@ impl ExampleSystemBuilder { system .sled_set_omicron_zones( *sled_id, - zones.to_omicron_zones_config( - BlueprintZoneFilter::ShouldBeRunning, - ), + zones.clone().into_running_omicron_zones_config(), ) .unwrap(); } @@ -546,7 +543,7 @@ impl ZoneCount { mod tests { use chrono::{NaiveDateTime, TimeZone, Utc}; use nexus_sled_agent_shared::inventory::{OmicronZoneConfig, ZoneKind}; - use nexus_types::deployment::BlueprintZoneConfig; + use nexus_types::deployment::{BlueprintZoneConfig, BlueprintZoneFilter}; use omicron_test_utils::dev::test_setup_log; use super::*; diff --git a/nexus/reconfigurator/planning/src/planner.rs b/nexus/reconfigurator/planning/src/planner.rs index 524ffaaa97..fc353d0c53 100644 --- a/nexus/reconfigurator/planning/src/planner.rs +++ b/nexus/reconfigurator/planning/src/planner.rs @@ -976,9 +976,8 @@ mod test { .blueprint_zones .get(&new_sled_id) .expect("blueprint should contain zones for new sled") - .to_omicron_zones_config( - BlueprintZoneFilter::ShouldBeRunning, - ), + .clone() + .into_running_omicron_zones_config(), ) .unwrap(); let collection = diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index 9b3e2fcb3c..7328cd3d83 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -547,33 +547,30 @@ pub struct BlueprintZonesConfig { pub zones: IdMap, } -impl From for OmicronZonesConfig { - fn from(config: BlueprintZonesConfig) -> Self { - Self { - generation: config.generation, - zones: config.zones.into_iter().map(From::from).collect(), - } - } -} - impl BlueprintZonesConfig { - /// Converts self to an [`OmicronZonesConfig`], applying the provided - /// [`BlueprintZoneFilter`]. + /// Converts self into [`OmicronZonesConfig`]. /// - /// The filter controls which zones should be exported into the resulting - /// [`OmicronZonesConfig`]. - pub fn to_omicron_zones_config( - &self, - filter: BlueprintZoneFilter, - ) -> OmicronZonesConfig { + /// [`OmicronZonesConfig`] is a format of the zones configuration that can + /// be passed to Sled Agents for deployment. + /// + /// This function is effectively a `From` implementation, but + /// is named slightly more explicitly, as it filters the blueprint + /// configuration to only consider zones that should be running. + pub fn into_running_omicron_zones_config(self) -> OmicronZonesConfig { OmicronZonesConfig { generation: self.generation, zones: self .zones - .iter() - .filter(|z| z.disposition.matches(filter)) - .cloned() - .map(OmicronZoneConfig::from) + .into_iter() + .filter_map(|z| { + if z.disposition + .matches(BlueprintZoneFilter::ShouldBeRunning) + { + Some(z.into()) + } else { + None + } + }) .collect(), } } @@ -899,6 +896,33 @@ pub struct BlueprintPhysicalDisksConfig { pub disks: IdMap, } +impl BlueprintPhysicalDisksConfig { + /// Converts self into [`OmicronPhysicalDisksConfig`]. + /// + /// [`OmicronPhysicalDisksConfig`] is a format of the disks configuration + /// that can be passed to Sled Agents for deployment. + /// + /// This function is effectively a `From` implementation, but + /// is named slightly more explicitly, as it filters the blueprint + /// configuration to only consider in-service disks. + pub fn into_in_service_disks(self) -> OmicronPhysicalDisksConfig { + OmicronPhysicalDisksConfig { + generation: self.generation, + disks: self + .disks + .into_iter() + .filter_map(|d| { + if d.disposition.matches(DiskFilter::InService) { + Some(d.into()) + } else { + None + } + }) + .collect(), + } + } +} + impl IdMappable for BlueprintPhysicalDiskConfig { type Id = PhysicalDiskUuid; @@ -927,19 +951,6 @@ impl From for OmicronPhysicalDiskConfig { } } -impl From for OmicronPhysicalDisksConfig { - fn from(value: BlueprintPhysicalDisksConfig) -> Self { - OmicronPhysicalDisksConfig { - generation: value.generation, - disks: value - .disks - .into_iter() - .map(OmicronPhysicalDiskConfig::from) - .collect(), - } - } -} - /// Information about Omicron datasets as recorded in a blueprint. #[derive( Debug, Clone, Eq, PartialEq, JsonSchema, Deserialize, Serialize, Diffable, @@ -950,7 +961,7 @@ pub struct BlueprintDatasetsConfig { } impl BlueprintDatasetsConfig { - /// Converts [Self] into [DatasetsConfig]. + /// Converts self into [DatasetsConfig]. /// /// [DatasetsConfig] is a format of the dataset configuration that can be /// passed to Sled Agents for deployment. diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 39b1d1d27b..c9bc0f339f 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -344,7 +344,7 @@ impl ServiceInner { // Ensure all the physical disks are initialized self.initialize_disks_on_sled( *sled_address, - config.disks.clone().into(), + config.disks.clone().into_in_service_disks(), ) .await?;