OmicronSledConfig: Just one Generation (#8067)

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. We don't yet ledger `OmicronSledConfig`s to disk, so
we're free to fiddle with the details of its fields without worrying
about backwards compatibility.

Fixes #7774.

Author: jgallagher

Cargo.lock

@@ -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

common/Cargo.toml

@@ -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,
 )]

common/src/disk.rs

@@ -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

nexus-sled-agent-shared/Cargo.toml

@@ -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.
     ///

nexus-sled-agent-shared/src/inventory.rs

@@ -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

nexus/inventory/Cargo.toml

@@ -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");

nexus/inventory/src/collector.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;
@@ -189,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,
@@ -306,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,
@@ -328,9 +333,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);

nexus/reconfigurator/execution/src/omicron_sled_config.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();
         }

nexus/reconfigurator/planning/src/example.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();

nexus/reconfigurator/planning/src/planner.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;
@@ -1155,8 +1154,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,
             };