OmicronSledConfig: Just one Generation

jgallagher · jgallagher · commit 7cd6e49c4d4d · 2025-04-29T16:10:36.000-04:00
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.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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
@@ -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
@@ -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<OmicronPhysicalDiskConfig>,
+    pub datasets: IdMap<DatasetConfig>,
+    pub zones: IdMap<OmicronZoneConfig>,
 }
 
 /// 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs
diff --git a/sled-agent/src/sim/sled_agent.rs b/sled-agent/src/sim/sled_agent.rs
diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs
