oxidecomputer · jgallagher · Mar 5, 2025 · Mar 4, 2025
diff --git a/nexus/db-queries/src/db/datastore/support_bundle.rs b/nexus/db-queries/src/db/datastore/support_bundle.rs
@@ -20,6 +20,7 @@ use crate::transaction_retry::OptionalError;
 use async_bb8_diesel::AsyncRunQueryDsl;
 use diesel::prelude::*;
 use futures::FutureExt;
+use nexus_types::deployment::BlueprintDatasetDisposition;
 use nexus_types::deployment::BlueprintZoneDisposition;
 use omicron_common::api::external;
 use omicron_common::api::external::CreateResult;
@@ -231,9 +232,7 @@ impl DataStore {
 
         // For this blueprint: The set of expunged debug datasets
         let invalid_datasets = blueprint
-            .all_omicron_datasets(
-                nexus_types::deployment::BlueprintDatasetFilter::Expunged,
-            )
+            .all_omicron_datasets(BlueprintDatasetDisposition::is_expunged)
             .filter_map(|(_sled_id, dataset_config)| {
                 if matches!(
                     dataset_config.kind,
@@ -479,8 +478,6 @@ mod test {
     use nexus_reconfigurator_planning::example::ExampleSystemBuilder;
     use nexus_reconfigurator_planning::example::SimRngState;
     use nexus_types::deployment::Blueprint;
-    use nexus_types::deployment::BlueprintDatasetDisposition;
-    use nexus_types::deployment::BlueprintDatasetFilter;
     use nexus_types::deployment::BlueprintZoneType;
     use omicron_common::api::external::LookupType;
     use omicron_common::api::internal::shared::DatasetKind::Debug as DebugDatasetKind;
@@ -536,9 +533,7 @@ mod test {
                     .iter()
                     .filter_map(|dataset| {
                         if !matches!(dataset.kind, DebugDatasetKind)
-                            || !dataset
-                                .disposition
-                                .matches(BlueprintDatasetFilter::InService)
+                            || !dataset.disposition.is_in_service()
                         {
                             return None;
                         };
@@ -945,17 +940,16 @@ mod test {
             .collect()
     }
 
-    fn get_debug_datasets_from_blueprint(
+    fn get_in_service_debug_datasets_from_blueprint(
         bp: &Blueprint,
-        filter: BlueprintDatasetFilter,
     ) -> Vec<DatasetUuid> {
         bp.sleds
             .values()
             .flat_map(|sled_config| {
                 let mut debug_datasets = vec![];
                 for dataset in sled_config.datasets_config.datasets.iter() {
                     if matches!(dataset.kind, DebugDatasetKind)
-                        && dataset.disposition.matches(filter)
+                        && dataset.disposition.is_in_service()
                     {
                         debug_datasets.push(dataset.id);
                     }
@@ -1026,10 +1020,7 @@ mod test {
             .get(0)
             .map(|id| *id)
             .expect("There should be a Nexus in the example blueprint");
-        let debug_datasets = get_debug_datasets_from_blueprint(
-            &bp1,
-            BlueprintDatasetFilter::InService,
-        );
+        let debug_datasets = get_in_service_debug_datasets_from_blueprint(&bp1);
         assert!(!debug_datasets.is_empty());
 
         // When we create a bundle, it should exist on a dataset provisioned by
@@ -1134,10 +1125,7 @@ mod test {
             .get(0)
             .map(|id| *id)
             .expect("There should be a Nexus in the example blueprint");
-        let debug_datasets = get_debug_datasets_from_blueprint(
-            &bp1,
-            BlueprintDatasetFilter::InService,
-        );
+        let debug_datasets = get_in_service_debug_datasets_from_blueprint(&bp1);
         assert!(!debug_datasets.is_empty());
 
         // When we create a bundle, it should exist on a dataset provisioned by
@@ -1238,10 +1226,7 @@ mod test {
 
         // Extract Nexus and Dataset information from the generated blueprint.
         let nexus_ids = get_in_service_nexuses_from_blueprint(&bp1);
-        let debug_datasets = get_debug_datasets_from_blueprint(
-            &bp1,
-            BlueprintDatasetFilter::InService,
-        );
+        let debug_datasets = get_in_service_debug_datasets_from_blueprint(&bp1);
         assert!(!debug_datasets.is_empty());
 
         // When we create a bundle, it should exist on a dataset provisioned by
@@ -1362,10 +1347,7 @@ mod test {
 
         // Extract Nexus and Dataset information from the generated blueprint.
         let nexus_ids = get_in_service_nexuses_from_blueprint(&bp1);
-        let debug_datasets = get_debug_datasets_from_blueprint(
-            &bp1,
-            BlueprintDatasetFilter::InService,
-        );
+        let debug_datasets = get_in_service_debug_datasets_from_blueprint(&bp1);
         assert!(!debug_datasets.is_empty());
 
         // When we create a bundle, it should exist on a dataset provisioned by

diff --git a/nexus/reconfigurator/blippy/src/checks.rs b/nexus/reconfigurator/blippy/src/checks.rs
@@ -7,7 +7,7 @@ use crate::blippy::Severity;
 use crate::blippy::SledKind;
 use nexus_sled_agent_shared::inventory::ZoneKind;
 use nexus_types::deployment::BlueprintDatasetConfig;
-use nexus_types::deployment::BlueprintDatasetFilter;
+use nexus_types::deployment::BlueprintDatasetDisposition;
 use nexus_types::deployment::BlueprintPhysicalDiskDisposition;
 use nexus_types::deployment::BlueprintSledConfig;
 use nexus_types::deployment::BlueprintZoneConfig;
@@ -491,7 +491,7 @@ fn check_datasets(blippy: &mut Blippy<'_>) {
     // should have been referenced by either a zone or a disk above.
     for (sled_id, dataset) in blippy
         .blueprint()
-        .all_omicron_datasets(BlueprintDatasetFilter::InService)
+        .all_omicron_datasets(BlueprintDatasetDisposition::is_in_service)
     {
         if !expected_datasets.contains(&dataset.id) {
             blippy.push_sled_note(
@@ -520,7 +520,7 @@ fn check_datasets(blippy: &mut Blippy<'_>) {
     // not have addresses.
     for (sled_id, dataset) in blippy
         .blueprint()
-        .all_omicron_datasets(BlueprintDatasetFilter::InService)
+        .all_omicron_datasets(BlueprintDatasetDisposition::is_in_service)
     {
         match dataset.kind {
             DatasetKind::Crucible => {

diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs
@@ -26,7 +26,6 @@ use nexus_sled_agent_shared::inventory::OmicronZoneDataset;
 use nexus_sled_agent_shared::inventory::ZoneKind;
 use nexus_types::deployment::Blueprint;
 use nexus_types::deployment::BlueprintDatasetDisposition;
-use nexus_types::deployment::BlueprintDatasetFilter;
 use nexus_types::deployment::BlueprintDatasetsConfig;
 use nexus_types::deployment::BlueprintPhysicalDiskConfig;
 use nexus_types::deployment::BlueprintPhysicalDiskDisposition;
@@ -889,16 +888,13 @@ impl<'a> BlueprintBuilder<'a> {
                 }
             }
         }
-        for dataset in editor.datasets(BlueprintDatasetFilter::All) {
-            match dataset.disposition {
-                BlueprintDatasetDisposition::Expunged => (),
-                BlueprintDatasetDisposition::InService => {
-                    return Err(Error::Planner(anyhow!(
-                        "expunged all disks but a dataset \
-                         is still in service: {dataset:?}"
-                    )));
-                }
-            }
+        if let Some(dataset) =
+            editor.datasets(BlueprintDatasetDisposition::is_in_service).next()
+        {
+            return Err(Error::Planner(anyhow!(
+                "expunged all disks but a dataset \
+                 is still in service: {dataset:?}"
+            )));
         }
         for zone_id in zones_ready_for_cleanup {
             editor

diff --git a/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs
@@ -10,7 +10,7 @@ use illumos_utils::zpool::ZpoolName;
 use itertools::Either;
 use nexus_sled_agent_shared::inventory::ZoneKind;
 use nexus_types::deployment::BlueprintDatasetConfig;
-use nexus_types::deployment::BlueprintDatasetFilter;
+use nexus_types::deployment::BlueprintDatasetDisposition;
 use nexus_types::deployment::BlueprintDatasetsConfig;
 use nexus_types::deployment::BlueprintPhysicalDiskConfig;
 use nexus_types::deployment::BlueprintPhysicalDiskDisposition;
@@ -256,10 +256,13 @@ impl SledEditor {
         }
     }
 
-    pub fn datasets(
+    pub fn datasets<F>(
         &self,
-        filter: BlueprintDatasetFilter,
-    ) -> impl Iterator<Item = &BlueprintDatasetConfig> {
+        mut filter: F,
+    ) -> impl Iterator<Item = &BlueprintDatasetConfig>
+    where
+        F: FnMut(BlueprintDatasetDisposition) -> bool,
+    {
         match &self.0 {
             InnerSledEditor::Active(editor) => {
                 Either::Left(editor.datasets(filter))
@@ -269,7 +272,7 @@ impl SledEditor {
                     .datasets
                     .datasets
                     .iter()
-                    .filter(move |disk| disk.disposition.matches(filter)),
+                    .filter(move |disk| filter(disk.disposition)),
             ),
         }
     }
@@ -503,10 +506,13 @@ impl ActiveSledEditor {
         self.disks.disks(filter)
     }
 
-    pub fn datasets(
+    pub fn datasets<F>(
         &self,
-        filter: BlueprintDatasetFilter,
-    ) -> impl Iterator<Item = &BlueprintDatasetConfig> {
+        filter: F,
+    ) -> impl Iterator<Item = &BlueprintDatasetConfig>
+    where
+        F: FnMut(BlueprintDatasetDisposition) -> bool,
+    {
         self.datasets.datasets(filter)
     }
 

diff --git a/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/datasets.rs b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/datasets.rs
@@ -7,7 +7,6 @@ use crate::planner::SledPlannerRng;
 use illumos_utils::zpool::ZpoolName;
 use nexus_types::deployment::BlueprintDatasetConfig;
 use nexus_types::deployment::BlueprintDatasetDisposition;
-use nexus_types::deployment::BlueprintDatasetFilter;
 use nexus_types::deployment::BlueprintDatasetsConfig;
 use nexus_types::deployment::id_map::{self, IdMap};
 use omicron_common::api::external::ByteCount;
@@ -184,14 +183,17 @@ impl DatasetsEditor {
         self.counts
     }
 
-    pub fn datasets(
+    pub fn datasets<F>(
         &self,
-        filter: BlueprintDatasetFilter,
-    ) -> impl Iterator<Item = &BlueprintDatasetConfig> {
+        mut filter: F,
+    ) -> impl Iterator<Item = &BlueprintDatasetConfig>
+    where
+        F: FnMut(BlueprintDatasetDisposition) -> bool,
+    {
         self.config
             .datasets
             .iter()
-            .filter(move |dataset| dataset.disposition.matches(filter))
+            .filter(move |dataset| filter(dataset.disposition))
     }
 
     // Private method; panics if given an ID that isn't present in
@@ -508,9 +510,11 @@ mod tests {
         // 1. Expunge that dataset
         // 2. Add a new dataset of the same kind
         // 3. Ensure the new dataset ID is freshly-generated
-        for dataset in config.datasets.iter().filter(|dataset| {
-            dataset.disposition.matches(BlueprintDatasetFilter::InService)
-        }) {
+        for dataset in config
+            .datasets
+            .iter()
+            .filter(|dataset| dataset.disposition.is_in_service())
+        {
             editor
                 .expunge(&dataset.pool.id(), &dataset.kind)
                 .expect("expunged dataset");
@@ -563,7 +567,7 @@ mod tests {
             // There should no longer be any in-service datasets on this zpool.
             assert!(
                 !editor
-                    .datasets(BlueprintDatasetFilter::InService)
+                    .datasets(BlueprintDatasetDisposition::is_in_service)
                     .any(|dataset| dataset.pool.id() == *zpool_id),
                 "in-service dataset remains after expunging zpool"
             );
@@ -573,9 +577,11 @@ mod tests {
         //
         // 1. Add a new dataset of the same kind
         // 2. Ensure the new dataset ID is freshly-generated
-        for dataset in config.datasets.iter().filter(|dataset| {
-            dataset.disposition.matches(BlueprintDatasetFilter::InService)
-        }) {
+        for dataset in config
+            .datasets
+            .iter()
+            .filter(|dataset| dataset.disposition.is_in_service())
+        {
             let new_dataset = PartialDatasetConfig {
                 name: DatasetName::new(
                     dataset.pool.clone(),

diff --git a/nexus/reconfigurator/rendezvous/src/lib.rs b/nexus/reconfigurator/rendezvous/src/lib.rs
@@ -11,7 +11,7 @@
 use nexus_db_queries::context::OpContext;
 use nexus_db_queries::db::DataStore;
 use nexus_types::deployment::Blueprint;
-use nexus_types::deployment::BlueprintDatasetFilter;
+use nexus_types::deployment::BlueprintDatasetDisposition;
 use nexus_types::internal_api::background::BlueprintRendezvousStats;
 use nexus_types::inventory::Collection;
 
@@ -35,7 +35,7 @@ pub async fn reconcile_blueprint_rendezvous_tables(
         datastore,
         blueprint.id,
         blueprint
-            .all_omicron_datasets(BlueprintDatasetFilter::All)
+            .all_omicron_datasets(BlueprintDatasetDisposition::any)
             .map(|(_sled_id, dataset)| dataset),
         &inventory_dataset_ids,
     )
@@ -45,7 +45,7 @@ pub async fn reconcile_blueprint_rendezvous_tables(
         opctx,
         datastore,
         blueprint
-            .all_omicron_datasets(BlueprintDatasetFilter::All)
+            .all_omicron_datasets(BlueprintDatasetDisposition::any)
             .map(|(_sled_id, dataset)| dataset),
         &inventory_dataset_ids,
     )