[reconfigurator-cli] Fix blueprint-edit failing if there are multiple…

… expunged zones with the same external IP (#7307) I tried to reproduce #7305 using just reconfigurator-cli's simulated example system. This required one change (adding external DNS zones, db90b4a), and while I was here I smoothed over a couple of minor ergonomic annoyances (275e24e is uncontroversial, I hope, and a147954 which is a little less clearly good - happy to back this one out if folks don't like it). With these changes, I can get to a reproduction: ``` generated RNG seed: discernibly-adaptive-grison 〉load-example loaded example system with: - collection: ed006741-bed9-4fd1-87e7-26c95a6f4dd9 - blueprint: 72183372-6429-4ab7-af83-5fea1a40bbcf 〉blueprint-show 72183372-6429-4ab7-af83-5fea1a40bbcf ... snip, finding an external DNS zone to expunge ... 〉blueprint-edit 72183372-6429-4ab7-af83-5fea1a40bbcf expunge-zone 34feb176-1129-4ca8-8df8-6e6cb1be2173 blueprint 9a5f95b4-7eb7-4b8d-b953-5fc9f403f330 created from blueprint 72183372-6429-4ab7-af83-5fea1a40bbcf: expunged zone 34feb176-1129-4ca8-8df8-6e6cb1be2173 from sled e78b0535-959d-4ba9-8d8c-ac3d591fc490 〉blueprint-plan 9a5f95b4-7eb7-4b8d-b953-5fc9f403f330 ... snip, finding the ID of the newly-generated blueprint ... 〉blueprint-diff 9a5f95b4-7eb7-4b8d-b953-5fc9f403f330 0a054c71-82cb-4589-afa3-09ced1e425b9 ... snip, finding the ID of the newly-added external DNS zone ... 〉blueprint-edit 0a054c71-82cb-4589-afa3-09ced1e425b9 expunge-zone 900df298-9c72-4a64-a4a7-7a92b722f68c error: adding omicron zone external IP: associating Omicron zone 900df298-9c72-4a64-a4a7-7a92b722f68c with Floating(OmicronZoneExternalFloatingIp { id: 1c962069-7ced-4162-a11f-acb40fd954f4 (external_ip), ip: 198.51.100.3 }) failed due to duplicates: duplicate entry: OmicronZoneExternalIpEntry { zone_id: 900df298-9c72-4a64-a4a7-7a92b722f68c (service), ip: Floating(OmicronZoneExternalFloatingIp { id: 1c962069-7ced-4162-a11f-acb40fd954f4 (external_ip), ip: 198.51.100.3 }) } conflicts with existing: [OmicronZoneExternalIpEntry { zone_id: 34feb176-1129-4ca8-8df8-6e6cb1be2173 (service), ip: Floating(OmicronZoneExternalFloatingIp { id: 1b31bc81-c2ef-4915-be62-c97bd7881b07 (external_ip), ip: 198.51.100.3 }) }] ``` Fixes #7305
oxidecomputer · Jan 9, 2025 · 667832c · 667832c
1 parent f63ed09
commit 667832c
Show file tree

Hide file tree

Showing 8 changed files with 534 additions and 15 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/dev-tools/reconfigurator-cli/Cargo.toml b/dev-tools/reconfigurator-cli/Cargo.toml
@@ -20,6 +20,7 @@ colored.workspace = true
 humantime.workspace = true
 indent_write.workspace = true
 internal-dns-types.workspace = true
+itertools.workspace = true
 nexus-inventory.workspace = true
 nexus-reconfigurator-planning.workspace = true
 nexus-reconfigurator-simulation.workspace = true

diff --git 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 indent_write::fmt::IndentWriter;
 use internal_dns_types::diff::DnsDiff;
+use itertools::Itertools;
 use log_capture::LogCapture;
 use nexus_inventory::CollectionBuilder;
 use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder;
@@ -120,8 +121,8 @@ impl ReconfiguratorSim {
         builder.set_internal_dns_version(parent_blueprint.internal_dns_version);
         builder.set_external_dns_version(parent_blueprint.external_dns_version);
 
-        for (_, zone) in
-            parent_blueprint.all_omicron_zones(BlueprintZoneFilter::All)
+        for (_, zone) in parent_blueprint
+            .all_omicron_zones(BlueprintZoneFilter::ShouldBeRunning)
         {
             if let Some((external_ip, nic)) =
                 zone.zone_type.external_networking()
@@ -404,7 +405,10 @@ struct BlueprintPlanArgs {
     /// id of the blueprint on which this one will be based
     parent_blueprint_id: Uuid,
     /// id of the inventory collection to use in planning
-    collection_id: CollectionUuid,
+    ///
+    /// Must be provided unless there is only one collection in the loaded
+    /// state.
+    collection_id: Option<CollectionUuid>,
 }
 
 #[derive(Debug, Args)]
@@ -431,7 +435,7 @@ enum BlueprintEditCommands {
     /// add a CockroachDB instance to a particular sled
     AddCockroach { sled_id: SledUuid },
     /// expunge a particular zone from a particular sled
-    ExpungeZone { sled_id: SledUuid, zone_id: OmicronZoneUuid },
+    ExpungeZone { zone_id: OmicronZoneUuid },
 }
 
 #[derive(Debug, Args)]
@@ -762,10 +766,25 @@ fn cmd_blueprint_plan(
     let parent_blueprint_id = args.parent_blueprint_id;
     let collection_id = args.collection_id;
     let parent_blueprint = system.get_blueprint(parent_blueprint_id)?;
-    let collection = system.get_collection(collection_id)?;
+    let collection = match collection_id {
+        Some(collection_id) => system.get_collection(collection_id)?,
+        None => {
+            let mut all_collections_iter = system.all_collections();
+            match all_collections_iter.len() {
+                0 => bail!("cannot plan blueprint with no loaded collections"),
+                1 => all_collections_iter.next().expect("iter length is 1"),
+                _ => bail!(
+                    "blueprint-plan: must specify collection ID (one of {:?})",
+                    all_collections_iter.map(|c| c.id).join(", ")
+                ),
+            }
+        }
+    };
 
     let creator = "reconfigurator-sim";
-    let planning_input = sim.planning_input(parent_blueprint)?;
+    let planning_input = sim
+        .planning_input(parent_blueprint)
+        .context("failed to construct planning input")?;
     let planner = Planner::new_based_on(
         sim.log.clone(),
         parent_blueprint,
@@ -799,7 +818,9 @@ fn cmd_blueprint_edit(
     let blueprint_id = args.blueprint_id;
     let blueprint = system.get_blueprint(blueprint_id)?;
     let creator = args.creator.as_deref().unwrap_or("reconfigurator-cli");
-    let planning_input = sim.planning_input(blueprint)?;
+    let planning_input = sim
+        .planning_input(blueprint)
+        .context("failed to create planning input")?;
 
     // TODO: We may want to do something other than just using the latest
     // collection -- add a way to specify which collection to use.
@@ -836,7 +857,20 @@ fn cmd_blueprint_edit(
                 .context("failed to add CockroachDB zone")?;
             format!("added CockroachDB zone to sled {}", sled_id)
         }
-        BlueprintEditCommands::ExpungeZone { sled_id, zone_id } => {
+        BlueprintEditCommands::ExpungeZone { zone_id } => {
+            let mut parent_sled_id = None;
+            for sled_id in builder.sled_ids_with_zones() {
+                if builder
+                    .current_sled_zones(sled_id, BlueprintZoneFilter::All)
+                    .any(|z| z.id == zone_id)
+                {
+                    parent_sled_id = Some(sled_id);
+                    break;
+                }
+            }
+            let Some(sled_id) = parent_sled_id else {
+                bail!("could not find parent sled for zone {zone_id}");
+            };
             builder
                 .sled_expunge_zone(sled_id, zone_id)
                 .context("failed to expunge zone")?;
@@ -1279,6 +1313,8 @@ fn cmd_load_example(
                     .num_nexus()
                     .map_or(NEXUS_REDUNDANCY, |n| n.into()),
             )
+            .external_dns_count(3)
+            .context("invalid external DNS zone count")?
             .create_zones(!args.no_zones)
             .create_disks_in_blueprint(!args.no_disks_in_blueprint)
             .build();

diff --git a/dev-tools/reconfigurator-cli/tests/input/cmds-expunge-newly-added.txt b/dev-tools/reconfigurator-cli/tests/input/cmds-expunge-newly-added.txt
@@ -0,0 +1,10 @@
+load-example
+
+blueprint-show 3f00b694-1b16-4aaa-8f78-e6b3a527b434
+blueprint-edit 3f00b694-1b16-4aaa-8f78-e6b3a527b434 expunge-zone 9995de32-dd52-4eb1-b0eb-141eb84bc739
+
+blueprint-show 366b0b68-d80e-4bc1-abd3-dc69837847e0
+blueprint-plan 366b0b68-d80e-4bc1-abd3-dc69837847e0
+
+blueprint-show 9c998c1d-1a7b-440a-ae0c-40f781dea6e2
+blueprint-edit 9c998c1d-1a7b-440a-ae0c-40f781dea6e2 expunge-zone d786ef4a-5acb-4f5d-a732-a00addf986b5
diff --git a/dev-tools/reconfigurator-cli/tests/output/cmd-example-stdout b/dev-tools/reconfigurator-cli/tests/output/cmd-example-stdout
@@ -91,15 +91,16 @@ parent:    02697f74-b14a-4418-90f0-c28b2a3a6aa9
     clickhouse        3a3f243b-7e9e-4818-bb7c-fe30966b2949   in service    fd00:1122:3344:102::23
     crucible          279b230f-5e77-4960-b08e-594c6f2f57c0   in service    fd00:1122:3344:102::2a
     crucible          4328425e-f5ba-436a-9e46-3f337f07671e   in service    fd00:1122:3344:102::2d
+    crucible          4e60ff64-155b-44e8-9d39-e6de8c5d5fd3   in service    fd00:1122:3344:102::2f
     crucible          61282e88-43b3-4011-9314-b0929880895a   in service    fd00:1122:3344:102::2b
     crucible          7537db8e-11c9-4a84-9dc7-b3ae7b657cc4   in service    fd00:1122:3344:102::2e
     crucible          b37ebcb3-533b-4fd7-9960-bb1ac511bea2   in service    fd00:1122:3344:102::27
-    crucible          b4c3734e-b6d8-47d8-a695-5dad2c21622e   in service    fd00:1122:3344:102::25
     crucible          b8aba012-d4b3-48e1-af2d-cf6265e02bd7   in service    fd00:1122:3344:102::2c
     crucible          e1b405aa-a32c-4410-8335-59237a7bc9ad   in service    fd00:1122:3344:102::28
     crucible          e696d6f8-c706-4ca7-8846-561f0323ccbf   in service    fd00:1122:3344:102::29
     crucible          f69e36ff-ea67-4d1f-bc73-3d2a0315c77f   in service    fd00:1122:3344:102::26
-    crucible_pantry   c5fefafb-d65c-44e0-b45e-d2097dca02d1   in service    fd00:1122:3344:102::24
+    crucible_pantry   b4c3734e-b6d8-47d8-a695-5dad2c21622e   in service    fd00:1122:3344:102::25
+    external_dns      c5fefafb-d65c-44e0-b45e-d2097dca02d1   in service    fd00:1122:3344:102::24
     internal_dns      426face8-6cc4-4ba0-b3a3-8492876ecd37   in service    fd00:1122:3344:1::1   
     internal_ntp      eaa48c21-f17c-41f7-9e85-fc6a10913b31   in service    fd00:1122:3344:102::21
     nexus             db1aa26e-7608-4ce0-933e-9968489f8a46   in service    fd00:1122:3344:102::22
@@ -132,13 +133,14 @@ parent:    02697f74-b14a-4418-90f0-c28b2a3a6aa9
     crucible          1a07a7f2-76ae-4670-8491-3383bb3e2d19   in service    fd00:1122:3344:103::2d
     crucible          1a7ddc8f-90c7-4842-81a9-2abfc76e3cb4   in service    fd00:1122:3344:103::26
     crucible          2f5dec78-6071-41c1-8f3f-2f4e98fdad0a   in service    fd00:1122:3344:103::25
-    crucible          41f7b32f-d85f-4cce-853c-144342cc8361   in service    fd00:1122:3344:103::24
     crucible          52960cc6-af73-4ae6-b776-b4bcc371fd68   in service    fd00:1122:3344:103::2c
     crucible          6914b9aa-5712-405c-817a-77b2e6c6a824   in service    fd00:1122:3344:103::27
     crucible          8e92b0f0-77b7-4b95-905f-653ee962b932   in service    fd00:1122:3344:103::2b
+    crucible          a2a98ae0-ee42-4933-9c4b-660123bc693d   in service    fd00:1122:3344:103::2e
     crucible          d9ea5125-d6f0-4bfd-9ebd-497569d91adf   in service    fd00:1122:3344:103::2a
     crucible          f7ced707-a517-4529-91fa-03dc7683f413   in service    fd00:1122:3344:103::29
-    crucible_pantry   25087c5b-58b9-46f2-9e4c-e9440c081111   in service    fd00:1122:3344:103::23
+    crucible_pantry   41f7b32f-d85f-4cce-853c-144342cc8361   in service    fd00:1122:3344:103::24
+    external_dns      25087c5b-58b9-46f2-9e4c-e9440c081111   in service    fd00:1122:3344:103::23
     internal_dns      5c1386b0-ed6b-4e09-8a65-7d9f47c41839   in service    fd00:1122:3344:2::1   
     internal_ntp      9ec70cc1-a22d-40df-9697-8a4db3c72d74   in service    fd00:1122:3344:103::21
     nexus             3bfd90d6-0640-4f63-a578-76277ce9c7c6   in service    fd00:1122:3344:103::22
@@ -170,14 +172,15 @@ parent:    02697f74-b14a-4418-90f0-c28b2a3a6aa9
     crucible          157d5b03-6897-4e80-9357-3cf733efe4b5   in service    fd00:1122:3344:101::28
     crucible          7341456c-4c6c-4bb7-8be4-2acac834886f   in service    fd00:1122:3344:101::2a
     crucible          793a6315-a07b-4fcf-a0b4-633d5c53b8cf   in service    fd00:1122:3344:101::27
+    crucible          7ce8eb07-58a7-4f1d-ba61-16db33b6fedd   in service    fd00:1122:3344:101::2e
     crucible          9915de3b-8104-40ca-a6b5-46132d26bb15   in service    fd00:1122:3344:101::29
     crucible          a975d276-7434-4def-8f5b-f250657d1040   in service    fd00:1122:3344:101::2c
     crucible          b41461de-6b60-4d35-ad90-336eb1fa9874   in service    fd00:1122:3344:101::26
     crucible          d1374f2f-e9ba-4046-ba0b-83da927ba0d3   in service    fd00:1122:3344:101::2b
     crucible          dc3c9584-44d8-4be6-b215-2df289f5763d   in service    fd00:1122:3344:101::25
     crucible          e70d6f37-d0a6-4309-93b5-4f2f72b779a7   in service    fd00:1122:3344:101::2d
-    crucible          efa9fb1c-9431-4072-877d-ff33d9d926ba   in service    fd00:1122:3344:101::24
-    crucible_pantry   761999e7-cf90-412c-91d8-f3247507edbc   in service    fd00:1122:3344:101::23
+    crucible_pantry   efa9fb1c-9431-4072-877d-ff33d9d926ba   in service    fd00:1122:3344:101::24
+    external_dns      761999e7-cf90-412c-91d8-f3247507edbc   in service    fd00:1122:3344:101::23
     internal_dns      a2708dbc-a751-4c26-a1ed-6eaadf3402cf   in service    fd00:1122:3344:3::1   
     internal_ntp      4bb07cd6-dc94-4601-ac22-c7ad972735b3   in service    fd00:1122:3344:101::21
     nexus             ba910747-f596-4088-a2d4-4372ee883dfd   in service    fd00:1122:3344:101::22

diff --git a/dev-tools/reconfigurator-cli/tests/output/cmd-expunge-newly-added-stderr b/dev-tools/reconfigurator-cli/tests/output/cmd-expunge-newly-added-stderr