Deterministic UUIDs, populate ExampleSystem datasets, sorting, check …

…BP diff in tests

nexus/reconfigurator/planning/src/blueprint_builder/builder.rs

@@ -711,17 +711,22 @@ impl<'a> BlueprintBuilder<'a> {
         let (mut additions, mut updates, expunges, removals) = {
             let mut datasets_builder = BlueprintSledDatasetsBuilder::new(
                 self.log.clone(),
+                &mut self.rng,
                 sled_id,
                 &self.datasets,
                 resources,
             );
 
             // Ensure each zpool has a "Debug" and "Zone Root" dataset.
-            let bp_zpools = self
+            let mut bp_zpools = self
                 .disks
                 .current_sled_disks(sled_id)
                 .map(|disk_config| disk_config.pool_id)
                 .collect::<Vec<ZpoolUuid>>();
+            // We iterate over the zpools in a deterministic order to ensure
+            // that "new Dataset UUIDs" are distributed in a reliable order.
+            bp_zpools.sort();
+
             for zpool_id in bp_zpools {
                 let zpool = ZpoolName::new_external(zpool_id);
                 let address = None;
@@ -1454,6 +1459,7 @@ struct BlueprintBuilderRng {
     // associated with a specific `TypedUuidKind`.
     blueprint_rng: UuidRng,
     zone_rng: TypedUuidRng<OmicronZoneKind>,
+    dataset_rng: TypedUuidRng<omicron_uuid_kinds::DatasetKind>,
     network_interface_rng: UuidRng,
     external_ip_rng: TypedUuidRng<ExternalIpKind>,
 }
@@ -1466,6 +1472,7 @@ impl BlueprintBuilderRng {
     fn new_from_parent(mut parent: StdRng) -> Self {
         let blueprint_rng = UuidRng::from_parent_rng(&mut parent, "blueprint");
         let zone_rng = TypedUuidRng::from_parent_rng(&mut parent, "zone");
+        let dataset_rng = TypedUuidRng::from_parent_rng(&mut parent, "dataset");
         let network_interface_rng =
             UuidRng::from_parent_rng(&mut parent, "network_interface");
         let external_ip_rng =
@@ -1474,6 +1481,7 @@ impl BlueprintBuilderRng {
         BlueprintBuilderRng {
             blueprint_rng,
             zone_rng,
+            dataset_rng,
             network_interface_rng,
             external_ip_rng,
         }
@@ -1773,6 +1781,7 @@ impl<'a> BlueprintDatasetsBuilder<'a> {
 #[derive(Debug)]
 struct BlueprintSledDatasetsBuilder<'a> {
     log: Logger,
+    rng: &'a mut BlueprintBuilderRng,
     blueprint_datasets:
         BTreeMap<ZpoolUuid, BTreeMap<DatasetKind, &'a BlueprintDatasetConfig>>,
     database_datasets:
@@ -1793,6 +1802,7 @@ struct BlueprintSledDatasetsBuilder<'a> {
 impl<'a> BlueprintSledDatasetsBuilder<'a> {
     pub fn new(
         log: Logger,
+        rng: &'a mut BlueprintBuilderRng,
         sled_id: SledUuid,
         datasets: &'a BlueprintDatasetsBuilder<'_>,
         resources: &'a SledResources,
@@ -1824,6 +1834,7 @@ impl<'a> BlueprintSledDatasetsBuilder<'a> {
 
         Self {
             log,
+            rng,
             blueprint_datasets,
             database_datasets,
             unchanged_datasets: BTreeMap::new(),
@@ -1887,7 +1898,7 @@ impl<'a> BlueprintSledDatasetsBuilder<'a> {
         let id = if let Some(old_config) = self.get_from_db(zpool_id, kind) {
             old_config.id
         } else {
-            DatasetUuid::new_v4()
+            self.rng.dataset_rng.next()
         };
 
         let new_config = make_config(id);

nexus/reconfigurator/planning/src/example.rs

@@ -89,6 +89,21 @@ impl ExampleSystem {
         }
 
         let blueprint = builder.build();
+
+        // We just ensured that a handful of datasets should exist in
+        // the blueprint, but they don't yet exist in the SystemDescription.
+        //
+        // Go back and add them so that the blueprint is consistent with
+        // inventory.
+        for (sled_id, datasets) in &blueprint.blueprint_datasets {
+            let sled = system.get_sled_mut(*sled_id).unwrap();
+
+            for dataset_config in datasets.datasets.values() {
+                let config = dataset_config.clone().try_into().unwrap();
+                sled.add_synthetic_dataset(config);
+            }
+        }
+
         let mut builder =
             system.to_collection_builder().expect("failed to build collection");
         builder.set_rng_seed((test_name, "ExampleSystem collection"));

nexus/reconfigurator/planning/src/planner.rs

@@ -845,6 +845,9 @@ mod test {
         assert_eq!(diff.zones.errors.len(), 0);
         assert_eq!(diff.physical_disks.added.len(), 0);
         assert_eq!(diff.physical_disks.removed.len(), 0);
+        assert_eq!(diff.datasets.added.len(), 0);
+        assert_eq!(diff.datasets.removed.len(), 0);
+        assert_eq!(diff.datasets.unchanged.len(), 3);
         verify_blueprint(&blueprint2);
 
         // Now add a new sled.
@@ -876,6 +879,8 @@ mod test {
             &diff.display().to_string(),
         );
         assert_eq!(diff.sleds_added.len(), 1);
+        assert_eq!(diff.physical_disks.added.len(), 1);
+        assert_eq!(diff.datasets.added.len(), 1);
         let sled_id = *diff.sleds_added.first().unwrap();
         let sled_zones = diff.zones.added.get(&sled_id).unwrap();
         // We have defined elsewhere that the first generation contains no
@@ -1021,6 +1026,7 @@ mod test {
             assert_eq!(collection.omicron_zones.len(), 1);
             blueprint.blueprint_zones.retain(|k, _v| keep_sled_id == *k);
             blueprint.blueprint_disks.retain(|k, _v| keep_sled_id == *k);
+            blueprint.blueprint_datasets.retain(|k, _v| keep_sled_id == *k);
 
             // Also remove all the networking resources for the zones we just
             // stripped out; i.e., only keep those for `keep_sled_id`.
@@ -1099,7 +1105,6 @@ mod test {
         assert_eq!(diff.sleds_removed.len(), 0);
         assert_eq!(diff.sleds_modified.len(), 1);
         let changed_sled_id = diff.sleds_modified.first().unwrap();
-
         // TODO-cleanup use `TypedUuid` everywhere
         assert_eq!(*changed_sled_id, sled_id);
         assert_eq!(diff.zones.removed.len(), 0);
@@ -1115,6 +1120,11 @@ mod test {
             }
         }
 
+        assert_eq!(diff.physical_disks.added.len(), 0);
+        assert_eq!(diff.physical_disks.removed.len(), 0);
+        assert_eq!(diff.datasets.added.len(), 1);
+        assert_eq!(diff.datasets.removed.len(), 0);
+
         logctx.cleanup_successful();
     }
 
@@ -1272,6 +1282,11 @@ mod test {
         );
         assert!(!diff.zones.removed.contains_key(sled_id));
 
+        assert_eq!(diff.physical_disks.added.len(), 1);
+        assert_eq!(diff.physical_disks.removed.len(), 0);
+        assert_eq!(diff.datasets.added.len(), 1);
+        assert_eq!(diff.datasets.removed.len(), 0);
+
         logctx.cleanup_successful();
     }
 
@@ -1361,6 +1376,13 @@ mod test {
             "Should have expunged this zone"
         );
 
+        assert_eq!(diff.physical_disks.added.len(), 0);
+        assert_eq!(diff.physical_disks.removed.len(), 1);
+        assert_eq!(diff.datasets.added.len(), 0);
+        // NOTE: Expunging a disk doesn't immediately delete datasets; see the
+        // "decommissioned_disk_cleaner" background task for more context.
+        assert_eq!(diff.datasets.removed.len(), 0);
+
         logctx.cleanup_successful();
     }
 

nexus/reconfigurator/planning/src/system.rs

@@ -12,7 +12,9 @@ use indexmap::IndexMap;
 use nexus_inventory::CollectionBuilder;
 use nexus_sled_agent_shared::inventory::Baseboard;
 use nexus_sled_agent_shared::inventory::Inventory;
+use nexus_sled_agent_shared::inventory::InventoryDataset;
 use nexus_sled_agent_shared::inventory::InventoryDisk;
+use nexus_sled_agent_shared::inventory::InventoryZpool;
 use nexus_sled_agent_shared::inventory::SledRole;
 use nexus_types::deployment::CockroachDbClusterVersion;
 use nexus_types::deployment::CockroachDbSettings;
@@ -207,6 +209,16 @@ impl SystemDescription {
         self
     }
 
+    pub fn get_sled_mut(
+        &mut self,
+        sled_id: SledUuid,
+    ) -> anyhow::Result<&mut Sled> {
+        let Some(sled) = self.sleds.get_mut(&sled_id) else {
+            bail!("Sled not found with id {sled_id}");
+        };
+        Ok(sled)
+    }
+
     /// Add a sled to the system, as described by a SledBuilder
     pub fn sled(&mut self, sled: SledBuilder) -> anyhow::Result<&mut Self> {
         let sled_id = sled.id.unwrap_or_else(SledUuid::new_v4);
@@ -450,11 +462,13 @@ pub struct SledHwInventory<'a> {
 /// This needs to be rich enough to generate a PlanningInput and inventory
 /// Collection.
 #[derive(Clone, Debug)]
-struct Sled {
+pub struct Sled {
     sled_id: SledUuid,
     sled_subnet: Ipv6Subnet<SLED_PREFIX>,
     inventory_sp: Option<(u16, SpState)>,
     inventory_sled_agent: Inventory,
+    // This represents "intended configuration of pools and datasets", rather
+    // than "observed status of pools and datasets".
     zpools: BTreeMap<ZpoolUuid, (SledDisk, Vec<DatasetConfig>)>,
     policy: SledPolicy,
 }
@@ -561,9 +575,13 @@ impl Sled {
                         slot: i64::try_from(i).unwrap(),
                     })
                     .collect(),
-                // Zpools & Datasets won't necessarily show up until our first
-                // request to provision storage, so we omit them.
-                zpools: vec![],
+                zpools: zpools
+                    .keys()
+                    .map(|id| InventoryZpool {
+                        id: *id,
+                        total_size: ByteCount::from_gibibytes_u32(100),
+                    })
+                    .collect(),
                 datasets: vec![],
             }
         };
@@ -713,6 +731,25 @@ impl Sled {
         }
     }
 
+    /// Adds a dataset to the system description.
+    ///
+    /// The inventory values for "available space" and "used space" are
+    /// made up, since this is a synthetic dataset.
+    pub fn add_synthetic_dataset(
+        &mut self,
+        config: omicron_common::disk::DatasetConfig,
+    ) {
+        self.inventory_sled_agent.datasets.push(InventoryDataset {
+            id: Some(config.id),
+            name: config.name.full_name(),
+            available: ByteCount::from_gibibytes_u32(1),
+            used: ByteCount::from_gibibytes_u32(0),
+            quota: config.quota,
+            reservation: config.reservation,
+            compression: config.compression.to_string(),
+        });
+    }
+
     fn sp_state(&self) -> Option<&(u16, SpState)> {
         self.inventory_sp.as_ref()
     }