diff --git a/nexus/db-queries/src/db/datastore/support_bundle.rs b/nexus/db-queries/src/db/datastore/support_bundle.rs index 6520c82307..1eddd3bbcd 100644 --- 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,9 +940,8 @@ mod test { .collect() } - fn get_debug_datasets_from_blueprint( + fn get_in_service_debug_datasets_from_blueprint( bp: &Blueprint, - filter: BlueprintDatasetFilter, ) -> Vec { bp.sleds .values() @@ -955,7 +949,7 @@ mod test { 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 index f5fd5ac5a4..29b9cb8bf0 100644 --- 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 index 43727dcd33..1ddc48f9d6 100644 --- 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 index fd96bd2a9f..123b660242 100644 --- 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( &self, - filter: BlueprintDatasetFilter, - ) -> impl Iterator { + mut filter: F, + ) -> impl Iterator + 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( &self, - filter: BlueprintDatasetFilter, - ) -> impl Iterator { + filter: F, + ) -> impl Iterator + 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 index 30e694578b..6a6cf59bd6 100644 --- 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( &self, - filter: BlueprintDatasetFilter, - ) -> impl Iterator { + mut filter: F, + ) -> impl Iterator + 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 index fab5d8abbb..948402ec5f 100644 --- 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, ) diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index 2d3991235d..1a41858dc3 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -264,10 +264,13 @@ impl Blueprint { } /// Iterate over the [`BlueprintDatasetsConfig`] instances in the blueprint. - pub fn all_omicron_datasets( + pub fn all_omicron_datasets( &self, - filter: BlueprintDatasetFilter, - ) -> impl Iterator { + mut filter: F, + ) -> impl Iterator + where + F: FnMut(BlueprintDatasetDisposition) -> bool, + { self.sleds .iter() .flat_map(move |(sled_id, config)| { @@ -277,7 +280,7 @@ impl Blueprint { .iter() .map(|dataset| (*sled_id, dataset)) }) - .filter(move |(_, d)| d.disposition.matches(filter)) + .filter(move |(_, d)| filter(d.disposition)) } /// Iterate over the ids of all sleds in the blueprint @@ -902,22 +905,6 @@ impl fmt::Display for BlueprintZoneImageSource { } } -/// Filters that apply to blueprint datasets. -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum BlueprintDatasetFilter { - // --- - // Prefer to keep this list in alphabetical order. - // --- - /// All datasets - All, - - /// Datasets that have been expunged. - Expunged, - - /// Datasets that are in-service. - InService, -} - /// The desired state of an Omicron-managed physical disk in a blueprint. #[derive( Debug, @@ -1135,8 +1122,7 @@ impl BlueprintDatasetsConfig { .datasets .into_iter() .filter_map(|d| { - if d.disposition.matches(BlueprintDatasetFilter::InService) - { + if d.disposition.is_in_service() { Some((d.id, d.into())) } else { None @@ -1189,19 +1175,23 @@ pub enum BlueprintDatasetDisposition { } impl BlueprintDatasetDisposition { - pub fn matches(self, filter: BlueprintDatasetFilter) -> bool { - match self { - Self::InService => match filter { - BlueprintDatasetFilter::All => true, - BlueprintDatasetFilter::Expunged => false, - BlueprintDatasetFilter::InService => true, - }, - Self::Expunged => match filter { - BlueprintDatasetFilter::All => true, - BlueprintDatasetFilter::Expunged => true, - BlueprintDatasetFilter::InService => false, - }, - } + /// Always returns true. + /// + /// This is intended for use with methods that take a filtering + /// closure operating on a `BlueprintDatasetDisposition` (e.g., + /// `Blueprint::all_omicron_datasets()`), allowing callers to make it clear + /// they accept any disposition via + /// + /// ```rust,ignore + /// blueprint.all_omicron_datasets(BlueprintDatasetDisposition::any) + /// ``` + pub fn any(self) -> bool { + true + } + + /// Returns true if `self` is `BlueprintDatasetDisposition::InService` + pub fn is_in_service(self) -> bool { + matches!(self, Self::InService) } /// Returns true if `self` is `BlueprintDatasetDisposition::Expunged`,