Add parent_blueprint to PlanningInput (#9481)

There are two main changes in this PR:

* `PlanningInput` now has a `parent_blueprint` field
* The `Planner` no longer takes a separate parent blueprint argument
(since it comes with the planning input)

There are a bunch of other mechanical changes, mostly related to
trafficking in `Arc<Blueprint>`s in various places.

The rationale for change this is to ensure correctness as we get into
#5552; I wrote this up in the new doc comment on
`PlanningInputFromDb::assemble()`; feedback welcome!

This is staged on top of #9473. I had tried to make this change before
doing that work, and updating the planner tests was a true nightmare.
Happy to report updating the tests after that change was quite easy!
(Didn't touch the planner tests at all, which only left a few that
constructed `PlanningInput`s outside the simulator.)

Author: jgallagher

dev-tools/reconfigurator-cli/src/lib.rs

@@ -77,6 +77,7 @@ use std::io::IsTerminal;
 use std::net::IpAddr;
 use std::num::ParseIntError;
 use std::str::FromStr;
+use std::sync::Arc;
 use swrite::{SWrite, swrite, swriteln};
 use tabled::Tabled;
 use tufaceous_artifact::ArtifactHash;
@@ -120,13 +121,13 @@ impl ReconfiguratorSim {
 
     fn planning_input(
         &self,
-        parent_blueprint: &Blueprint,
+        parent_blueprint: &Arc<Blueprint>,
     ) -> anyhow::Result<PlanningInput> {
         let state = self.current_state();
         let mut builder = state
             .system()
             .description()
-            .to_planning_input_builder()
+            .to_planning_input_builder(Arc::clone(parent_blueprint))
             .context("generating planning input builder")?;
 
         // The internal and external DNS numbers that go here are supposed to be
@@ -371,7 +372,6 @@ impl ReconfiguratorSim {
             .context("failed to construct planning input")?;
         let planner = Planner::new_based_on(
             self.log.clone(),
-            parent_blueprint,
             &planning_input,
             creator,
             collection,
@@ -1735,10 +1735,14 @@ fn cmd_sled_list(
     }
 
     let state = sim.current_state();
+    let parent_blueprint = state
+        .system()
+        .resolve_and_get_blueprint(BlueprintId::Target)
+        .expect("target blueprint is always present");
     let planning_input = state
         .system()
         .description()
-        .to_planning_input_builder()
+        .to_planning_input_builder(Arc::clone(parent_blueprint))
         .context("failed to generate planning input")?
         .build();
     let rows = planning_input.all_sleds(SledFilter::Commissioned).map(
@@ -1823,8 +1827,12 @@ fn cmd_sled_show(
         description.sled_rot_pending_persistent_boot_preference(sled_id)?;
     let rot_transient_boot_preference =
         description.sled_rot_transient_boot_preference(sled_id)?;
+    let parent_blueprint = state
+        .system()
+        .resolve_and_get_blueprint(BlueprintId::Target)
+        .expect("target blueprint is always present");
     let planning_input = description
-        .to_planning_input_builder()
+        .to_planning_input_builder(Arc::clone(parent_blueprint))
         .context("failed to generate planning_input builder")?
         .build();
     let sled = planning_input.sled_lookup(args.filter, sled_id)?;

live-tests/tests/test_nexus_add_remove.rs

@@ -30,6 +30,7 @@ use omicron_test_utils::dev::poll::CondCheckError;
 use omicron_test_utils::dev::poll::wait_for_condition;
 use slog::{debug, info};
 use std::net::SocketAddrV6;
+use std::sync::Arc;
 use std::time::Duration;
 
 // TODO-coverage This test could check other stuff:
@@ -49,15 +50,23 @@ async fn test_nexus_add_remove(lc: &LiveTestContext) {
     let opctx = lc.opctx();
     let datastore = lc.datastore();
 
+    let (_, parent_blueprint) = datastore
+        .blueprint_target_get_current_full(opctx)
+        .await
+        .expect("obtained current target 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");
+    let planning_input = PlanningInputFromDb::assemble(
+        &opctx,
+        &datastore,
+        planner_config,
+        Arc::new(parent_blueprint),
+    )
+    .await
+    .expect("planning input");
     let initial_nexus_clients = lc.all_internal_nexus_clients().await.unwrap();
     let nexus = initial_nexus_clients.first().expect("internal Nexus client");
 
@@ -271,17 +280,20 @@ async fn test_nexus_add_remove(lc: &LiveTestContext) {
 
     // Now run through the planner.
     info!(log, "running through planner");
-    let planning_input =
-        PlanningInputFromDb::assemble(&opctx, &datastore, planner_config)
-            .await
-            .expect("planning input");
     let (_, parent_blueprint) = datastore
         .blueprint_target_get_current_full(opctx)
         .await
         .expect("getting latest target blueprint");
+    let planning_input = PlanningInputFromDb::assemble(
+        &opctx,
+        &datastore,
+        planner_config,
+        Arc::new(parent_blueprint),
+    )
+    .await
+    .expect("planning input");
     let planner = Planner::new_based_on(
         log.clone(),
-        &parent_blueprint,
         &planning_input,
         "live test suite",
         &latest_collection,

live-tests/tests/test_nexus_handoff.rs

@@ -36,6 +36,7 @@ use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::net::IpAddr;
 use std::net::SocketAddr;
+use std::sync::Arc;
 use std::time::Duration;
 
 #[live_test]
@@ -163,10 +164,18 @@ async fn test_nexus_handoff(lc: &LiveTestContext) {
         .await
         .expect("obtained latest reconfigurator config")
         .map_or_else(PlannerConfig::default, |cs| cs.config.planner_config);
-    let planning_input =
-        PlanningInputFromDb::assemble(opctx, datastore, planner_config)
-            .await
-            .expect("planning input");
+    let (_, parent_blueprint) = datastore
+        .blueprint_target_get_current_full(opctx)
+        .await
+        .expect("getting latest target blueprint");
+    let planning_input = PlanningInputFromDb::assemble(
+        opctx,
+        datastore,
+        planner_config,
+        Arc::new(parent_blueprint),
+    )
+    .await
+    .expect("planning input");
     let (_blueprint_initial, blueprint_new_nexus) =
         blueprint_edit_current_target(
             log,

nexus/db-queries/src/db/datastore/rack.rs

@@ -1203,12 +1203,15 @@ mod test {
         system: &SystemDescription,
         test_name: &str,
     ) -> BlueprintBuilder<'static> {
-        static EMPTY_BLUEPRINT: LazyLock<Blueprint> = LazyLock::new(|| {
-            BlueprintBuilder::build_empty("EMPTY_BLUEPRINT static")
-        });
+        static EMPTY_BLUEPRINT: LazyLock<Arc<Blueprint>> =
+            LazyLock::new(|| {
+                Arc::new(BlueprintBuilder::build_empty(
+                    "EMPTY_BLUEPRINT static",
+                ))
+            });
 
         let planning_input = system
-            .to_planning_input_builder()
+            .to_planning_input_builder(Arc::clone(&EMPTY_BLUEPRINT))
             .expect("created planning input builder")
             .build();
 

nexus/db-queries/src/db/datastore/vpc.rs

@@ -3003,6 +3003,7 @@ mod tests {
     use slog::info;
     use std::net::Ipv4Addr;
     use std::net::Ipv6Addr;
+    use std::sync::Arc;
 
     // Test that we detect the right error condition and return None when we
     // fail to insert a VPC due to VNI exhaustion.
@@ -3283,10 +3284,6 @@ mod tests {
             datastore.sled_upsert(sled_update).await.expect("upserting sled");
         }
         sled_ids.sort_unstable();
-        let planning_input = system
-            .to_planning_input_builder()
-            .expect("creating planning builder")
-            .build();
 
         // Helper to convert a zone's nic into an insertable nic.
         let db_nic_from_zone = |zone_config: &BlueprintZoneConfig| {
@@ -3314,9 +3311,14 @@ mod tests {
         };
 
         // Create an initial, empty blueprint, and make it the target.
-        let bp0 = BlueprintBuilder::build_empty("test");
+        let bp0 = Arc::new(BlueprintBuilder::build_empty("test"));
         bp_insert_and_make_target(&opctx, &datastore, &bp0).await;
 
+        let planning_input = system
+            .to_planning_input_builder(Arc::clone(&bp0))
+            .expect("creating planning builder")
+            .build();
+
         // Our blueprint doesn't describe any services, so we shouldn't find any
         // sled IDs running services.
         assert_service_sled_ids(&datastore, &[]).await;

nexus/reconfigurator/planning/src/blueprint_builder/builder.rs

@@ -2641,7 +2641,7 @@ pub mod test {
             }
         }
 
-        let blueprint2 = builder.build(BlueprintSource::Test);
+        let blueprint2 = Arc::new(builder.build(BlueprintSource::Test));
         verify_blueprint(&blueprint2, &example.input);
         let summary = blueprint2.diff_since_blueprint(&blueprint1);
         println!(
@@ -2658,7 +2658,11 @@ pub mod test {
 
         let _ =
             example.system.sled(SledBuilder::new().id(new_sled_id)).unwrap();
-        let input = example.system.to_planning_input_builder().unwrap().build();
+        let input = example
+            .system
+            .to_planning_input_builder(Arc::clone(&blueprint2))
+            .unwrap()
+            .build();
         let mut builder = BlueprintBuilder::new_based_on(
             &logctx.log,
             &blueprint2,

nexus/reconfigurator/planning/src/example.rs

@@ -10,6 +10,7 @@ use std::fmt;
 use std::hash::Hash;
 use std::net::IpAddr;
 use std::net::Ipv4Addr;
+use std::sync::Arc;
 
 use crate::blueprint_builder::BlueprintBuilder;
 use crate::blueprint_editor::ExternalNetworkingAllocator;
@@ -182,7 +183,7 @@ pub struct ExampleSystem {
     pub input: PlanningInput,
     pub collection: Collection,
     /// The initial blueprint that was used to describe the system.
-    pub initial_blueprint: Blueprint,
+    pub initial_blueprint: Arc<Blueprint>,
 }
 
 /// Returns a collection, planning input, and blueprint describing a pretty
@@ -531,16 +532,17 @@ impl ExampleSystemBuilder {
         }
 
         let mut input_builder = system
-            .to_planning_input_builder()
+            .to_planning_input_builder(Arc::new(
+                // Start with an empty blueprint.
+                BlueprintBuilder::build_empty_seeded(
+                    "test suite",
+                    rng.blueprint1_rng,
+                ),
+            ))
             .expect("failed to make planning input builder");
 
         let base_input = input_builder.clone().build();
-
-        // Start with an empty blueprint.
-        let initial_blueprint = BlueprintBuilder::build_empty_seeded(
-            "test suite",
-            rng.blueprint1_rng,
-        );
+        let initial_blueprint = Arc::clone(&base_input.parent_blueprint());
 
         // Now make a blueprint and collection with some zones on each sled.
         let mut builder = BlueprintBuilder::new_based_on(

nexus/reconfigurator/planning/src/planner.rs

@@ -175,7 +175,6 @@ pub struct Planner<'a> {
 impl<'a> Planner<'a> {
     pub fn new_based_on(
         log: Logger,
-        parent_blueprint: &'a Blueprint,
         input: &'a PlanningInput,
         creator: &str,
         // NOTE: Right now, we just assume that this is the latest inventory
@@ -185,7 +184,7 @@ impl<'a> Planner<'a> {
     ) -> anyhow::Result<Planner<'a>> {
         let mut blueprint = BlueprintBuilder::new_based_on(
             &log,
-            parent_blueprint,
+            input.parent_blueprint(),
             creator,
             rng,
         )?;

nexus/reconfigurator/planning/src/system.rs

@@ -30,6 +30,7 @@ use nexus_sled_agent_shared::inventory::SledRole;
 use nexus_sled_agent_shared::inventory::ZoneImageResolverInventory;
 use nexus_sled_agent_shared::inventory::ZoneKind;
 use nexus_sled_agent_shared::inventory::ZoneManifestBootInventory;
+use nexus_types::deployment::Blueprint;
 use nexus_types::deployment::ClickhousePolicy;
 use nexus_types::deployment::CockroachDbClusterVersion;
 use nexus_types::deployment::CockroachDbSettings;
@@ -1140,6 +1141,7 @@ impl SystemDescription {
     /// NICs.
     pub fn to_planning_input_builder(
         &self,
+        parent_blueprint: Arc<Blueprint>,
     ) -> anyhow::Result<PlanningInputBuilder> {
         let policy = Policy {
             external_ips: self.external_ip_policy.clone(),
@@ -1159,6 +1161,7 @@ impl SystemDescription {
             planner_config: self.planner_config,
         };
         let mut builder = PlanningInputBuilder::new(
+            parent_blueprint,
             policy,
             self.internal_dns_version,
             self.external_dns_version,