From f4fe3a9081efd9587fab75a8cb28f7047fa6b3a7 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Mon, 6 Jan 2025 16:35:12 -0500 Subject: [PATCH 1/6] rename DatasetName::dataset() -> kind() Fixes #7115. --- common/src/disk.rs | 4 +--- nexus/reconfigurator/blippy/src/checks.rs | 2 +- .../planning/src/blueprint_editor/sled_editor.rs | 2 +- .../planning/src/blueprint_editor/sled_editor/datasets.rs | 4 ++-- sled-agent/src/artifact_store.rs | 2 +- sled-agent/src/rack_setup/service.rs | 6 +++--- sled-agent/src/services.rs | 2 +- sled-storage/src/manager.rs | 2 +- 8 files changed, 11 insertions(+), 13 deletions(-) diff --git a/common/src/disk.rs b/common/src/disk.rs index df6efe31963..c2e341f4d69 100644 --- a/common/src/disk.rs +++ b/common/src/disk.rs @@ -112,9 +112,7 @@ impl DatasetName { &self.pool_name } - // TODO(https://github.com/oxidecomputer/omicron/issues/7115): Rename - // this to "kind? - pub fn dataset(&self) -> &DatasetKind { + pub fn kind(&self) -> &DatasetKind { &self.kind } diff --git a/nexus/reconfigurator/blippy/src/checks.rs b/nexus/reconfigurator/blippy/src/checks.rs index f5673cb77c7..b7a6a5ec18f 100644 --- a/nexus/reconfigurator/blippy/src/checks.rs +++ b/nexus/reconfigurator/blippy/src/checks.rs @@ -431,7 +431,7 @@ fn check_datasets(blippy: &mut Blippy<'_>) { Some(dataset) => { match sled_datasets .get(&dataset.pool().id()) - .and_then(|by_zpool| by_zpool.get(dataset.dataset())) + .and_then(|by_zpool| by_zpool.get(dataset.kind())) { Some(dataset) => { expected_datasets.insert(dataset.id); diff --git a/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs index 995cc306a5d..3355390dc9e 100644 --- a/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs +++ b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs @@ -460,7 +460,7 @@ impl ActiveSledEditor { } if let Some(dataset) = config.filesystem_dataset() { - self.datasets.expunge(&dataset.pool().id(), dataset.dataset())?; + self.datasets.expunge(&dataset.pool().id(), dataset.kind())?; } if let Some(dataset) = config.zone_type.durable_dataset() { self.datasets diff --git a/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/datasets.rs b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/datasets.rs index 211af59aabe..d164a26a843 100644 --- a/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/datasets.rs +++ b/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor/datasets.rs @@ -70,7 +70,7 @@ impl DatasetIdsBackfillFromDb { 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) + (zpool_id, config.name.kind().clone(), config.id) }) }, ); @@ -162,7 +162,7 @@ impl PartialDatasetConfig { pub fn for_transient_zone(name: DatasetName) -> Self { assert!( - matches!(name.dataset(), DatasetKind::TransientZone { .. }), + matches!(name.kind(), DatasetKind::TransientZone { .. }), "for_transient_zone called with incorrect dataset kind: {name:?}" ); Self { diff --git a/sled-agent/src/artifact_store.rs b/sled-agent/src/artifact_store.rs index 5b78c0099d1..dc0331457db 100644 --- a/sled-agent/src/artifact_store.rs +++ b/sled-agent/src/artifact_store.rs @@ -453,7 +453,7 @@ pub(crate) fn filter_dataset_mountpoints( config .datasets .into_values() - .filter(|dataset| *dataset.name.dataset() == DatasetKind::Update) + .filter(|dataset| *dataset.name.kind() == DatasetKind::Update) .map(|dataset| dataset.name.mountpoint(root)) } diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 7bb6bffe024..6ca4e8bdf92 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -834,13 +834,13 @@ impl ServiceInner { // to usage by specific zones. for dataset in sled_config.datasets.datasets.values() { let duplicate = datasets.insert( - (dataset.name.pool().id(), dataset.name.dataset().clone()), + (dataset.name.pool().id(), dataset.name.kind().clone()), NexusTypes::DatasetCreateRequest { zpool_id: dataset.name.pool().id().into_untyped_uuid(), dataset_id: dataset.id, request: NexusTypes::DatasetPutRequest { address: None, - kind: dataset.name.dataset().clone(), + kind: dataset.name.kind().clone(), }, }, ); @@ -1559,7 +1559,7 @@ pub(crate) fn build_initial_blueprint_from_sled_configs( disposition: BlueprintDatasetDisposition::InService, id: d.id, pool: d.name.pool().clone(), - kind: d.name.dataset().clone(), + kind: d.name.kind().clone(), address, compression: d.inner.compression, quota: d.inner.quota, diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 7536824da32..54b0a559cd3 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -3703,7 +3703,7 @@ impl ServiceManager { }; check_property("zoned", zoned, "on")?; check_property("canmount", canmount, "on")?; - if dataset.dataset().dataset_should_be_encrypted() { + if dataset.kind().dataset_should_be_encrypted() { check_property("encryption", encryption, "aes-256-gcm")?; } diff --git a/sled-storage/src/manager.rs b/sled-storage/src/manager.rs index d6ffd42d0a0..f1739fe5f47 100644 --- a/sled-storage/src/manager.rs +++ b/sled-storage/src/manager.rs @@ -1054,7 +1054,7 @@ impl StorageManager { let mountpoint_root = &self.resources.disks().mount_config().root; let mountpoint_path = config.name.mountpoint(mountpoint_root); let details = DatasetCreationDetails { - zoned: config.name.dataset().zoned(), + zoned: config.name.kind().zoned(), mountpoint: Mountpoint::Path(mountpoint_path), full_name: config.name.full_name(), }; From 9bd87be62aa8550330329f19cec72a13ddb884e5 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Mon, 6 Jan 2025 17:18:20 -0500 Subject: [PATCH 2/6] RSS: only set addresses on Crucible datasets --- sled-agent/src/rack_setup/plan/sled.rs | 2 +- sled-agent/src/rack_setup/service.rs | 110 ++++++++++++++++++++----- sled-agent/src/sim/server.rs | 12 +-- 3 files changed, 96 insertions(+), 28 deletions(-) diff --git a/sled-agent/src/rack_setup/plan/sled.rs b/sled-agent/src/rack_setup/plan/sled.rs index 3d06a91e27f..eb447e605ca 100644 --- a/sled-agent/src/rack_setup/plan/sled.rs +++ b/sled-agent/src/rack_setup/plan/sled.rs @@ -26,7 +26,7 @@ pub struct Plan { } impl Plan { - pub async fn create( + pub fn create( log: &Logger, config: &Config, bootstrap_addrs: BTreeSet, diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 6ca4e8bdf92..5d87440da69 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -797,7 +797,8 @@ impl ServiceInner { let blueprint = build_initial_blueprint_from_plan( &sled_configs_by_id, service_plan, - ); + ) + .map_err(SetupServiceError::ConvertPlanToBlueprint)?; info!(self.log, "Nexus address: {}", nexus_address.to_string()); @@ -1287,8 +1288,7 @@ impl ServiceInner { config, bootstrap_addrs, config.trust_quorum_peers.is_some(), - ) - .await; + ); let config = &sled_plan.config; rss_step.update(RssStep::InitTrustQuorum); @@ -1520,7 +1520,7 @@ fn build_sled_configs_by_id( fn build_initial_blueprint_from_plan( sled_configs_by_id: &BTreeMap, service_plan: &ServicePlan, -) -> Blueprint { +) -> anyhow::Result { build_initial_blueprint_from_sled_configs( sled_configs_by_id, service_plan.dns_config.generation, @@ -1530,7 +1530,7 @@ fn build_initial_blueprint_from_plan( pub(crate) fn build_initial_blueprint_from_sled_configs( sled_configs_by_id: &BTreeMap, internal_dns_version: Generation, -) -> Blueprint { +) -> anyhow::Result { let blueprint_disks: BTreeMap<_, _> = sled_configs_by_id .iter() .map(|(sled_id, sled_config)| (*sled_id, sled_config.disks.clone())) @@ -1541,17 +1541,29 @@ pub(crate) fn build_initial_blueprint_from_sled_configs( let mut datasets = BTreeMap::new(); for d in sled_config.datasets.datasets.values() { // Only the "Crucible" dataset needs to know the address - let address = sled_config.zones.iter().find_map(|z| { - if let BlueprintZoneType::Crucible( - blueprint_zone_type::Crucible { address, dataset }, - ) = &z.zone_type - { - if &dataset.pool_name == d.name.pool() { - return Some(*address); - } - }; + let address = if *d.name.kind() == DatasetKind::Crucible { + let address = sled_config.zones.iter().find_map(|z| { + if let BlueprintZoneType::Crucible( + blueprint_zone_type::Crucible { address, dataset }, + ) = &z.zone_type + { + if &dataset.pool_name == d.name.pool() { + return Some(*address); + } + }; + None + }); + if address.is_some() { + address + } else { + bail!( + "could not find Crucible zone for zpool {}", + d.name.pool() + ) + } + } else { None - }); + }; datasets.insert( d.id, @@ -1599,7 +1611,7 @@ pub(crate) fn build_initial_blueprint_from_sled_configs( sled_state.insert(*sled_id, SledState::Active); } - Blueprint { + Ok(Blueprint { id: Uuid::new_v4(), blueprint_zones, blueprint_disks, @@ -1621,7 +1633,7 @@ pub(crate) fn build_initial_blueprint_from_sled_configs( time_created: Utc::now(), creator: "RSS".to_string(), comment: "initial blueprint from rack setup".to_string(), - } + }) } /// Facilitates creating a sequence of OmicronZonesConfig objects for each sled @@ -1719,7 +1731,7 @@ impl<'a> OmicronZonesConfigGenerator<'a> { #[cfg(test)] mod test { - use super::{Config, OmicronZonesConfigGenerator}; + use super::*; use crate::rack_setup::plan::service::{Plan as ServicePlan, SledInfo}; use nexus_sled_agent_shared::inventory::{ Baseboard, Inventory, InventoryDisk, OmicronZoneType, @@ -1778,9 +1790,8 @@ mod test { ) } - fn make_test_service_plan() -> ServicePlan { - let rss_config = Config::test_config(); - let fake_sleds = vec![ + fn make_fake_sleds() -> Vec { + vec![ make_sled_info( SledUuid::new_v4(), Ipv6Subnet::::new( @@ -1795,7 +1806,12 @@ mod test { ), 5, ), - ]; + ] + } + + fn make_test_service_plan() -> ServicePlan { + let rss_config = Config::test_config(); + let fake_sleds = make_fake_sleds(); let service_plan = ServicePlan::create_transient(&rss_config, fake_sleds) .expect("failed to create service plan"); @@ -1913,4 +1929,54 @@ mod test { } assert!(v6_nfound > v5_nfound); } + + #[test] + fn only_crucible_datasets_have_addresses() { + let logctx = omicron_test_utils::dev::test_setup_log( + "only_crucible_datasets_have_addresses", + ); + + let fake_sleds = make_fake_sleds(); + + let rss_config = Config::test_config(); + let use_trust_quorum = false; + let sled_plan = SledPlan::create( + &logctx.log, + &rss_config, + fake_sleds + .iter() + .map(|sled_info| *sled_info.sled_address.ip()) + .collect(), + use_trust_quorum, + ); + let service_plan = + ServicePlan::create_transient(&rss_config, fake_sleds) + .expect("created service plan"); + + let sled_configs_by_id = + build_sled_configs_by_id(&sled_plan, &service_plan) + .expect("built sled configs"); + let blueprint = build_initial_blueprint_from_plan( + &sled_configs_by_id, + &service_plan, + ) + .expect("built blueprint"); + + for dataset_config in blueprint.blueprint_datasets.into_values() { + for dataset in dataset_config.datasets.values() { + match dataset.kind { + DatasetKind::Crucible => assert!( + dataset.address.is_some(), + "crucible datasets should have addresses" + ), + _ => assert_eq!( + dataset.address, None, + "non-crucible datasets should not have addresses" + ), + } + } + } + + logctx.cleanup_successful(); + } } diff --git a/sled-agent/src/sim/server.rs b/sled-agent/src/sim/server.rs index 3833d5ca7c6..0d3a682b57e 100644 --- a/sled-agent/src/sim/server.rs +++ b/sled-agent/src/sim/server.rs @@ -15,7 +15,7 @@ use crate::rack_setup::{ from_ipaddr_to_external_floating_ip, from_sockaddr_to_external_floating_addr, }; -use anyhow::anyhow; +use anyhow::{anyhow, Context as _}; use crucible_agent_client::types::State as RegionState; use illumos_utils::zpool::ZpoolName; use internal_dns_types::config::DnsConfigBuilder; @@ -564,11 +564,13 @@ pub async fn run_standalone_server( }, ); + let blueprint = build_initial_blueprint_from_sled_configs( + &sled_configs, + internal_dns_version, + ) + .context("could not construct initial blueprint")?; let rack_init_request = NexusTypes::RackInitializationRequest { - blueprint: build_initial_blueprint_from_sled_configs( - &sled_configs, - internal_dns_version, - ), + blueprint, physical_disks, zpools, datasets, From 53696f2a5375489a4b1ff13f4ee05caac4281da4 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 7 Jan 2025 09:27:49 -0500 Subject: [PATCH 3/6] fix other dataset() -> kind() changes in tests --- nexus/reconfigurator/blippy/src/checks.rs | 4 ++-- .../reconfigurator/planning/src/blueprint_builder/builder.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nexus/reconfigurator/blippy/src/checks.rs b/nexus/reconfigurator/blippy/src/checks.rs index b7a6a5ec18f..bd79d8fd450 100644 --- a/nexus/reconfigurator/blippy/src/checks.rs +++ b/nexus/reconfigurator/blippy/src/checks.rs @@ -1294,7 +1294,7 @@ mod tests { == durable_zone.zone_type.durable_dataset().unwrap().kind); let root_dataset = root_zone.filesystem_dataset().unwrap(); let matches_root = (dataset.pool == *root_dataset.pool()) - && (dataset.kind == *root_dataset.dataset()); + && (dataset.kind == *root_dataset.kind()); !matches_durable && !matches_root }); @@ -1465,7 +1465,7 @@ mod tests { if (dataset.pool == durable_dataset.dataset.pool_name && dataset.kind == durable_dataset.kind) || (dataset.pool == *root_dataset.pool() - && dataset.kind == *root_dataset.dataset()) + && dataset.kind == *root_dataset.kind()) { Some(Note { severity: Severity::Fatal, diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs index 0aaadb624d7..09a6de84efd 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs @@ -2790,7 +2790,7 @@ pub mod test { { for dataset in dataset_configs { let id = dataset.id; - let kind = dataset.name.dataset(); + let kind = dataset.name.kind(); let by_kind: &mut BTreeMap<_, _> = input_dataset_ids.entry(*zpool_id).or_default(); let prev = by_kind.insert(kind.clone(), id); From dabf6d499b21405e0ac2b3975f72c96eb0f3090a Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 7 Jan 2025 10:47:17 -0500 Subject: [PATCH 4/6] add blippy checks for dataset addresses --- nexus/reconfigurator/blippy/src/blippy.rs | 30 ++++ nexus/reconfigurator/blippy/src/checks.rs | 169 ++++++++++++++++++++++ 2 files changed, 199 insertions(+) diff --git a/nexus/reconfigurator/blippy/src/blippy.rs b/nexus/reconfigurator/blippy/src/blippy.rs index 9e9cc84b32b..4e105fa9cac 100644 --- a/nexus/reconfigurator/blippy/src/blippy.rs +++ b/nexus/reconfigurator/blippy/src/blippy.rs @@ -19,6 +19,7 @@ use omicron_uuid_kinds::SledUuid; use omicron_uuid_kinds::ZpoolUuid; use std::collections::BTreeSet; use std::net::IpAddr; +use std::net::SocketAddrV6; #[derive(Debug, Clone, PartialEq, Eq)] pub struct Note { @@ -169,6 +170,17 @@ pub enum SledKind { OrphanedDataset { dataset: BlueprintDatasetConfig }, /// A dataset claims to be on a zpool that does not exist. DatasetOnNonexistentZpool { dataset: BlueprintDatasetConfig }, + /// A Crucible dataset does not have its `address` set to its corresponding + /// Crucible zone. + CrucibleDatasetWithIncorrectAddress { + dataset: BlueprintDatasetConfig, + expected_address: SocketAddrV6, + }, + /// A non-Crucible dataset has an address. + NonCrucibleDatasetWithAddress { + dataset: BlueprintDatasetConfig, + address: SocketAddrV6, + }, } impl fmt::Display for SledKind { @@ -352,6 +364,24 @@ impl fmt::Display for SledKind { dataset.kind, dataset.id, dataset.pool ) } + SledKind::CrucibleDatasetWithIncorrectAddress { + dataset, + expected_address, + } => { + write!( + f, + "Crucible dataset {} has bad address {:?} (expected {})", + dataset.id, dataset.address, expected_address, + ) + } + SledKind::NonCrucibleDatasetWithAddress { dataset, address } => { + write!( + f, + "non-Crucible dataset ({:?} {}) has an address: {} \ + (only Crucible datasets should have addresses)", + dataset.kind, dataset.id, address, + ) + } } } } diff --git a/nexus/reconfigurator/blippy/src/checks.rs b/nexus/reconfigurator/blippy/src/checks.rs index bd79d8fd450..faf456c73ae 100644 --- a/nexus/reconfigurator/blippy/src/checks.rs +++ b/nexus/reconfigurator/blippy/src/checks.rs @@ -6,10 +6,12 @@ use crate::blippy::Blippy; use crate::blippy::Severity; use crate::blippy::SledKind; use nexus_sled_agent_shared::inventory::ZoneKind; +use nexus_types::deployment::blueprint_zone_type; use nexus_types::deployment::BlueprintDatasetConfig; use nexus_types::deployment::BlueprintDatasetFilter; use nexus_types::deployment::BlueprintZoneConfig; use nexus_types::deployment::BlueprintZoneFilter; +use nexus_types::deployment::BlueprintZoneType; use nexus_types::deployment::OmicronZoneExternalIp; use omicron_common::address::DnsSubnet; use omicron_common::address::Ipv6Subnet; @@ -413,6 +415,10 @@ fn check_datasets(blippy: &mut Blippy<'_>) { } } + // In a check below, we want to look up Crucible zones by zpool; build that + // map as we perform the next set of checks. + let mut crucible_zone_by_zpool = BTreeMap::new(); + // There should be a dataset for every dataset referenced by a running zone // (filesystem or durable). for (sled_id, zone_config) in blippy @@ -471,6 +477,21 @@ fn check_datasets(blippy: &mut Blippy<'_>) { ); } } + + if dataset.kind == DatasetKind::Crucible { + match &zone_config.zone_type { + BlueprintZoneType::Crucible(crucible_zone_config) => { + crucible_zone_by_zpool.insert( + dataset.dataset.pool_name.id(), + crucible_zone_config, + ); + } + _ => unreachable!( + "zone_type.durable_dataset() returned Crucible for \ + non-Crucible zone type" + ), + } + } } } @@ -533,6 +554,50 @@ fn check_datasets(blippy: &mut Blippy<'_>) { continue; } } + + // All Crucible datasets should have their address set to the address of the + // Crucible zone on their same zpool, and all non-Crucible datasets should + // not have addresses. + for (sled_id, dataset) in blippy + .blueprint() + .all_omicron_datasets(BlueprintDatasetFilter::InService) + { + match dataset.kind { + DatasetKind::Crucible => { + let Some(blueprint_zone_type::Crucible { address, .. }) = + crucible_zone_by_zpool.get(&dataset.pool.id()) + else { + // We already checked above that all datasets have + // corresponding zones, so a failure to find the zone for + // this dataset would have already produced a note; just + // skip it. + continue; + }; + if dataset.address != Some(*address) { + blippy.push_sled_note( + sled_id, + Severity::Fatal, + SledKind::CrucibleDatasetWithIncorrectAddress { + dataset: dataset.clone(), + expected_address: *address, + }, + ); + } + } + _ => { + if let Some(address) = dataset.address { + blippy.push_sled_note( + sled_id, + Severity::Fatal, + SledKind::NonCrucibleDatasetWithAddress { + dataset: dataset.clone(), + address, + }, + ); + } + } + } + } } #[cfg(test)] @@ -1561,4 +1626,108 @@ mod tests { logctx.cleanup_successful(); } + + #[test] + fn test_dataset_with_bad_address() { + static TEST_NAME: &str = "test_dataset_with_bad_address"; + let logctx = test_setup_log(TEST_NAME); + let (_, _, mut blueprint) = example(&logctx.log, TEST_NAME); + + let crucible_addr_by_zpool = blueprint + .all_omicron_zones(BlueprintZoneFilter::ShouldBeRunning) + .filter_map(|(_, z)| match z.zone_type { + BlueprintZoneType::Crucible( + blueprint_zone_type::Crucible { address, .. }, + ) => { + let zpool_id = z + .zone_type + .durable_zpool() + .expect("crucible zone has durable zpool") + .id(); + Some((zpool_id, address)) + } + _ => None, + }) + .collect::>(); + + // We have three ways a dataset address can be wrong: + // + // * A Crucible dataset has no address + // * A Crucible dataset has an address but it doesn't match its zone + // * A non-Crucible dataset has an address + // + // Make all three kinds of modifications to three different datasets and + // ensure we see all three noted. + let mut cleared_crucible_addr = false; + let mut changed_crucible_addr = false; + let mut set_non_crucible_addr = false; + let mut expected_notes = Vec::new(); + + for (sled_id, datasets_config) in + blueprint.blueprint_datasets.iter_mut() + { + for dataset in datasets_config.datasets.values_mut() { + match dataset.kind { + DatasetKind::Crucible => { + let bad_address = if !cleared_crucible_addr { + cleared_crucible_addr = true; + None + } else if !changed_crucible_addr { + changed_crucible_addr = true; + Some("[1234:5678:9abc::]:0".parse().unwrap()) + } else { + continue; + }; + + dataset.address = bad_address; + let expected_address = *crucible_addr_by_zpool + .get(&dataset.pool.id()) + .expect("found crucible zone for zpool"); + expected_notes.push(Note { + severity: Severity::Fatal, + kind: Kind::Sled { + sled_id: *sled_id, + kind: SledKind::CrucibleDatasetWithIncorrectAddress { + dataset: dataset.clone(), + expected_address, + }, + }, + }); + } + _ => { + if !set_non_crucible_addr { + set_non_crucible_addr = true; + let address = "[::1]:0".parse().unwrap(); + dataset.address = Some(address); + expected_notes.push(Note { + severity: Severity::Fatal, + kind: Kind::Sled { + sled_id: *sled_id, + kind: SledKind::NonCrucibleDatasetWithAddress { + dataset: dataset.clone(), + address, + }, + }, + }); + } + } + } + } + } + + // We should have modified 3 datasets. + assert_eq!(expected_notes.len(), 3); + + let report = + Blippy::new(&blueprint).into_report(BlippyReportSortKey::Kind); + eprintln!("{}", report.display()); + for note in expected_notes { + assert!( + report.notes().contains(¬e), + "did not find expected note {note:?}" + ); + } + + logctx.cleanup_successful(); + } } From 9283a4a4a24003a4b13289312fc19ea2e5870ea1 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 7 Jan 2025 10:52:38 -0500 Subject: [PATCH 5/6] change sled-agent test to use blippy --- Cargo.lock | 1 + sled-agent/Cargo.toml | 1 + sled-agent/src/rack_setup/service.rs | 24 +++++++++--------------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe8ac9704f3..0562fd3f580 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7302,6 +7302,7 @@ dependencies = [ "mg-admin-client", "nexus-client", "nexus-config", + "nexus-reconfigurator-blippy", "nexus-sled-agent-shared", "nexus-types", "omicron-common", diff --git a/sled-agent/Cargo.toml b/sled-agent/Cargo.toml index 88c758dc31c..12efddda079 100644 --- a/sled-agent/Cargo.toml +++ b/sled-agent/Cargo.toml @@ -119,6 +119,7 @@ guppy.workspace = true hex-literal.workspace = true http.workspace = true hyper.workspace = true +nexus-reconfigurator-blippy.workspace = true omicron-test-utils.workspace = true pretty_assertions.workspace = true rcgen.workspace = true diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 5d87440da69..308bc2f7009 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -1733,6 +1733,7 @@ impl<'a> OmicronZonesConfigGenerator<'a> { mod test { use super::*; use crate::rack_setup::plan::service::{Plan as ServicePlan, SledInfo}; + use nexus_reconfigurator_blippy::{Blippy, BlippyReportSortKey}; use nexus_sled_agent_shared::inventory::{ Baseboard, Inventory, InventoryDisk, OmicronZoneType, OmicronZonesConfig, SledRole, @@ -1931,9 +1932,9 @@ mod test { } #[test] - fn only_crucible_datasets_have_addresses() { + fn rss_blueprint_is_blippy_clean() { let logctx = omicron_test_utils::dev::test_setup_log( - "only_crucible_datasets_have_addresses", + "rss_blueprint_is_blippy_clean", ); let fake_sleds = make_fake_sleds(); @@ -1962,19 +1963,12 @@ mod test { ) .expect("built blueprint"); - for dataset_config in blueprint.blueprint_datasets.into_values() { - for dataset in dataset_config.datasets.values() { - match dataset.kind { - DatasetKind::Crucible => assert!( - dataset.address.is_some(), - "crucible datasets should have addresses" - ), - _ => assert_eq!( - dataset.address, None, - "non-crucible datasets should not have addresses" - ), - } - } + let report = + Blippy::new(&blueprint).into_report(BlippyReportSortKey::Kind); + + if !report.notes().is_empty() { + eprintln!("{}", report.display()); + panic!("RSS blueprint should have no blippy notes"); } logctx.cleanup_successful(); From 5acca22a3b3d7167f240cd33c0e929ad162076e5 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 7 Jan 2025 13:01:13 -0500 Subject: [PATCH 6/6] add third fake sled to RSS blippy test --- sled-agent/src/rack_setup/service.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 308bc2f7009..37a49b930a0 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -1807,6 +1807,13 @@ mod test { ), 5, ), + make_sled_info( + SledUuid::new_v4(), + Ipv6Subnet::::new( + "fd00:1122:3344:103::1".parse().unwrap(), + ), + 5, + ), ] }