From 453311a880075b9f89626bb20cca1c1cd85ffb4f Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Fri, 20 Sep 2024 19:23:37 -0400 Subject: [PATCH] Add inventory `Collection` to `BlueprintBuilder` (#6624) A follow up PR will use this for clickhouse keeper allocation. --- Cargo.lock | 2 + dev-tools/reconfigurator-cli/Cargo.toml | 1 + dev-tools/reconfigurator-cli/src/main.rs | 8 ++++ live-tests/Cargo.toml | 1 + live-tests/tests/common/reconfigurator.rs | 3 ++ live-tests/tests/test_nexus_add_remove.rs | 8 ++++ .../db-queries/src/db/datastore/deployment.rs | 16 +++++++ nexus/db-queries/src/db/datastore/vpc.rs | 6 +++ nexus/reconfigurator/execution/src/dns.rs | 3 ++ .../planning/src/blueprint_builder/builder.rs | 48 +++++++++++++++---- .../planning/src/blueprint_builder/zones.rs | 2 + nexus/reconfigurator/planning/src/example.rs | 7 +++ nexus/reconfigurator/planning/src/planner.rs | 1 + 13 files changed, 97 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 620bdee096..146c721e22 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6590,6 +6590,7 @@ dependencies = [ "nexus-config", "nexus-db-model", "nexus-db-queries", + "nexus-inventory", "nexus-reconfigurator-planning", "nexus-reconfigurator-preparation", "nexus-sled-agent-shared", @@ -8963,6 +8964,7 @@ dependencies = [ "indexmap 2.4.0", "nexus-client", "nexus-db-queries", + "nexus-inventory", "nexus-reconfigurator-execution", "nexus-reconfigurator-planning", "nexus-reconfigurator-preparation", diff --git a/dev-tools/reconfigurator-cli/Cargo.toml b/dev-tools/reconfigurator-cli/Cargo.toml index f6b225a5c9..b24c0eef36 100644 --- a/dev-tools/reconfigurator-cli/Cargo.toml +++ b/dev-tools/reconfigurator-cli/Cargo.toml @@ -19,6 +19,7 @@ dns-service-client.workspace = true dropshot.workspace = true humantime.workspace = true indexmap.workspace = true +nexus-inventory.workspace = true nexus-reconfigurator-planning.workspace = true nexus-reconfigurator-execution.workspace = true nexus-sled-agent-shared.workspace = true diff --git a/dev-tools/reconfigurator-cli/src/main.rs b/dev-tools/reconfigurator-cli/src/main.rs index 13e4617679..f444be514b 100644 --- a/dev-tools/reconfigurator-cli/src/main.rs +++ b/dev-tools/reconfigurator-cli/src/main.rs @@ -12,6 +12,7 @@ use clap::ValueEnum; use clap::{Args, Parser, Subcommand}; use dns_service_client::DnsDiff; use indexmap::IndexMap; +use nexus_inventory::CollectionBuilder; use nexus_reconfigurator_execution::blueprint_external_dns_config; use nexus_reconfigurator_execution::blueprint_internal_dns_config; use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder; @@ -736,10 +737,17 @@ fn cmd_blueprint_edit( let blueprint = sim.blueprint_lookup(blueprint_id)?; let creator = args.creator.as_deref().unwrap_or("reconfigurator-cli"); let planning_input = sim.planning_input(blueprint)?; + let latest_collection = sim + .collections + .iter() + .max_by_key(|(_, c)| c.time_started) + .map(|(_, c)| c.clone()) + .unwrap_or_else(|| CollectionBuilder::new("sim").build()); let mut builder = BlueprintBuilder::new_based_on( &sim.log, blueprint, &planning_input, + &latest_collection, creator, ) .context("creating blueprint builder")?; diff --git a/live-tests/Cargo.toml b/live-tests/Cargo.toml index e0eaf2c338..f731f248d0 100644 --- a/live-tests/Cargo.toml +++ b/live-tests/Cargo.toml @@ -23,6 +23,7 @@ nexus-client.workspace = true nexus-config.workspace = true nexus-db-model.workspace = true nexus-db-queries.workspace = true +nexus-inventory.workspace = true nexus-reconfigurator-planning.workspace = true nexus-reconfigurator-preparation.workspace = true nexus-sled-agent-shared.workspace = true diff --git a/live-tests/tests/common/reconfigurator.rs b/live-tests/tests/common/reconfigurator.rs index 8f2560bb49..ae060b5b4c 100644 --- a/live-tests/tests/common/reconfigurator.rs +++ b/live-tests/tests/common/reconfigurator.rs @@ -8,6 +8,7 @@ use anyhow::{ensure, Context}; use nexus_client::types::BlueprintTargetSet; use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder; use nexus_types::deployment::{Blueprint, PlanningInput}; +use nexus_types::inventory::Collection; use slog::{debug, info}; /// Modify the system by editing the current target blueprint @@ -35,6 +36,7 @@ use slog::{debug, info}; pub async fn blueprint_edit_current_target( log: &slog::Logger, planning_input: &PlanningInput, + collection: &Collection, nexus: &nexus_client::Client, edit_fn: &dyn Fn(&mut BlueprintBuilder) -> Result<(), anyhow::Error>, ) -> Result<(Blueprint, Blueprint), anyhow::Error> { @@ -68,6 +70,7 @@ pub async fn blueprint_edit_current_target( log, &blueprint1, &planning_input, + &collection, "test-suite", ) .context("creating BlueprintBuilder")?; diff --git a/live-tests/tests/test_nexus_add_remove.rs b/live-tests/tests/test_nexus_add_remove.rs index 70e55b704a..1113f5fb3b 100644 --- a/live-tests/tests/test_nexus_add_remove.rs +++ b/live-tests/tests/test_nexus_add_remove.rs @@ -12,6 +12,7 @@ use futures::TryStreamExt; use live_tests_macros::live_test; use nexus_client::types::Saga; use nexus_client::types::SagaState; +use nexus_inventory::CollectionBuilder; use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder; use nexus_reconfigurator_planning::blueprint_builder::EnsureMultiple; use nexus_reconfigurator_preparation::PlanningInputFromDb; @@ -43,6 +44,11 @@ async fn test_nexus_add_remove(lc: &LiveTestContext) { let planning_input = PlanningInputFromDb::assemble(&opctx, &datastore) .await .expect("planning input"); + let collection = datastore + .inventory_get_latest_collection(opctx) + .await + .expect("latest inventory collection") + .unwrap_or_else(|| CollectionBuilder::new("test").build()); let initial_nexus_clients = lc.all_internal_nexus_clients().await.unwrap(); let nexus = initial_nexus_clients.first().expect("internal Nexus client"); @@ -54,6 +60,7 @@ async fn test_nexus_add_remove(lc: &LiveTestContext) { let (blueprint1, blueprint2) = blueprint_edit_current_target( log, &planning_input, + &collection, &nexus, &|builder: &mut BlueprintBuilder| { let nnexus = builder @@ -123,6 +130,7 @@ async fn test_nexus_add_remove(lc: &LiveTestContext) { let _ = blueprint_edit_current_target( log, &planning_input, + &collection, &nexus, &|builder: &mut BlueprintBuilder| { builder diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index d413f9507a..12cbed4652 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -1513,6 +1513,7 @@ mod tests { use super::*; use crate::db::datastore::test_utils::datastore_test; use nexus_inventory::now_db_precision; + use nexus_inventory::CollectionBuilder; use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder; use nexus_reconfigurator_planning::blueprint_builder::Ensure; use nexus_reconfigurator_planning::blueprint_builder::EnsureMultiple; @@ -1815,6 +1816,7 @@ mod tests { &logctx.log, &blueprint1, &planning_input, + &collection, "test", ) .expect("failed to create builder"); @@ -1987,6 +1989,9 @@ mod tests { .unwrap_err(); assert!(err.to_string().contains("no target blueprint set")); + // Create an initial empty collection + let collection = CollectionBuilder::new("test").build(); + // Create three blueprints: // * `blueprint1` has no parent // * `blueprint2` and `blueprint3` both have `blueprint1` as parent @@ -1998,6 +2003,7 @@ mod tests { &logctx.log, &blueprint1, &EMPTY_PLANNING_INPUT, + &collection, "test2", ) .expect("failed to create builder") @@ -2006,6 +2012,7 @@ mod tests { &logctx.log, &blueprint1, &EMPTY_PLANNING_INPUT, + &collection, "test3", ) .expect("failed to create builder") @@ -2105,6 +2112,7 @@ mod tests { &logctx.log, &blueprint3, &EMPTY_PLANNING_INPUT, + &collection, "test3", ) .expect("failed to create builder") @@ -2137,6 +2145,9 @@ mod tests { let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; + // Create an initial empty collection + let collection = CollectionBuilder::new("test").build(); + // Create an initial blueprint and a child. let blueprint1 = BlueprintBuilder::build_empty_with_sleds( std::iter::empty(), @@ -2146,6 +2157,7 @@ mod tests { &logctx.log, &blueprint1, &EMPTY_PLANNING_INPUT, + &collection, "test2", ) .expect("failed to create builder") @@ -2248,6 +2260,9 @@ mod tests { let mut db = test_setup_database(&logctx.log).await; let (opctx, datastore) = datastore_test(&logctx, &db).await; + // Create an initial empty collection + let collection = CollectionBuilder::new("test").build(); + // Create an initial blueprint and a child. let blueprint1 = BlueprintBuilder::build_empty_with_sleds( std::iter::empty(), @@ -2257,6 +2272,7 @@ mod tests { &logctx.log, &blueprint1, &EMPTY_PLANNING_INPUT, + &collection, "test2", ) .expect("failed to create builder") diff --git a/nexus/db-queries/src/db/datastore/vpc.rs b/nexus/db-queries/src/db/datastore/vpc.rs index b21a7cf6aa..ca0a4f5085 100644 --- a/nexus/db-queries/src/db/datastore/vpc.rs +++ b/nexus/db-queries/src/db/datastore/vpc.rs @@ -2240,12 +2240,17 @@ mod tests { // sled IDs running services. assert_service_sled_ids(&datastore, &[]).await; + // Build an initial empty collection + let collection = + system.to_collection_builder().expect("collection builder").build(); + // Create a blueprint that has a Nexus on our third sled. let bp1 = { let mut builder = BlueprintBuilder::new_based_on( &logctx.log, &bp0, &planning_input, + &collection, "test", ) .expect("created blueprint builder"); @@ -2315,6 +2320,7 @@ mod tests { &logctx.log, &bp2, &planning_input, + &collection, "test", ) .expect("created blueprint builder"); diff --git a/nexus/reconfigurator/execution/src/dns.rs b/nexus/reconfigurator/execution/src/dns.rs index 9b0b95a0e0..4654643576 100644 --- a/nexus/reconfigurator/execution/src/dns.rs +++ b/nexus/reconfigurator/execution/src/dns.rs @@ -474,6 +474,7 @@ mod test { use nexus_db_queries::context::OpContext; use nexus_db_queries::db::DataStore; use nexus_inventory::now_db_precision; + use nexus_inventory::CollectionBuilder; use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder; use nexus_reconfigurator_planning::blueprint_builder::EnsureMultiple; use nexus_reconfigurator_planning::example::example; @@ -1544,10 +1545,12 @@ mod test { builder.build() }; + let collection = CollectionBuilder::new("test").build(); let mut builder = BlueprintBuilder::new_based_on( &log, &blueprint, &planning_input, + &collection, "test suite", ) .unwrap(); diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs index e9025d4df6..19450c0f80 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs @@ -34,6 +34,7 @@ use nexus_types::deployment::SledResources; use nexus_types::deployment::ZpoolFilter; use nexus_types::deployment::ZpoolName; use nexus_types::external_api::views::SledState; +use nexus_types::inventory::Collection; use omicron_common::address::get_internal_dns_server_addresses; use omicron_common::address::get_sled_address; use omicron_common::address::get_switch_zone_address; @@ -204,6 +205,10 @@ pub struct BlueprintBuilder<'a> { /// previous blueprint, on which this one will be based parent_blueprint: &'a Blueprint, + /// The latest inventory collection + #[allow(unused)] + collection: &'a Collection, + // These fields are used to allocate resources for sleds. input: &'a PlanningInput, sled_ip_allocators: BTreeMap, @@ -294,6 +299,7 @@ impl<'a> BlueprintBuilder<'a> { log: &Logger, parent_blueprint: &'a Blueprint, input: &'a PlanningInput, + inventory: &'a Collection, creator: &str, ) -> anyhow::Result> { let log = log.new(o!( @@ -334,6 +340,7 @@ impl<'a> BlueprintBuilder<'a> { Ok(BlueprintBuilder { log, parent_blueprint, + collection: inventory, input, sled_ip_allocators: BTreeMap::new(), external_networking, @@ -1530,6 +1537,7 @@ pub mod test { use crate::example::ExampleSystem; use crate::system::SledBuilder; use expectorate::assert_contents; + use nexus_inventory::CollectionBuilder; use nexus_types::deployment::BlueprintOrCollectionZoneConfig; use nexus_types::deployment::BlueprintZoneFilter; use nexus_types::deployment::OmicronZoneNetworkResources; @@ -1600,9 +1608,15 @@ pub mod test { input: &PlanningInput, test_name: &'static str, ) { - let builder = - BlueprintBuilder::new_based_on(&log, &blueprint, &input, test_name) - .expect("failed to create builder"); + let collection = CollectionBuilder::new("test").build(); + let builder = BlueprintBuilder::new_based_on( + &log, + &blueprint, + &input, + &collection, + test_name, + ) + .expect("failed to create builder"); let child_blueprint = builder.build(); verify_blueprint(&child_blueprint); let diff = child_blueprint.diff_since_blueprint(&blueprint); @@ -1666,6 +1680,7 @@ pub mod test { &logctx.log, blueprint1, &example.input, + &example.collection, "test_basic", ) .expect("failed to create builder"); @@ -1702,6 +1717,7 @@ pub mod test { &logctx.log, &blueprint2, &input, + &example.collection, "test_basic", ) .expect("failed to create builder"); @@ -1811,7 +1827,7 @@ pub mod test { static TEST_NAME: &str = "blueprint_builder_test_prune_decommissioned_sleds"; let logctx = test_setup_log(TEST_NAME); - let (_, input, mut blueprint1) = + let (collection, input, mut blueprint1) = example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS); verify_blueprint(&blueprint1); @@ -1840,6 +1856,7 @@ pub mod test { &logctx.log, &blueprint1, &input, + &collection, "test_prune_decommissioned_sleds", ) .expect("created builder") @@ -1866,6 +1883,7 @@ pub mod test { &logctx.log, &blueprint2, &input, + &collection, "test_prune_decommissioned_sleds", ) .expect("created builder") @@ -1895,7 +1913,8 @@ pub mod test { fn test_add_physical_disks() { static TEST_NAME: &str = "blueprint_builder_test_add_physical_disks"; let logctx = test_setup_log(TEST_NAME); - let (_, input, _) = example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS); + let (collection, input, _) = + example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS); let input = { // Clear out the external networking records from `input`, since // we're building an empty blueprint. @@ -1918,6 +1937,7 @@ pub mod test { &logctx.log, &parent, &input, + &collection, "test", ) .expect("failed to create builder"); @@ -1998,6 +2018,7 @@ pub mod test { &logctx.log, &parent, &input, + &collection, "test", ) .expect("failed to create builder"); @@ -2098,6 +2119,7 @@ pub mod test { &logctx.log, &parent, &input, + &collection, "test", ) .expect("failed to create builder"); @@ -2116,6 +2138,7 @@ pub mod test { &logctx.log, &parent, &input, + &collection, "test", ) .expect("failed to create builder"); @@ -2149,6 +2172,7 @@ pub mod test { &logctx.log, &parent, &input, + &collection, "test", ) .expect("failed to create builder"); @@ -2179,7 +2203,7 @@ pub mod test { "blueprint_builder_test_invalid_parent_blueprint_\ two_zones_with_same_external_ip"; let logctx = test_setup_log(TEST_NAME); - let (_, input, mut parent) = + let (collection, input, mut parent) = example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS); // We should fail if the parent blueprint claims to contain two @@ -2213,6 +2237,7 @@ pub mod test { &logctx.log, &parent, &input, + &collection, "test", ) { Ok(_) => panic!("unexpected success"), @@ -2231,7 +2256,7 @@ pub mod test { "blueprint_builder_test_invalid_parent_blueprint_\ two_nexus_zones_with_same_nic_ip"; let logctx = test_setup_log(TEST_NAME); - let (_, input, mut parent) = + let (collection, input, mut parent) = example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS); // We should fail if the parent blueprint claims to contain two @@ -2265,6 +2290,7 @@ pub mod test { &logctx.log, &parent, &input, + &collection, "test", ) { Ok(_) => panic!("unexpected success"), @@ -2283,7 +2309,7 @@ pub mod test { "blueprint_builder_test_invalid_parent_blueprint_\ two_zones_with_same_vnic_mac"; let logctx = test_setup_log(TEST_NAME); - let (_, input, mut parent) = + let (collection, input, mut parent) = example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS); // We should fail if the parent blueprint claims to contain two @@ -2317,6 +2343,7 @@ pub mod test { &logctx.log, &parent, &input, + &collection, "test", ) { Ok(_) => panic!("unexpected success"), @@ -2335,7 +2362,8 @@ pub mod test { let logctx = test_setup_log(TEST_NAME); // Discard the example blueprint and start with an empty one. - let (_, input, _) = example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS); + let (collection, input, _) = + example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS); let input = { // Clear out the external networking records from `input`, since // we're building an empty blueprint. @@ -2368,6 +2396,7 @@ pub mod test { &logctx.log, &parent, &input, + &collection, "test", ) .expect("constructed builder"); @@ -2409,6 +2438,7 @@ pub mod test { &logctx.log, &parent, &input, + &collection, "test", ) .expect("constructed builder"); diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs b/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs index 25fa819d6d..593c57c331 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/zones.rs @@ -304,6 +304,7 @@ mod tests { &logctx.log, &blueprint_initial, &input2, + &example.collection, "the_test", ) .expect("creating blueprint builder"); @@ -475,6 +476,7 @@ mod tests { &logctx.log, &blueprint, &input2, + &example.collection, "the_test", ) .expect("creating blueprint builder"); diff --git a/nexus/reconfigurator/planning/src/example.rs b/nexus/reconfigurator/planning/src/example.rs index 5d4f3483ff..ea9cf129c7 100644 --- a/nexus/reconfigurator/planning/src/example.rs +++ b/nexus/reconfigurator/planning/src/example.rs @@ -55,11 +55,18 @@ impl ExampleSystem { (test_name, "ExampleSystem initial"), ); + // Start with an empty collection + let collection = system + .to_collection_builder() + .expect("failed to build collection") + .build(); + // Now make a blueprint and collection with some zones on each sled. let mut builder = BlueprintBuilder::new_based_on( log, &initial_blueprint, &base_input, + &collection, "test suite", ) .unwrap(); diff --git a/nexus/reconfigurator/planning/src/planner.rs b/nexus/reconfigurator/planning/src/planner.rs index 69aab88c94..a27a3443f7 100644 --- a/nexus/reconfigurator/planning/src/planner.rs +++ b/nexus/reconfigurator/planning/src/planner.rs @@ -71,6 +71,7 @@ impl<'a> Planner<'a> { &log, parent_blueprint, input, + inventory, creator, )?; Ok(Planner { log, input, blueprint, inventory })