From 7cd6e49c4d4d83e8cb687bea1ceae8705c7767b2 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 29 Apr 2025 16:10:36 -0400 Subject: [PATCH 1/4] OmicronSledConfig: Just one `Generation` This is somewhat extracted from #8064, but can be landed independently and will make some of the followup sled-agent-config-reconciler PRs a little cleaner. Fixes #7774. --- Cargo.lock | 3 + common/Cargo.toml | 1 + common/src/disk.rs | 17 ++++ nexus-sled-agent-shared/Cargo.toml | 1 + nexus-sled-agent-shared/src/inventory.rs | 25 ++++-- nexus/inventory/Cargo.toml | 1 + nexus/inventory/src/collector.rs | 30 ++++--- .../execution/src/omicron_sled_config.rs | 31 ++++++- nexus/reconfigurator/planning/src/example.rs | 7 +- nexus/reconfigurator/planning/src/planner.rs | 15 ++-- nexus/test-utils/src/lib.rs | 33 +++----- nexus/types/src/deployment.rs | 83 ++++++++----------- sled-agent/src/rack_setup/service.rs | 47 ++++------- sled-agent/src/sim/sled_agent.rs | 18 +++- sled-agent/src/sled_agent.rs | 24 +++++- 15 files changed, 196 insertions(+), 140 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6f660297fa4..323cdc38421 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6210,6 +6210,7 @@ dependencies = [ "gateway-client", "gateway-messages", "gateway-test-utils", + "id-map", "nexus-sled-agent-shared", "nexus-types", "omicron-common", @@ -6548,6 +6549,7 @@ name = "nexus-sled-agent-shared" version = "0.1.0" dependencies = [ "daft", + "id-map", "illumos-utils", "omicron-common", "omicron-passwords", @@ -7093,6 +7095,7 @@ dependencies = [ "futures", "hex", "http", + "id-map", "ipnetwork", "libc", "macaddr", diff --git a/common/Cargo.toml b/common/Cargo.toml index da1b648c333..2e1cb217743 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -25,6 +25,7 @@ dropshot.workspace = true futures.workspace = true hex.workspace = true http.workspace = true +id-map.workspace = true ipnetwork.workspace = true lldp_protocol.workspace = true macaddr.workspace = true diff --git a/common/src/disk.rs b/common/src/disk.rs index adef569d5ab..b866d447822 100644 --- a/common/src/disk.rs +++ b/common/src/disk.rs @@ -7,6 +7,7 @@ use anyhow::bail; use camino::{Utf8Path, Utf8PathBuf}; use daft::Diffable; +use id_map::IdMappable; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::ZpoolUuid; @@ -42,6 +43,14 @@ pub struct OmicronPhysicalDiskConfig { pub pool_id: ZpoolUuid, } +impl IdMappable for OmicronPhysicalDiskConfig { + type Id = PhysicalDiskUuid; + + fn id(&self) -> Self::Id { + self.id + } +} + #[derive( Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq, Hash, )] @@ -337,6 +346,14 @@ pub struct DatasetConfig { pub inner: SharedDatasetConfig, } +impl IdMappable for DatasetConfig { + type Id = DatasetUuid; + + fn id(&self) -> Self::Id { + self.id + } +} + #[derive( Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq, Hash, )] diff --git a/nexus-sled-agent-shared/Cargo.toml b/nexus-sled-agent-shared/Cargo.toml index 043a2eddb61..d68dbe33e93 100644 --- a/nexus-sled-agent-shared/Cargo.toml +++ b/nexus-sled-agent-shared/Cargo.toml @@ -8,6 +8,7 @@ workspace = true [dependencies] daft.workspace = true +id-map.workspace = true illumos-utils.workspace = true omicron-common.workspace = true omicron-passwords.workspace = true diff --git a/nexus-sled-agent-shared/src/inventory.rs b/nexus-sled-agent-shared/src/inventory.rs index 5b8fdb39133..70672f68df8 100644 --- a/nexus-sled-agent-shared/src/inventory.rs +++ b/nexus-sled-agent-shared/src/inventory.rs @@ -7,14 +7,16 @@ use std::net::{IpAddr, Ipv6Addr, SocketAddr, SocketAddrV6}; use daft::Diffable; +use id_map::IdMap; +use id_map::IdMappable; use omicron_common::{ api::{ external::{ByteCount, Generation}, internal::shared::{NetworkInterface, SourceNatConfig}, }, disk::{ - DatasetManagementStatus, DatasetsConfig, DiskManagementStatus, - DiskVariant, OmicronPhysicalDisksConfig, + DatasetConfig, DatasetManagementStatus, DiskManagementStatus, + DiskVariant, OmicronPhysicalDiskConfig, }, zpool_name::ZpoolName, }; @@ -130,13 +132,12 @@ pub enum SledRole { /// Describes the set of Reconfigurator-managed configuration elements of a sled // TODO this struct should have a generation number; at the moment, each of // the fields has a separete one internally. -#[derive( - Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq, Hash, -)] +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq)] pub struct OmicronSledConfig { - pub disks_config: OmicronPhysicalDisksConfig, - pub datasets_config: DatasetsConfig, - pub zones_config: OmicronZonesConfig, + pub generation: Generation, + pub disks: IdMap, + pub datasets: IdMap, + pub zones: IdMap, } /// Result of the currently-synchronous `omicron_config_put` endpoint. @@ -190,6 +191,14 @@ pub struct OmicronZoneConfig { pub image_source: OmicronZoneImageSource, } +impl IdMappable for OmicronZoneConfig { + type Id = OmicronZoneUuid; + + fn id(&self) -> Self::Id { + self.id + } +} + impl OmicronZoneConfig { /// Returns the underlay IP address associated with this zone. /// diff --git a/nexus/inventory/Cargo.toml b/nexus/inventory/Cargo.toml index b5ca91283e5..8df62e16d9c 100644 --- a/nexus/inventory/Cargo.toml +++ b/nexus/inventory/Cargo.toml @@ -17,6 +17,7 @@ clickhouse-admin-types.workspace = true futures.workspace = true gateway-client.workspace = true gateway-messages.workspace = true +id-map.workspace = true nexus-sled-agent-shared.workspace = true nexus-types.workspace = true omicron-common.workspace = true diff --git a/nexus/inventory/src/collector.rs b/nexus/inventory/src/collector.rs index 36a97a26ddf..5aff15f6400 100644 --- a/nexus/inventory/src/collector.rs +++ b/nexus/inventory/src/collector.rs @@ -408,15 +408,13 @@ mod test { use super::Collector; use crate::StaticSledAgentEnumerator; use gateway_messages::SpPort; + use id_map::IdMap; use nexus_sled_agent_shared::inventory::OmicronSledConfig; use nexus_sled_agent_shared::inventory::OmicronZoneConfig; use nexus_sled_agent_shared::inventory::OmicronZoneImageSource; use nexus_sled_agent_shared::inventory::OmicronZoneType; - use nexus_sled_agent_shared::inventory::OmicronZonesConfig; use nexus_types::inventory::Collection; use omicron_common::api::external::Generation; - use omicron_common::disk::DatasetsConfig; - use omicron_common::disk::OmicronPhysicalDisksConfig; use omicron_common::zpool_name::ZpoolName; use omicron_sled_agent::sim; use omicron_uuid_kinds::OmicronZoneUuid; @@ -595,19 +593,19 @@ mod test { let zone_address = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0); client .omicron_config_put(&OmicronSledConfig { - disks_config: OmicronPhysicalDisksConfig::default(), - datasets_config: DatasetsConfig::default(), - zones_config: OmicronZonesConfig { - generation: Generation::from(3), - zones: vec![OmicronZoneConfig { - id: zone_id, - zone_type: OmicronZoneType::Oximeter { - address: zone_address, - }, - filesystem_pool: Some(filesystem_pool), - image_source: OmicronZoneImageSource::InstallDataset, - }], - }, + generation: Generation::from(3), + disks: IdMap::default(), + datasets: IdMap::default(), + zones: [OmicronZoneConfig { + id: zone_id, + zone_type: OmicronZoneType::Oximeter { + address: zone_address, + }, + filesystem_pool: Some(filesystem_pool), + image_source: OmicronZoneImageSource::InstallDataset, + }] + .into_iter() + .collect(), }) .await .expect("failed to write initial zone version to fake sled agent"); diff --git a/nexus/reconfigurator/execution/src/omicron_sled_config.rs b/nexus/reconfigurator/execution/src/omicron_sled_config.rs index 85fbd523031..5f42fca2fa9 100644 --- a/nexus/reconfigurator/execution/src/omicron_sled_config.rs +++ b/nexus/reconfigurator/execution/src/omicron_sled_config.rs @@ -145,6 +145,7 @@ fn parse_config_result( mod tests { use super::*; use id_map::IdMap; + use nexus_sled_agent_shared::inventory::OmicronZonesConfig; use nexus_sled_agent_shared::inventory::SledRole; use nexus_test_utils_macros::nexus_test; use nexus_types::deployment::BlueprintDatasetConfig; @@ -163,7 +164,9 @@ mod tests { use omicron_common::api::external::Generation; use omicron_common::api::internal::shared::DatasetKind; use omicron_common::disk::CompressionAlgorithm; + use omicron_common::disk::DatasetsConfig; use omicron_common::disk::DiskIdentity; + use omicron_common::disk::OmicronPhysicalDisksConfig; use omicron_common::zpool_name::ZpoolName; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::OmicronZoneUuid; @@ -328,9 +331,31 @@ mod tests { let in_service_config = sled_config.clone().into_in_service_sled_config(); - assert_eq!(observed_disks, in_service_config.disks_config); - assert_eq!(observed_datasets, in_service_config.datasets_config); - assert_eq!(observed_zones, in_service_config.zones_config); + assert_eq!( + observed_disks, + OmicronPhysicalDisksConfig { + generation: in_service_config.generation, + disks: in_service_config.disks.into_iter().collect(), + } + ); + assert_eq!( + observed_datasets, + DatasetsConfig { + generation: in_service_config.generation, + datasets: in_service_config + .datasets + .into_iter() + .map(|d| (d.id, d)) + .collect(), + } + ); + assert_eq!( + observed_zones, + OmicronZonesConfig { + generation: in_service_config.generation, + zones: in_service_config.zones.into_iter().collect(), + } + ); // We expect to see each single in-service item we supplied as input. assert_eq!(observed_disks.disks.len(), 1); diff --git a/nexus/reconfigurator/planning/src/example.rs b/nexus/reconfigurator/planning/src/example.rs index 7a031b8c81c..538884fb45d 100644 --- a/nexus/reconfigurator/planning/src/example.rs +++ b/nexus/reconfigurator/planning/src/example.rs @@ -14,6 +14,7 @@ use crate::planner::rng::PlannerRng; use crate::system::SledBuilder; use crate::system::SystemDescription; use nexus_inventory::CollectionBuilderRng; +use nexus_sled_agent_shared::inventory::OmicronZonesConfig; use nexus_types::deployment::Blueprint; use nexus_types::deployment::OmicronZoneNic; use nexus_types::deployment::PlanningInput; @@ -484,10 +485,14 @@ impl ExampleSystemBuilder { } for (sled_id, sled_cfg) in &blueprint.sleds { + let sled_cfg = sled_cfg.clone().into_in_service_sled_config(); system .sled_set_omicron_zones( *sled_id, - sled_cfg.clone().into_in_service_sled_config().zones_config, + OmicronZonesConfig { + generation: sled_cfg.generation, + zones: sled_cfg.zones.into_iter().collect(), + }, ) .unwrap(); } diff --git a/nexus/reconfigurator/planning/src/planner.rs b/nexus/reconfigurator/planning/src/planner.rs index 1ee0f8ddda0..d863438bab1 100644 --- a/nexus/reconfigurator/planning/src/planner.rs +++ b/nexus/reconfigurator/planning/src/planner.rs @@ -996,6 +996,7 @@ pub(crate) mod test { use clickhouse_admin_types::ClickhouseKeeperClusterMembership; use clickhouse_admin_types::KeeperId; use expectorate::assert_contents; + use nexus_sled_agent_shared::inventory::OmicronZonesConfig; use nexus_types::deployment::BlueprintDatasetDisposition; use nexus_types::deployment::BlueprintDiffSummary; use nexus_types::deployment::BlueprintPhysicalDiskDisposition; @@ -1180,16 +1181,18 @@ pub(crate) mod test { // example.collection -- this should be addressed via API improvements. example .system - .sled_set_omicron_zones( - new_sled_id, - blueprint4 + .sled_set_omicron_zones(new_sled_id, { + let sled_cfg = blueprint4 .sleds .get(&new_sled_id) .expect("blueprint should contain zones for new sled") .clone() - .into_in_service_sled_config() - .zones_config, - ) + .into_in_service_sled_config(); + OmicronZonesConfig { + generation: sled_cfg.generation, + zones: sled_cfg.zones.into_iter().collect(), + } + }) .unwrap(); let collection = example.system.to_collection_builder().unwrap().build(); diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index fc2d9898ac7..b9b4e22eb21 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -35,7 +35,6 @@ use nexus_config::NexusConfig; use nexus_db_queries::db::pub_test_utils::crdb; use nexus_sled_agent_shared::inventory::OmicronSledConfig; use nexus_sled_agent_shared::inventory::OmicronZoneDataset; -use nexus_sled_agent_shared::inventory::OmicronZonesConfig; use nexus_sled_agent_shared::recovery_silo::RecoverySiloConfig; use nexus_test_interface::NexusServer; use nexus_types::deployment::Blueprint; @@ -70,8 +69,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::DatasetsConfig; -use omicron_common::disk::OmicronPhysicalDisksConfig; use omicron_common::zpool_name::ZpoolName; use omicron_sled_agent::sim; use omicron_test_utils::dev; @@ -1089,19 +1086,17 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { client .omicron_config_put(&OmicronSledConfig { + generation: Generation::new().next(), // Sending no disks or datasets is probably wrong, but there are // a lot of inconsistencies with this in nexus-test. - disks_config: OmicronPhysicalDisksConfig::default(), - datasets_config: DatasetsConfig::default(), - zones_config: OmicronZonesConfig { - zones: self - .blueprint_zones - .clone() - .into_iter() - .map(From::from) - .collect(), - generation: Generation::new().next(), - }, + disks: IdMap::default(), + datasets: IdMap::default(), + zones: self + .blueprint_zones + .clone() + .into_iter() + .map(From::from) + .collect(), }) .await .expect("Failed to configure sled agent with our zones"); @@ -1135,13 +1130,11 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { client .omicron_config_put(&OmicronSledConfig { + generation: Generation::new().next(), // As above, sending no disks or datasets is probably wrong - disks_config: OmicronPhysicalDisksConfig::default(), - datasets_config: DatasetsConfig::default(), - zones_config: OmicronZonesConfig { - zones: vec![], - generation: Generation::new().next(), - }, + disks: IdMap::default(), + datasets: IdMap::default(), + zones: IdMap::default(), }) .await .expect("Failed to configure sled agent with our zones"); diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index 769f74a46f7..db069f75925 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -23,7 +23,6 @@ use daft::Diffable; use nexus_sled_agent_shared::inventory::OmicronSledConfig; use nexus_sled_agent_shared::inventory::OmicronZoneConfig; use nexus_sled_agent_shared::inventory::OmicronZoneImageSource; -use nexus_sled_agent_shared::inventory::OmicronZonesConfig; use nexus_sled_agent_shared::inventory::ZoneKind; use omicron_common::api::external::ByteCount; use omicron_common::api::external::Generation; @@ -31,10 +30,8 @@ use omicron_common::api::internal::shared::DatasetKind; use omicron_common::disk::CompressionAlgorithm; use omicron_common::disk::DatasetConfig; use omicron_common::disk::DatasetName; -use omicron_common::disk::DatasetsConfig; use omicron_common::disk::DiskIdentity; use omicron_common::disk::OmicronPhysicalDiskConfig; -use omicron_common::disk::OmicronPhysicalDisksConfig; use omicron_common::disk::SharedDatasetConfig; use omicron_uuid_kinds::BlueprintUuid; use omicron_uuid_kinds::DatasetUuid; @@ -668,53 +665,41 @@ impl BlueprintSledConfig { /// is named slightly more explicitly, as it filters the blueprint /// configuration to only consider components that should be in-service. pub fn into_in_service_sled_config(self) -> OmicronSledConfig { - // TODO OmicronSledConfig should have a single generation; for now we - // reuse our generation for all three subfields. Tracked by - // https://github.com/oxidecomputer/omicron/issues/7774 - let generation = self.sled_agent_generation; OmicronSledConfig { - disks_config: OmicronPhysicalDisksConfig { - generation, - disks: self - .disks - .into_iter() - .filter_map(|disk| { - if disk.disposition.is_in_service() { - Some(disk.into()) - } else { - None - } - }) - .collect(), - }, - datasets_config: DatasetsConfig { - generation, - datasets: self - .datasets - .into_iter() - .filter_map(|dataset| { - if dataset.disposition.is_in_service() { - Some((dataset.id, dataset.into())) - } else { - None - } - }) - .collect(), - }, - zones_config: OmicronZonesConfig { - generation, - zones: self - .zones - .into_iter() - .filter_map(|zone| { - if zone.disposition.is_in_service() { - Some(zone.into()) - } else { - None - } - }) - .collect(), - }, + generation: self.sled_agent_generation, + disks: self + .disks + .into_iter() + .filter_map(|disk| { + if disk.disposition.is_in_service() { + Some(disk.into()) + } else { + None + } + }) + .collect(), + datasets: self + .datasets + .into_iter() + .filter_map(|dataset| { + if dataset.disposition.is_in_service() { + Some(dataset.into()) + } else { + None + } + }) + .collect(), + zones: self + .zones + .into_iter() + .filter_map(|zone| { + if zone.disposition.is_in_service() { + Some(zone.into()) + } else { + None + } + }) + .collect(), } } diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 9164b10b228..0700e20ae1e 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -106,9 +106,7 @@ use omicron_common::api::internal::shared::LldpAdminStatus; use omicron_common::backoff::{ BackoffError, retry_notify, retry_policy_internal_service_aggressive, }; -use omicron_common::disk::{ - DatasetKind, DatasetsConfig, OmicronPhysicalDisksConfig, -}; +use omicron_common::disk::DatasetKind; use omicron_common::ledger::{self, Ledger, Ledgerable}; use omicron_ddm_admin_client::{Client as DdmAdminClient, DdmError}; use omicron_uuid_kinds::BlueprintUuid; @@ -372,9 +370,10 @@ impl ServiceInner { info!( log, "attempting to set sled's config"; - "disks" => ?sled_config.disks_config, - "datasets" => ?sled_config.datasets_config, - "zones" => ?sled_config.zones_config, + "generation" => %sled_config.generation, + "disks" => ?sled_config.disks, + "datasets" => ?sled_config.datasets, + "zones" => ?sled_config.zones, ); let result = client.omicron_config_put(&sled_config).await; let error = match result { @@ -464,12 +463,8 @@ impl ServiceInner { log, "ignoring attempt to initialize config because \ the server seems to be newer"; - "attempted_disks_generation" => - i64::from(&sled_config.disks_config.generation), - "attempted_datasets_generation" => - i64::from(&sled_config.datasets_config.generation), - "attempted_zones_generation" => - i64::from(&sled_config.zones_config.generation), + "attempted_generation" => + i64::from(&sled_config.generation), "req_id" => &response.request_id, "server_message" => &response.message, ); @@ -535,25 +530,17 @@ impl ServiceInner { })? .clone(); - // TODO OmicronSledConfig should have a single generation; for - // now we reuse our zone generation for all three subfields. - // Tracked by - // https://github.com/oxidecomputer/omicron/issues/7774 - let generation = zones_config.generation; let sled_config = OmicronSledConfig { - disks_config: OmicronPhysicalDisksConfig { - generation, - disks: config - .disks - .iter() - .map(|c| c.clone().into()) - .collect(), - }, - datasets_config: DatasetsConfig { - generation, - datasets: config.datasets.clone(), - }, - zones_config, + // We bump the zone generation as we step through phases of + // RSS; use that as the overall sled config generation. + generation: zones_config.generation, + disks: config + .disks + .iter() + .map(|c| c.clone().into()) + .collect(), + datasets: config.datasets.values().cloned().collect(), + zones: zones_config.zones.iter().cloned().collect(), }; self.set_config_on_sled(*sled_address, sled_config).await?; diff --git a/sled-agent/src/sim/sled_agent.rs b/sled-agent/src/sim/sled_agent.rs index 98174dbdd61..9ab9444932c 100644 --- a/sled-agent/src/sim/sled_agent.rs +++ b/sled-agent/src/sim/sled_agent.rs @@ -906,15 +906,27 @@ impl SledAgent { &self, config: OmicronSledConfig, ) -> Result { + let disks_config = OmicronPhysicalDisksConfig { + generation: config.generation, + disks: config.disks.into_iter().collect(), + }; + let datasets_config = DatasetsConfig { + generation: config.generation, + datasets: config.datasets.into_iter().map(|d| (d.id, d)).collect(), + }; + let zones_config = OmicronZonesConfig { + generation: config.generation, + zones: config.zones.into_iter().collect(), + }; let (disks, datasets) = { let mut storage = self.storage.lock(); let DisksManagementResult { status: disks } = - storage.omicron_physical_disks_ensure(config.disks_config)?; + storage.omicron_physical_disks_ensure(disks_config)?; let DatasetsManagementResult { status: datasets } = - storage.datasets_ensure(config.datasets_config)?; + storage.datasets_ensure(datasets_config)?; (disks, datasets) }; - *self.fake_zones.lock().unwrap() = config.zones_config; + *self.fake_zones.lock().unwrap() = zones_config; Ok(OmicronSledConfigResult { disks, datasets }) } diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index 9eda7e4feb1..e1976c2b29a 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -970,8 +970,14 @@ impl SledAgent { &self, config: OmicronSledConfig, ) -> Result { - let disks = - self.omicron_physical_disks_ensure(config.disks_config).await?; + // Until the config-reconciler work lands: unpack the unified config + // into the three split configs for indepenedent ledgering. + let disks_config = OmicronPhysicalDisksConfig { + generation: config.generation, + disks: config.disks.into_iter().collect(), + }; + + let disks = self.omicron_physical_disks_ensure(disks_config).await?; // If we only had partial success deploying disks, don't proceed. if disks.has_error() { @@ -981,7 +987,12 @@ impl SledAgent { }); } - let datasets = self.datasets_ensure(config.datasets_config).await?; + let datasets_config = DatasetsConfig { + generation: config.generation, + datasets: config.datasets.into_iter().map(|d| (d.id, d)).collect(), + }; + + let datasets = self.datasets_ensure(datasets_config).await?; // If we only had partial success deploying datasets, don't proceed. if datasets.has_error() { @@ -991,7 +1002,12 @@ impl SledAgent { }); } - self.omicron_zones_ensure(config.zones_config).await?; + let zones_config = OmicronZonesConfig { + generation: config.generation, + zones: config.zones.into_iter().collect(), + }; + + self.omicron_zones_ensure(zones_config).await?; Ok(OmicronSledConfigResult { disks: disks.status, From e32766ff04615afe8d5bc05ae392ce66b56fe93c Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 30 Apr 2025 09:40:16 -0400 Subject: [PATCH 2/4] patch up tests that assume config generations --- .../execution/src/omicron_sled_config.rs | 4 +++- .../tasks/support_bundle_collector.rs | 9 ++++++- nexus/test-utils/src/resource_helpers.rs | 24 +++++++++++++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/nexus/reconfigurator/execution/src/omicron_sled_config.rs b/nexus/reconfigurator/execution/src/omicron_sled_config.rs index 5f42fca2fa9..64e8fdc3088 100644 --- a/nexus/reconfigurator/execution/src/omicron_sled_config.rs +++ b/nexus/reconfigurator/execution/src/omicron_sled_config.rs @@ -192,6 +192,8 @@ mod tests { _ => panic!("Unexpected address type for sled agent (wanted IPv6)"), }; let sim_sled_agent = &cptestctx.sled_agents[0].sled_agent(); + let sim_sled_agent_config_generation = + sim_sled_agent.omicron_zones_list().generation; let sleds_by_id = BTreeMap::from([( sim_sled_agent.id, @@ -309,7 +311,7 @@ mod tests { let sled_config = BlueprintSledConfig { state: SledState::Active, - sled_agent_generation: Generation::new().next(), + sled_agent_generation: sim_sled_agent_config_generation.next(), disks, datasets, zones, diff --git a/nexus/src/app/background/tasks/support_bundle_collector.rs b/nexus/src/app/background/tasks/support_bundle_collector.rs index 7748e8b1346..764d575a582 100644 --- a/nexus/src/app/background/tasks/support_bundle_collector.rs +++ b/nexus/src/app/background/tasks/support_bundle_collector.rs @@ -1155,8 +1155,15 @@ mod test { ) }) .collect(); + + // Read current sled config generation from zones (this will change + // slightly once the simulator knows how to keep the unified config + // and be a little less weird) + let current_generation = + cptestctx.first_sled_agent().omicron_zones_list().generation; + let dataset_config = DatasetsConfig { - generation: Generation::new().next(), + generation: current_generation.next(), datasets, }; diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index d580140132a..7501472d41d 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -1307,13 +1307,33 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { .collect(), }; + // ControlPlaneTestContext sets up initial configs; find the max + // generation of all our sleds and use that as the base for any new + // configs we produce. + let generation = cptestctx + .all_sled_agents() + .filter_map(|agent| { + if input_sleds.contains(&agent.sled_agent.id) { + Some(agent + .sled_agent + .datasets_config_list() + .expect( + "dataset config populated by ControlPlaneTestContext", + ) + .generation) + } else { + None + } + }) + .max() + .expect("at least one sled specified"); + let mut sleds = BTreeMap::new(); for sled_id in input_sleds { sleds.insert(sled_id, PerSledDiskState { zpools: vec![] }); } - let mut disk_test = - Self { cptestctx, sleds, generation: Generation::new() }; + let mut disk_test = Self { cptestctx, sleds, generation }; for sled_id in disk_test.sleds.keys().cloned().collect::>() From d15448a0328bf68de38aba7b080314939d7b836e Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 30 Apr 2025 09:52:23 -0400 Subject: [PATCH 3/4] clippy: unused import --- nexus/src/app/background/tasks/support_bundle_collector.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/nexus/src/app/background/tasks/support_bundle_collector.rs b/nexus/src/app/background/tasks/support_bundle_collector.rs index 764d575a582..9003f19038d 100644 --- a/nexus/src/app/background/tasks/support_bundle_collector.rs +++ b/nexus/src/app/background/tasks/support_bundle_collector.rs @@ -949,7 +949,6 @@ mod test { use nexus_test_utils::SLED_AGENT_UUID; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external::ByteCount; - use omicron_common::api::external::Generation; use omicron_common::api::internal::shared::DatasetKind; use omicron_common::disk::DatasetConfig; use omicron_common::disk::DatasetName; From 4c86adade807e33321af9ca774f3521a259816ce Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 30 Apr 2025 11:26:07 -0400 Subject: [PATCH 4/4] openapi --- openapi/sled-agent.json | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index e63c9ae81d4..c3f1cbcee51 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -4521,6 +4521,24 @@ ], "additionalProperties": false }, + "IdMapDatasetConfig": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/DatasetConfig" + } + }, + "IdMapOmicronPhysicalDiskConfig": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/OmicronPhysicalDiskConfig" + } + }, + "IdMapOmicronZoneConfig": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/OmicronZoneConfig" + } + }, "ImportExportPolicy": { "description": "Define policy relating to the import and export of prefixes from a BGP peer.", "oneOf": [ @@ -5440,20 +5458,24 @@ "description": "Describes the set of Reconfigurator-managed configuration elements of a sled", "type": "object", "properties": { - "datasets_config": { - "$ref": "#/components/schemas/DatasetsConfig" + "datasets": { + "$ref": "#/components/schemas/IdMapDatasetConfig" }, - "disks_config": { - "$ref": "#/components/schemas/OmicronPhysicalDisksConfig" + "disks": { + "$ref": "#/components/schemas/IdMapOmicronPhysicalDiskConfig" }, - "zones_config": { - "$ref": "#/components/schemas/OmicronZonesConfig" + "generation": { + "$ref": "#/components/schemas/Generation" + }, + "zones": { + "$ref": "#/components/schemas/IdMapOmicronZoneConfig" } }, "required": [ - "datasets_config", - "disks_config", - "zones_config" + "datasets", + "disks", + "generation", + "zones" ] }, "OmicronSledConfigResult": {