Skip to content

Commit

Permalink
[reconfigurator] BlueprintBuilder cleanup 5/5 - Convert discretionary…
Browse files Browse the repository at this point in the history
… ensure_zone_multiple_* to add_zone_* (#7107)

All of the callers of the `ensure_zone_multiple_*` methods to add
discretionary zones were, in practice, wanting to _add_ a specific
number of zones. This rewords all of those methods to be imperative,
allowing us to remove a pretty significant chunk of now-unnecessary
checks and logic.

Builds on and is staged on top of #7106.
  • Loading branch information
jgallagher authored Nov 25, 2024
1 parent 1cf9437 commit 106e722
Show file tree
Hide file tree
Showing 18 changed files with 395 additions and 876 deletions.
32 changes: 4 additions & 28 deletions dev-tools/reconfigurator-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@ use indent_write::fmt::IndentWriter;
use internal_dns_types::diff::DnsDiff;
use nexus_inventory::CollectionBuilder;
use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder;
use nexus_reconfigurator_planning::blueprint_builder::EnsureMultiple;
use nexus_reconfigurator_planning::example::ExampleSystemBuilder;
use nexus_reconfigurator_planning::planner::Planner;
use nexus_reconfigurator_planning::system::{SledBuilder, SystemDescription};
use nexus_reconfigurator_simulation::SimState;
use nexus_reconfigurator_simulation::SimStateBuilder;
use nexus_reconfigurator_simulation::Simulator;
use nexus_sled_agent_shared::inventory::ZoneKind;
use nexus_types::deployment::execution;
use nexus_types::deployment::execution::blueprint_external_dns_config;
use nexus_types::deployment::execution::blueprint_internal_dns_config;
Expand Down Expand Up @@ -819,37 +817,15 @@ fn cmd_blueprint_edit(

let label = match args.edit_command {
BlueprintEditCommands::AddNexus { sled_id } => {
let current = builder
.sled_num_running_zones_of_kind(sled_id, ZoneKind::Nexus);
let added = builder
.sled_ensure_zone_multiple_nexus(sled_id, current + 1)
builder
.sled_add_zone_nexus(sled_id)
.context("failed to add Nexus zone")?;
assert_matches::assert_matches!(
added,
EnsureMultiple::Changed {
added: 1,
updated: 0,
expunged: 0,
removed: 0
}
);
format!("added Nexus zone to sled {}", sled_id)
}
BlueprintEditCommands::AddCockroach { sled_id } => {
let current = builder
.sled_num_running_zones_of_kind(sled_id, ZoneKind::CockroachDb);
let added = builder
.sled_ensure_zone_multiple_cockroachdb(sled_id, current + 1)
builder
.sled_add_zone_cockroachdb(sled_id)
.context("failed to add CockroachDB zone")?;
assert_matches::assert_matches!(
added,
EnsureMultiple::Changed {
added: 1,
updated: 0,
expunged: 0,
removed: 0
}
);
format!("added CockroachDB zone to sled {}", sled_id)
}
BlueprintEditCommands::ExpungeZone { sled_id, zone_id } => {
Expand Down
17 changes: 2 additions & 15 deletions live-tests/tests/test_nexus_add_remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
mod common;

use anyhow::Context;
use assert_matches::assert_matches;
use common::reconfigurator::blueprint_edit_current_target;
use common::LiveTestContext;
use futures::TryStreamExt;
Expand All @@ -14,7 +13,6 @@ 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;
use nexus_sled_agent_shared::inventory::ZoneKind;
use nexus_types::deployment::SledFilter;
Expand Down Expand Up @@ -63,20 +61,9 @@ async fn test_nexus_add_remove(lc: &LiveTestContext) {
&collection,
&nexus,
&|builder: &mut BlueprintBuilder| {
let nnexus = builder
.sled_num_running_zones_of_kind(sled_id, ZoneKind::Nexus);
let count = builder
.sled_ensure_zone_multiple_nexus(sled_id, nnexus + 1)
builder
.sled_add_zone_nexus(sled_id)
.context("adding Nexus zone")?;
assert_matches!(
count,
EnsureMultiple::Changed {
added: 1,
removed: 0,
updated: 0,
expunged: 0
}
);
Ok(())
},
)
Expand Down
14 changes: 2 additions & 12 deletions nexus/db-queries/src/db/datastore/vpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3130,12 +3130,7 @@ mod tests {
.expect("ensured disks");
}
builder
.sled_ensure_zone_multiple_nexus_with_config(
sled_ids[2],
1,
false,
Vec::new(),
)
.sled_add_zone_nexus_with_config(sled_ids[2], false, Vec::new())
.expect("added nexus to third sled");
builder.build()
};
Expand Down Expand Up @@ -3201,12 +3196,7 @@ mod tests {
.expect("created blueprint builder");
for &sled_id in &sled_ids {
builder
.sled_ensure_zone_multiple_nexus_with_config(
sled_id,
1,
false,
Vec::new(),
)
.sled_add_zone_nexus_with_config(sled_id, false, Vec::new())
.expect("added nexus to third sled");
}
builder.build()
Expand Down
26 changes: 1 addition & 25 deletions nexus/reconfigurator/execution/src/dns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@ mod test {
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::ExampleSystemBuilder;
use nexus_reconfigurator_preparation::PlanningInputFromDb;
use nexus_sled_agent_shared::inventory::OmicronZoneConfig;
Expand Down Expand Up @@ -1384,30 +1383,7 @@ mod test {
.unwrap();
let sled_id =
blueprint.sleds().next().expect("expected at least one sled");
let nalready =
builder.sled_num_running_zones_of_kind(sled_id, ZoneKind::Nexus);
let rv = builder
.sled_ensure_zone_multiple_nexus(sled_id, nalready + 1)
.unwrap();
assert_eq!(
rv,
EnsureMultiple::Changed {
added: 1,
updated: 0,
expunged: 0,
removed: 0
}
);
builder
.sled_ensure_zone_datasets(
sled_id,
&planning_input
.sled_lookup(SledFilter::InService, sled_id)
.expect("found sled")
.resources,
)
.expect("ensured datasets");

builder.sled_add_zone_nexus(sled_id).unwrap();
let blueprint2 = builder.build();
eprintln!("blueprint2: {}", blueprint2.display());
// Figure out the id of the new zone.
Expand Down
Loading

0 comments on commit 106e722

Please sign in to comment.