From 604a52d4836f79d7ce8ad89472f32e64dc49c237 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 18 Nov 2025 10:21:22 -0500 Subject: [PATCH] BlueprintBuilder::new_based_on(): remove PlanningInput argument --- dev-tools/reconfigurator-cli/src/lib.rs | 15 ++-- live-tests/tests/common/reconfigurator.rs | 4 +- live-tests/tests/test_nexus_add_remove.rs | 2 - live-tests/tests/test_nexus_handoff.rs | 11 --- .../db-queries/src/db/datastore/deployment.rs | 15 ---- .../deployment/external_networking.rs | 2 - nexus/db-queries/src/db/datastore/rack.rs | 2 +- nexus/db-queries/src/db/datastore/vpc.rs | 2 - nexus/reconfigurator/execution/src/dns.rs | 75 +++---------------- nexus/reconfigurator/execution/src/sagas.rs | 2 - .../planning/src/blueprint_builder/builder.rs | 65 ++++++++-------- nexus/reconfigurator/planning/src/example.rs | 3 +- nexus/reconfigurator/planning/src/planner.rs | 52 +++++++++---- .../tasks/crdb_node_id_collector.rs | 4 +- nexus/tests/integration_tests/quiesce.rs | 14 +--- 15 files changed, 95 insertions(+), 173 deletions(-) diff --git a/dev-tools/reconfigurator-cli/src/lib.rs b/dev-tools/reconfigurator-cli/src/lib.rs index 7742a7227b4..bd91e44b32e 100644 --- a/dev-tools/reconfigurator-cli/src/lib.rs +++ b/dev-tools/reconfigurator-cli/src/lib.rs @@ -2223,14 +2223,13 @@ fn cmd_blueprint_edit( .map(|c| c.clone()) .unwrap_or_else(|| CollectionBuilder::new("sim").build()); - let mut builder = BlueprintBuilder::new_based_on( - &sim.log, - blueprint, - &planning_input, - creator, - rng, - ) - .context("creating blueprint builder")?; + let mut builder = + BlueprintBuilder::new_based_on(&sim.log, blueprint, creator, rng) + .context("creating blueprint builder")?; + + // We're acting as the planner here, so we need to update the builder for + // any static changes in the input (e.g., new sleds, new DNS versions, ...). + Planner::update_builder_from_planning_input(&mut builder, &planning_input); if let Some(comment) = args.comment { builder.comment(comment); diff --git a/live-tests/tests/common/reconfigurator.rs b/live-tests/tests/common/reconfigurator.rs index 50656d22950..7e97cb5a569 100644 --- a/live-tests/tests/common/reconfigurator.rs +++ b/live-tests/tests/common/reconfigurator.rs @@ -12,7 +12,7 @@ use nexus_lockstep_client::types::{ }; use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder; use nexus_reconfigurator_planning::planner::PlannerRng; -use nexus_types::deployment::{Blueprint, BlueprintSource, PlanningInput}; +use nexus_types::deployment::{Blueprint, BlueprintSource}; use nexus_types::external_api::views::SledState; use nexus_types::inventory::Collection; use omicron_test_utils::dev::poll::{CondCheckError, wait_for_condition}; @@ -80,7 +80,6 @@ pub async fn blueprint_load_target_enabled( /// their test environment. pub async fn blueprint_edit_current_target( log: &slog::Logger, - planning_input: &PlanningInput, nexus: &nexus_lockstep_client::Client, edit_fn: &dyn Fn(&mut BlueprintBuilder) -> Result<(), anyhow::Error>, ) -> Result<(Blueprint, Blueprint), anyhow::Error> { @@ -92,7 +91,6 @@ pub async fn blueprint_edit_current_target( let mut builder = BlueprintBuilder::new_based_on( log, &blueprint1, - &planning_input, "test-suite", PlannerRng::from_entropy(), ) diff --git a/live-tests/tests/test_nexus_add_remove.rs b/live-tests/tests/test_nexus_add_remove.rs index 34711b8a8d9..9b15da962e1 100644 --- a/live-tests/tests/test_nexus_add_remove.rs +++ b/live-tests/tests/test_nexus_add_remove.rs @@ -68,7 +68,6 @@ async fn test_nexus_add_remove(lc: &LiveTestContext) { let sled_id = *commissioned_sled_ids.first().expect("any sled id"); let (blueprint1, blueprint2) = blueprint_edit_current_target( log, - &planning_input, &nexus, &|builder: &mut BlueprintBuilder| { // We have to tell the builder what image source to use for the new @@ -188,7 +187,6 @@ async fn test_nexus_add_remove(lc: &LiveTestContext) { // Now expunge the zone we just created. let (_blueprint2, blueprint3) = blueprint_edit_current_target( log, - &planning_input, &nexus, &|builder: &mut BlueprintBuilder| { builder diff --git a/live-tests/tests/test_nexus_handoff.rs b/live-tests/tests/test_nexus_handoff.rs index 1f9698867ad..5dfe8e4188f 100644 --- a/live-tests/tests/test_nexus_handoff.rs +++ b/live-tests/tests/test_nexus_handoff.rs @@ -170,7 +170,6 @@ async fn test_nexus_handoff(lc: &LiveTestContext) { let (_blueprint_initial, blueprint_new_nexus) = blueprint_edit_current_target( log, - &planning_input, &nexus, &|builder: &mut BlueprintBuilder| { let mut external_networking_alloc = @@ -264,14 +263,9 @@ async fn test_nexus_handoff(lc: &LiveTestContext) { info!(log, "created demo saga"; "demo_saga" => ?demo_saga); // Now update the target blueprint to trigger a handoff. - let planning_input = - PlanningInputFromDb::assemble(opctx, datastore, planner_config) - .await - .expect("planning input"); let (_blueprint_new_nexus, blueprint_handoff) = blueprint_edit_current_target( log, - &planning_input, &nexus, &|builder: &mut BlueprintBuilder| { builder.set_nexus_generation(next_generation); @@ -427,16 +421,11 @@ async fn test_nexus_handoff(lc: &LiveTestContext) { info!(log, "all new Nexus instances report running!"); // Clean up: expunge the old Nexus instances. - let planning_input = - PlanningInputFromDb::assemble(opctx, datastore, planner_config) - .await - .expect("planning input"); let new_nexus = new_nexus_clients.values().next().expect("one new Nexus client"); let (_blueprint_handoff, blueprint_cleanup) = blueprint_edit_current_target( log, - &planning_input, new_nexus, &|builder: &mut BlueprintBuilder| { for (id, current_zone) in ¤t_nexus_zones { diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 3b5f406cd86..bb6fd74db0a 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -3159,7 +3159,6 @@ mod tests { use nexus_types::deployment::ExpectedActiveRotSlot; use nexus_types::deployment::PendingMgsUpdate; use nexus_types::deployment::PlanningInput; - use nexus_types::deployment::PlanningInputBuilder; use nexus_types::deployment::SledDetails; use nexus_types::deployment::SledDisk; use nexus_types::deployment::SledFilter; @@ -3191,16 +3190,12 @@ mod tests { use std::mem; use std::net::Ipv6Addr; use std::sync::Arc; - use std::sync::LazyLock; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; use std::time::Duration; use tufaceous_artifact::ArtifactHash; use tufaceous_artifact::ArtifactVersion; - static EMPTY_PLANNING_INPUT: LazyLock = - LazyLock::new(|| PlanningInputBuilder::empty_input()); - #[derive(Default)] pub struct NetworkResourceControlFlow { pub target_check_done: Option>, @@ -3448,7 +3443,6 @@ mod tests { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint1, - &planning_input, "test", PlannerRng::from_entropy(), ) @@ -3795,7 +3789,6 @@ mod tests { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint2, - &planning_input, "dummy", PlannerRng::from_entropy(), ) @@ -3851,7 +3844,6 @@ mod tests { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint3, - &planning_input, "dummy", PlannerRng::from_entropy(), ) @@ -3904,7 +3896,6 @@ mod tests { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint4, - &planning_input, "dummy", PlannerRng::from_entropy(), ) @@ -3961,7 +3952,6 @@ mod tests { let blueprint6 = BlueprintBuilder::new_based_on( &logctx.log, &blueprint5, - &planning_input, "dummy", PlannerRng::from_entropy(), ) @@ -4032,7 +4022,6 @@ mod tests { let blueprint2 = BlueprintBuilder::new_based_on( &logctx.log, &blueprint1, - &EMPTY_PLANNING_INPUT, "test2", PlannerRng::from_entropy(), ) @@ -4041,7 +4030,6 @@ mod tests { let blueprint3 = BlueprintBuilder::new_based_on( &logctx.log, &blueprint1, - &EMPTY_PLANNING_INPUT, "test3", PlannerRng::from_entropy(), ) @@ -4141,7 +4129,6 @@ mod tests { let blueprint4 = BlueprintBuilder::new_based_on( &logctx.log, &blueprint3, - &EMPTY_PLANNING_INPUT, "test3", PlannerRng::from_entropy(), ) @@ -4180,7 +4167,6 @@ mod tests { let blueprint2 = BlueprintBuilder::new_based_on( &logctx.log, &blueprint1, - &EMPTY_PLANNING_INPUT, "test2", PlannerRng::from_entropy(), ) @@ -4365,7 +4351,6 @@ mod tests { let blueprint2 = BlueprintBuilder::new_based_on( &logctx.log, &blueprint1, - &example_system.input, &format!("{test_name}-2"), PlannerRng::from_entropy(), ) diff --git a/nexus/db-queries/src/db/datastore/deployment/external_networking.rs b/nexus/db-queries/src/db/datastore/deployment/external_networking.rs index 0723873433c..94689d211aa 100644 --- a/nexus/db-queries/src/db/datastore/deployment/external_networking.rs +++ b/nexus/db-queries/src/db/datastore/deployment/external_networking.rs @@ -1421,7 +1421,6 @@ mod tests { let mut builder = BlueprintBuilder::new_based_on( &opctx.log, &bp1, - &input, TEST_NAME, PlannerRng::from_entropy(), ) @@ -1466,7 +1465,6 @@ mod tests { let mut builder = BlueprintBuilder::new_based_on( &opctx.log, &bp2, - &input, TEST_NAME, PlannerRng::from_entropy(), ) diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index 274a26cb3b5..79b44c86020 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -1208,7 +1208,6 @@ mod test { let mut builder = BlueprintBuilder::new_based_on( log, &EMPTY_BLUEPRINT, - &planning_input, test_name, PlannerRng::from_entropy(), ) @@ -1217,6 +1216,7 @@ mod test { for (sled_id, sled_resources) in planning_input.all_sled_resources(SledFilter::InService) { + builder.ensure_sled_exists(sled_id, sled_resources.subnet); builder .sled_add_disks(sled_id, &sled_resources) .expect("added disks"); diff --git a/nexus/db-queries/src/db/datastore/vpc.rs b/nexus/db-queries/src/db/datastore/vpc.rs index 682da75e243..30d4d1a3487 100644 --- a/nexus/db-queries/src/db/datastore/vpc.rs +++ b/nexus/db-queries/src/db/datastore/vpc.rs @@ -3324,7 +3324,6 @@ mod tests { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &bp0, - &planning_input, "test", PlannerRng::from_entropy(), ) @@ -3417,7 +3416,6 @@ mod tests { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &bp2, - &planning_input, "test", PlannerRng::from_entropy(), ) diff --git a/nexus/reconfigurator/execution/src/dns.rs b/nexus/reconfigurator/execution/src/dns.rs index 45813489d7f..2e6b2264d3b 100644 --- a/nexus/reconfigurator/execution/src/dns.rs +++ b/nexus/reconfigurator/execution/src/dns.rs @@ -322,7 +322,6 @@ mod test { use internal_dns_types::names::BOUNDARY_NTP_DNS_NAME; use internal_dns_types::names::DNS_ZONE; use internal_dns_types::names::ServiceName; - use nexus_db_model::DbMetadataNexusState; use nexus_db_model::DnsGroup; use nexus_db_model::Silo; use nexus_db_queries::authn; @@ -334,7 +333,6 @@ mod test { use nexus_reconfigurator_planning::blueprint_editor::ExternalNetworkingAllocator; use nexus_reconfigurator_planning::example::ExampleSystemBuilder; use nexus_reconfigurator_planning::planner::PlannerRng; - use nexus_reconfigurator_preparation::PlanningInputFromDb; use nexus_sled_agent_shared::inventory::OmicronZoneConfig; use nexus_sled_agent_shared::inventory::OmicronZoneImageSource; use nexus_sled_agent_shared::inventory::OmicronZoneType; @@ -352,18 +350,13 @@ mod test { use nexus_types::deployment::BlueprintZoneDisposition; use nexus_types::deployment::BlueprintZoneImageSource; use nexus_types::deployment::BlueprintZoneType; - use nexus_types::deployment::CockroachDbClusterVersion; use nexus_types::deployment::CockroachDbPreserveDowngrade; - use nexus_types::deployment::CockroachDbSettings; + use nexus_types::deployment::ExternalIpPolicy; pub use nexus_types::deployment::OmicronZoneExternalFloatingAddr; pub use nexus_types::deployment::OmicronZoneExternalFloatingIp; pub use nexus_types::deployment::OmicronZoneExternalSnatIp; use nexus_types::deployment::OximeterReadMode; - use nexus_types::deployment::OximeterReadPolicy; use nexus_types::deployment::PendingMgsUpdates; - use nexus_types::deployment::PlannerConfig; - use nexus_types::deployment::SledFilter; - use nexus_types::deployment::TufRepoPolicy; use nexus_types::deployment::blueprint_zone_type; use nexus_types::external_api::params; use nexus_types::external_api::shared; @@ -376,6 +369,7 @@ mod test { use nexus_types::internal_api::params::DnsRecord; use nexus_types::internal_api::params::Srv; use nexus_types::silo::silo_dns_name; + use omicron_common::address::IpRange; use omicron_common::address::Ipv6Subnet; use omicron_common::address::RACK_PREFIX; use omicron_common::address::REPO_DEPOT_PORT; @@ -384,12 +378,6 @@ mod test { use omicron_common::address::get_switch_zone_address; use omicron_common::api::external::Generation; use omicron_common::api::external::IdentityMetadataCreateParams; - use omicron_common::policy::BOUNDARY_NTP_REDUNDANCY; - use omicron_common::policy::COCKROACHDB_REDUNDANCY; - use omicron_common::policy::CRUCIBLE_PANTRY_REDUNDANCY; - use omicron_common::policy::INTERNAL_DNS_REDUNDANCY; - use omicron_common::policy::NEXUS_REDUNDANCY; - use omicron_common::policy::OXIMETER_REDUNDANCY; use omicron_common::zpool_name::ZpoolName; use omicron_test_utils::dev::test_setup_log; use omicron_uuid_kinds::BlueprintUuid; @@ -1514,60 +1502,19 @@ mod test { // Now, go through the motions of provisioning a new Nexus zone. // We do this directly with BlueprintBuilder to avoid the planner // deciding to make other unrelated changes. - let sled_rows = datastore - .sled_list_all_batched(&opctx, SledFilter::Commissioned) - .await - .unwrap(); - let zpool_rows = - datastore.zpool_list_all_external_batched(&opctx).await.unwrap(); let ip_pool_range_rows = fetch_all_service_ip_pool_ranges(&datastore, &opctx).await; - let active_nexus_zones = datastore - .get_db_metadata_nexus_in_state( - &opctx, - vec![DbMetadataNexusState::Active], - ) - .await - .internal_context("fetching active nexuses") - .unwrap() - .into_iter() - .map(|z| z.nexus_id()) - .collect(); - let planning_input = PlanningInputFromDb { - sled_rows: &sled_rows, - zpool_rows: &zpool_rows, - ip_pool_range_rows: &ip_pool_range_rows, - external_dns_external_ips: BTreeSet::new(), - internal_dns_version: dns_initial_internal.generation.into(), - external_dns_version: dns_latest_external.generation.into(), - // These are not used because we're not actually going through - // the planner. - cockroachdb_settings: &CockroachDbSettings::empty(), - external_ip_rows: &[], - service_nic_rows: &[], - target_boundary_ntp_zone_count: BOUNDARY_NTP_REDUNDANCY, - target_nexus_zone_count: NEXUS_REDUNDANCY, - target_internal_dns_zone_count: INTERNAL_DNS_REDUNDANCY, - target_oximeter_zone_count: OXIMETER_REDUNDANCY, - target_cockroachdb_zone_count: COCKROACHDB_REDUNDANCY, - target_cockroachdb_cluster_version: - CockroachDbClusterVersion::POLICY, - target_crucible_pantry_zone_count: CRUCIBLE_PANTRY_REDUNDANCY, - clickhouse_policy: None, - oximeter_read_policy: OximeterReadPolicy::new(1), - tuf_repo: TufRepoPolicy::initial(), - old_repo: TufRepoPolicy::initial(), - planner_config: PlannerConfig::default(), - active_nexus_zones, - not_yet_nexus_zones: BTreeSet::new(), - log, - } - .build() - .unwrap(); + let external_ip_policy = { + let mut builder = ExternalIpPolicy::builder(); + for range in ip_pool_range_rows { + let range = IpRange::try_from(&range).unwrap(); + builder.push_service_pool_range(range).unwrap(); + } + builder.build() + }; let mut builder = BlueprintBuilder::new_based_on( &log, &blueprint, - &planning_input, "test suite", PlannerRng::from_entropy(), ) @@ -1590,7 +1537,7 @@ mod test { let new_nexus_external_ip = ExternalNetworkingAllocator::from_current_zones( &builder, - planning_input.external_ip_policy(), + &external_ip_policy, ) .expect("constructed ExternalNetworkingAllocator") .for_new_nexus() diff --git a/nexus/reconfigurator/execution/src/sagas.rs b/nexus/reconfigurator/execution/src/sagas.rs index 07fd6064a2c..d973c2c75fe 100644 --- a/nexus/reconfigurator/execution/src/sagas.rs +++ b/nexus/reconfigurator/execution/src/sagas.rs @@ -139,7 +139,6 @@ mod test { let mut builder = BlueprintBuilder::new_based_on( log, &blueprint1, - &example.input, "test suite", PlannerRng::from_entropy(), ) @@ -195,7 +194,6 @@ mod test { let mut builder = BlueprintBuilder::new_based_on( log, &blueprint2, - &example.input, "test suite", PlannerRng::from_entropy(), ) diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs index 35023d421fb..e410f4730e8 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs @@ -47,8 +47,6 @@ use nexus_types::deployment::OmicronZoneExternalSnatIp; use nexus_types::deployment::OximeterReadMode; use nexus_types::deployment::PendingMgsUpdate; use nexus_types::deployment::PendingMgsUpdates; -use nexus_types::deployment::PlanningInput; -use nexus_types::deployment::SledFilter; use nexus_types::deployment::SledResources; use nexus_types::deployment::TufRepoContentsError; use nexus_types::deployment::ZpoolName; @@ -59,8 +57,10 @@ use omicron_common::address::CLICKHOUSE_HTTP_PORT; use omicron_common::address::DNS_HTTP_PORT; use omicron_common::address::DNS_PORT; use omicron_common::address::DnsSubnet; +use omicron_common::address::Ipv6Subnet; use omicron_common::address::NTP_PORT; use omicron_common::address::ReservedRackSubnet; +use omicron_common::address::SLED_PREFIX; use omicron_common::api::external::Generation; use omicron_common::api::external::Vni; use omicron_common::api::internal::shared::NetworkInterface; @@ -568,7 +568,6 @@ impl<'a> BlueprintBuilder<'a> { pub fn new_based_on( log: &Logger, parent_blueprint: &'a Blueprint, - input: &PlanningInput, creator: &str, mut rng: PlannerRng, ) -> anyhow::Result> { @@ -587,16 +586,6 @@ impl<'a> BlueprintBuilder<'a> { sled_editors.insert(*sled_id, editor); } - // 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, details) in input.all_sleds(SledFilter::Commissioned) { - if let Entry::Vacant(slot) = sled_editors.entry(sled_id) { - slot.insert(SledEditor::for_new_active( - details.resources.subnet, - )); - } - } - // Copy the Oximeter read policy from our parent blueprint so we can // track changes to it. let oximeter_read_policy = OximeterReadPolicy { @@ -615,16 +604,15 @@ impl<'a> BlueprintBuilder<'a> { sled_editors, cockroachdb_setting_preserve_downgrade: parent_blueprint .cockroachdb_setting_preserve_downgrade, - cockroachdb_fingerprint: input - .cockroachdb_settings() - .state_fingerprint + cockroachdb_fingerprint: parent_blueprint + .cockroachdb_fingerprint .clone(), pending_mgs_updates: parent_blueprint.pending_mgs_updates.clone(), target_release_minimum_generation: parent_blueprint .target_release_minimum_generation, nexus_generation: parent_blueprint.nexus_generation, - internal_dns_version: input.internal_dns_version(), - external_dns_version: input.external_dns_version(), + internal_dns_version: parent_blueprint.internal_dns_version, + external_dns_version: parent_blueprint.external_dns_version, creator: creator.to_owned(), operations: Vec::new(), comments: Vec::new(), @@ -640,6 +628,29 @@ impl<'a> BlueprintBuilder<'a> { self.new_blueprint_id } + /// Ensure a commissioned sled exists in this builder. + pub fn ensure_sled_exists( + &mut self, + sled_id: SledUuid, + sled_subnet: Ipv6Subnet, + ) { + if let Entry::Vacant(slot) = self.sled_editors.entry(sled_id) { + slot.insert(SledEditor::for_new_active(sled_subnet)); + } + } + + pub fn set_cockroachdb_fingerprint(&mut self, fingerprint: String) { + self.cockroachdb_fingerprint = fingerprint; + } + + pub fn set_internal_dns_version(&mut self, version: Generation) { + self.internal_dns_version = version; + } + + pub fn set_external_dns_version(&mut self, version: Generation) { + self.external_dns_version = version; + } + /// Helper method to construct an empty [`ClickhouseClusterConfig`] using /// this builder's internal RNG (for reproducible testing, primarily). pub fn make_empty_clickhouse_cluster_config( @@ -2583,6 +2594,8 @@ pub mod test { use nexus_types::deployment::BlueprintDatasetDisposition; use nexus_types::deployment::ExternalIpPolicy; use nexus_types::deployment::OmicronZoneNetworkResources; + use nexus_types::deployment::PlanningInput; + use nexus_types::deployment::SledFilter; use nexus_types::external_api::views::SledPolicy; use omicron_common::address::IpRange; use omicron_test_utils::dev::test_setup_log; @@ -2624,7 +2637,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint1, - &example.input, "test_basic", rng.next_planner_rng(), ) @@ -2675,7 +2687,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint2, - &input, "test_basic", rng.next_planner_rng(), ) @@ -2684,6 +2695,7 @@ pub mod test { .sled_lookup(SledFilter::Commissioned, new_sled_id) .unwrap() .resources; + builder.ensure_sled_exists(new_sled_id, new_sled_resources.subnet); builder.sled_add_disks(new_sled_id, &new_sled_resources).unwrap(); builder .sled_ensure_zone_ntp( @@ -2843,7 +2855,6 @@ pub mod test { let mut blueprint2 = BlueprintBuilder::new_based_on( &logctx.log, &blueprint1, - &input, "test_decommissioned_sleds", rng.next_planner_rng(), ) @@ -2880,7 +2891,6 @@ pub mod test { let blueprint3 = BlueprintBuilder::new_based_on( &logctx.log, &blueprint2, - &input, "test_decommissioned_sleds", rng.next_planner_rng(), ) @@ -2915,7 +2925,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &parent, - &input, "test", rng.next_planner_rng(), ) @@ -3015,7 +3024,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint, - &input, "test", rng.next_planner_rng(), ) @@ -3069,7 +3077,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint, - &input, "test", rng.next_planner_rng(), ) @@ -3108,7 +3115,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint, - &input, "test", rng.next_planner_rng(), ) @@ -3149,7 +3155,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &parent, - &input, "test", rng.next_planner_rng(), ) @@ -3267,7 +3272,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &parent, - &input, "test", rng.next_planner_rng(), ) @@ -3297,7 +3301,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &parent, - &input, "test", rng.next_planner_rng(), ) @@ -3352,7 +3355,6 @@ pub mod test { let builder = BlueprintBuilder::new_based_on( &logctx.log, &parent, - &input, "test", rng.next_planner_rng(), ) @@ -3427,7 +3429,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &parent, - &input, "test", rng.next_planner_rng(), ) @@ -3469,7 +3470,6 @@ pub mod test { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &parent, - &input, "test", rng.next_planner_rng(), ) @@ -3515,7 +3515,6 @@ pub mod test { let mut blueprint_builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint1, - &system.input, TEST_NAME, rng.next_planner_rng(), ) diff --git a/nexus/reconfigurator/planning/src/example.rs b/nexus/reconfigurator/planning/src/example.rs index 4f1b1a10c67..7f566d0db46 100644 --- a/nexus/reconfigurator/planning/src/example.rs +++ b/nexus/reconfigurator/planning/src/example.rs @@ -13,6 +13,7 @@ use std::net::Ipv4Addr; use crate::blueprint_builder::BlueprintBuilder; use crate::blueprint_editor::ExternalNetworkingAllocator; +use crate::planner::Planner; use crate::planner::rng::PlannerRng; use crate::system::RotStateOverrides; use crate::system::SledBuilder; @@ -545,11 +546,11 @@ impl ExampleSystemBuilder { let mut builder = BlueprintBuilder::new_based_on( &self.log, &initial_blueprint, - &base_input, "test suite", rng.blueprint2_rng, ) .unwrap(); + Planner::update_builder_from_planning_input(&mut builder, &base_input); let discretionary_sled_count = base_input.all_sled_ids(SledFilter::Discretionary).count(); diff --git a/nexus/reconfigurator/planning/src/planner.rs b/nexus/reconfigurator/planning/src/planner.rs index efbfb1a82d8..1b65b628cf8 100644 --- a/nexus/reconfigurator/planning/src/planner.rs +++ b/nexus/reconfigurator/planning/src/planner.rs @@ -183,16 +183,53 @@ impl<'a> Planner<'a> { inventory: &'a Collection, rng: PlannerRng, ) -> anyhow::Result> { - let blueprint = BlueprintBuilder::new_based_on( + let mut blueprint = BlueprintBuilder::new_based_on( &log, parent_blueprint, - input, creator, rng, )?; + Self::update_builder_from_planning_input(&mut blueprint, input); Ok(Planner { log, input, blueprint, inventory }) } + /// Update the internal state of `builder` to reflect changes between its + /// current blueprint and `input`. + /// + /// This method does not implement any real planning logic; it performs + /// strictly straightforward "update this value in the blueprint", such as + /// noting the latest internal and external DNS versions. + /// + /// This method is public and does not take `&self` so other planner-like + /// entities (`reconfigurator-cli` and tests) can easily reuse it if they + /// want to update a `BlueprintBuilder` they're using without running + /// through the planner proper. + pub fn update_builder_from_planning_input( + builder: &mut BlueprintBuilder, + input: &PlanningInput, + ) { + // Ensure we have an entry for every commissioned sled. + // + // If `input` has new sleds that weren't in `builder`'s parent + // blueprint, this will create empty configs for them. + for (sled_id, details) in input.all_sleds(SledFilter::Commissioned) { + builder.ensure_sled_exists(sled_id, details.resources.subnet); + } + + // Update various blueprint fields that track changes to the system but + // that don't require any planning logic. + builder.set_cockroachdb_fingerprint( + input.cockroachdb_settings().state_fingerprint.clone(), + ); + builder.set_internal_dns_version(input.internal_dns_version()); + builder.set_external_dns_version(input.external_dns_version()); + let oximeter_read_policy = input.oximeter_read_settings(); + builder.set_oximeter_read_policy( + oximeter_read_policy.version.into(), + oximeter_read_policy.mode, + ); + } + pub fn plan(mut self) -> Result { let report = self.do_plan()?; Ok(self.blueprint.build(BlueprintSource::Planner(Arc::new(report)))) @@ -2350,18 +2387,8 @@ impl<'a> Planner<'a> { } fn do_plan_clickhouse_cluster_settings(&mut self) { - // Generate a new cluster config based on the policy and parent - // blueprint. let new_config = self.generate_current_clickhouse_cluster_config(); self.blueprint.set_clickhouse_cluster_config(new_config); - - // Not directly clickhouse cluster settings, but closely related: also - // update how we read from Oximeter based on the input policy. - let oximeter_read_policy = self.input.oximeter_read_settings(); - self.blueprint.set_oximeter_read_policy( - oximeter_read_policy.version.into(), - oximeter_read_policy.mode, - ); } fn generate_current_clickhouse_cluster_config( @@ -3416,7 +3443,6 @@ pub(crate) mod test { let mut blueprint_builder = BlueprintBuilder::new_based_on( &logctx.log, &blueprint1, - &input, TEST_NAME, PlannerRng::from_entropy(), ) diff --git a/nexus/src/app/background/tasks/crdb_node_id_collector.rs b/nexus/src/app/background/tasks/crdb_node_id_collector.rs index f9563a17299..74f0935d496 100644 --- a/nexus/src/app/background/tasks/crdb_node_id_collector.rs +++ b/nexus/src/app/background/tasks/crdb_node_id_collector.rs @@ -264,9 +264,8 @@ mod tests { let log = &logctx.log; // Build an example system with one sled. - let (example_system, bp0) = + let (_, bp0) = ExampleSystemBuilder::new(log, TEST_NAME).nsleds(1).build(); - let input = example_system.input; // `ExampleSystemBuilder` doesn't place any cockroach nodes; assert so // we bail out early if that changes. @@ -287,7 +286,6 @@ mod tests { let mut builder = BlueprintBuilder::new_based_on( log, &bp0, - &input, TEST_NAME, PlannerRng::from_entropy(), ) diff --git a/nexus/tests/integration_tests/quiesce.rs b/nexus/tests/integration_tests/quiesce.rs index 97f33b03be7..0512724858e 100644 --- a/nexus/tests/integration_tests/quiesce.rs +++ b/nexus/tests/integration_tests/quiesce.rs @@ -7,12 +7,10 @@ use nexus_auth::context::OpContext; use nexus_lockstep_client::types::QuiesceState; use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder; use nexus_reconfigurator_planning::planner::PlannerRng; -use nexus_reconfigurator_preparation::PlanningInputFromDb; use nexus_test_interface::NexusServer; use nexus_test_utils_macros::nexus_test; use nexus_types::deployment::BlueprintSource; use nexus_types::deployment::BlueprintTargetSet; -use nexus_types::deployment::PlannerConfig; use omicron_test_utils::dev::poll::CondCheckError; use omicron_test_utils::dev::poll::wait_for_condition; use omicron_uuid_kinds::GenericUuid; @@ -35,16 +33,7 @@ async fn test_quiesce(cptestctx: &ControlPlaneTestContext) { let nexus_client = nexus_lockstep_client::Client::new(&nexus_lockstep_url, log.clone()); - // Collect what we need to modify the blueprint. - let planner_config = datastore - .reconfigurator_config_get_latest(&opctx) - .await - .expect("obtained latest reconfigurator config") - .map_or_else(PlannerConfig::default, |c| c.config.planner_config); - let planning_input = - PlanningInputFromDb::assemble(&opctx, &datastore, planner_config) - .await - .expect("planning input"); + // Load the current target blueprint. let target_blueprint = nexus .blueprint_target_view(&opctx) .await @@ -59,7 +48,6 @@ async fn test_quiesce(cptestctx: &ControlPlaneTestContext) { let mut builder = BlueprintBuilder::new_based_on( log, &blueprint1, - &planning_input, "test-suite", PlannerRng::from_entropy(), )