From 91b514530872bb8a8f34e51dcdf3eac99f7ae4fc Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Fri, 19 Apr 2024 16:37:51 -0400 Subject: [PATCH 1/4] nexus-test-utils: Generate Blueprint zones, derive Omicron zones --- nexus/test-utils/src/lib.rs | 176 ++++++++++++++++++---------------- nexus/types/src/deployment.rs | 9 ++ 2 files changed, 103 insertions(+), 82 deletions(-) diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index cd71bd2cb8e..2dd388b884b 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -25,9 +25,11 @@ use nexus_config::MgdConfig; use nexus_config::NexusConfig; use nexus_config::NUM_INITIAL_RESERVED_IP_ADDRESSES; use nexus_test_interface::NexusServer; +use nexus_types::deployment::blueprint_zone_type; use nexus_types::deployment::Blueprint; use nexus_types::deployment::BlueprintZoneConfig; use nexus_types::deployment::BlueprintZoneDisposition; +use nexus_types::deployment::BlueprintZoneType; use nexus_types::deployment::BlueprintZonesConfig; use nexus_types::external_api::params::UserId; use nexus_types::internal_api::params::Certificate; @@ -35,9 +37,7 @@ use nexus_types::internal_api::params::DatasetCreateRequest; use nexus_types::internal_api::params::DatasetKind; use nexus_types::internal_api::params::DatasetPutRequest; use nexus_types::internal_api::params::RecoverySiloConfig; -use nexus_types::inventory::OmicronZoneConfig; use nexus_types::inventory::OmicronZoneDataset; -use nexus_types::inventory::OmicronZoneType; use nexus_types::inventory::OmicronZonesConfig; use omicron_common::address::DNS_OPTE_IPV4_SUBNET; use omicron_common::address::NEXUS_OPTE_IPV4_SUBNET; @@ -198,17 +198,13 @@ impl RackInitRequestBuilder { fn add_service_to_dns( &mut self, - zone_id: Uuid, + zone_id: OmicronZoneUuid, address: SocketAddrV6, service_name: internal_dns::ServiceName, ) { let zone = self .internal_dns_config - .host_zone( - // TODO-cleanup use TypedUuid everywhere - OmicronZoneUuid::from_untyped_uuid(zone_id), - *address.ip(), - ) + .host_zone(zone_id, *address.ip()) .expect("Failed to set up DNS for {kind}"); self.internal_dns_config .service_backend_zone(service_name, &zone, address.port()) @@ -279,8 +275,8 @@ pub struct ControlPlaneTestContextBuilder<'a, N: NexusServer> { pub internal_dns: Option, dns_config: Option, initial_blueprint_id: Option, - omicron_zones: Vec, - omicron_zones2: Vec, + blueprint_zones: Vec, + blueprint_zones2: Vec, pub silo_name: Option, pub user_name: Option, @@ -324,8 +320,8 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { internal_dns: None, dns_config: None, initial_blueprint_id: None, - omicron_zones: Vec::new(), - omicron_zones2: Vec::new(), + blueprint_zones: Vec::new(), + blueprint_zones2: Vec::new(), silo_name: None, user_name: None, } @@ -410,13 +406,16 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { .to_string() .parse() .unwrap(); - self.omicron_zones.push(OmicronZoneConfig { - id: dataset_id, + self.blueprint_zones.push(BlueprintZoneConfig { + disposition: BlueprintZoneDisposition::InService, + id: OmicronZoneUuid::from_untyped_uuid(dataset_id), underlay_address: *address.ip(), - zone_type: OmicronZoneType::CockroachDb { - address: address.to_string(), - dataset: OmicronZoneDataset { pool_name }, - }, + zone_type: BlueprintZoneType::CockroachDb( + blueprint_zone_type::CockroachDb { + address, + dataset: OmicronZoneDataset { pool_name }, + }, + ), }); self.database = Some(database); } @@ -459,13 +458,16 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { .to_string() .parse() .unwrap(); - self.omicron_zones.push(OmicronZoneConfig { - id: dataset_id, + self.blueprint_zones.push(BlueprintZoneConfig { + disposition: BlueprintZoneDisposition::InService, + id: OmicronZoneUuid::from_untyped_uuid(dataset_id), underlay_address: *address.ip(), - zone_type: OmicronZoneType::Clickhouse { - address: address.to_string(), - dataset: OmicronZoneDataset { pool_name }, - }, + zone_type: BlueprintZoneType::Clickhouse( + blueprint_zone_type::Clickhouse { + address, + dataset: OmicronZoneDataset { pool_name }, + }, + ), }); } @@ -637,17 +639,19 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { .expect("ran out of MAC addresses"); let external_address = self.config.deployment.dropshot_external.dropshot.bind_address.ip(); - let nexus_id = self.config.deployment.id; + let nexus_id = + OmicronZoneUuid::from_untyped_uuid(self.config.deployment.id); self.rack_init_builder.add_service_to_dns( nexus_id, address, internal_dns::ServiceName::Nexus, ); - self.omicron_zones.push(OmicronZoneConfig { + self.blueprint_zones.push(BlueprintZoneConfig { + disposition: BlueprintZoneDisposition::InService, id: nexus_id, underlay_address: *address.ip(), - zone_type: OmicronZoneType::Nexus { + zone_type: BlueprintZoneType::Nexus(blueprint_zone_type::Nexus { external_dns_servers: self .config .deployment @@ -655,14 +659,16 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { .clone(), external_ip: external_address, external_tls: self.config.deployment.dropshot_external.tls, - internal_address: address.to_string(), + internal_address: address, nic: NetworkInterface { id: Uuid::new_v4(), ip: NEXUS_OPTE_IPV4_SUBNET .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 1) .unwrap() .into(), - kind: NetworkInterfaceKind::Service { id: nexus_id }, + kind: NetworkInterfaceKind::Service { + id: nexus_id.into_untyped_uuid(), + }, mac, name: format!("nexus-{}", nexus_id).parse().unwrap(), primary: true, @@ -670,7 +676,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { subnet: (*NEXUS_OPTE_IPV4_SUBNET).into(), vni: Vni::SERVICES_VNI, }, - }, + }), }); self.nexus_internal = Some(nexus_internal); @@ -741,24 +747,15 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { let blueprint = { let mut blueprint_zones = BTreeMap::new(); for (maybe_sled_agent, zones) in [ - (self.sled_agent.as_ref(), &self.omicron_zones), - (self.sled_agent2.as_ref(), &self.omicron_zones2), + (self.sled_agent.as_ref(), &self.blueprint_zones), + (self.sled_agent2.as_ref(), &self.blueprint_zones2), ] { if let Some(sa) = maybe_sled_agent { blueprint_zones.insert( sa.sled_agent.id, BlueprintZonesConfig { generation: Generation::new().next(), - zones: zones - .iter() - .map(|z| { - // All initial zones are in-service - BlueprintZoneConfig::from_omicron_zone_config( - z.clone(), - BlueprintZoneDisposition::InService, - ).unwrap() - }) - .collect(), + zones: zones.clone(), }, ); } @@ -889,9 +886,9 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { switch_location: SwitchLocation, ) { let (field, zones) = if switch_location == SwitchLocation::Switch0 { - (&self.sled_agent, &self.omicron_zones) + (&self.sled_agent, &self.blueprint_zones) } else { - (&self.sled_agent2, &self.omicron_zones2) + (&self.sled_agent2, &self.blueprint_zones2) }; // Tell our Sled Agent to report the zones that we configured. @@ -904,7 +901,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { ); client .omicron_zones_put(&OmicronZonesConfig { - zones: zones.clone(), + zones: zones.clone().into_iter().map(From::from).collect(), generation: Generation::new().next(), }) .await @@ -925,18 +922,19 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { panic!("Expected IPv6 Pantry Address"); }; - let zone_id = Uuid::new_v4(); + let zone_id = OmicronZoneUuid::new_v4(); self.rack_init_builder.add_service_to_dns( zone_id, address, internal_dns::ServiceName::CruciblePantry, ); - self.omicron_zones.push(OmicronZoneConfig { + self.blueprint_zones.push(BlueprintZoneConfig { + disposition: BlueprintZoneDisposition::InService, id: zone_id, underlay_address: *address.ip(), - zone_type: OmicronZoneType::CruciblePantry { - address: address.to_string(), - }, + zone_type: BlueprintZoneType::CruciblePantry( + blueprint_zone_type::CruciblePantry { address }, + ), }); } @@ -959,7 +957,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { .mac_addrs .next() .expect("ran out of MAC addresses"); - let zone_id = Uuid::new_v4(); + let zone_id = OmicronZoneUuid::new_v4(); self.rack_init_builder.add_service_to_dns( zone_id, dropshot_address, @@ -971,28 +969,35 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { .to_string() .parse() .unwrap(); - self.omicron_zones.push(OmicronZoneConfig { + self.blueprint_zones.push(BlueprintZoneConfig { + disposition: BlueprintZoneDisposition::InService, id: zone_id, underlay_address: *dropshot_address.ip(), - zone_type: OmicronZoneType::ExternalDns { - dataset: OmicronZoneDataset { pool_name }, - dns_address: dns_address.to_string(), - http_address: dropshot_address.to_string(), - nic: NetworkInterface { - id: Uuid::new_v4(), - ip: DNS_OPTE_IPV4_SUBNET - .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 1) - .unwrap() - .into(), - kind: NetworkInterfaceKind::Service { id: zone_id }, - mac, - name: format!("external-dns-{}", zone_id).parse().unwrap(), - primary: true, - slot: 0, - subnet: (*DNS_OPTE_IPV4_SUBNET).into(), - vni: Vni::SERVICES_VNI, + zone_type: BlueprintZoneType::ExternalDns( + blueprint_zone_type::ExternalDns { + dataset: OmicronZoneDataset { pool_name }, + dns_address: dns_address.into(), + http_address: dropshot_address, + nic: NetworkInterface { + id: Uuid::new_v4(), + ip: DNS_OPTE_IPV4_SUBNET + .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES as u32 + 1) + .unwrap() + .into(), + kind: NetworkInterfaceKind::Service { + id: zone_id.into_untyped_uuid(), + }, + mac, + name: format!("external-dns-{}", zone_id) + .parse() + .unwrap(), + primary: true, + slot: 0, + subnet: (*DNS_OPTE_IPV4_SUBNET).into(), + vni: Vni::SERVICES_VNI, + }, }, - }, + ), }); self.external_dns = Some(dns); @@ -1003,13 +1008,17 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { let log = self.logctx.log.new(o!("component" => "internal_dns_server")); let dns = dns_server::TransientServer::new(&log).await.unwrap(); - let SocketAddr::V6(address) = dns.dropshot_server.local_addr() else { + let SocketAddr::V6(dns_address) = dns.dns_server.local_address() else { panic!("Unsupported IPv4 DNS address"); }; - let zone_id = Uuid::new_v4(); + let SocketAddr::V6(http_address) = dns.dropshot_server.local_addr() + else { + panic!("Unsupported IPv4 DNS address"); + }; + let zone_id = OmicronZoneUuid::new_v4(); self.rack_init_builder.add_service_to_dns( zone_id, - address, + http_address, internal_dns::ServiceName::InternalDns, ); @@ -1018,16 +1027,19 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { .to_string() .parse() .unwrap(); - self.omicron_zones.push(OmicronZoneConfig { + self.blueprint_zones.push(BlueprintZoneConfig { + disposition: BlueprintZoneDisposition::InService, id: zone_id, - underlay_address: *address.ip(), - zone_type: OmicronZoneType::InternalDns { - dataset: OmicronZoneDataset { pool_name }, - dns_address: dns.dns_server.local_address().to_string(), - http_address: address.to_string(), - gz_address: Ipv6Addr::LOCALHOST, - gz_address_index: 0, - }, + underlay_address: *http_address.ip(), + zone_type: BlueprintZoneType::InternalDns( + blueprint_zone_type::InternalDns { + dataset: OmicronZoneDataset { pool_name }, + dns_address, + http_address, + gz_address: Ipv6Addr::LOCALHOST, + gz_address_index: 0, + }, + ), }); self.internal_dns = Some(dns); diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index 3a8e6e40662..0bbafbdd3c5 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -319,6 +319,15 @@ pub struct BlueprintZonesConfig { pub zones: Vec, } +impl From for OmicronZonesConfig { + fn from(config: BlueprintZonesConfig) -> Self { + Self { + generation: config.generation, + zones: config.zones.into_iter().map(From::from).collect(), + } + } +} + impl BlueprintZonesConfig { /// Sorts the list of zones stored in this configuration. /// From 75d3373c1c46154c59add0380209fb65080428d4 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Fri, 19 Apr 2024 16:57:07 -0400 Subject: [PATCH 2/4] Remove from_omicron_zone_config in datasets test --- .../reconfigurator/execution/src/datasets.rs | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/nexus/reconfigurator/execution/src/datasets.rs b/nexus/reconfigurator/execution/src/datasets.rs index 9dd3da50df8..5f1f738f3ba 100644 --- a/nexus/reconfigurator/execution/src/datasets.rs +++ b/nexus/reconfigurator/execution/src/datasets.rs @@ -141,8 +141,10 @@ mod tests { use nexus_db_model::SledSystemHardware; use nexus_db_model::SledUpdate; use nexus_db_model::Zpool; + use nexus_reconfigurator_planning::example::example; use nexus_test_utils_macros::nexus_test; use nexus_types::deployment::BlueprintZoneDisposition; + use nexus_types::deployment::BlueprintZoneFilter; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::ZpoolUuid; use sled_agent_client::types::OmicronZoneDataset; @@ -156,6 +158,8 @@ mod tests { async fn test_ensure_crucible_dataset_records_exist( cptestctx: &ControlPlaneTestContext, ) { + const TEST_NAME: &str = "test_ensure_crucible_dataset_records_exist"; + // Set up. let nexus = &cptestctx.server.apictx().nexus; let datastore = nexus.datastore(); @@ -165,9 +169,8 @@ mod tests { ); let opctx = &opctx; - // Use the standard representative inventory collection. - let representative = nexus_inventory::examples::representative(); - let collection = representative.builder.build(); + // Use the standard example system. + let (collection, _, blueprint) = example(&opctx.log, TEST_NAME, 5); // Record the sleds and zpools contained in this collection. let rack_id = Uuid::new_v4(); @@ -226,22 +229,16 @@ mod tests { 0 ); - // Convert the collection zones into blueprint zones. - let all_omicron_zones = collection - .all_omicron_zones() - .map(|z| { - BlueprintZoneConfig::from_omicron_zone_config( - z.clone(), - BlueprintZoneDisposition::InService, - ) - .expect("failed to convert to blueprint zone config") - }) + // Collect all the blueprint zones. + let all_omicron_zones = blueprint + .all_omicron_zones(BlueprintZoneFilter::All) + .map(|(_, zone)| zone) .collect::>(); let ndatasets_inserted = ensure_crucible_dataset_records_exist( opctx, datastore, - all_omicron_zones.iter(), + all_omicron_zones.iter().copied(), ) .await .expect("failed to ensure crucible datasets"); @@ -262,7 +259,7 @@ mod tests { let ndatasets_inserted = ensure_crucible_dataset_records_exist( opctx, datastore, - all_omicron_zones.iter(), + all_omicron_zones.iter().copied(), ) .await .expect("failed to ensure crucible datasets"); @@ -312,7 +309,7 @@ mod tests { let ndatasets_inserted = ensure_crucible_dataset_records_exist( opctx, datastore, - all_omicron_zones.iter().chain(std::iter::once(&new_zone)), + all_omicron_zones.iter().copied().chain(std::iter::once(&new_zone)), ) .await .expect("failed to ensure crucible datasets"); From b7919d42b2a4bf7d9edf0096528053ed7019eb9d Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Fri, 19 Apr 2024 17:28:33 -0400 Subject: [PATCH 3/4] avoid from_omicron_zone_config in blueprint diffs The "before" side of a blueprint diff can be a collection or a blueprint. This makes that explicit by wrapping the "before" cases in two-variant enums instead of converting the collection zones into blueprint zones (as that will soon become messier). --- nexus/types/src/deployment.rs | 289 ++++++++++++++++++++++++++-------- 1 file changed, 222 insertions(+), 67 deletions(-) diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index 0bbafbdd3c5..568d534796d 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -189,7 +189,11 @@ impl Blueprint { ) -> Result { BlueprintDiff::new( DiffBeforeMetadata::Blueprint(Box::new(before.metadata())), - before.typed_blueprint_zones(), + before + .typed_blueprint_zones() + .into_iter() + .map(|(sled_id, zones)| (sled_id, zones.into())) + .collect(), self.metadata(), self.typed_blueprint_zones(), ) @@ -212,33 +216,9 @@ impl Blueprint { .omicron_zones .iter() .map(|(sled_id, zones_found)| { - let zones = zones_found - .zones - .zones - .iter() - .map(|z| { - BlueprintZoneConfig::from_omicron_zone_config( - z.clone(), - BlueprintZoneDisposition::InService, - ) - .map_err(|err| { - BlueprintDiffError { - before_meta: DiffBeforeMetadata::Collection { - id: before.id, - }, - after_meta: Box::new(self.metadata()), - errors: vec![BlueprintDiffSingleError::InvalidOmicronZoneType(err)], - } - }) - }) - .collect::, _>>()?; - let zones = BlueprintZonesConfig { - generation: zones_found.zones.generation, - zones, - }; - Ok((*sled_id, zones)) + (*sled_id, zones_found.zones.clone().into()) }) - .collect::>()?; + .collect(); BlueprintDiff::new( DiffBeforeMetadata::Collection { id: before.id }, @@ -359,10 +339,45 @@ impl BlueprintZonesConfig { } } -fn zone_sort_key(z: &BlueprintZoneConfig) -> impl Ord { +trait ZoneSortKey { + fn kind(&self) -> ZoneKind; + fn id(&self) -> OmicronZoneUuid; +} + +impl ZoneSortKey for BlueprintZoneConfig { + fn kind(&self) -> ZoneKind { + self.zone_type.kind() + } + + fn id(&self) -> OmicronZoneUuid { + self.id + } +} + +impl ZoneSortKey for OmicronZoneConfig { + fn kind(&self) -> ZoneKind { + self.zone_type.kind() + } + + fn id(&self) -> OmicronZoneUuid { + OmicronZoneUuid::from_untyped_uuid(self.id) + } +} + +impl ZoneSortKey for BlueprintOrCollectionZoneConfig { + fn kind(&self) -> ZoneKind { + BlueprintOrCollectionZoneConfig::kind(self) + } + + fn id(&self) -> OmicronZoneUuid { + BlueprintOrCollectionZoneConfig::id(self) + } +} + +fn zone_sort_key(z: &T) -> impl Ord { // First sort by kind, then by ID. This makes it so that zones of the same // kind (e.g. Crucible zones) are grouped together. - (z.zone_type.kind(), z.id) + (z.kind(), z.id()) } /// "Should never happen" errors from converting an [`OmicronZoneType`] into a @@ -821,7 +836,7 @@ impl BlueprintDiff { /// data is valid. fn new( before_meta: DiffBeforeMetadata, - before_zones: BTreeMap, + before_zones: BTreeMap, after_meta: BlueprintMetadata, after_zones: BTreeMap, ) -> Result { @@ -861,8 +876,9 @@ impl BlueprintDiff { /// Iterate over sleds only present in the first blueprint of a diff pub fn sleds_removed( &self, - ) -> impl ExactSizeIterator + '_ - { + ) -> impl ExactSizeIterator< + Item = (SledUuid, &BlueprintOrCollectionZonesConfig), + > + '_ { self.sleds.removed.iter().map(|(sled_id, zones)| (*sled_id, zones)) } @@ -891,7 +907,7 @@ impl BlueprintDiff { #[derive(Debug)] struct DiffSleds { added: BTreeMap, - removed: BTreeMap, + removed: BTreeMap, modified: BTreeMap, unchanged: BTreeMap, } @@ -903,7 +919,7 @@ impl DiffSleds { /// The return value only contains the sleds that are present in both /// blueprints. fn new( - before: BTreeMap, + before: BTreeMap, mut after: BTreeMap, errors: &mut Vec, ) -> Self { @@ -918,7 +934,7 @@ impl DiffSleds { after_z.sort(); if before_z == after_z { - unchanged.insert(sled_id, before_z); + unchanged.insert(sled_id, after_z); } else { let sled_modified = DiffSledModified::new( sled_id, before_z, after_z, errors, @@ -1063,6 +1079,144 @@ impl DiffBeforeMetadata { } } +/// Single sled's zones config for "before" version within a [`BlueprintDiff`]. +#[derive(Clone, Debug)] +pub enum BlueprintOrCollectionZonesConfig { + /// The diff was made from a collection. + Collection(OmicronZonesConfig), + /// The diff was made from a blueprint. + Blueprint(BlueprintZonesConfig), +} + +impl BlueprintOrCollectionZonesConfig { + fn sort(&mut self) { + match self { + BlueprintOrCollectionZonesConfig::Collection(z) => { + z.zones.sort_unstable_by_key(zone_sort_key) + } + BlueprintOrCollectionZonesConfig::Blueprint(z) => z.sort(), + } + } + + fn generation(&self) -> Generation { + match self { + BlueprintOrCollectionZonesConfig::Collection(z) => z.generation, + BlueprintOrCollectionZonesConfig::Blueprint(z) => z.generation, + } + } + + fn zones( + &self, + ) -> Box + '_> { + match self { + BlueprintOrCollectionZonesConfig::Collection(zc) => { + Box::new(zc.zones.iter().map(|z| z.clone().into())) + } + BlueprintOrCollectionZonesConfig::Blueprint(zc) => { + Box::new(zc.zones.iter().map(|z| z.clone().into())) + } + } + } +} + +impl From for BlueprintOrCollectionZonesConfig { + fn from(zc: OmicronZonesConfig) -> Self { + Self::Collection(zc) + } +} + +impl From for BlueprintOrCollectionZonesConfig { + fn from(zc: BlueprintZonesConfig) -> Self { + Self::Blueprint(zc) + } +} + +impl PartialEq for BlueprintOrCollectionZonesConfig { + fn eq(&self, other: &BlueprintZonesConfig) -> bool { + match self { + BlueprintOrCollectionZonesConfig::Collection(z) => { + // BlueprintZonesConfig contains more information than + // OmicronZonesConfig. We compare them by lowering the + // BlueprintZonesConfig into an OmicronZonesConfig. + let lowered = OmicronZonesConfig::from(other.clone()); + z.eq(&lowered) + } + BlueprintOrCollectionZonesConfig::Blueprint(z) => z.eq(other), + } + } +} + +/// Single zone config for "before" version within a [`BlueprintDiff`]. +#[derive(Clone, Debug)] +pub enum BlueprintOrCollectionZoneConfig { + /// The diff was made from a collection. + Collection(OmicronZoneConfig), + /// The diff was made from a blueprint. + Blueprint(BlueprintZoneConfig), +} + +impl From for BlueprintOrCollectionZoneConfig { + fn from(zc: OmicronZoneConfig) -> Self { + Self::Collection(zc) + } +} + +impl From for BlueprintOrCollectionZoneConfig { + fn from(zc: BlueprintZoneConfig) -> Self { + Self::Blueprint(zc) + } +} + +impl BlueprintOrCollectionZoneConfig { + fn id(&self) -> OmicronZoneUuid { + match self { + BlueprintOrCollectionZoneConfig::Collection(z) => z.id(), + BlueprintOrCollectionZoneConfig::Blueprint(z) => z.id(), + } + } + + fn kind(&self) -> ZoneKind { + match self { + BlueprintOrCollectionZoneConfig::Collection(z) => z.kind(), + BlueprintOrCollectionZoneConfig::Blueprint(z) => z.kind(), + } + } + + fn disposition(&self) -> BlueprintZoneDisposition { + match self { + // All zones from inventory collection are assumed to be in-service. + BlueprintOrCollectionZoneConfig::Collection(_) => { + BlueprintZoneDisposition::InService + } + BlueprintOrCollectionZoneConfig::Blueprint(z) => z.disposition, + } + } + + fn underlay_address(&self) -> Ipv6Addr { + match self { + BlueprintOrCollectionZoneConfig::Collection(z) => { + z.underlay_address + } + BlueprintOrCollectionZoneConfig::Blueprint(z) => z.underlay_address, + } + } + + fn is_zone_type_equal(&self, other: &BlueprintZoneType) -> bool { + match self { + BlueprintOrCollectionZoneConfig::Collection(z) => { + // BlueprintZoneType contains more information than + // OmicronZoneType. We compare them by lowering the + // BlueprintZoneType into an OmicronZoneType. + let lowered = OmicronZoneType::from(other.clone()); + z.zone_type == lowered + } + BlueprintOrCollectionZoneConfig::Blueprint(z) => { + z.zone_type == *other + } + } + } +} + /// Describes a sled that appeared on both sides of a diff and is changed. #[derive(Clone, Debug)] pub struct DiffSledModified { @@ -1073,20 +1227,20 @@ pub struct DiffSledModified { /// generation of the "zones" configuration on the right side pub generation_after: Generation, zones_added: Vec, - zones_removed: Vec, + zones_removed: Vec, zones_common: Vec, } impl DiffSledModified { fn new( sled_id: SledUuid, - before: BlueprintZonesConfig, + before: BlueprintOrCollectionZonesConfig, after: BlueprintZonesConfig, errors: &mut Vec, ) -> Self { // Assemble separate summaries of the zones, indexed by zone id. let before_by_id: HashMap<_, _> = - before.zones.into_iter().map(|zone| (zone.id, zone)).collect(); + before.zones().map(|zone| (zone.id(), zone)).collect(); let mut after_by_id: HashMap<_, _> = after.zones.into_iter().map(|zone| (zone.id, zone)).collect(); @@ -1096,7 +1250,7 @@ impl DiffSledModified { // Now go through each zone and compare them. for (zone_id, zone_before) in before_by_id { if let Some(zone_after) = after_by_id.remove(&zone_id) { - let before_kind = zone_before.zone_type.kind(); + let before_kind = zone_before.kind(); let after_kind = zone_after.zone_type.kind(); if before_kind != after_kind { @@ -1132,7 +1286,7 @@ impl DiffSledModified { Self { sled_id, - generation_before: before.generation, + generation_before: before.generation(), generation_after: after.generation, zones_added, zones_removed, @@ -1150,7 +1304,8 @@ impl DiffSledModified { /// Iterate over zones removed between the blueprints pub fn zones_removed( &self, - ) -> impl ExactSizeIterator + '_ { + ) -> impl ExactSizeIterator + '_ + { self.zones_removed.iter() } @@ -1178,7 +1333,7 @@ impl DiffSledModified { #[derive(Debug, Clone)] pub struct DiffZoneCommon { /// full zone configuration before - pub zone_before: BlueprintZoneConfig, + pub zone_before: BlueprintOrCollectionZoneConfig, /// full zone configuration after pub zone_after: BlueprintZoneConfig, } @@ -1198,14 +1353,14 @@ impl DiffZoneCommon { /// changed. #[inline] pub fn config_changed(&self) -> bool { - self.zone_before.underlay_address != self.zone_after.underlay_address - || self.zone_before.zone_type != self.zone_after.zone_type + self.zone_before.underlay_address() != self.zone_after.underlay_address + || !self.zone_before.is_zone_type_equal(&self.zone_after.zone_type) } /// Returns true if the [`BlueprintZoneDisposition`] for the zone changed. #[inline] pub fn disposition_changed(&self) -> bool { - self.zone_before.disposition != self.zone_after.disposition + self.zone_before.disposition() != self.zone_after.disposition } } @@ -1261,7 +1416,7 @@ mod table_display { for zone in &sled_zones.zones { add_zone_record( ZONE_INDENT.to_string(), - zone, + &zone.clone().into(), section, ); } @@ -1361,7 +1516,7 @@ mod table_display { for (sled_id, sled_zones) in diff.sleds_unchanged() { add_whole_sled_records( sled_id, - sled_zones, + &sled_zones.clone().into(), WholeSledKind::Unchanged, section, ); @@ -1405,7 +1560,7 @@ mod table_display { for (sled_id, sled_zones) in diff.sleds_added() { add_whole_sled_records( sled_id, - sled_zones, + &sled_zones.clone().into(), WholeSledKind::Added, section, ); @@ -1497,25 +1652,25 @@ mod table_display { fn add_whole_sled_records( sled_id: SledUuid, - sled_zones: &BlueprintZonesConfig, + sled_zones: &BlueprintOrCollectionZonesConfig, kind: WholeSledKind, section: &mut StSectionBuilder, ) { let heading = format!( "{}{SLED_INDENT}sled {sled_id}: blueprint zones at generation {}", kind.prefix(), - sled_zones.generation, + sled_zones.generation(), ); let prefix = kind.prefix(); let status = kind.status(); section.make_subsection(SectionSpacing::Always, heading, |s2| { // Also add another section for zones. - for zone in &sled_zones.zones { + for zone in sled_zones.zones() { match status { Some(status) => { add_zone_record_with_status( format!("{prefix}{ZONE_INDENT}"), - zone, + &zone, status, s2, ); @@ -1523,7 +1678,7 @@ mod table_display { None => { add_zone_record( format!("{prefix}{ZONE_INDENT}"), - zone, + &zone, s2, ); } @@ -1610,7 +1765,7 @@ mod table_display { for zone in modified.zones_added() { add_zone_record_with_status( format!("{ADDED_PREFIX}{ZONE_INDENT}"), - zone, + &zone.clone().into(), ADDED, s2, ); @@ -1634,30 +1789,30 @@ mod table_display { /// This is the meat-and-potatoes of the diff display. fn add_zone_record( first_column: String, - zone: &BlueprintZoneConfig, + zone: &BlueprintOrCollectionZoneConfig, section: &mut StSectionBuilder, ) { section.push_record(vec![ first_column, - zone.zone_type.kind().to_string(), - zone.id.to_string(), - zone.disposition.to_string(), - zone.underlay_address.to_string(), + zone.kind().to_string(), + zone.id().to_string(), + zone.disposition().to_string(), + zone.underlay_address().to_string(), ]); } fn add_zone_record_with_status( first_column: String, - zone: &BlueprintZoneConfig, + zone: &BlueprintOrCollectionZoneConfig, status: &str, section: &mut StSectionBuilder, ) { section.push_record(vec![ first_column, - zone.zone_type.kind().to_string(), - zone.id.to_string(), - zone.disposition.to_string(), - zone.underlay_address.to_string(), + zone.kind().to_string(), + zone.id().to_string(), + zone.disposition().to_string(), + zone.underlay_address().to_string(), status.to_string(), ]); } @@ -1683,13 +1838,13 @@ mod table_display { ); let mut what_changed = Vec::new(); - if before.zone_type != after.zone_type { + if !before.is_zone_type_equal(&after.zone_type) { what_changed.push(ZONE_TYPE_CONFIG); } - if before.disposition != after.disposition { + if before.disposition() != after.disposition { what_changed.push(DISPOSITION); } - if before.underlay_address != after.underlay_address { + if before.underlay_address() != after.underlay_address { what_changed.push(UNDERLAY_IP); } debug_assert!( From b7fe215bdb4818d81f55677156b4e13bf90ad035 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sun, 21 Apr 2024 13:53:11 -0400 Subject: [PATCH 4/4] make BlueprintOrCollection* methods pub --- .../planning/src/blueprint_builder/zones.rs | 2 +- nexus/types/src/deployment.rs | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs b/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs index c0e0918503f..c9015f8b273 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs @@ -408,7 +408,7 @@ mod tests { assert_eq!(sled_modified.zones_modified().count(), 1); let modified_zone = sled_modified.zones_modified().next().unwrap(); - assert_eq!(modified_zone.zone_before.id, existing_zone_id); + assert_eq!(modified_zone.zone_before.id(), existing_zone_id); } else { assert_eq!(sled_id, control_sled_id); diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index 568d534796d..36f87a2eb3e 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -1089,7 +1089,7 @@ pub enum BlueprintOrCollectionZonesConfig { } impl BlueprintOrCollectionZonesConfig { - fn sort(&mut self) { + pub fn sort(&mut self) { match self { BlueprintOrCollectionZonesConfig::Collection(z) => { z.zones.sort_unstable_by_key(zone_sort_key) @@ -1098,14 +1098,14 @@ impl BlueprintOrCollectionZonesConfig { } } - fn generation(&self) -> Generation { + pub fn generation(&self) -> Generation { match self { BlueprintOrCollectionZonesConfig::Collection(z) => z.generation, BlueprintOrCollectionZonesConfig::Blueprint(z) => z.generation, } } - fn zones( + pub fn zones( &self, ) -> Box + '_> { match self { @@ -1168,21 +1168,21 @@ impl From for BlueprintOrCollectionZoneConfig { } impl BlueprintOrCollectionZoneConfig { - fn id(&self) -> OmicronZoneUuid { + pub fn id(&self) -> OmicronZoneUuid { match self { BlueprintOrCollectionZoneConfig::Collection(z) => z.id(), BlueprintOrCollectionZoneConfig::Blueprint(z) => z.id(), } } - fn kind(&self) -> ZoneKind { + pub fn kind(&self) -> ZoneKind { match self { BlueprintOrCollectionZoneConfig::Collection(z) => z.kind(), BlueprintOrCollectionZoneConfig::Blueprint(z) => z.kind(), } } - fn disposition(&self) -> BlueprintZoneDisposition { + pub fn disposition(&self) -> BlueprintZoneDisposition { match self { // All zones from inventory collection are assumed to be in-service. BlueprintOrCollectionZoneConfig::Collection(_) => { @@ -1192,7 +1192,7 @@ impl BlueprintOrCollectionZoneConfig { } } - fn underlay_address(&self) -> Ipv6Addr { + pub fn underlay_address(&self) -> Ipv6Addr { match self { BlueprintOrCollectionZoneConfig::Collection(z) => { z.underlay_address @@ -1201,7 +1201,7 @@ impl BlueprintOrCollectionZoneConfig { } } - fn is_zone_type_equal(&self, other: &BlueprintZoneType) -> bool { + pub fn is_zone_type_equal(&self, other: &BlueprintZoneType) -> bool { match self { BlueprintOrCollectionZoneConfig::Collection(z) => { // BlueprintZoneType contains more information than