From e73a30ea190d0770cf395a12850d3d6c96b176bf Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 5 Dec 2024 18:12:49 -0500 Subject: [PATCH] [reconfigurator] Introduce a combined `SledEditor` inside `BlueprintBuilder` (#7204) This is a step on the road to #7078. The current `BlueprintBuilder` internals would not at all be amenable to squishing the disparate maps in `Blueprint` together, so this PR tries to rework those internals. `BlueprintBuilder` now does its own map squishing (inside `new_based_on()`), combining state+zones+disks+datasets into one helper (`SledEditor`). All modifications are preformed via those editors, then `BlueprintBuilder::build()` breaks the combined editors' results back out into four maps for `Blueprint`. There should be only one functional change in this PR (marked with a `TODO-john`): previously, expunging a zone checked that that zone had not been modified in the current planning cycle. I'm not sure that's really necessary; it felt like some defensiveness born out of the complexity of the builder itself, maybe? I think we could put it back (inside `ZonesEditor`, presumably) if folks think it would be good to keep. My intention was to change the tests as little as possible to ensure I didn't break anything as I was moving functionality around, so `new_based_on()` and `build()` have some arguably-pretty-gross concessions to behave exactly the way the builder did before these changes. I'd like to _remove_ those concessions, but those will be nontrivial behavioral changes that I don't want to try to roll in with all of this cleanup. I think I landed this pretty well; there are only a few expectorate changes (due to the slightly reworked `ZonesEditor` producing a different ordering for a few tests), and none in `omdb` (which is where I've often seen incidental planner changes bubble out). Apologies for the size of the PR. I'd lightly recommend that the new `blueprint_editor/` module and its subcomponents should be pretty quickly reviewable and should be looked at first; they're _supposed_ to be simple and obvious, and are largely ported over from the prior storage / disks / datasets / zones editors. I did rework how we're handling #6645 backwards compat to try to reduce how much we need to pass `SledResources` around, so that complicates things in `DatasetsEditor` some, but not too bad IMO. (I also added a test for this, since I changed it and I don't think we had one before.) The `BlueprintBuilter` changes should then be more or less the natural way one would use a collection of `SledEditor`s without changing its API or behavior (yet!). --- Cargo.lock | 1 + common/src/disk.rs | 4 + .../planning/src/blueprint_builder/builder.rs | 975 +++++++++--------- .../builder/datasets_editor.rs | 348 ------- .../blueprint_builder/builder/disks_editor.rs | 195 ---- .../builder/storage_editor.rs | 206 ---- .../planning/src/blueprint_builder/mod.rs | 1 - .../planning/src/blueprint_builder/zones.rs | 527 ---------- .../planning/src/blueprint_editor.rs | 14 + .../src/blueprint_editor/sled_editor.rs | 329 ++++++ .../blueprint_editor/sled_editor/datasets.rs | 398 +++++++ .../src/blueprint_editor/sled_editor/disks.rs | 145 +++ .../src/blueprint_editor/sled_editor/zones.rs | 181 ++++ nexus/reconfigurator/planning/src/example.rs | 4 +- nexus/reconfigurator/planning/src/lib.rs | 1 + nexus/reconfigurator/planning/src/planner.rs | 10 +- nexus/types/Cargo.toml | 1 + nexus/types/src/deployment.rs | 31 + 18 files changed, 1620 insertions(+), 1751 deletions(-) delete mode 100644 nexus/reconfigurator/planning/src/blueprint_builder/builder/datasets_editor.rs delete mode 100644 nexus/reconfigurator/planning/src/blueprint_builder/builder/disks_editor.rs delete mode 100644 nexus/reconfigurator/planning/src/blueprint_builder/builder/storage_editor.rs delete mode 100644 nexus/reconfigurator/planning/src/blueprint_builder/zones.rs create mode 100644 nexus/reconfigurator/planning/src/blueprint_editor.rs create mode 100644 nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs create mode 100644 nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/datasets.rs create mode 100644 nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/disks.rs create mode 100644 nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/zones.rs diff --git a/Cargo.lock b/Cargo.lock index f3c69e4d14..f5e44318bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6271,6 +6271,7 @@ dependencies = [ "gateway-client", "http", "humantime", + "illumos-utils", "internal-dns-types", "ipnetwork", "newtype-uuid", diff --git a/common/src/disk.rs b/common/src/disk.rs index 3500d4dabb..99c2b2db7b 100644 --- a/common/src/disk.rs +++ b/common/src/disk.rs @@ -103,6 +103,10 @@ impl DatasetName { Self { pool_name, kind } } + pub fn into_parts(self) -> (ZpoolName, DatasetKind) { + (self.pool_name, self.kind) + } + pub fn pool(&self) -> &ZpoolName { &self.pool_name } diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs index eb50ab19fd..394133132b 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs @@ -4,21 +4,28 @@ //! Low-level facility for generating Blueprints +use crate::blueprint_editor::DatasetIdsBackfillFromDb; +use crate::blueprint_editor::EditedSled; +use crate::blueprint_editor::SledEditError; +use crate::blueprint_editor::SledEditor; use crate::ip_allocator::IpAllocator; use crate::planner::rng::PlannerRng; use crate::planner::zone_needs_expungement; use crate::planner::ZoneExpungeReason; use anyhow::anyhow; +use anyhow::bail; +use anyhow::Context as _; use clickhouse_admin_types::OXIMETER_CLUSTER; -use datasets_editor::BlueprintDatasetsEditError; use ipnet::IpAdd; use nexus_inventory::now_db_precision; use nexus_sled_agent_shared::inventory::OmicronZoneDataset; use nexus_sled_agent_shared::inventory::ZoneKind; use nexus_types::deployment::blueprint_zone_type; use nexus_types::deployment::Blueprint; +use nexus_types::deployment::BlueprintDatasetsConfig; use nexus_types::deployment::BlueprintPhysicalDiskConfig; use nexus_types::deployment::BlueprintPhysicalDiskDisposition; +use nexus_types::deployment::BlueprintPhysicalDisksConfig; use nexus_types::deployment::BlueprintZoneConfig; use nexus_types::deployment::BlueprintZoneDisposition; use nexus_types::deployment::BlueprintZoneFilter; @@ -33,6 +40,7 @@ use nexus_types::deployment::OmicronZoneExternalSnatIp; use nexus_types::deployment::PlanningInput; use nexus_types::deployment::SledDetails; use nexus_types::deployment::SledFilter; +use nexus_types::deployment::SledLookupErrorKind; use nexus_types::deployment::SledResources; use nexus_types::deployment::ZpoolFilter; use nexus_types::deployment::ZpoolName; @@ -62,16 +70,16 @@ use slog::error; use slog::info; use slog::o; use slog::Logger; +use std::collections::btree_map::Entry; use std::collections::BTreeMap; use std::collections::BTreeSet; use std::collections::HashSet; use std::fmt; -use std::mem; +use std::iter; use std::net::IpAddr; use std::net::Ipv6Addr; use std::net::SocketAddr; use std::net::SocketAddrV6; -use storage_editor::BlueprintStorageEditor; use thiserror::Error; use super::clickhouse::ClickhouseAllocator; @@ -80,13 +88,6 @@ use super::external_networking::BuilderExternalNetworking; use super::external_networking::ExternalNetworkingChoice; use super::external_networking::ExternalSnatNetworkingChoice; use super::internal_dns::DnsSubnetAllocator; -use super::zones::is_already_expunged; -use super::zones::BuilderZoneState; -use super::zones::BuilderZonesConfig; - -mod datasets_editor; -mod disks_editor; -mod storage_editor; /// Errors encountered while assembling blueprints #[derive(Debug, Error)] @@ -125,8 +126,12 @@ pub enum Error { TooManyDnsServers, #[error("planner produced too many {kind:?} zones")] TooManyZones { kind: ZoneKind }, - #[error(transparent)] - BlueprintDatasetsEditError(#[from] BlueprintDatasetsEditError), + #[error("error editing sled {sled_id}")] + SledEditError { + sled_id: SledUuid, + #[source] + err: SledEditError, + }, } /// Describes the result of an idempotent "ensure" operation @@ -197,12 +202,12 @@ impl EditCounts { *self != Self::zeroes() } - pub fn accum(self, other: Self) -> Self { + pub fn difference_since(self, other: Self) -> Self { Self { - added: self.added + other.added, - updated: self.updated + other.updated, - expunged: self.expunged + other.expunged, - removed: self.removed + other.removed, + added: self.added - other.added, + updated: self.updated - other.updated, + expunged: self.expunged - other.expunged, + removed: self.removed - other.removed, } } } @@ -223,11 +228,18 @@ pub struct SledEditCounts { } impl SledEditCounts { - fn accum(self, other: Self) -> Self { + fn has_nonzero_counts(&self) -> bool { + let Self { disks, datasets, zones } = self; + disks.has_nonzero_counts() + || datasets.has_nonzero_counts() + || zones.has_nonzero_counts() + } + + fn difference_since(self, other: Self) -> Self { Self { - disks: self.disks.accum(other.disks), - datasets: self.datasets.accum(other.datasets), - zones: self.zones.accum(other.zones), + disks: self.disks.difference_since(other.disks), + datasets: self.datasets.difference_since(other.datasets), + zones: self.zones.difference_since(other.zones), } } } @@ -349,9 +361,7 @@ pub struct BlueprintBuilder<'a> { // These fields will become part of the final blueprint. See the // corresponding fields in `Blueprint`. - pub(super) zones: BlueprintZonesBuilder<'a>, - storage: BlueprintStorageEditor, - sled_state: BTreeMap, + sled_editors: BTreeMap, cockroachdb_setting_preserve_downgrade: CockroachDbPreserveDowngrade, creator: String, @@ -400,6 +410,28 @@ impl<'a> BlueprintBuilder<'a> { (sled_id, config) }) .collect::>(); + let blueprint_disks = blueprint_zones + .keys() + .copied() + .map(|sled_id| { + let config = BlueprintPhysicalDisksConfig { + generation: Generation::new(), + disks: Vec::new(), + }; + (sled_id, config) + }) + .collect(); + let blueprint_datasets = blueprint_zones + .keys() + .copied() + .map(|sled_id| { + let config = BlueprintDatasetsConfig { + generation: Generation::new(), + datasets: BTreeMap::new(), + }; + (sled_id, config) + }) + .collect(); let num_sleds = blueprint_zones.len(); let sled_state = blueprint_zones .keys() @@ -410,8 +442,8 @@ impl<'a> BlueprintBuilder<'a> { Blueprint { id: rng.next_blueprint(), blueprint_zones, - blueprint_disks: BTreeMap::new(), - blueprint_datasets: BTreeMap::new(), + blueprint_disks, + blueprint_datasets, sled_state, parent_blueprint_id: None, internal_dns_version: Generation::new(), @@ -440,30 +472,105 @@ impl<'a> BlueprintBuilder<'a> { "parent_id" => parent_blueprint.id.to_string(), )); - // Prefer the sled state from our parent blueprint for sleds - // that were in it; there may be new sleds in `input`, in which - // case we'll use their current state as our starting point. - let mut sled_state = parent_blueprint.sled_state.clone(); - let mut commissioned_sled_ids = BTreeSet::new(); - for (sled_id, details) in input.all_sleds(SledFilter::Commissioned) { - commissioned_sled_ids.insert(sled_id); - sled_state.entry(sled_id).or_insert(details.state); + // Helper to build a `PreexistingDatasetIds` for a given sled. This will + // go away with https://github.com/oxidecomputer/omicron/issues/6645. + let build_preexisting_dataset_ids = + |sled_id| -> anyhow::Result { + match input.sled_lookup(SledFilter::All, sled_id) { + Ok(details) => { + DatasetIdsBackfillFromDb::build(&details.resources) + .with_context(|| { + format!( + "failed building map of preexisting \ + dataset IDs for sled {sled_id}" + ) + }) + } + Err(err) => match err.kind() { + SledLookupErrorKind::Missing => { + Ok(DatasetIdsBackfillFromDb::empty()) + } + SledLookupErrorKind::Filtered { .. } => unreachable!( + "SledFilter::All should not filter anything out" + ), + }, + } + }; + + // Squish the disparate maps in our parent blueprint into one map of + // `SledEditor`s. + let mut sled_editors = BTreeMap::new(); + for (sled_id, zones) in &parent_blueprint.blueprint_zones { + // Prefer the sled state from our parent blueprint for sleds + // that were in it. + let state = match parent_blueprint.sled_state.get(sled_id).copied() + { + Some(state) => state, + None => { + // If we have zones but no state for a sled, we assume + // it was removed by an earlier version of the planner + // (which pruned decommissioned sleds from + // `sled_state`). Check that all of its zones are + // expunged, which is a prerequisite for + // decommissioning. If any zones aren't, then we don't + // know what to do: the state is missing but we can't + // assume "decommissioned", so fail. + if zones.are_all_zones_expunged() { + SledState::Decommissioned + } else { + bail!( + "sled {sled_id} is missing in parent blueprint \ + sled_state map, but has non-expunged zones" + ); + } + } + }; + + // If we don't have disks/datasets entries, we'll start with an + // empty config and rely on `sled_ensure_{disks,datasets}` calls to + // populate it. It's also possible our parent blueprint removed + // entries because our sled has been expunged, in which case we + // won't do any further editing and what we fill in here is + // irrelevant. + let disks = parent_blueprint + .blueprint_disks + .get(sled_id) + .cloned() + .unwrap_or_else(|| BlueprintPhysicalDisksConfig { + generation: Generation::new(), + disks: Vec::new(), + }); + let datasets = parent_blueprint + .blueprint_datasets + .get(sled_id) + .cloned() + .unwrap_or_else(|| BlueprintDatasetsConfig { + generation: Generation::new(), + datasets: BTreeMap::new(), + }); + let editor = SledEditor::new( + state, + zones.clone(), + disks, + datasets.clone(), + build_preexisting_dataset_ids(*sled_id)?, + ) + .with_context(|| { + format!("failed to construct SledEditor for sled {sled_id}") + })?; + sled_editors.insert(*sled_id, editor); } - // Make a garbage collection pass through `sled_state`. We want to keep - // any sleds which either: - // - // 1. do not have a desired state of `Decommissioned` - // 2. do have a desired state of `Decommissioned` and are still included - // in our input's list of commissioned sleds - // - // Sleds that don't fall into either of these cases have reached the - // actual `Decommissioned` state, which means we no longer need to carry - // forward that desired state. - sled_state.retain(|sled_id, state| { - *state != SledState::Decommissioned - || commissioned_sled_ids.contains(sled_id) - }); + // Add new, empty `SledEditor`s for any commissioned sleds in our input + // that weren't in the parent blueprint. (These are newly-added sleds.) + for sled_id in input.all_sled_ids(SledFilter::Commissioned) { + if let Entry::Vacant(slot) = sled_editors.entry(sled_id) { + slot.insert(SledEditor::new_empty( + SledState::Active, + build_preexisting_dataset_ids(sled_id)?, + )); + } + } Ok(BlueprintBuilder { log, @@ -473,12 +580,7 @@ impl<'a> BlueprintBuilder<'a> { sled_ip_allocators: BTreeMap::new(), external_networking: OnceCell::new(), internal_dns_subnets: OnceCell::new(), - zones: BlueprintZonesBuilder::new(parent_blueprint), - storage: BlueprintStorageEditor::new( - parent_blueprint.blueprint_disks.clone(), - parent_blueprint.blueprint_datasets.clone(), - ), - sled_state, + sled_editors, cockroachdb_setting_preserve_downgrade: parent_blueprint .cockroachdb_setting_preserve_downgrade, creator: creator.to_owned(), @@ -514,12 +616,12 @@ impl<'a> BlueprintBuilder<'a> { )?; BuilderExternalNetworking::new( - self.zones - .current_zones(BlueprintZoneFilter::ShouldBeRunning) - .flat_map(|(_sled_id, zone_config)| zone_config), - self.zones - .current_zones(BlueprintZoneFilter::Expunged) - .flat_map(|(_sled_id, zone_config)| zone_config), + self.sled_editors.values().flat_map(|editor| { + editor.zones(BlueprintZoneFilter::ShouldBeRunning) + }), + self.sled_editors.values().flat_map(|editor| { + editor.zones(BlueprintZoneFilter::Expunged) + }), self.input.service_ip_pool_ranges(), ) }) @@ -534,9 +636,9 @@ impl<'a> BlueprintBuilder<'a> { ) -> Result<&mut DnsSubnetAllocator, Error> { self.internal_dns_subnets.get_or_try_init(|| { DnsSubnetAllocator::new( - self.zones - .current_zones(BlueprintZoneFilter::ShouldBeRunning) - .flat_map(|(_sled_id, zone_config)| zone_config), + self.sled_editors.values().flat_map(|editor| { + editor.zones(BlueprintZoneFilter::ShouldBeRunning) + }), self.input, ) })?; @@ -546,8 +648,8 @@ impl<'a> BlueprintBuilder<'a> { /// Iterates over the list of sled IDs for which we have zones. /// /// This may include decommissioned sleds. - pub fn sled_ids_with_zones(&self) -> impl Iterator { - self.zones.sled_ids_with_zones() + pub fn sled_ids_with_zones(&self) -> impl Iterator + '_ { + self.sled_editors.keys().copied() } pub fn current_sled_zones( @@ -555,20 +657,82 @@ impl<'a> BlueprintBuilder<'a> { sled_id: SledUuid, filter: BlueprintZoneFilter, ) -> impl Iterator { - self.zones.current_sled_zones(sled_id, filter).map(|(config, _)| config) + let Some(editor) = self.sled_editors.get(&sled_id) else { + return Box::new(iter::empty()) + as Box>; + }; + Box::new(editor.zones(filter)) } /// Assemble a final [`Blueprint`] based on the contents of the builder pub fn build(mut self) -> Blueprint { + let blueprint_id = self.rng.next_blueprint(); + // Collect the Omicron zones config for all sleds, including sleds that // are no longer in service and need expungement work. - let blueprint_zones = self - .zones - .into_zones_map(self.input.all_sled_ids(SledFilter::Commissioned)); - let (blueprint_disks, blueprint_datasets) = - self.storage.into_blueprint_maps( - self.input.all_sled_ids(SledFilter::InService), - ); + let mut sled_state = BTreeMap::new(); + let mut blueprint_zones = BTreeMap::new(); + let mut blueprint_disks = BTreeMap::new(); + let mut blueprint_datasets = BTreeMap::new(); + for (sled_id, editor) in self.sled_editors { + let EditedSled { zones, disks, datasets, state, edit_counts } = + editor.finalize(); + sled_state.insert(sled_id, state); + blueprint_disks.insert(sled_id, disks); + blueprint_datasets.insert(sled_id, datasets); + blueprint_zones.insert(sled_id, zones); + if edit_counts.has_nonzero_counts() { + debug!( + self.log, "sled modified in new blueprint"; + "sled_id" => %sled_id, + "blueprint_id" => %blueprint_id, + "disk_edits" => ?edit_counts.disks, + "dataset_edits" => ?edit_counts.datasets, + "zone_edits" => ?edit_counts.zones, + ); + } else { + debug!( + self.log, "sled unchanged in new blueprint"; + "sled_id" => %sled_id, + "blueprint_id" => %blueprint_id, + ); + } + } + // Preserving backwards compatibility, for now: prune sled_state of any + // fully decommissioned sleds, which we determine by the state being + // `Decommissioned` _and_ the sled is no longer in our PlanningInput's + // list of commissioned sleds. + let commissioned_sled_ids = self + .input + .all_sled_ids(SledFilter::Commissioned) + .collect::>(); + sled_state.retain(|sled_id, state| { + *state != SledState::Decommissioned + || commissioned_sled_ids.contains(sled_id) + }); + // Preserving backwards compatibility, for now: disks should only + // have entries for in-service sleds, and expunged disks should be + // removed entirely. + let in_service_sled_ids = self + .input + .all_sled_ids(SledFilter::InService) + .collect::>(); + blueprint_disks.retain(|sled_id, disks_config| { + if !in_service_sled_ids.contains(sled_id) { + return false; + } + + disks_config.disks.retain(|config| match config.disposition { + BlueprintPhysicalDiskDisposition::InService => true, + BlueprintPhysicalDiskDisposition::Expunged => false, + }); + + true + }); + // Preserving backwards compatibility, for now: datasets should only + // have entries for in-service sleds. + blueprint_datasets + .retain(|sled_id, _| in_service_sled_ids.contains(sled_id)); // If we have the clickhouse cluster setup enabled via policy and we // don't yet have a `ClickhouseClusterConfiguration`, then we must create @@ -623,11 +787,11 @@ impl<'a> BlueprintBuilder<'a> { } }); Blueprint { - id: self.rng.next_blueprint(), + id: blueprint_id, blueprint_zones, blueprint_disks, blueprint_datasets, - sled_state: self.sled_state, + sled_state, parent_blueprint_id: Some(self.parent_blueprint.id), internal_dns_version: self.input.internal_dns_version(), external_dns_version: self.input.external_dns_version(), @@ -655,8 +819,14 @@ impl<'a> BlueprintBuilder<'a> { &mut self, sled_id: SledUuid, desired_state: SledState, - ) { - self.sled_state.insert(sled_id, desired_state); + ) -> Result<(), Error> { + let editor = self.sled_editors.get_mut(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to set sled state for unknown sled {sled_id}" + )) + })?; + editor.set_state(desired_state); + Ok(()) } /// Within tests, set an RNG for deterministic results. @@ -698,12 +868,16 @@ impl<'a> BlueprintBuilder<'a> { "sled_id" => sled_id.to_string(), )); + let editor = self.sled_editors.get_mut(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to expunge zones for unknown sled {sled_id}" + )) + })?; + // Do any zones need to be marked expunged? let mut zones_to_expunge = BTreeMap::new(); - let sled_zones = - self.zones.current_sled_zones(sled_id, BlueprintZoneFilter::All); - for (zone_config, state) in sled_zones { + for zone_config in editor.zones(BlueprintZoneFilter::All) { let zone_id = zone_config.id; let log = log.new(o!( "zone_id" => zone_id.to_string() @@ -715,12 +889,13 @@ impl<'a> BlueprintBuilder<'a> { continue; }; - let is_expunged = - is_already_expunged(zone_config, state).map_err(|error| { - Error::Planner(anyhow!(error).context(format!( - "for sled {sled_id}, error computing zones to expunge" - ))) - })?; + // TODO-john we lost the check for "are we expunging a zone we + // modified in this planner iteration" - do we need that? + let is_expunged = match zone_config.disposition { + BlueprintZoneDisposition::InService + | BlueprintZoneDisposition::Quiesced => false, + BlueprintZoneDisposition::Expunged => true, + }; if !is_expunged { match reason { @@ -778,34 +953,13 @@ impl<'a> BlueprintBuilder<'a> { return Ok(zones_to_expunge); } - let sled_resources = self.sled_resources(sled_id)?; - let mut sled_storage = self.storage.sled_storage_editor( - sled_id, - sled_resources, - &mut self.rng, - )?; - // Now expunge all the zones that need it. - let removed_zones = { - let change = self.zones.change_sled_zones(sled_id); - change - .expunge_zones(zones_to_expunge.keys().cloned().collect()) - .map_err(|error| { - Error::Planner(anyhow!(error).context(format!( - "for sled {sled_id}, error expunging zones" - ))) - })? - }; - - // Also expunge the datasets of all removed zones. - for zone in removed_zones { - sled_storage.expunge_zone_datasets(zone); + for zone_id in zones_to_expunge.keys() { + editor + .expunge_zone(&zone_id) + .map_err(|err| Error::SledEditError { sled_id, err })?; } - // We're done with `sled_storage`; drop it so the borrow checker is okay - // with calling other methods on `self` below. - mem::drop(sled_storage); - // Finally, add comments describing what happened. // // Group the zones by their reason for expungement. @@ -869,12 +1023,17 @@ impl<'a> BlueprintBuilder<'a> { resources: &SledResources, ) -> Result { // These are the disks known to our (last?) blueprint - let mut sled_storage = self.storage.sled_storage_editor( - sled_id, - resources, - &mut self.rng, - )?; - let blueprint_disk_ids = sled_storage.disk_ids().collect::>(); + let editor = self.sled_editors.get_mut(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to ensure disks for unknown sled {sled_id}" + )) + })?; + let initial_counts = editor.edit_counts(); + + let blueprint_disk_ids = editor + .disks(DiskFilter::InService) + .map(|config| config.id) + .collect::>(); // These are the in-service disks as we observed them in the database, // during the planning phase @@ -887,42 +1046,28 @@ impl<'a> BlueprintBuilder<'a> { // blueprint for (disk_id, (zpool, disk)) in database_disks { database_disk_ids.insert(disk_id); - sled_storage.ensure_disk(BlueprintPhysicalDiskConfig { - disposition: BlueprintPhysicalDiskDisposition::InService, - identity: disk.disk_identity.clone(), - id: disk_id, - pool_id: *zpool, - }); + editor.ensure_disk( + BlueprintPhysicalDiskConfig { + disposition: BlueprintPhysicalDiskDisposition::InService, + identity: disk.disk_identity.clone(), + id: disk_id, + pool_id: *zpool, + }, + &mut self.rng, + ); } // Remove any disks that appear in the blueprint, but not the database - let mut zones_to_expunge = BTreeSet::new(); for disk_id in blueprint_disk_ids { if !database_disk_ids.contains(&disk_id) { - if let Some(expunged_zpool) = sled_storage.remove_disk(&disk_id) - { - zones_to_expunge.extend( - self.zones - .zones_using_zpool( - sled_id, - BlueprintZoneFilter::ShouldBeRunning, - &expunged_zpool, - ) - .map(|zone| zone.id), - ); - } + editor + .expunge_disk(&disk_id) + .map_err(|err| Error::SledEditError { sled_id, err })?; } } - let mut edit_counts: SledEditCounts = sled_storage.finalize().into(); + let final_counts = editor.edit_counts(); - // Expunging a zpool necessarily requires also expunging any zones that - // depended on it. - for zone_id in zones_to_expunge { - edit_counts = - edit_counts.accum(self.sled_expunge_zone(sled_id, zone_id)?); - } - - Ok(edit_counts) + Ok(final_counts.difference_since(initial_counts)) } /// Ensure that a sled in the blueprint has all the datasets it needs for @@ -942,31 +1087,32 @@ impl<'a> BlueprintBuilder<'a> { pub fn sled_ensure_zone_datasets( &mut self, sled_id: SledUuid, - resources: &SledResources, ) -> Result { - let mut sled_storage = self.storage.sled_storage_editor( - sled_id, - resources, - &mut self.rng, - )?; + let editor = self.sled_editors.get_mut(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to ensure zone datasets for unknown sled {sled_id}" + )) + })?; - // Ensure that datasets needed for zones exist. - for (zone, _zone_state) in self - .zones - .current_sled_zones(sled_id, BlueprintZoneFilter::ShouldBeRunning) - { - sled_storage.ensure_zone_datasets(zone); - } + let initial_counts = editor.edit_counts(); + editor + .ensure_datasets_for_running_zones(&mut self.rng) + .map_err(|err| Error::SledEditError { sled_id, err })?; + let final_counts = editor.edit_counts(); - let StorageEditCounts { disks: disk_edits, datasets: dataset_edits } = - sled_storage.finalize(); + let SledEditCounts { disks, datasets, zones } = + final_counts.difference_since(initial_counts); debug_assert_eq!( - disk_edits, + disks, EditCounts::zeroes(), - "we only edited datasets, not disks" + "we only edited datasets" ); - - Ok(dataset_edits.into()) + debug_assert_eq!( + zones, + EditCounts::zeroes(), + "we only edited datasets" + ); + Ok(datasets.into()) } fn next_internal_dns_gz_address_index(&self, sled_id: SledUuid) -> u32 { @@ -1077,10 +1223,16 @@ impl<'a> BlueprintBuilder<'a> { sled_id: SledUuid, ) -> Result { // If there's already an NTP zone on this sled, do nothing. - let has_ntp = self - .zones - .current_sled_zones(sled_id, BlueprintZoneFilter::ShouldBeRunning) - .any(|(z, _)| z.zone_type.is_ntp()); + let has_ntp = { + let editor = self.sled_editors.get(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to ensure NTP zone for unknown sled {sled_id}" + )) + })?; + editor + .zones(BlueprintZoneFilter::ShouldBeRunning) + .any(|z| z.zone_type.is_ntp()) + }; if has_ntp { return Ok(Ensure::NotNeeded); } @@ -1114,10 +1266,13 @@ impl<'a> BlueprintBuilder<'a> { let pool_name = ZpoolName::new_external(zpool_id); // If this sled already has a Crucible zone on this pool, do nothing. - let has_crucible_on_this_pool = self - .zones - .current_sled_zones(sled_id, BlueprintZoneFilter::ShouldBeRunning) - .any(|(z, _)| { + let has_crucible_on_this_pool = { + let editor = self.sled_editors.get(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to ensure crucible zone for unknown sled {sled_id}" + )) + })?; + editor.zones(BlueprintZoneFilter::ShouldBeRunning).any(|z| { matches!( &z.zone_type, BlueprintZoneType::Crucible(blueprint_zone_type::Crucible { @@ -1126,7 +1281,8 @@ impl<'a> BlueprintBuilder<'a> { }) if dataset.pool_name == pool_name ) - }); + }) + }; if has_crucible_on_this_pool { return Ok(Ensure::NotNeeded); } @@ -1172,9 +1328,12 @@ impl<'a> BlueprintBuilder<'a> { sled_id: SledUuid, kind: ZoneKind, ) -> usize { - self.zones - .current_sled_zones(sled_id, BlueprintZoneFilter::ShouldBeRunning) - .filter(|(z, _)| z.zone_type.kind() == kind) + let Some(editor) = self.sled_editors.get(&sled_id) else { + return 0; + }; + editor + .zones(BlueprintZoneFilter::ShouldBeRunning) + .filter(|z| z.zone_type.kind() == kind) .count() } @@ -1461,20 +1620,18 @@ impl<'a> BlueprintBuilder<'a> { dns_servers: Vec, domain: Option, ) -> Result<(), Error> { - // Check the sled id and return an appropriate error if it's invalid. - let _ = self.sled_resources(sled_id)?; - - let sled_zones = self.zones.change_sled_zones(sled_id); + let editor = self.sled_editors.get_mut(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to promote NTP zone on unknown sled {sled_id}" + )) + })?; // Find the internal NTP zone and expunge it. - let mut internal_ntp_zone_id_iter = sled_zones - .iter_zones(BlueprintZoneFilter::ShouldBeRunning) - .filter_map(|config| { - if matches!( - config.zone().zone_type, - BlueprintZoneType::InternalNtp(_) - ) { - Some(config.zone().id) + let mut internal_ntp_zone_id_iter = editor + .zones(BlueprintZoneFilter::ShouldBeRunning) + .filter_map(|zone| { + if matches!(zone.zone_type, BlueprintZoneType::InternalNtp(_)) { + Some(zone.id) } else { None } @@ -1496,7 +1653,7 @@ impl<'a> BlueprintBuilder<'a> { std::mem::drop(internal_ntp_zone_id_iter); // Expunge the internal NTP zone. - sled_zones.expunge_zone(internal_ntp_zone_id).map_err(|error| { + editor.expunge_zone(&internal_ntp_zone_id).map_err(|error| { Error::Planner(anyhow!(error).context(format!( "error expunging internal NTP zone from sled {sled_id}" ))) @@ -1559,31 +1716,18 @@ impl<'a> BlueprintBuilder<'a> { sled_id: SledUuid, zone_id: OmicronZoneUuid, ) -> Result { - let sled_resources = self.sled_resources(sled_id)?; - - let sled_zones = self.zones.change_sled_zones(sled_id); - let (builder_config, did_expunge) = - sled_zones.expunge_zone(zone_id).map_err(|error| { - Error::Planner( - anyhow!(error) - .context("failed to expunge zone from sled {sled_id}"), - ) - })?; - let zone_config = builder_config.zone(); - - let mut storage = self.storage.sled_storage_editor( - sled_id, - sled_resources, - &mut self.rng, - )?; - storage.expunge_zone_datasets(zone_config); - - let mut edit_counts: SledEditCounts = storage.finalize().into(); - if did_expunge { - edit_counts.zones.expunged += 1; - } + let editor = self.sled_editors.get_mut(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to expunge zone on unknown sled {sled_id}" + )) + })?; + let initial_counts = editor.edit_counts(); + editor + .expunge_zone(&zone_id) + .map_err(|err| Error::SledEditError { sled_id, err })?; + let final_counts = editor.edit_counts(); - Ok(edit_counts) + Ok(final_counts.difference_since(initial_counts)) } fn sled_add_zone( @@ -1591,30 +1735,25 @@ impl<'a> BlueprintBuilder<'a> { sled_id: SledUuid, zone: BlueprintZoneConfig, ) -> Result<(), Error> { - // Check the sled id and return an appropriate error if it's invalid. - let sled_resources = self.sled_resources(sled_id)?; - let mut sled_storage = self.storage.sled_storage_editor( - sled_id, - sled_resources, - &mut self.rng, - )?; - sled_storage.ensure_zone_datasets(&zone); - - let sled_zones = self.zones.change_sled_zones(sled_id); - sled_zones.add_zone(zone).map_err(|error| { - Error::Planner( - anyhow!(error) - .context(format!("error adding zone to sled {sled_id}")), - ) + let editor = self.sled_editors.get_mut(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to add zone on unknown sled {sled_id}" + )) })?; - - Ok(()) + editor + .add_zone(zone, &mut self.rng) + .map_err(|err| Error::SledEditError { sled_id, err }) } /// Returns a newly-allocated underlay address suitable for use by Omicron /// zones fn sled_alloc_ip(&mut self, sled_id: SledUuid) -> Result { let sled_subnet = self.sled_resources(sled_id)?.subnet; + let editor = self.sled_editors.get(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to allocate underlay IP for unknown sled {sled_id}" + )) + })?; let allocator = self.sled_ip_allocators.entry(sled_id).or_insert_with(|| { let sled_subnet_addr = sled_subnet.net().prefix(); @@ -1640,10 +1779,7 @@ impl<'a> BlueprintBuilder<'a> { // Record each of the sled's zones' underlay IPs as // allocated. - for (z, _) in self - .zones - .current_sled_zones(sled_id, BlueprintZoneFilter::All) - { + for z in editor.zones(BlueprintZoneFilter::All) { allocator.reserve(z.underlay_ip()); } @@ -1653,15 +1789,6 @@ impl<'a> BlueprintBuilder<'a> { allocator.alloc().ok_or(Error::OutOfAddresses { sled_id }) } - #[cfg(test)] - pub(crate) fn sled_select_zpool_for_tests( - &self, - sled_id: SledUuid, - zone_kind: ZoneKind, - ) -> Result { - self.sled_select_zpool(sled_id, zone_kind) - } - /// Selects a zpool for this zone type. /// /// This zpool may be used for either durable storage or transient @@ -1674,14 +1801,17 @@ impl<'a> BlueprintBuilder<'a> { sled_id: SledUuid, zone_kind: ZoneKind, ) -> Result { + let editor = self.sled_editors.get(&sled_id).ok_or_else(|| { + Error::Planner(anyhow!( + "tried to select zpool for unknown sled {sled_id}" + )) + })?; + // We'll check both the disks available to this sled per our current // blueprint and the list of all in-service zpools on this sled per our // planning input, and only pick zpools that are available in both. - let current_sled_disks = self - .storage - .current_sled_disks(&sled_id) - .ok_or(Error::NoAvailableZpool { sled_id, kind: zone_kind })? - .values() + let current_sled_disks = editor + .disks(DiskFilter::InService) .map(|disk_config| disk_config.pool_id) .collect::>(); @@ -1758,157 +1888,6 @@ impl<'a> BlueprintBuilder<'a> { } } -/// Helper for working with sets of zones on each sled -/// -/// Tracking the set of zones is slightly non-trivial because we need to bump -/// the per-sled generation number iff the zones are changed. So we need to -/// keep track of whether we've changed the zones relative to the parent -/// blueprint. We do this by keeping a copy of any [`BlueprintZonesConfig`] -/// that we've changed and a _reference_ to the parent blueprint's zones. This -/// struct makes it easy for callers iterate over the right set of zones. -pub(super) struct BlueprintZonesBuilder<'a> { - changed_zones: BTreeMap, - parent_zones: &'a BTreeMap, -} - -impl<'a> BlueprintZonesBuilder<'a> { - pub fn new(parent_blueprint: &'a Blueprint) -> BlueprintZonesBuilder { - BlueprintZonesBuilder { - changed_zones: BTreeMap::new(), - parent_zones: &parent_blueprint.blueprint_zones, - } - } - - /// Returns a mutable reference to a sled's Omicron zones *because* we're - /// going to change them. - /// - /// This updates internal data structures, and it is recommended that it be - /// called only when the caller actually wishes to make changes to zones. - /// But making no changes after calling this does not result in a changed - /// blueprint. (In particular, the generation number is only updated if - /// the state of any zones was updated.) - pub fn change_sled_zones( - &mut self, - sled_id: SledUuid, - ) -> &mut BuilderZonesConfig { - self.changed_zones.entry(sled_id).or_insert_with(|| { - if let Some(old_sled_zones) = self.parent_zones.get(&sled_id) { - BuilderZonesConfig::from_parent(old_sled_zones) - } else { - BuilderZonesConfig::new() - } - }) - } - - /// Iterates over the list of sled IDs for which we have zones. - /// - /// This may include decommissioned sleds. - pub fn sled_ids_with_zones(&self) -> impl Iterator { - let mut sled_ids = - self.changed_zones.keys().copied().collect::>(); - for &sled_id in self.parent_zones.keys() { - sled_ids.insert(sled_id); - } - sled_ids.into_iter() - } - - /// Iterates over the list of `current_sled_zones` for all sled IDs for - /// which we have zones. - /// - /// This may include decommissioned sleds. - pub fn current_zones( - &self, - filter: BlueprintZoneFilter, - ) -> impl Iterator)> { - let sled_ids = self.sled_ids_with_zones(); - sled_ids.map(move |sled_id| { - let zones = self - .current_sled_zones(sled_id, filter) - .map(|(zone_config, _)| zone_config) - .collect(); - (sled_id, zones) - }) - } - - /// Iterates over the list of Omicron zones currently configured for this - /// sled in the blueprint that's being built, along with each zone's state - /// in the builder. - pub fn current_sled_zones( - &self, - sled_id: SledUuid, - filter: BlueprintZoneFilter, - ) -> Box + '_> - { - if let Some(sled_zones) = self.changed_zones.get(&sled_id) { - Box::new( - sled_zones.iter_zones(filter).map(|z| (z.zone(), z.state())), - ) - } else if let Some(parent_zones) = self.parent_zones.get(&sled_id) { - Box::new(parent_zones.zones.iter().filter_map(move |z| { - if z.disposition.matches(filter) { - Some((z, BuilderZoneState::Unchanged)) - } else { - None - } - })) - } else { - Box::new(std::iter::empty()) - } - } - - /// Builds a set of all zones whose filesystem or durable dataset reside on - /// the given `zpool`. - pub fn zones_using_zpool<'b>( - &'b self, - sled_id: SledUuid, - filter: BlueprintZoneFilter, - zpool: &'b ZpoolName, - ) -> impl Iterator + 'b { - self.current_sled_zones(sled_id, filter).filter_map( - move |(config, _state)| { - if Some(zpool) == config.filesystem_pool.as_ref() - || Some(zpool) == config.zone_type.durable_zpool() - { - Some(config) - } else { - None - } - }, - ) - } - - /// Produces an owned map of zones for the sleds recorded in this blueprint - /// plus any newly-added sleds - pub fn into_zones_map( - self, - added_sled_ids: impl Iterator, - ) -> BTreeMap { - // Start with self.changed_zones, which contains entries for any - // sled whose zones config is changing in this blueprint. - let mut zones = self - .changed_zones - .into_iter() - .map(|(sled_id, zones)| (sled_id, zones.build())) - .collect::>(); - - // Carry forward any zones from our parent blueprint. This may include - // zones for decommissioned sleds. - for (sled_id, parent_zones) in self.parent_zones { - zones.entry(*sled_id).or_insert_with(|| parent_zones.clone()); - } - - // Finally, insert any newly-added sleds. - for sled_id in added_sled_ids { - zones.entry(sled_id).or_insert_with(|| BlueprintZonesConfig { - generation: Generation::new(), - zones: vec![], - }); - } - - zones - } -} - #[cfg(test)] pub mod test { use super::*; @@ -2035,9 +2014,13 @@ pub mod test { } } - // All commissioned disks should have debug and zone root datasets. + // All disks should have debug and zone root datasets. for (sled_id, disk_config) in &blueprint.blueprint_disks { for disk in &disk_config.disks { + eprintln!( + "checking datasets for sled {sled_id} disk {}", + disk.id + ); let zpool = ZpoolName::new_external(disk.pool_id); let datasets = datasets_for_sled(&blueprint, *sled_id); @@ -2074,10 +2057,8 @@ pub mod test { } let datasets = datasets_for_sled(&blueprint, sled_id); - let zpool = zone_config.filesystem_pool.as_ref().unwrap(); - let kind = DatasetKind::TransientZone { - name: storage_editor::zone_name(&zone_config), - }; + let (zpool, kind) = + zone_config.filesystem_dataset().unwrap().into_parts(); let dataset = find_dataset(&datasets, &zpool, kind); assert_eq!( dataset.disposition, @@ -2256,9 +2237,7 @@ pub mod test { for pool_id in new_sled_resources.zpools.keys() { builder.sled_ensure_zone_crucible(new_sled_id, *pool_id).unwrap(); } - builder - .sled_ensure_zone_datasets(new_sled_id, new_sled_resources) - .unwrap(); + builder.sled_ensure_zone_datasets(new_sled_id).unwrap(); let blueprint3 = builder.build(); verify_blueprint(&blueprint3); @@ -2381,7 +2360,7 @@ pub mod test { // Generate a new blueprint. This sled should still be included: even // though the desired state is decommissioned, the current state is // still active, so we should carry it forward. - let blueprint2 = BlueprintBuilder::new_based_on( + let mut blueprint2 = BlueprintBuilder::new_based_on( &logctx.log, &blueprint1, &input, @@ -2399,11 +2378,21 @@ pub mod test { ); // Change the input to mark the sled decommissioned. (Normally realizing - // blueprint2 would make this change.) + // blueprint2 would make this change.) We must also mark all its zones + // expunged to avoid tripping over an invalid state check in + // `new_based_on()`. let mut builder = input.into_builder(); builder.sleds_mut().get_mut(&decommision_sled_id).unwrap().state = SledState::Decommissioned; let input = builder.build(); + for z in &mut blueprint2 + .blueprint_zones + .get_mut(&decommision_sled_id) + .unwrap() + .zones + { + z.disposition = BlueprintZoneDisposition::Expunged; + } // Generate a new blueprint. This desired sled state should no longer be // present: it has reached the terminal decommissioned state, so there's @@ -2468,9 +2457,11 @@ pub mod test { // not have any disks in them. for sled_id in input.all_sled_ids(SledFilter::InService) { let disks = builder - .storage - .current_sled_disks(&sled_id) - .expect("found disks config for sled"); + .sled_editors + .get(&sled_id) + .unwrap() + .disks(DiskFilter::All) + .collect::>(); assert!( disks.is_empty(), "expected empty disks for sled {sled_id}, got {disks:?}" @@ -2505,19 +2496,14 @@ pub mod test { ); } - let new_disks = builder - .storage - .into_blueprint_maps(input.all_sled_ids(SledFilter::InService)) - .0; // We should have disks and a generation bump for every sled. let parent_disk_gens = parent .blueprint_disks .iter() .map(|(&sled_id, config)| (sled_id, config.generation)); for (sled_id, parent_gen) in parent_disk_gens { - let new_sled_disks = new_disks - .get(&sled_id) - .expect("found child entry for sled present in parent"); + let EditedSled { disks: new_sled_disks, .. } = + builder.sled_editors.remove(&sled_id).unwrap().finalize(); assert_eq!(new_sled_disks.generation, parent_gen.next()); assert_eq!( new_sled_disks.disks.len(), @@ -2577,11 +2563,8 @@ pub mod test { // Before we make any modifications, there should be no work to do. // // If we haven't changed inputs, the output should be the same! - for (sled_id, resources) in - input.all_sled_resources(SledFilter::Commissioned) - { - let r = - builder.sled_ensure_zone_datasets(sled_id, resources).unwrap(); + for sled_id in input.all_sled_ids(SledFilter::Commissioned) { + let r = builder.sled_ensure_zone_datasets(sled_id).unwrap(); assert_eq!(r, EnsureMultiple::NotNeeded); } @@ -2591,48 +2574,32 @@ pub mod test { .all_sled_ids(SledFilter::Commissioned) .next() .expect("at least one sled present"); - let sled_details = - input.sled_lookup(SledFilter::Commissioned, sled_id).unwrap(); - let crucible_zone_id = builder - .zones - .current_sled_zones(sled_id, BlueprintZoneFilter::ShouldBeRunning) - .find_map(|(zone_config, _)| { + let editor = + builder.sled_editors.get_mut(&sled_id).expect("found sled"); + let crucible_zone_id = editor + .zones(BlueprintZoneFilter::ShouldBeRunning) + .find_map(|zone_config| { if zone_config.zone_type.is_crucible() { return Some(zone_config.id); } None }) .expect("at least one crucible must be present"); - let change = builder.zones.change_sled_zones(sled_id); println!("Expunging crucible zone: {crucible_zone_id}"); - let expunged_zones = - change.expunge_zones(BTreeSet::from([crucible_zone_id])).unwrap(); - assert_eq!(expunged_zones.len(), 1); + + let initial_counts = editor.edit_counts(); + editor.expunge_zone(&crucible_zone_id).expect("expunged crucible"); + let changed_counts = + editor.edit_counts().difference_since(initial_counts); // In the case of Crucible, we have a durable dataset and a transient // zone filesystem, so we expect two datasets to be expunged. - let r = builder - .storage - .sled_storage_editor( - sled_id, - &sled_details.resources, - &mut builder.rng, - ) - .unwrap() - .expunge_zone_datasets(&expunged_zones[0]); assert_eq!( - r, - EnsureMultiple::Changed { - added: 0, - updated: 0, - expunged: 2, - removed: 0 - } + changed_counts.datasets, + EditCounts { added: 0, updated: 0, expunged: 2, removed: 0 } ); // Once the datasets are expunged, no further changes will be proposed. - let r = builder - .sled_ensure_zone_datasets(sled_id, &sled_details.resources) - .unwrap(); + let r = builder.sled_ensure_zone_datasets(sled_id).unwrap(); assert_eq!(r, EnsureMultiple::NotNeeded); let blueprint = builder.build(); @@ -2649,9 +2616,7 @@ pub mod test { // While the datasets still exist in the input (effectively, the db) we // cannot remove them. - let r = builder - .sled_ensure_zone_datasets(sled_id, &sled_details.resources) - .unwrap(); + let r = builder.sled_ensure_zone_datasets(sled_id).unwrap(); assert_eq!(r, EnsureMultiple::NotNeeded); let blueprint = builder.build(); @@ -2703,11 +2668,7 @@ pub mod test { // Now, we should see the datasets "removed" from the blueprint, since // we no longer need to keep around records of their expungement. - let sled_details = - input.sled_lookup(SledFilter::Commissioned, sled_id).unwrap(); - let r = builder - .sled_ensure_zone_datasets(sled_id, &sled_details.resources) - .unwrap(); + let r = builder.sled_ensure_zone_datasets(sled_id).unwrap(); // TODO(https://github.com/oxidecomputer/omicron/issues/6646): // Because of the workaround for #6646, we don't actually remove @@ -2950,9 +2911,7 @@ pub mod test { .sled_add_zone_cockroachdb(target_sled_id) .expect("added CRDB zone"); } - builder - .sled_ensure_zone_datasets(target_sled_id, sled_resources) - .unwrap(); + builder.sled_ensure_zone_datasets(target_sled_id).unwrap(); let blueprint = builder.build(); verify_blueprint(&blueprint); @@ -3003,4 +2962,92 @@ pub mod test { logctx.cleanup_successful(); } + + // This test can go away with + // https://github.com/oxidecomputer/omicron/issues/6645; for now, it + // confirms we maintain the compatibility layer it needs. + #[test] + fn test_backcompat_reuse_existing_database_dataset_ids() { + static TEST_NAME: &str = + "backcompat_reuse_existing_database_dataset_ids"; + let logctx = test_setup_log(TEST_NAME); + + // Start with the standard example blueprint. + let (collection, input, mut parent) = example(&logctx.log, TEST_NAME); + + // `parent` was not created prior to the addition of disks and datasets, + // so it should have datasets for all the disks and zones, and the + // dataset IDs should match the input. + let mut input_dataset_ids = BTreeMap::new(); + let mut input_ndatasets = 0; + for (_, resources) in input.all_sled_resources(SledFilter::All) { + for (zpool_id, dataset_configs) in + resources.all_datasets(ZpoolFilter::All) + { + for dataset in dataset_configs { + let id = dataset.id; + let kind = dataset.name.dataset(); + let by_kind: &mut BTreeMap<_, _> = + input_dataset_ids.entry(*zpool_id).or_default(); + let prev = by_kind.insert(kind.clone(), id); + input_ndatasets += 1; + assert!(prev.is_none()); + } + } + } + // We should have 3 datasets per disk (debug + zone root + crucible), + // plus some number of datasets for discretionary zones. We'll just + // check that we have more than 3 per disk. + assert!( + input_ndatasets + > 3 * usize::from(SledBuilder::DEFAULT_NPOOLS) + * ExampleSystemBuilder::DEFAULT_N_SLEDS, + "too few datasets: {input_ndatasets}" + ); + + // Now _remove_ the blueprint datasets entirely, to emulate a + // pre-dataset-addition blueprint. + parent.blueprint_datasets = BTreeMap::new(); + + // Build a new blueprint. + let mut builder = BlueprintBuilder::new_based_on( + &logctx.log, + &parent, + &input, + &collection, + TEST_NAME, + ) + .expect("failed to create builder"); + + // Ensure disks and datasets. This should repopulate the datasets. + for (sled_id, resources) in input.all_sled_resources(SledFilter::All) { + builder + .sled_ensure_disks(sled_id, resources) + .expect("ensured disks"); + builder + .sled_ensure_zone_datasets(sled_id) + .expect("ensured zone datasets"); + } + let output = builder.build(); + + // Repeat the logic above on our new blueprint; it should have the same + // number of datasets, and they should all have identical IDs. + let mut output_dataset_ids = BTreeMap::new(); + let mut output_ndatasets = 0; + for datasets in output.blueprint_datasets.values() { + for (id, dataset) in &datasets.datasets { + let zpool_id = dataset.pool.id(); + let kind = dataset.kind.clone(); + let by_kind: &mut BTreeMap<_, _> = + output_dataset_ids.entry(zpool_id).or_default(); + let prev = by_kind.insert(kind, *id); + output_ndatasets += 1; + assert!(prev.is_none()); + } + } + assert_eq!(input_ndatasets, output_ndatasets); + assert_eq!(input_dataset_ids, output_dataset_ids); + + logctx.cleanup_successful(); + } } diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder/datasets_editor.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder/datasets_editor.rs deleted file mode 100644 index 160b841a88..0000000000 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder/datasets_editor.rs +++ /dev/null @@ -1,348 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! Helper for editing the datasets of a Blueprint - -use super::EditCounts; -use crate::planner::PlannerRng; -use illumos_utils::zpool::ZpoolName; -use nexus_types::deployment::BlueprintDatasetConfig; -use nexus_types::deployment::BlueprintDatasetDisposition; -use nexus_types::deployment::BlueprintDatasetsConfig; -use nexus_types::deployment::SledResources; -use nexus_types::deployment::ZpoolFilter; -use omicron_common::api::external::ByteCount; -use omicron_common::api::external::Generation; -use omicron_common::disk::CompressionAlgorithm; -use omicron_common::disk::DatasetKind; -use omicron_common::disk::DatasetName; -use omicron_common::disk::GzipLevel; -use omicron_uuid_kinds::DatasetUuid; -use omicron_uuid_kinds::SledUuid; -use omicron_uuid_kinds::ZpoolUuid; -use std::collections::btree_map::Entry; -use std::collections::BTreeMap; -use std::collections::BTreeSet; -use std::net::SocketAddrV6; - -#[derive(Debug, thiserror::Error)] -pub enum BlueprintDatasetsEditError { - #[error( - "{data_source} inconsistency: multiple datasets with kind {kind:?} \ - on zpool {zpool_id}: {id1}, {id2}" - )] - MultipleDatasetsOfKind { - data_source: &'static str, - zpool_id: ZpoolUuid, - kind: DatasetKind, - id1: DatasetUuid, - id2: DatasetUuid, - }, -} - -/// Helper for working with sets of datasets on each sled -/// -/// Tracking the set of datasets is slightly non-trivial because we need to -/// bump the per-sled generation number iff the datasets are changed. So -/// we need to keep track of whether we've changed the datasets relative -/// to the parent blueprint. -#[derive(Debug)] -pub(super) struct BlueprintDatasetsEditor { - current: BTreeMap, - changed: BTreeSet, -} - -impl BlueprintDatasetsEditor { - pub fn new(current: BTreeMap) -> Self { - Self { current, changed: BTreeSet::new() } - } - - /// Get a helper to edit the datasets of a specific sled. - /// - /// If any changes are made via the returned editor, the sled will be - /// recorded as needing a generation bump in its dataset config when the - /// editor is dropped. - pub fn sled_datasets_editor<'a>( - &'a mut self, - sled_id: SledUuid, - sled_resources: &SledResources, - rng: &'a mut PlannerRng, - ) -> Result, BlueprintDatasetsEditError> { - let config = self - .current - .entry(sled_id) - .or_insert_with(empty_blueprint_datasets_config); - - // Gather all dataset IDs known to the database. - // - // See the comment below where this is used; this is a - // backwards-compatibility layer for - // https://github.com/oxidecomputer/omicron/issues/6645. - let database_dataset_ids = build_dataset_kind_id_map( - "database", - sled_resources.all_datasets(ZpoolFilter::InService).flat_map( - |(&zpool_id, configs)| { - configs.iter().map(move |config| { - (zpool_id, config.name.dataset().clone(), config.id) - }) - }, - ), - )?; - - SledDatasetsEditor::new( - rng, - database_dataset_ids, - sled_id, - config, - &mut self.changed, - ) - } - - pub fn build( - mut self, - sled_ids: impl Iterator, - ) -> BTreeMap { - sled_ids - .map(|sled_id| { - let config = match self.current.remove(&sled_id) { - Some(mut config) => { - // Bump generation number for any sled whose - // DatasetsConfig changed - if self.changed.contains(&sled_id) { - config.generation = config.generation.next(); - } - config - } - None => empty_blueprint_datasets_config(), - }; - (sled_id, config) - }) - .collect() - } -} - -#[derive(Debug)] -pub(super) struct SledDatasetsEditor<'a> { - rng: &'a mut PlannerRng, - blueprint_dataset_ids: - BTreeMap>, - database_dataset_ids: - BTreeMap>, - config: &'a mut BlueprintDatasetsConfig, - counts: EditCounts, - sled_id: SledUuid, - parent_changed_set: &'a mut BTreeSet, -} - -impl Drop for SledDatasetsEditor<'_> { - fn drop(&mut self) { - if self.counts.has_nonzero_counts() { - self.parent_changed_set.insert(self.sled_id); - } - } -} - -impl<'a> SledDatasetsEditor<'a> { - fn new( - rng: &'a mut PlannerRng, - database_dataset_ids: BTreeMap< - ZpoolUuid, - BTreeMap, - >, - sled_id: SledUuid, - config: &'a mut BlueprintDatasetsConfig, - parent_changed_set: &'a mut BTreeSet, - ) -> Result { - let blueprint_dataset_ids = build_dataset_kind_id_map( - "parent blueprint", - config.datasets.values().map(|dataset| { - (dataset.pool.id(), dataset.kind.clone(), dataset.id) - }), - )?; - Ok(Self { - rng, - blueprint_dataset_ids, - database_dataset_ids, - config, - counts: EditCounts::zeroes(), - sled_id, - parent_changed_set, - }) - } - - pub fn expunge_datasets_if(&mut self, mut expunge_if: F) -> usize - where - F: FnMut(&BlueprintDatasetConfig) -> bool, - { - let mut num_expunged = 0; - - for dataset in self.config.datasets.values_mut() { - match dataset.disposition { - // Already expunged; ignore - BlueprintDatasetDisposition::Expunged => continue, - // Potentially expungeable - BlueprintDatasetDisposition::InService => (), - } - if expunge_if(&*dataset) { - dataset.disposition = BlueprintDatasetDisposition::Expunged; - num_expunged += 1; - self.counts.expunged += 1; - } - } - - num_expunged - } - - pub fn ensure_debug_dataset(&mut self, zpool: ZpoolName) { - const DEBUG_QUOTA_SIZE_GB: u32 = 100; - - let address = None; - let quota = Some(ByteCount::from_gibibytes_u32(DEBUG_QUOTA_SIZE_GB)); - let reservation = None; - - self.ensure_dataset( - DatasetName::new(zpool, DatasetKind::Debug), - address, - quota, - reservation, - CompressionAlgorithm::GzipN { level: GzipLevel::new::<9>() }, - ) - } - - pub fn ensure_zone_root_dataset(&mut self, zpool: ZpoolName) { - let address = None; - let quota = None; - let reservation = None; - - self.ensure_dataset( - DatasetName::new(zpool, DatasetKind::TransientZoneRoot), - address, - quota, - reservation, - CompressionAlgorithm::Off, - ) - } - - /// Ensures a dataset exists on this sled. - /// - /// - If the dataset exists in the blueprint already, use it. - /// - Otherwise, if the dataset exists in the database, re-use the UUID, but - /// add it to the blueprint. - /// - Otherwise, create a new dataset in the blueprint, which will propagate - /// to the database during execution. - pub fn ensure_dataset( - &mut self, - dataset: DatasetName, - address: Option, - quota: Option, - reservation: Option, - compression: CompressionAlgorithm, - ) { - let zpool_id = dataset.pool().id(); - let kind = dataset.dataset(); - - let make_config = |id: DatasetUuid| BlueprintDatasetConfig { - disposition: BlueprintDatasetDisposition::InService, - id, - pool: dataset.pool().clone(), - kind: kind.clone(), - address, - quota, - reservation, - compression, - }; - - // Is this dataset already in the blueprint? If so, update it if it's - // changed. - if let Some(existing_id) = self - .blueprint_dataset_ids - .get(&zpool_id) - .and_then(|kind_to_id| kind_to_id.get(kind)) - { - // We built `self.blueprint_dataset_ids` based on the contents of - // `self.config.datasets`, so we know we can unwrap this `get_mut`. - let old_config = self.config.datasets.get_mut(existing_id).expect( - "internal inconsistency: \ - entry in blueprint_dataset_ids but not current", - ); - let new_config = make_config(*existing_id); - - if new_config != *old_config { - *old_config = new_config; - self.counts.updated += 1; - } - - return; - } - - // Is there a dataset ID matching this one in the database? If so, use - // that. - // - // TODO(https://github.com/oxidecomputer/omicron/issues/6645): We - // could avoid reading from the datastore if we were confident all - // provisioned datasets existed in the parent blueprint. - let id = self - .database_dataset_ids - .get(&zpool_id) - .and_then(|kind_to_id| kind_to_id.get(kind)) - .copied() - .unwrap_or_else(|| self.rng.next_dataset()); - - self.config.datasets.insert(id, make_config(id)); - self.counts.added += 1; - - // We updated our config, so also record this ID in our "present in - // the blueprint" map. We know the entry doesn't exist or we would have - // found it when we checked above. - self.blueprint_dataset_ids - .entry(zpool_id) - .or_default() - .insert(kind.clone(), id); - } - - /// Consume this editor, returning a summary of changes made. - pub fn finalize(self) -> EditCounts { - self.counts - } -} - -fn build_dataset_kind_id_map( - data_source: &'static str, - iter: impl Iterator, -) -> Result< - BTreeMap>, - BlueprintDatasetsEditError, -> { - let mut kind_id_map: BTreeMap< - ZpoolUuid, - BTreeMap, - > = BTreeMap::new(); - for (zpool_id, kind, dataset_id) in iter { - let dataset_ids_by_kind = kind_id_map.entry(zpool_id).or_default(); - match dataset_ids_by_kind.entry(kind) { - Entry::Vacant(slot) => { - slot.insert(dataset_id); - } - Entry::Occupied(prev) => { - return Err( - BlueprintDatasetsEditError::MultipleDatasetsOfKind { - data_source, - zpool_id, - kind: prev.key().clone(), - id1: *prev.get(), - id2: dataset_id, - }, - ); - } - } - } - Ok(kind_id_map) -} - -fn empty_blueprint_datasets_config() -> BlueprintDatasetsConfig { - BlueprintDatasetsConfig { - generation: Generation::new(), - datasets: BTreeMap::new(), - } -} diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder/disks_editor.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder/disks_editor.rs deleted file mode 100644 index 7c5c4c318f..0000000000 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder/disks_editor.rs +++ /dev/null @@ -1,195 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! Helper for editing the disks of a Blueprint - -use super::EditCounts; -use nexus_types::deployment::BlueprintPhysicalDiskConfig; -use nexus_types::deployment::BlueprintPhysicalDisksConfig; -use omicron_common::api::external::Generation; -use omicron_uuid_kinds::PhysicalDiskUuid; -use omicron_uuid_kinds::SledUuid; -use std::collections::btree_map::Entry; -use std::collections::BTreeMap; -use std::collections::BTreeSet; - -/// Helper for working with sets of disks on each sled -/// -/// Tracking the set of disks is slightly non-trivial because we need to -/// bump the per-sled generation number iff the disks are changed. So -/// we need to keep track of whether we've changed the disks relative -/// to the parent blueprint. -#[derive(Debug)] -pub(super) struct BlueprintDisksEditor { - current: BTreeMap, - changed: BTreeSet, -} - -impl BlueprintDisksEditor { - pub fn new( - current: BTreeMap, - ) -> Self { - let current = current - .into_iter() - .map(|(sled_id, config)| (sled_id, config.into())) - .collect(); - Self { current, changed: BTreeSet::new() } - } - - /// Get a helper to edit the disks of a specific sled. - /// - /// If any changes are made via the returned editor, the sled will be - /// recorded as needing a generation bump in its disk config when the editor - /// is dropped. - pub fn sled_disks_editor( - &mut self, - sled_id: SledUuid, - ) -> SledDisksEditor<'_> { - let config = - self.current.entry(sled_id).or_insert_with(DisksConfig::empty); - SledDisksEditor::new(sled_id, config, &mut self.changed) - } - - pub fn current_sled_disks( - &self, - sled_id: &SledUuid, - ) -> Option<&BTreeMap> { - let config = self.current.get(sled_id)?; - Some(&config.disks) - } - - /// Compile all edits into a new map suitable for a blueprint's - /// `blueprint_disks`, bumping the generation number for any sleds whose - /// disk config changed. - /// - /// Only sleds listed in `sled_ids` will be present in the returned map. - /// This primarily allows the caller to drop sleds that are no longer in - /// service. (Any new sleds will be given an empty set of disks, but - /// presumably any new sleds will have _some_ disks that will have already - /// been populated via a relevant `sled_disks_editor()` call.) - pub fn build( - mut self, - sled_ids: impl Iterator, - ) -> BTreeMap { - sled_ids - .map(|sled_id| { - let config = match self.current.remove(&sled_id) { - Some(mut config) => { - // Bump generation number for any sled whose DisksConfig - // changed - if self.changed.contains(&sled_id) { - config.generation = config.generation.next() - } - config.into() - } - None => DisksConfig::empty().into(), - }; - (sled_id, config) - }) - .collect() - } -} - -#[derive(Debug)] -pub(super) struct SledDisksEditor<'a> { - config: &'a mut DisksConfig, - counts: EditCounts, - sled_id: SledUuid, - parent_changed_set: &'a mut BTreeSet, -} - -impl Drop for SledDisksEditor<'_> { - fn drop(&mut self) { - if self.counts.has_nonzero_counts() { - self.parent_changed_set.insert(self.sled_id); - } - } -} - -impl<'a> SledDisksEditor<'a> { - fn new( - sled_id: SledUuid, - config: &'a mut DisksConfig, - parent_changed_set: &'a mut BTreeSet, - ) -> Self { - Self { - config, - counts: EditCounts::zeroes(), - sled_id, - parent_changed_set, - } - } - - pub fn disk_ids(&self) -> impl Iterator + '_ { - self.config.disks.keys().copied() - } - - pub fn ensure_disk(&mut self, disk: BlueprintPhysicalDiskConfig) { - let disk_id = disk.id; - match self.config.disks.entry(disk_id) { - Entry::Vacant(slot) => { - slot.insert(disk); - self.counts.added += 1; - } - Entry::Occupied(mut slot) => { - if *slot.get() != disk { - slot.insert(disk); - self.counts.updated += 1; - } - } - } - } - - pub fn remove_disk( - &mut self, - disk_id: &PhysicalDiskUuid, - ) -> Option { - let old = self.config.disks.remove(disk_id); - if old.is_some() { - self.counts.removed += 1; - } - old - } - - pub fn finalize(self) -> EditCounts { - self.counts - } -} - -// We want add and remove to be cheap and easy to check whether they performed -// the requested operation, so we'll internally convert from the vec of disks to -// a map of disks keyed by disk ID. -#[derive(Debug)] -struct DisksConfig { - generation: Generation, - disks: BTreeMap, -} - -impl DisksConfig { - fn empty() -> Self { - Self { generation: Generation::new(), disks: BTreeMap::new() } - } -} - -impl From for BlueprintPhysicalDisksConfig { - fn from(config: DisksConfig) -> Self { - BlueprintPhysicalDisksConfig { - generation: config.generation, - disks: config.disks.into_values().collect(), - } - } -} - -impl From for DisksConfig { - fn from(config: BlueprintPhysicalDisksConfig) -> Self { - Self { - generation: config.generation, - disks: config - .disks - .into_iter() - .map(|disk| (disk.id, disk)) - .collect(), - } - } -} diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder/storage_editor.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder/storage_editor.rs deleted file mode 100644 index 2119656da3..0000000000 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder/storage_editor.rs +++ /dev/null @@ -1,206 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! Helper for editing the storage (disks and datasets) of a Blueprint - -use crate::planner::PlannerRng; - -use super::datasets_editor::BlueprintDatasetsEditError; -use super::datasets_editor::BlueprintDatasetsEditor; -use super::datasets_editor::SledDatasetsEditor; -use super::disks_editor::BlueprintDisksEditor; -use super::disks_editor::SledDisksEditor; -use super::EnsureMultiple; -use super::StorageEditCounts; -use illumos_utils::zpool::ZpoolName; -use nexus_types::deployment::blueprint_zone_type; -use nexus_types::deployment::BlueprintDatasetsConfig; -use nexus_types::deployment::BlueprintPhysicalDiskConfig; -use nexus_types::deployment::BlueprintPhysicalDisksConfig; -use nexus_types::deployment::BlueprintZoneConfig; -use nexus_types::deployment::BlueprintZoneType; -use nexus_types::deployment::SledResources; -use omicron_common::disk::CompressionAlgorithm; -use omicron_common::disk::DatasetKind; -use omicron_common::disk::DatasetName; -use omicron_uuid_kinds::PhysicalDiskUuid; -use omicron_uuid_kinds::SledUuid; -use std::collections::BTreeMap; - -#[derive(Debug)] -pub(super) struct BlueprintStorageEditor { - disks: BlueprintDisksEditor, - datasets: BlueprintDatasetsEditor, -} - -impl BlueprintStorageEditor { - pub fn new( - disks: BTreeMap, - datasets: BTreeMap, - ) -> Self { - Self { - disks: BlueprintDisksEditor::new(disks), - datasets: BlueprintDatasetsEditor::new(datasets), - } - } - - pub fn sled_storage_editor<'a>( - &'a mut self, - sled_id: SledUuid, - sled_resources: &SledResources, - rng: &'a mut PlannerRng, - ) -> Result, BlueprintDatasetsEditError> { - let disks = self.disks.sled_disks_editor(sled_id); - let datasets = - self.datasets.sled_datasets_editor(sled_id, sled_resources, rng)?; - Ok(SledStorageEditor { disks, datasets }) - } - - pub fn current_sled_disks( - &self, - sled_id: &SledUuid, - ) -> Option<&BTreeMap> { - self.disks.current_sled_disks(sled_id) - } - - pub fn into_blueprint_maps( - self, - sled_ids: impl Iterator + Clone, - ) -> ( - BTreeMap, - BTreeMap, - ) { - (self.disks.build(sled_ids.clone()), self.datasets.build(sled_ids)) - } -} - -#[derive(Debug)] -pub(super) struct SledStorageEditor<'a> { - disks: SledDisksEditor<'a>, - datasets: SledDatasetsEditor<'a>, -} - -impl SledStorageEditor<'_> { - pub fn disk_ids(&self) -> impl Iterator + '_ { - self.disks.disk_ids() - } - - pub fn ensure_disk(&mut self, disk: BlueprintPhysicalDiskConfig) { - let zpool = ZpoolName::new_external(disk.pool_id); - - self.disks.ensure_disk(disk); - self.datasets.ensure_debug_dataset(zpool.clone()); - self.datasets.ensure_zone_root_dataset(zpool); - } - - pub fn remove_disk( - &mut self, - disk_id: &PhysicalDiskUuid, - ) -> Option { - let Some(disk) = self.disks.remove_disk(disk_id) else { - return None; - }; - self.datasets - .expunge_datasets_if(|dataset| dataset.pool.id() == disk.pool_id); - Some(ZpoolName::new_external(disk.pool_id)) - } - - pub fn ensure_zone_datasets(&mut self, zone: &BlueprintZoneConfig) { - // TODO check that zpools are on valid disks? - - // Dataset for transient zone filesystem - if let Some(fs_zpool) = &zone.filesystem_pool { - let name = zone_name(&zone); - let address = None; - let quota = None; - let reservation = None; - self.datasets.ensure_dataset( - DatasetName::new( - fs_zpool.clone(), - DatasetKind::TransientZone { name }, - ), - address, - quota, - reservation, - CompressionAlgorithm::Off, - ); - } - - // Dataset for durable dataset co-located with zone - if let Some(dataset) = zone.zone_type.durable_dataset() { - let zpool = &dataset.dataset.pool_name; - - if let Some(fs_zpool) = &zone.filesystem_pool { - debug_assert_eq!( - zpool, fs_zpool, - "zone has durable dataset and transient root \ - on different zpools" - ); - } - - let address = match zone.zone_type { - BlueprintZoneType::Crucible( - blueprint_zone_type::Crucible { address, .. }, - ) => Some(address), - _ => None, - }; - let quota = None; - let reservation = None; - self.datasets.ensure_dataset( - DatasetName::new(zpool.clone(), dataset.kind), - address, - quota, - reservation, - CompressionAlgorithm::Off, - ); - } - } - - pub fn expunge_zone_datasets( - &mut self, - zone: &BlueprintZoneConfig, - ) -> EnsureMultiple { - let mut expunged = 0; - - if zone.filesystem_pool.is_some() { - let name = zone_name(&zone); - let kind = DatasetKind::TransientZone { name }; - expunged += self.datasets.expunge_datasets_if(|dataset_config| { - dataset_config.kind == kind - }); - } - - if let Some(dataset) = zone.zone_type.durable_dataset() { - expunged += self.datasets.expunge_datasets_if(|dataset_config| { - dataset_config.pool == dataset.dataset.pool_name - && dataset_config.kind == dataset.kind - }); - } - - if expunged == 0 { - EnsureMultiple::NotNeeded - } else { - EnsureMultiple::Changed { - added: 0, - updated: 0, - expunged, - removed: 0, - } - } - } - - pub fn finalize(self) -> StorageEditCounts { - StorageEditCounts { - disks: self.disks.finalize(), - datasets: self.datasets.finalize(), - } - } -} - -pub(super) fn zone_name(zone: &BlueprintZoneConfig) -> String { - illumos_utils::zone::zone_name( - zone.zone_type.kind().zone_prefix(), - Some(zone.id), - ) -} diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/mod.rs b/nexus/reconfigurator/planning/src/blueprint_builder/mod.rs index 725835f4ae..bab6476456 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/mod.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/mod.rs @@ -8,7 +8,6 @@ mod builder; mod clickhouse; mod external_networking; mod internal_dns; -mod zones; pub use builder::*; pub use clickhouse::{ClickhouseAllocator, ClickhouseZonesThatShouldBeRunning}; diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs b/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs deleted file mode 100644 index 672331ab81..0000000000 --- a/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs +++ /dev/null @@ -1,527 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -use std::collections::BTreeSet; - -use nexus_types::deployment::{ - BlueprintZoneConfig, BlueprintZoneDisposition, BlueprintZoneFilter, - BlueprintZonesConfig, -}; -use omicron_common::api::external::Generation; -use omicron_uuid_kinds::OmicronZoneUuid; -use thiserror::Error; - -#[derive(Debug)] -#[must_use] -pub(super) struct BuilderZonesConfig { - // The current generation -- this is bumped at blueprint build time and is - // otherwise not exposed to callers. - generation: Generation, - - // The list of zones, along with their state. - zones: Vec, -} - -impl BuilderZonesConfig { - pub(super) fn new() -> Self { - Self { - // Note that the first generation is reserved to mean the one - // containing no zones. See - // OmicronZonesConfig::INITIAL_GENERATION. - // - // Since we're currently assuming that creating a new - // `BuilderZonesConfig` means that we're going to add new zones - // shortly, we start with Generation::new() here. It'll get - // bumped up to the next one in `Self::build`. - generation: Generation::new(), - zones: vec![], - } - } - - pub(super) fn from_parent(parent: &BlueprintZonesConfig) -> Self { - Self { - // We'll bump this up at build time. - generation: parent.generation, - - zones: parent - .zones - .iter() - .map(|zone| BuilderZoneConfig { - zone: zone.clone(), - state: BuilderZoneState::Unchanged, - }) - .collect(), - } - } - - pub(super) fn add_zone( - &mut self, - zone: BlueprintZoneConfig, - ) -> Result<(), BuilderZonesConfigError> { - if self.zones.iter().any(|z| z.zone.id == zone.id) { - // We shouldn't be trying to add zones that already exist -- - // something went wrong in the planner logic. - return Err(BuilderZonesConfigError::AddExistingZone { - zone_id: zone.id, - }); - }; - - self.zones - .push(BuilderZoneConfig { zone, state: BuilderZoneState::Added }); - Ok(()) - } - - // On success, returns the now-expunged zone and whether or not it was set - // to expunged (as opposed to already being marked expunged). - pub(super) fn expunge_zone( - &mut self, - zone_id: OmicronZoneUuid, - ) -> Result<(&BuilderZoneConfig, bool), BuilderZonesConfigError> { - let zone = self - .zones - .iter_mut() - .find(|zone| zone.zone.id == zone_id) - .ok_or_else(|| { - let mut unmatched = BTreeSet::new(); - unmatched.insert(zone_id); - BuilderZonesConfigError::ExpungeUnmatchedZones { unmatched } - })?; - - // Check that the zone is expungeable. Typically, zones passed - // in here should have had this check done to them already, but - // in case they're not, or in case something else about those - // zones changed in between, check again. - let needs_expunged = !is_already_expunged(&zone.zone, zone.state)?; - - if needs_expunged { - zone.zone.disposition = BlueprintZoneDisposition::Expunged; - zone.state = BuilderZoneState::Modified; - } - - Ok((&*zone, needs_expunged)) - } - - pub(super) fn expunge_zones( - &mut self, - mut zones: BTreeSet, - ) -> Result, BuilderZonesConfigError> { - let mut removed = Vec::new(); - - for zone in &mut self.zones { - if zones.remove(&zone.zone.id) { - // Check that the zone is expungeable. Typically, zones passed - // in here should have had this check done to them already, but - // in case they're not, or in case something else about those - // zones changed in between, check again. - is_already_expunged(&zone.zone, zone.state)?; - zone.zone.disposition = BlueprintZoneDisposition::Expunged; - zone.state = BuilderZoneState::Modified; - removed.push(&zone.zone); - } - } - - // All zones passed in should have been found -- are there any left - // over? - if !zones.is_empty() { - return Err(BuilderZonesConfigError::ExpungeUnmatchedZones { - unmatched: zones, - }); - } - - Ok(removed) - } - - pub(super) fn iter_zones( - &self, - filter: BlueprintZoneFilter, - ) -> impl Iterator { - self.zones.iter().filter(move |z| z.zone().disposition.matches(filter)) - } - - pub(super) fn build(self) -> BlueprintZonesConfig { - // Only bump the generation if any zones have been changed. - let generation = if self - .zones - .iter() - .any(|z| z.state != BuilderZoneState::Unchanged) - { - self.generation.next() - } else { - self.generation - }; - - let mut ret = BlueprintZonesConfig { - generation, - zones: self.zones.into_iter().map(|z| z.zone).collect(), - }; - ret.sort(); - ret - } -} - -pub(super) fn is_already_expunged( - zone: &BlueprintZoneConfig, - state: BuilderZoneState, -) -> Result { - match zone.disposition { - BlueprintZoneDisposition::InService - | BlueprintZoneDisposition::Quiesced => { - if state != BuilderZoneState::Unchanged { - // We shouldn't be trying to expunge zones that have also been - // changed in this blueprint -- something went wrong in the planner - // logic. - return Err(BuilderZonesConfigError::ExpungeModifiedZone { - zone_id: zone.id, - state, - }); - } - Ok(false) - } - BlueprintZoneDisposition::Expunged => { - // Treat expungement as idempotent. - Ok(true) - } - } -} - -#[derive(Debug)] -pub(super) struct BuilderZoneConfig { - zone: BlueprintZoneConfig, - state: BuilderZoneState, -} - -impl BuilderZoneConfig { - pub(super) fn zone(&self) -> &BlueprintZoneConfig { - &self.zone - } - - pub(super) fn state(&self) -> BuilderZoneState { - self.state - } -} - -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub(super) enum BuilderZoneState { - Unchanged, - Modified, - Added, -} - -#[derive(Clone, Debug, PartialEq, Eq, Error)] -pub(super) enum BuilderZonesConfigError { - #[error("attempted to add zone that already exists: {zone_id}")] - AddExistingZone { zone_id: OmicronZoneUuid }, - #[error( - "attempted to expunge zone {zone_id} that was in state {state:?} \ - (can only expunge unchanged zones)" - )] - ExpungeModifiedZone { zone_id: OmicronZoneUuid, state: BuilderZoneState }, - #[error( - "while expunging zones, not all zones provided were found: {unmatched:?}" - )] - ExpungeUnmatchedZones { unmatched: BTreeSet }, -} - -#[cfg(test)] -mod tests { - use std::{ - collections::BTreeMap, - net::{Ipv6Addr, SocketAddrV6}, - }; - - use maplit::btreeset; - use nexus_sled_agent_shared::inventory::ZoneKind; - use nexus_types::deployment::SledDisk; - use nexus_types::external_api::views::PhysicalDiskPolicy; - use nexus_types::external_api::views::PhysicalDiskState; - use nexus_types::{ - deployment::{ - blueprint_zone_type, BlueprintZoneType, SledDetails, SledFilter, - SledResources, - }, - external_api::views::{SledPolicy, SledState}, - }; - use omicron_common::address::Ipv6Subnet; - use omicron_common::disk::DiskIdentity; - use omicron_test_utils::dev::test_setup_log; - use omicron_uuid_kinds::PhysicalDiskUuid; - use omicron_uuid_kinds::ZpoolUuid; - - use crate::{ - blueprint_builder::{test::verify_blueprint, BlueprintBuilder, Ensure}, - example::{ExampleSystemBuilder, SimRngState}, - planner::rng::PlannerRng, - }; - - use super::*; - - /// A test focusing on `BlueprintZonesBuilder` and its internal logic. - #[test] - fn test_builder_zones() { - static TEST_NAME: &str = "blueprint_test_builder_zones"; - let logctx = test_setup_log(TEST_NAME); - - let mut rng = SimRngState::from_seed(TEST_NAME); - let (example, blueprint_initial) = ExampleSystemBuilder::new_with_rng( - &logctx.log, - rng.next_system_rng(), - ) - .build(); - - // Add a completely bare sled to the input. - let (new_sled_id, input2) = { - let mut sled_id_rng = rng.next_sled_id_rng(); - let new_sled_id = sled_id_rng.next(); - - let mut input = example.input.clone().into_builder(); - - input - .add_sled( - new_sled_id, - SledDetails { - policy: SledPolicy::provisionable(), - state: SledState::Active, - resources: SledResources { - subnet: Ipv6Subnet::new( - "fd00:1::".parse().unwrap(), - ), - zpools: BTreeMap::from([( - ZpoolUuid::new_v4(), - ( - SledDisk { - disk_identity: DiskIdentity { - vendor: String::from("fake-vendor"), - serial: String::from("fake-serial"), - model: String::from("fake-model"), - }, - disk_id: PhysicalDiskUuid::new_v4(), - policy: PhysicalDiskPolicy::InService, - state: PhysicalDiskState::Active, - }, - // Datasets: Leave empty - vec![], - ), - )]), - }, - }, - ) - .expect("adding new sled"); - - (new_sled_id, input.build()) - }; - - let existing_sled_id = example - .input - .all_sled_ids(SledFilter::Commissioned) - .next() - .expect("at least one sled present"); - - let mut builder = BlueprintBuilder::new_based_on( - &logctx.log, - &blueprint_initial, - &input2, - &example.collection, - "the_test", - ) - .expect("creating blueprint builder"); - builder.set_rng(PlannerRng::from_seed((TEST_NAME, "bp2"))); - let new_sled_resources = &input2 - .sled_lookup(SledFilter::Commissioned, new_sled_id) - .unwrap() - .resources; - - // Test adding a new sled with an NTP zone. - builder.sled_ensure_disks(new_sled_id, new_sled_resources).unwrap(); - assert_eq!( - builder.sled_ensure_zone_ntp(new_sled_id).unwrap(), - Ensure::Added - ); - - // Iterate over the zones for the sled and ensure that the NTP zone is - // present. - { - let mut zones = builder.zones.current_sled_zones( - new_sled_id, - BlueprintZoneFilter::ShouldBeRunning, - ); - let (_, state) = zones.next().expect("exactly one zone for sled"); - assert!(zones.next().is_none(), "exactly one zone for sled"); - assert_eq!( - state, - BuilderZoneState::Added, - "NTP zone should have been added" - ); - } - - // Now, test adding a new zone (Oximeter, picked arbitrarily) to an - // existing sled. - let filesystem_pool = builder - .sled_select_zpool_for_tests(existing_sled_id, ZoneKind::Oximeter) - .expect("chose zpool for new zone"); - let change = builder.zones.change_sled_zones(existing_sled_id); - let new_zone_id = OmicronZoneUuid::new_v4(); - change - .add_zone(BlueprintZoneConfig { - disposition: BlueprintZoneDisposition::InService, - id: new_zone_id, - filesystem_pool: Some(filesystem_pool), - zone_type: BlueprintZoneType::Oximeter( - blueprint_zone_type::Oximeter { - address: SocketAddrV6::new( - Ipv6Addr::UNSPECIFIED, - 0, - 0, - 0, - ), - }, - ), - }) - .expect("adding new zone"); - - // Attempt to expunge one of the other zones on the sled. - let existing_zone_id = change - .iter_zones(BlueprintZoneFilter::ShouldBeRunning) - .find(|z| z.zone.id != new_zone_id) - .expect("at least one existing zone") - .zone - .id; - change - .expunge_zones(btreeset! { existing_zone_id }) - .expect("expunging existing zone"); - // Do it again to ensure that expunging an already-expunged zone is - // idempotent, even within the same blueprint. - change - .expunge_zones(btreeset! { existing_zone_id }) - .expect("expunging already-expunged zone"); - // But expunging a zone that doesn't exist should fail. - let non_existent_zone_id = OmicronZoneUuid::new_v4(); - let non_existent_set = btreeset! { non_existent_zone_id }; - let error = change - .expunge_zones(non_existent_set.clone()) - .expect_err("expunging non-existent zone"); - assert_eq!( - error, - BuilderZonesConfigError::ExpungeUnmatchedZones { - unmatched: non_existent_set - } - ); - - { - // Iterate over the zones and ensure that the Oximeter zone is - // present, and marked added. - let mut zones = builder.zones.current_sled_zones( - existing_sled_id, - BlueprintZoneFilter::ShouldBeRunning, - ); - zones - .find_map(|(z, state)| { - if z.id == new_zone_id { - assert_eq!( - state, - BuilderZoneState::Added, - "new zone ID {new_zone_id} should be marked added" - ); - Some(()) - } else { - None - } - }) - .expect("new zone ID should be present"); - } - - // Attempt to expunge the newly added Oximeter zone. This should fail - // because we only support expunging zones that are unchanged from the - // parent blueprint. - let error = builder - .zones - .change_sled_zones(existing_sled_id) - .expunge_zones(btreeset! { new_zone_id }) - .expect_err("expunging a new zone should fail"); - assert_eq!( - error, - BuilderZonesConfigError::ExpungeModifiedZone { - zone_id: new_zone_id, - state: BuilderZoneState::Added - } - ); - - // Ensure all datasets are created for the zones we've provisioned - for (sled_id, resources) in - input2.all_sled_resources(SledFilter::Commissioned) - { - builder.sled_ensure_zone_datasets(sled_id, resources).unwrap(); - } - - // Now build the blueprint and ensure that all the changes we described - // above are present. - let blueprint = builder.build(); - verify_blueprint(&blueprint); - let diff = blueprint.diff_since_blueprint(&blueprint_initial); - println!("expecting new NTP and Oximeter zones:\n{}", diff.display()); - - // No sleds were removed. - assert_eq!(diff.sleds_removed.len(), 0); - - // One sled was added. - assert_eq!(diff.sleds_added.len(), 1); - let sled_id = diff.sleds_added.first().unwrap(); - assert_eq!(*sled_id, new_sled_id); - let new_sled_zones = diff.zones.added.get(sled_id).unwrap(); - // The generation number should be newer than the initial default. - assert_eq!( - new_sled_zones.generation_after.unwrap(), - Generation::new().next() - ); - assert_eq!(new_sled_zones.zones.len(), 1); - - // TODO: AJS - See comment above - we don't actually use the control sled anymore - // so the comparison was changed. - // One sled was modified: existing_sled_id - assert_eq!(diff.sleds_modified.len(), 1, "1 sled modified"); - for sled_id in &diff.sleds_modified { - assert_eq!(*sled_id, existing_sled_id); - let added = diff.zones.added.get(sled_id).unwrap(); - assert_eq!( - added.generation_after.unwrap(), - added.generation_before.unwrap().next() - ); - assert_eq!(added.zones.len(), 1); - let added_zone = &added.zones[0]; - assert_eq!(added_zone.id(), new_zone_id); - - assert!(!diff.zones.removed.contains_key(sled_id)); - let modified = diff.zones.modified.get(sled_id).unwrap(); - assert_eq!(modified.zones.len(), 1); - let modified_zone = &modified.zones[0]; - assert_eq!(modified_zone.zone.id(), existing_zone_id); - } - - // Test a no-op change. - { - let mut builder = BlueprintBuilder::new_based_on( - &logctx.log, - &blueprint, - &input2, - &example.collection, - "the_test", - ) - .expect("creating blueprint builder"); - builder.set_rng(PlannerRng::from_seed((TEST_NAME, "bp2"))); - - // This call by itself shouldn't bump the generation number. - builder.zones.change_sled_zones(existing_sled_id); - - let blueprint_noop = builder.build(); - verify_blueprint(&blueprint_noop); - let diff = blueprint_noop.diff_since_blueprint(&blueprint); - println!("expecting a noop:\n{}", diff.display()); - - assert!(diff.sleds_modified.is_empty(), "no sleds modified"); - assert!(diff.sleds_added.is_empty(), "no sleds added"); - assert!(diff.sleds_removed.is_empty(), "no sleds removed"); - } - - logctx.cleanup_successful(); - } -} diff --git a/nexus/reconfigurator/planning/src/blueprint_editor.rs b/nexus/reconfigurator/planning/src/blueprint_editor.rs new file mode 100644 index 0000000000..652b541de1 --- /dev/null +++ b/nexus/reconfigurator/planning/src/blueprint_editor.rs @@ -0,0 +1,14 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! High-level facilities for editing Blueprints +//! +//! See crate-level documentation for details. + +mod sled_editor; + +pub(crate) use sled_editor::DatasetIdsBackfillFromDb; +pub(crate) use sled_editor::EditedSled; +pub(crate) use sled_editor::SledEditError; +pub(crate) use sled_editor::SledEditor; diff --git a/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs new file mode 100644 index 0000000000..13094b97a4 --- /dev/null +++ b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs @@ -0,0 +1,329 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Support for editing the blueprint details of a single sled. + +use crate::blueprint_builder::SledEditCounts; +use crate::planner::PlannerRng; +use illumos_utils::zpool::ZpoolName; +use nexus_types::deployment::blueprint_zone_type; +use nexus_types::deployment::BlueprintDatasetsConfig; +use nexus_types::deployment::BlueprintPhysicalDiskConfig; +use nexus_types::deployment::BlueprintPhysicalDisksConfig; +use nexus_types::deployment::BlueprintZoneConfig; +use nexus_types::deployment::BlueprintZoneFilter; +use nexus_types::deployment::BlueprintZoneType; +use nexus_types::deployment::BlueprintZonesConfig; +use nexus_types::deployment::DiskFilter; +use nexus_types::external_api::views::SledState; +use omicron_uuid_kinds::OmicronZoneUuid; +use omicron_uuid_kinds::PhysicalDiskUuid; + +mod datasets; +mod disks; +mod zones; + +pub(crate) use self::datasets::DatasetIdsBackfillFromDb; + +pub use self::datasets::DatasetsEditError; +pub use self::datasets::MultipleDatasetsOfKind; +pub use self::disks::DisksEditError; +pub use self::disks::DuplicateDiskId; +pub use self::zones::DuplicateZoneId; +pub use self::zones::ZonesEditError; + +use self::datasets::DatasetsEditor; +use self::datasets::PartialDatasetConfig; +use self::disks::DisksEditor; +use self::zones::ZonesEditor; + +#[derive(Debug, thiserror::Error)] +pub enum SledInputError { + #[error(transparent)] + DuplicateZoneId(#[from] DuplicateZoneId), + #[error(transparent)] + DuplicateDiskId(#[from] DuplicateDiskId), + #[error(transparent)] + MultipleDatasetsOfKind(#[from] MultipleDatasetsOfKind), +} + +#[derive(Debug, thiserror::Error)] +pub enum SledEditError { + #[error("failed to edit disks")] + EditDisks(#[from] DisksEditError), + #[error("failed to edit datasets")] + EditDatasetsError(#[from] DatasetsEditError), + #[error("failed to edit zones")] + EditZones(#[from] ZonesEditError), + #[error( + "invalid configuration for zone {zone_id}: \ + filesystem root zpool ({fs_zpool}) and durable dataset zpool \ + ({dur_zpool}) should be the same" + )] + ZoneInvalidZpoolCombination { + zone_id: OmicronZoneUuid, + fs_zpool: ZpoolName, + dur_zpool: ZpoolName, + }, + #[error( + "invalid configuration for zone {zone_id}: \ + zpool ({zpool}) is not present in this sled's disks" + )] + ZoneOnNonexistentZpool { zone_id: OmicronZoneUuid, zpool: ZpoolName }, +} + +#[derive(Debug)] +pub(crate) struct SledEditor { + state: SledState, + zones: ZonesEditor, + disks: DisksEditor, + datasets: DatasetsEditor, +} + +#[derive(Debug)] +pub(crate) struct EditedSled { + pub state: SledState, + pub zones: BlueprintZonesConfig, + pub disks: BlueprintPhysicalDisksConfig, + pub datasets: BlueprintDatasetsConfig, + pub edit_counts: SledEditCounts, +} + +impl SledEditor { + pub fn new( + state: SledState, + zones: BlueprintZonesConfig, + disks: BlueprintPhysicalDisksConfig, + datasets: BlueprintDatasetsConfig, + preexisting_dataset_ids: DatasetIdsBackfillFromDb, + ) -> Result { + Ok(Self { + state, + zones: zones.try_into()?, + disks: disks.try_into()?, + datasets: DatasetsEditor::new(datasets, preexisting_dataset_ids)?, + }) + } + + pub fn new_empty( + state: SledState, + preexisting_dataset_ids: DatasetIdsBackfillFromDb, + ) -> Self { + Self { + state, + zones: ZonesEditor::empty(), + disks: DisksEditor::empty(), + datasets: DatasetsEditor::empty(preexisting_dataset_ids), + } + } + + pub fn finalize(self) -> EditedSled { + let (disks, disks_counts) = self.disks.finalize(); + let (datasets, datasets_counts) = self.datasets.finalize(); + let (zones, zones_counts) = self.zones.finalize(); + EditedSled { + state: self.state, + zones, + disks, + datasets, + edit_counts: SledEditCounts { + disks: disks_counts, + datasets: datasets_counts, + zones: zones_counts, + }, + } + } + + pub fn edit_counts(&self) -> SledEditCounts { + SledEditCounts { + disks: self.disks.edit_counts(), + datasets: self.datasets.edit_counts(), + zones: self.zones.edit_counts(), + } + } + + pub fn set_state(&mut self, new_state: SledState) { + self.state = new_state; + } + + pub fn disks( + &self, + filter: DiskFilter, + ) -> impl Iterator { + self.disks.disks(filter) + } + + pub fn zones( + &self, + filter: BlueprintZoneFilter, + ) -> impl Iterator { + self.zones.zones(filter) + } + + pub fn ensure_disk( + &mut self, + disk: BlueprintPhysicalDiskConfig, + rng: &mut PlannerRng, + ) { + let zpool = ZpoolName::new_external(disk.pool_id); + + self.disks.ensure(disk); + + // Every disk also gets a Debug and Transient Zone Root dataset; ensure + // both of those exist as well. + let debug = PartialDatasetConfig::for_debug(zpool.clone()); + let zone_root = PartialDatasetConfig::for_transient_zone_root(zpool); + + self.datasets.ensure_in_service(debug, rng); + self.datasets.ensure_in_service(zone_root, rng); + } + + pub fn expunge_disk( + &mut self, + disk_id: &PhysicalDiskUuid, + ) -> Result<(), SledEditError> { + let zpool_id = self.disks.expunge(disk_id)?; + + // When we expunge a disk, we must also expunge any datasets on it, and + // any zones that relied on those datasets. + self.datasets.expunge_all_on_zpool(&zpool_id); + self.zones.expunge_all_on_zpool(&zpool_id); + + Ok(()) + } + + pub fn add_zone( + &mut self, + zone: BlueprintZoneConfig, + rng: &mut PlannerRng, + ) -> Result<(), SledEditError> { + // Ensure we can construct the configs for the datasets for this zone. + let datasets = ZoneDatasetConfigs::new(&self.disks, &zone)?; + + // Actually add the zone and its datasets. + self.zones.add_zone(zone)?; + datasets.ensure_in_service(&mut self.datasets, rng); + + Ok(()) + } + + pub fn expunge_zone( + &mut self, + zone_id: &OmicronZoneUuid, + ) -> Result<(), SledEditError> { + let (did_expunge, config) = self.zones.expunge(zone_id)?; + + // If we didn't actually expunge the zone in this edit, we don't + // move on and expunge its datasets. This is to guard against + // accidentally exposing a different zone's datasets (if that zone has + // happens to have the same dataset kind as us and is running on the + // same zpool as us, which is only possible if we were previously + // expunged). + // + // This wouldn't be necessary if `config` tracked its dataset IDs + // explicitly instead of only recording its zpool; once we fix that we + // should be able to remove this check. + if !did_expunge { + return Ok(()); + } + + if let Some(dataset) = config.filesystem_dataset() { + self.datasets.expunge(&dataset.pool().id(), dataset.dataset())?; + } + if let Some(dataset) = config.zone_type.durable_dataset() { + self.datasets + .expunge(&dataset.dataset.pool_name.id(), &dataset.kind)?; + } + + Ok(()) + } + + /// Backwards compatibility / test helper: If we're given a blueprint that + /// has zones but wasn't created via `SledEditor`, it might not have + /// datasets for all its zones. This method backfills them. + pub fn ensure_datasets_for_running_zones( + &mut self, + rng: &mut PlannerRng, + ) -> Result<(), SledEditError> { + for zone in self.zones.zones(BlueprintZoneFilter::ShouldBeRunning) { + ZoneDatasetConfigs::new(&self.disks, zone)? + .ensure_in_service(&mut self.datasets, rng); + } + Ok(()) + } +} + +#[derive(Debug)] +struct ZoneDatasetConfigs { + filesystem: Option, + durable: Option, +} + +impl ZoneDatasetConfigs { + fn new( + disks: &DisksEditor, + zone: &BlueprintZoneConfig, + ) -> Result { + let filesystem_dataset = zone + .filesystem_dataset() + .map(|dataset| PartialDatasetConfig::for_transient_zone(dataset)); + let durable_dataset = zone.zone_type.durable_dataset().map(|dataset| { + // `dataset` records include an optional socket address, which is + // only applicable for durable datasets backing crucible. This this + // is a little fishy and might go away with + // https://github.com/oxidecomputer/omicron/issues/6998. + let address = match &zone.zone_type { + BlueprintZoneType::Crucible( + blueprint_zone_type::Crucible { address, .. }, + ) => Some(*address), + _ => None, + }; + PartialDatasetConfig::for_durable_zone( + dataset.dataset.pool_name.clone(), + dataset.kind, + address, + ) + }); + + // Ensure that if this zone has both kinds of datasets, they reside on + // the same zpool. + if let (Some(fs), Some(dur)) = (&filesystem_dataset, &durable_dataset) { + if fs.zpool() != dur.zpool() { + return Err(SledEditError::ZoneInvalidZpoolCombination { + zone_id: zone.id, + fs_zpool: fs.zpool().clone(), + dur_zpool: dur.zpool().clone(), + }); + } + } + + // Ensure that if we have a zpool, we have a matching disk (i.e., a zone + // can't be added if it has a dataset on a zpool that we don't have) + if let Some(dataset) = + filesystem_dataset.as_ref().or(durable_dataset.as_ref()) + { + if !disks.contains_zpool(&dataset.zpool().id()) { + return Err(SledEditError::ZoneOnNonexistentZpool { + zone_id: zone.id, + zpool: dataset.zpool().clone(), + }); + } + } + + Ok(Self { filesystem: filesystem_dataset, durable: durable_dataset }) + } + + fn ensure_in_service( + self, + datasets: &mut DatasetsEditor, + rng: &mut PlannerRng, + ) { + if let Some(dataset) = self.filesystem { + datasets.ensure_in_service(dataset, rng); + } + if let Some(dataset) = self.durable { + datasets.ensure_in_service(dataset, rng); + } + } +} diff --git a/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/datasets.rs b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/datasets.rs new file mode 100644 index 0000000000..3830f02233 --- /dev/null +++ b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/datasets.rs @@ -0,0 +1,398 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use crate::blueprint_builder::EditCounts; +use crate::planner::PlannerRng; +use illumos_utils::zpool::ZpoolName; +use nexus_types::deployment::BlueprintDatasetConfig; +use nexus_types::deployment::BlueprintDatasetDisposition; +use nexus_types::deployment::BlueprintDatasetsConfig; +use nexus_types::deployment::SledResources; +use nexus_types::deployment::ZpoolFilter; +use omicron_common::api::external::ByteCount; +use omicron_common::api::external::Generation; +use omicron_common::disk::CompressionAlgorithm; +use omicron_common::disk::DatasetKind; +use omicron_common::disk::DatasetName; +use omicron_common::disk::GzipLevel; +use omicron_uuid_kinds::DatasetUuid; +use omicron_uuid_kinds::ZpoolUuid; +use std::collections::btree_map::Entry; +use std::collections::BTreeMap; +use std::net::SocketAddrV6; + +#[derive(Debug, thiserror::Error)] +#[error( + "invalid blueprint input: multiple datasets with kind {kind:?} \ + on zpool {zpool_id}: {id1}, {id2}" +)] +pub struct MultipleDatasetsOfKind { + zpool_id: ZpoolUuid, + kind: DatasetKind, + id1: DatasetUuid, + id2: DatasetUuid, +} + +#[derive(Debug, thiserror::Error)] +pub enum DatasetsEditError { + #[error( + "tried to expunge nonexistent dataset: \ + zpool {zpool_id}, kind {kind}" + )] + ExpungeNonexistentDataset { zpool_id: ZpoolUuid, kind: DatasetKind }, +} + +/// TODO(): In between +/// the addition of datasets to blueprints and knowing all deployed system +/// have _generated_ a blueprint that populates datasets, we are in a sticky +/// situation where a dataset might have already existed in CRDB with an ID, +/// but the blueprint system doesn't know about it. We accept a map of all +/// existing dataset IDs, and then when determining the ID of a dataset, +/// we'll try these in order: +/// +/// 1. Is the dataset in our blueprint already? If so, use its ID. +/// 2. Is the dataset in `preexisting_database_ids`? If so, use that ID. +/// 3. Generate a new random ID. +#[derive(Debug)] +pub(crate) struct DatasetIdsBackfillFromDb( + BTreeMap>, +); + +impl DatasetIdsBackfillFromDb { + pub fn build( + resources: &SledResources, + ) -> Result { + let iter = resources.all_datasets(ZpoolFilter::InService).flat_map( + |(&zpool_id, configs)| { + configs.iter().map(move |config| { + (zpool_id, config.name.dataset().clone(), config.id) + }) + }, + ); + + let mut kind_id_map: BTreeMap< + ZpoolUuid, + BTreeMap, + > = BTreeMap::new(); + + for (zpool_id, kind, dataset_id) in iter { + let dataset_ids_by_kind = kind_id_map.entry(zpool_id).or_default(); + match dataset_ids_by_kind.entry(kind) { + Entry::Vacant(slot) => { + slot.insert(dataset_id); + } + Entry::Occupied(prev) => { + return Err(MultipleDatasetsOfKind { + zpool_id, + kind: prev.key().clone(), + id1: *prev.get(), + id2: dataset_id, + }); + } + } + } + Ok(Self(kind_id_map)) + } + + pub fn empty() -> Self { + Self(BTreeMap::new()) + } +} + +impl DatasetIdsBackfillFromDb { + fn get( + &self, + zpool_id: &ZpoolUuid, + kind: &DatasetKind, + ) -> Option { + self.0.get(zpool_id).and_then(|by_kind| by_kind.get(kind).copied()) + } +} + +/// Container for most of the information needed to construct a +/// `BlueprintDatasetConfig`. +/// +/// Omitted from this set are the disposition (in practice, this will typically +/// be "in service", as one constructs a `PartialDatasetConfig` to describe a +/// dataset that should be in service) and the ID. Dataset IDs are a little +/// tricky at the moment (see `DatasetIdsBackfillFromDb` above), so they're +/// determined internally by `DatasetsEditor`. +#[derive(Debug)] +pub(crate) struct PartialDatasetConfig { + pub name: DatasetName, + pub address: Option, + pub quota: Option, + pub reservation: Option, + pub compression: CompressionAlgorithm, +} + +impl PartialDatasetConfig { + pub fn zpool(&self) -> &ZpoolName { + self.name.pool() + } + + pub fn for_debug(zpool: ZpoolName) -> Self { + const DEBUG_QUOTA_SIZE_GB: u32 = 100; + + Self { + name: DatasetName::new(zpool, DatasetKind::Debug), + address: None, + quota: Some(ByteCount::from_gibibytes_u32(DEBUG_QUOTA_SIZE_GB)), + reservation: None, + compression: CompressionAlgorithm::GzipN { + level: GzipLevel::new::<9>(), + }, + } + } + + pub fn for_transient_zone_root(zpool: ZpoolName) -> Self { + Self { + name: DatasetName::new(zpool, DatasetKind::TransientZoneRoot), + address: None, + quota: None, + reservation: None, + compression: CompressionAlgorithm::Off, + } + } + + pub fn for_transient_zone(name: DatasetName) -> Self { + assert!( + matches!(name.dataset(), DatasetKind::TransientZone { .. }), + "for_transient_zone called with incorrect dataset kind: {name:?}" + ); + Self { + name, + address: None, + quota: None, + reservation: None, + compression: CompressionAlgorithm::Off, + } + } + + pub fn for_durable_zone( + zpool: ZpoolName, + kind: DatasetKind, + address: Option, + ) -> Self { + Self { + name: DatasetName::new(zpool, kind), + address, + quota: None, + reservation: None, + compression: CompressionAlgorithm::Off, + } + } +} + +#[derive(Debug)] +pub(super) struct DatasetsEditor { + preexisting_dataset_ids: DatasetIdsBackfillFromDb, + config: BlueprintDatasetsConfig, + by_zpool_and_kind: BTreeMap>, + counts: EditCounts, +} + +impl DatasetsEditor { + pub fn new( + config: BlueprintDatasetsConfig, + preexisting_dataset_ids: DatasetIdsBackfillFromDb, + ) -> Result { + let mut by_zpool_and_kind = BTreeMap::new(); + for dataset in config.datasets.values() { + let by_kind: &mut BTreeMap<_, _> = + by_zpool_and_kind.entry(dataset.pool.id()).or_default(); + match by_kind.entry(dataset.kind.clone()) { + Entry::Vacant(slot) => { + slot.insert(dataset.id); + } + Entry::Occupied(prev) => { + return Err(MultipleDatasetsOfKind { + zpool_id: dataset.pool.id(), + kind: dataset.kind.clone(), + id1: *prev.get(), + id2: dataset.id, + }); + } + } + } + Ok(Self { + preexisting_dataset_ids, + config, + by_zpool_and_kind, + counts: EditCounts::zeroes(), + }) + } + + pub fn empty(preexisting_dataset_ids: DatasetIdsBackfillFromDb) -> Self { + Self { + preexisting_dataset_ids, + config: BlueprintDatasetsConfig { + generation: Generation::new(), + datasets: BTreeMap::new(), + }, + by_zpool_and_kind: BTreeMap::new(), + counts: EditCounts::zeroes(), + } + } + + pub fn finalize(self) -> (BlueprintDatasetsConfig, EditCounts) { + let mut config = self.config; + if self.counts.has_nonzero_counts() { + config.generation = config.generation.next(); + } + (config, self.counts) + } + + pub fn edit_counts(&self) -> EditCounts { + self.counts + } + + // If there is a dataset of the given `kind` on the given `zpool`, return + // its ID. + // + // This prefers IDs we already have; if we don't have one, it falls back to + // backfilling based on IDs recorded in the database from before blueprints + // tracked datasets (see `DatasetIdsBackfillFromDb` above). + fn get_id( + &self, + zpool: &ZpoolUuid, + kind: &DatasetKind, + ) -> Option { + if let Some(blueprint_id) = self + .by_zpool_and_kind + .get(zpool) + .and_then(|by_kind| by_kind.get(kind).copied()) + { + return Some(blueprint_id); + }; + if let Some(preexisting_database_id) = + self.preexisting_dataset_ids.get(zpool, kind) + { + return Some(preexisting_database_id); + }; + None + } + + fn expunge_impl( + dataset: &mut BlueprintDatasetConfig, + counts: &mut EditCounts, + ) { + match dataset.disposition { + BlueprintDatasetDisposition::InService => { + dataset.disposition = BlueprintDatasetDisposition::Expunged; + counts.expunged += 1; + } + BlueprintDatasetDisposition::Expunged => { + // already expunged; nothing to do + } + } + } + + /// Expunge a dataset identified by its zpool + kind combo. + /// + /// TODO-cleanup This seems fishy. We require that there is at most one + /// dataset of a given `DatasetKind` on a given zpool at a time, but over + /// time we might have had multiple. For example: + /// + /// * Blueprint A: Nexus 1 is on zpool 12 + /// * Blueprint B: Nexus 1 is expunged + /// * Blueprint C: Nexus 2 is added and is placed on zpool 12 + /// + /// When we go to plan Blueprint D, if Nexus 1 is still being carried + /// forward, it will already be expunged (which is fine). If we then try to + /// expunge it again, which should be idempotent, expunging its + /// datasets would incorrectly expunge Nexus 2's datasets (because we'd look + /// up "the dataset with kind Nexus on zpool 12"). We should probably take + /// an explicit dataset ID here, but that would require + /// `BlueprintZoneConfig` to track its dataset IDs explicitly instead of + /// only tracking their zpools. + pub fn expunge( + &mut self, + zpool: &ZpoolUuid, + kind: &DatasetKind, + ) -> Result<(), DatasetsEditError> { + let Some(id) = self + .by_zpool_and_kind + .get(zpool) + .and_then(|by_kind| by_kind.get(kind)) + else { + return Err(DatasetsEditError::ExpungeNonexistentDataset { + zpool_id: *zpool, + kind: kind.clone(), + }); + }; + let dataset = self + .config + .datasets + .get_mut(id) + .expect("by_zpool_and_kind and config out of sync"); + Self::expunge_impl(dataset, &mut self.counts); + Ok(()) + } + + pub fn expunge_all_on_zpool(&mut self, zpool: &ZpoolUuid) { + let Some(by_kind) = self.by_zpool_and_kind.get(zpool) else { + return; + }; + + for id in by_kind.values() { + let dataset = self + .config + .datasets + .get_mut(id) + .expect("by_zpool_and_kind and config out of sync"); + Self::expunge_impl(dataset, &mut self.counts); + } + } + + pub fn ensure_in_service( + &mut self, + dataset: PartialDatasetConfig, + rng: &mut PlannerRng, + ) { + // Convert the partial config into a full config by finding or + // generating its ID. + let dataset = { + let PartialDatasetConfig { + name, + address, + quota, + reservation, + compression, + } = dataset; + let (pool, kind) = name.into_parts(); + let id = self + .get_id(&pool.id(), &kind) + .unwrap_or_else(|| rng.next_dataset()); + BlueprintDatasetConfig { + disposition: BlueprintDatasetDisposition::InService, + id, + pool, + kind, + address, + quota, + reservation, + compression, + } + }; + + // Add or update our config with this new dataset info. + match self.config.datasets.entry(dataset.id) { + Entry::Vacant(slot) => { + self.by_zpool_and_kind + .entry(dataset.pool.id()) + .or_default() + .insert(dataset.kind.clone(), dataset.id); + slot.insert(dataset); + self.counts.added += 1; + } + Entry::Occupied(mut prev) => { + if *prev.get() != dataset { + prev.insert(dataset); + self.counts.updated += 1; + } + } + } + } +} diff --git a/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/disks.rs b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/disks.rs new file mode 100644 index 0000000000..f7c0dcba36 --- /dev/null +++ b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/disks.rs @@ -0,0 +1,145 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use crate::blueprint_builder::EditCounts; +use nexus_types::deployment::BlueprintPhysicalDiskConfig; +use nexus_types::deployment::BlueprintPhysicalDiskDisposition; +use nexus_types::deployment::BlueprintPhysicalDisksConfig; +use nexus_types::deployment::DiskFilter; +use omicron_common::api::external::Generation; +use omicron_uuid_kinds::PhysicalDiskUuid; +use omicron_uuid_kinds::ZpoolUuid; +use std::collections::btree_map::Entry; +use std::collections::BTreeMap; + +#[derive(Debug, thiserror::Error)] +pub enum DisksEditError { + #[error("tried to expunge nonexistent disk {id}")] + ExpungeNonexistentDisk { id: PhysicalDiskUuid }, +} + +#[derive(Debug, thiserror::Error)] +#[error( + "invalid blueprint input: duplicate disk ID {id} \ + (zpools: {zpool1:?}, {zpool2:?})" +)] +pub struct DuplicateDiskId { + pub id: PhysicalDiskUuid, + pub zpool1: ZpoolUuid, + pub zpool2: ZpoolUuid, +} + +#[derive(Debug)] +pub(super) struct DisksEditor { + generation: Generation, + disks: BTreeMap, + counts: EditCounts, +} + +impl DisksEditor { + pub fn empty() -> Self { + Self { + generation: Generation::new(), + disks: BTreeMap::new(), + counts: EditCounts::zeroes(), + } + } + + pub fn finalize(self) -> (BlueprintPhysicalDisksConfig, EditCounts) { + let mut generation = self.generation; + if self.counts.has_nonzero_counts() { + generation = generation.next(); + } + + ( + BlueprintPhysicalDisksConfig { + generation, + disks: self.disks.into_values().collect(), + }, + self.counts, + ) + } + + pub fn edit_counts(&self) -> EditCounts { + self.counts + } + + pub fn disks( + &self, + filter: DiskFilter, + ) -> impl Iterator { + self.disks + .values() + .filter(move |config| config.disposition.matches(filter)) + } + + pub fn contains_zpool(&self, zpool_id: &ZpoolUuid) -> bool { + self.disks.values().any(|disk| disk.pool_id == *zpool_id) + } + + pub fn ensure(&mut self, disk: BlueprintPhysicalDiskConfig) { + match self.disks.entry(disk.id) { + Entry::Vacant(slot) => { + slot.insert(disk); + self.counts.added += 1; + } + Entry::Occupied(mut slot) => { + if *slot.get() != disk { + slot.insert(disk); + self.counts.updated += 1; + } + } + } + } + + pub fn expunge( + &mut self, + disk_id: &PhysicalDiskUuid, + ) -> Result { + let config = self.disks.get_mut(disk_id).ok_or_else(|| { + DisksEditError::ExpungeNonexistentDisk { id: *disk_id } + })?; + + match config.disposition { + BlueprintPhysicalDiskDisposition::InService => { + config.disposition = BlueprintPhysicalDiskDisposition::Expunged; + self.counts.expunged += 1; + } + BlueprintPhysicalDiskDisposition::Expunged => { + // expunge is idempotent; do nothing + } + } + + Ok(config.pool_id) + } +} + +impl TryFrom for DisksEditor { + type Error = DuplicateDiskId; + + fn try_from( + config: BlueprintPhysicalDisksConfig, + ) -> Result { + let mut disks = BTreeMap::new(); + for disk in config.disks { + match disks.entry(disk.id) { + Entry::Vacant(slot) => { + slot.insert(disk); + } + Entry::Occupied(prev) => { + return Err(DuplicateDiskId { + id: disk.id, + zpool1: disk.pool_id, + zpool2: prev.get().pool_id, + }); + } + } + } + Ok(Self { + generation: config.generation, + disks, + counts: EditCounts::zeroes(), + }) + } +} diff --git a/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/zones.rs b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/zones.rs new file mode 100644 index 0000000000..5a5c7a1807 --- /dev/null +++ b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/zones.rs @@ -0,0 +1,181 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use crate::blueprint_builder::EditCounts; +use nexus_sled_agent_shared::inventory::ZoneKind; +use nexus_types::deployment::BlueprintZoneConfig; +use nexus_types::deployment::BlueprintZoneDisposition; +use nexus_types::deployment::BlueprintZoneFilter; +use nexus_types::deployment::BlueprintZonesConfig; +use omicron_common::api::external::Generation; +use omicron_uuid_kinds::OmicronZoneUuid; +use omicron_uuid_kinds::ZpoolUuid; +use std::collections::btree_map::Entry; +use std::collections::BTreeMap; + +#[derive(Debug, thiserror::Error)] +pub enum ZonesEditError { + #[error( + "tried to add duplicate zone ID {id} (kinds: {kind1:?}, {kind2:?})" + )] + AddDuplicateZoneId { id: OmicronZoneUuid, kind1: ZoneKind, kind2: ZoneKind }, + #[error("tried to expunge nonexistent zone {id}")] + ExpungeNonexistentZone { id: OmicronZoneUuid }, +} + +#[derive(Debug, thiserror::Error)] +#[error( + "invalid blueprint input: duplicate zone ID {id} \ + (kinds: {kind1:?}, {kind2:?})" +)] +pub struct DuplicateZoneId { + pub id: OmicronZoneUuid, + pub kind1: ZoneKind, + pub kind2: ZoneKind, +} + +#[derive(Debug)] +pub(super) struct ZonesEditor { + generation: Generation, + zones: BTreeMap, + counts: EditCounts, +} + +impl ZonesEditor { + pub fn empty() -> Self { + Self { + generation: Generation::new(), + zones: BTreeMap::new(), + counts: EditCounts::zeroes(), + } + } + + pub fn finalize(self) -> (BlueprintZonesConfig, EditCounts) { + let mut generation = self.generation; + if self.counts.has_nonzero_counts() { + generation = generation.next(); + } + let mut config = BlueprintZonesConfig { + generation, + zones: self.zones.into_values().collect(), + }; + config.sort(); + (config, self.counts) + } + + pub fn edit_counts(&self) -> EditCounts { + self.counts + } + + pub fn zones( + &self, + filter: BlueprintZoneFilter, + ) -> impl Iterator { + self.zones + .values() + .filter(move |config| config.disposition.matches(filter)) + } + + pub fn add_zone( + &mut self, + zone: BlueprintZoneConfig, + ) -> Result<(), ZonesEditError> { + match self.zones.entry(zone.id) { + Entry::Vacant(slot) => { + slot.insert(zone); + self.counts.added += 1; + Ok(()) + } + Entry::Occupied(prev) => { + // We shouldn't be trying to add zones that already exist -- + // something went wrong in the planner logic. + Err(ZonesEditError::AddDuplicateZoneId { + id: zone.id, + kind1: zone.zone_type.kind(), + kind2: prev.get().zone_type.kind(), + }) + } + } + } + + /// Expunge a zone, returning `true` if the zone was expunged and `false` if + /// the zone was already expunged, along with the updated zone config. + pub fn expunge( + &mut self, + zone_id: &OmicronZoneUuid, + ) -> Result<(bool, &BlueprintZoneConfig), ZonesEditError> { + let config = self.zones.get_mut(zone_id).ok_or_else(|| { + ZonesEditError::ExpungeNonexistentZone { id: *zone_id } + })?; + + let did_expunge = Self::expunge_impl(config, &mut self.counts); + + Ok((did_expunge, &*config)) + } + + fn expunge_impl( + config: &mut BlueprintZoneConfig, + counts: &mut EditCounts, + ) -> bool { + match config.disposition { + BlueprintZoneDisposition::InService + | BlueprintZoneDisposition::Quiesced => { + config.disposition = BlueprintZoneDisposition::Expunged; + counts.expunged += 1; + true + } + BlueprintZoneDisposition::Expunged => { + // expunge is idempotent; do nothing + false + } + } + } + + pub fn expunge_all_on_zpool(&mut self, zpool: &ZpoolUuid) { + for config in self.zones.values_mut() { + // Expunge this zone if its filesystem or durable dataset are on + // this zpool. (If it has both, they should be on the _same_ zpool, + // but that's not strictly required by this method - we'll expunge a + // zone that depends on this zpool in any way.) + let fs_is_on_zpool = config + .filesystem_pool + .as_ref() + .map_or(false, |pool| pool.id() == *zpool); + let dd_is_on_zpool = config + .zone_type + .durable_zpool() + .map_or(false, |pool| pool.id() == *zpool); + if fs_is_on_zpool || dd_is_on_zpool { + Self::expunge_impl(config, &mut self.counts); + } + } + } +} + +impl TryFrom for ZonesEditor { + type Error = DuplicateZoneId; + + fn try_from(config: BlueprintZonesConfig) -> Result { + let mut zones = BTreeMap::new(); + for zone in config.zones { + match zones.entry(zone.id) { + Entry::Vacant(slot) => { + slot.insert(zone); + } + Entry::Occupied(prev) => { + return Err(DuplicateZoneId { + id: zone.id, + kind1: zone.zone_type.kind(), + kind2: prev.get().zone_type.kind(), + }); + } + } + } + Ok(Self { + generation: config.generation, + zones, + counts: EditCounts::zeroes(), + }) + } +} diff --git a/nexus/reconfigurator/planning/src/example.rs b/nexus/reconfigurator/planning/src/example.rs index 3848934d19..dfba3f9992 100644 --- a/nexus/reconfigurator/planning/src/example.rs +++ b/nexus/reconfigurator/planning/src/example.rs @@ -453,9 +453,7 @@ impl ExampleSystemBuilder { .unwrap(); } } - builder - .sled_ensure_zone_datasets(sled_id, &sled_resources) - .unwrap(); + builder.sled_ensure_zone_datasets(sled_id).unwrap(); } let blueprint = builder.build(); diff --git a/nexus/reconfigurator/planning/src/lib.rs b/nexus/reconfigurator/planning/src/lib.rs index a5a47c933d..f6c521c0f8 100644 --- a/nexus/reconfigurator/planning/src/lib.rs +++ b/nexus/reconfigurator/planning/src/lib.rs @@ -7,6 +7,7 @@ //! See docs/reconfigurator.adoc for an overview. pub mod blueprint_builder; +pub mod blueprint_editor; pub mod example; mod ip_allocator; pub mod planner; diff --git a/nexus/reconfigurator/planning/src/planner.rs b/nexus/reconfigurator/planning/src/planner.rs index 9bdb29048b..56fc671667 100644 --- a/nexus/reconfigurator/planning/src/planner.rs +++ b/nexus/reconfigurator/planning/src/planner.rs @@ -160,7 +160,7 @@ impl<'a> Planner<'a> { if all_zones_expunged && num_instances_assigned == 0 { self.blueprint - .set_sled_state(sled_id, SledState::Decommissioned); + .set_sled_state(sled_id, SledState::Decommissioned)?; } } @@ -362,17 +362,13 @@ impl<'a> Planner<'a> { } fn do_plan_datasets(&mut self) -> Result<(), Error> { - for (sled_id, sled_resources) in - self.input.all_sled_resources(SledFilter::InService) - { + for sled_id in self.input.all_sled_ids(SledFilter::InService) { if let EnsureMultiple::Changed { added, updated, expunged, removed, - } = self - .blueprint - .sled_ensure_zone_datasets(sled_id, &sled_resources)? + } = self.blueprint.sled_ensure_zone_datasets(sled_id)? { info!( &self.log, diff --git a/nexus/types/Cargo.toml b/nexus/types/Cargo.toml index 5f21652feb..8990b0b83b 100644 --- a/nexus/types/Cargo.toml +++ b/nexus/types/Cargo.toml @@ -21,6 +21,7 @@ dropshot.workspace = true futures.workspace = true http.workspace = true humantime.workspace = true +illumos-utils.workspace = true ipnetwork.workspace = true newtype_derive.workspace = true omicron-uuid-kinds.workspace = true diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index 3a17f69863..a487fea2ce 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -736,6 +736,17 @@ impl BlueprintZoneConfig { pub fn underlay_ip(&self) -> Ipv6Addr { self.zone_type.underlay_ip() } + + /// Returns the dataset used for the the zone's (transient) root filesystem. + pub fn filesystem_dataset(&self) -> Option { + let pool_name = self.filesystem_pool.clone()?; + let name = illumos_utils::zone::zone_name( + self.zone_type.kind().zone_prefix(), + Some(self.id), + ); + let kind = DatasetKind::TransientZone { name }; + Some(DatasetName::new(pool_name, kind)) + } } impl From for OmicronZoneConfig { @@ -917,6 +928,26 @@ pub enum BlueprintPhysicalDiskDisposition { Expunged, } +impl BlueprintPhysicalDiskDisposition { + /// Returns true if the disk disposition matches this filter. + pub fn matches(self, filter: DiskFilter) -> bool { + match self { + Self::InService => match filter { + DiskFilter::All => true, + DiskFilter::InService => true, + // TODO remove this variant? + DiskFilter::ExpungedButActive => false, + }, + Self::Expunged => match filter { + DiskFilter::All => true, + DiskFilter::InService => false, + // TODO remove this variant? + DiskFilter::ExpungedButActive => true, + }, + } + } +} + /// Information about an Omicron physical disk as recorded in a bluerprint. #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq)] pub struct BlueprintPhysicalDiskConfig {