Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[reconfigurator-cli] Fix blueprint-edit failing if there are multiple expunged zones with the same external IP #7307

Merged
merged 8 commits into from
Jan 9, 2025
Merged

[reconfigurator-cli] Fix blueprint-edit failing if there are multiple expunged zones with the same external IP #7307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
db90b4a
add external DNS zones to reconfigurator-cli's example system
jgallagher Jan 6, 2025
275e24e
expunge-zone: no longer require specifying parent sled ID
jgallagher Jan 6, 2025
a147954
blueprint-plan: allow omitted collection ID if exactly 1 exists
jgallagher Jan 6, 2025
36d82d4
add test and fix for expunge/plan/expunge external-dns
jgallagher Jan 6, 2025
839899e
check in new test files
jgallagher Jan 6, 2025
c43fbd4
remove fake comments from test input
jgallagher Jan 6, 2025
e653b3a
Merge branch 'main' into john/reconfigurator-cli-ergo
jgallagher Jan 8, 2025
c52efb6
update test input IDs for new RNGs
jgallagher Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions dev-tools/reconfigurator-cli/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 44 additions & 8 deletions dev-tools/reconfigurator-cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)]
Expand All @@ -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)]
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Comment on lines +863 to +865
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this match all zones or some subset?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think All is right - in this case the user passed in an explicit zone ID, and we're just trying to find it. This would allow someone to try to use reconfigurator-cli to expunge an already-expunged zones, but that seems fine, I think?

{
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")?;
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 10 additions & 0 deletions dev-tools/reconfigurator-cli/tests/input/cmds-expunge-newly-added.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
15 changes: 9 additions & 6 deletions dev-tools/reconfigurator-cli/tests/output/cmd-example-stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Empty file added dev-tools/reconfigurator-cli/tests/output/cmd-expunge-newly-added-stderr
View file
Open in desktop
Empty file.
Loading
Loading