Skip to content

Commit

Permalink
Remove ability to generate a Blueprint from an inventory collection (#…
Browse files Browse the repository at this point in the history 
…5583)

We want to add information in blueprints, particularly
`BlueprintZoneConfig` and `BlueprintZoneType`, that is not present in
the sled-agent types `OmicronZoneConfig` and `OmicronZoneType`. Today on
main conversion between those types is bidirectional. As we add to
blueprints, we will continue to be able to convert a
`BlueprintZoneConfig` into an `OmicronZoneConfig`, but the opposite
direction will become more and more difficult as callers need to provide
the additional information required for blueprints.

We have enough users of the inventory -> blueprint direction that it's
quite painful to try to add to blueprints, so this PR attempts to knock
out one of the more common uses: converting an inventory collection into
a blueprint via `BlueprintBuilder::build_initial_from_collection()`.
This method is removed, and all the remaining changes are fallout from
that. Most uses of this have been replaced by either the blueprint
produced by `ExampleSystem` or the new
`BlueprintBuilder::build_empty_with_sleds()` helper for constructing an
empty blueprint. One test needed the full blueprint-from-inventory, so
that one actually gained a new use of converting OmicronZoneConfig ->
BlueprintZoneConfig. (Not ideal, but fine for now!)
  • Loading branch information
@jgallagher
jgallagher authored Apr 20, 2024
1 parent a080dff commit 84e9c27
Show file tree
Hide file tree
Showing 16 changed files with 448 additions and 906 deletions.
31 changes: 0 additions & 31 deletions dev-tools/omdb/src/bin/omdb/nexus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ enum BlueprintsCommands {
Delete(BlueprintIdArgs),
/// Interact with the current target blueprint
Target(BlueprintsTargetArgs),
/// Generate an initial blueprint from a specific inventory collection
GenerateFromCollection(CollectionIdArgs),
/// Generate a new blueprint
Regenerate,
/// Import a blueprint
Expand Down Expand Up @@ -361,15 +359,6 @@ impl NexusArgs {
let token = omdb.check_allow_destructive()?;
cmd_nexus_blueprints_regenerate(&client, token).await
}
NexusCommands::Blueprints(BlueprintsArgs {
command: BlueprintsCommands::GenerateFromCollection(args),
}) => {
let token = omdb.check_allow_destructive()?;
cmd_nexus_blueprints_generate_from_collection(
&client, args, token,
)
.await
}
NexusCommands::Blueprints(BlueprintsArgs {
command: BlueprintsCommands::Import(args),
}) => {
Expand Down Expand Up @@ -1134,26 +1123,6 @@ async fn cmd_nexus_blueprints_target_set_enabled(
Ok(())
}

async fn cmd_nexus_blueprints_generate_from_collection(
client: &nexus_client::Client,
args: &CollectionIdArgs,
_destruction_token: DestructiveOperationToken,
) -> Result<(), anyhow::Error> {
let blueprint = client
.blueprint_generate_from_collection(
&nexus_client::types::CollectionId {
collection_id: args.collection_id,
},
)
.await
.context("creating blueprint from collection id")?;
eprintln!(
"created blueprint {} from collection id {}",
blueprint.id, args.collection_id
);
Ok(())
}

async fn cmd_nexus_blueprints_regenerate(
client: &nexus_client::Client,
_destruction_token: DestructiveOperationToken,
Expand Down
37 changes: 0 additions & 37 deletions dev-tools/reconfigurator-cli/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,6 @@ fn process_entry(sim: &mut ReconfiguratorSim, entry: String) -> LoopResult {
Commands::InventoryList => cmd_inventory_list(sim),
Commands::InventoryGenerate => cmd_inventory_generate(sim),
Commands::BlueprintList => cmd_blueprint_list(sim),
Commands::BlueprintFromInventory(args) => {
cmd_blueprint_from_inventory(sim, args)
}
Commands::BlueprintEdit(args) => cmd_blueprint_edit(sim, args),
Commands::BlueprintPlan(args) => cmd_blueprint_plan(sim, args),
Commands::BlueprintShow(args) => cmd_blueprint_show(sim, args),
Expand Down Expand Up @@ -374,8 +371,6 @@ enum Commands {

/// list all blueprints
BlueprintList,
/// generate a blueprint that represents the contents of an inventory
BlueprintFromInventory(InventoryArgs),
/// run planner to generate a new blueprint
BlueprintPlan(BlueprintPlanArgs),
/// edit contents of a blueprint directly
Expand Down Expand Up @@ -718,38 +713,6 @@ fn cmd_blueprint_list(
Ok(Some(table))
}

fn cmd_blueprint_from_inventory(
sim: &mut ReconfiguratorSim,
args: InventoryArgs,
) -> anyhow::Result<Option<String>> {
let collection_id = args.collection_id;
let collection = sim
.collections
.get(&collection_id)
.ok_or_else(|| anyhow!("no such collection: {}", collection_id))?;
let dns_version = Generation::new();
let planning_input = sim
.system
.to_planning_input_builder()
.context("generating planning_input builder")?
.build();
let creator = "reconfigurator-sim";
let blueprint = BlueprintBuilder::build_initial_from_collection(
collection,
dns_version,
dns_version,
planning_input.all_sled_ids(SledFilter::All),
creator,
)
.context("building collection")?;
let rv = format!(
"generated blueprint {} from inventory collection {}",
blueprint.id, collection_id
);
sim.blueprint_insert_new(blueprint);
Ok(Some(rv))
}

fn cmd_blueprint_plan(
sim: &mut ReconfiguratorSim,
args: BlueprintPlanArgs,
Expand Down
136 changes: 50 additions & 86 deletions nexus/db-queries/src/db/datastore/deployment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1263,12 +1263,12 @@ mod tests {
use nexus_inventory::now_db_precision;
use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder;
use nexus_reconfigurator_planning::blueprint_builder::Ensure;
use nexus_reconfigurator_planning::example::example;
use nexus_test_utils::db::test_setup_database;
use nexus_types::deployment::BlueprintZoneDisposition;
use nexus_types::deployment::BlueprintZoneFilter;
use nexus_types::deployment::PlanningInput;
use nexus_types::deployment::PlanningInputBuilder;
use nexus_types::deployment::Policy;
use nexus_types::deployment::SledDetails;
use nexus_types::deployment::SledDisk;
use nexus_types::deployment::SledFilter;
Expand All @@ -1279,7 +1279,6 @@ mod tests {
use nexus_types::external_api::views::SledState;
use nexus_types::inventory::Collection;
use omicron_common::address::Ipv6Subnet;
use omicron_common::api::external::Generation;
use omicron_common::disk::DiskIdentity;
use omicron_test_utils::dev;
use omicron_uuid_kinds::PhysicalDiskUuid;
Expand All @@ -1288,6 +1287,7 @@ mod tests {
use pretty_assertions::assert_eq;
use rand::thread_rng;
use rand::Rng;
use slog::Logger;
use std::mem;
use std::net::Ipv6Addr;

Expand Down Expand Up @@ -1359,65 +1359,32 @@ mod tests {
}
}

// Create a `Policy` that contains all the sleds found in `collection`
fn policy_from_collection(collection: &Collection) -> Policy {
Policy {
service_ip_pool_ranges: Vec::new(),
target_nexus_zone_count: collection
.all_omicron_zones()
.filter(|z| z.zone_type.is_nexus())
.count(),
}
}
fn representative(
log: &Logger,
test_name: &str,
) -> (Collection, PlanningInput, Blueprint) {
// We'll start with an example system.
let (mut base_collection, planning_input, mut blueprint) =
example(log, test_name, 3);

fn representative() -> (Collection, PlanningInput, Blueprint) {
// We'll start with a representative collection...
// Take a more thorough collection representative (includes SPs,
// etc.)...
let mut collection =
nexus_inventory::examples::representative().builder.build();

// ...and then mutate it such that the omicron zones it reports match
// the sled agent IDs it reports. Steal the sled agent info and drop the
// fake sled-agent IDs:
let mut empty_map = BTreeMap::new();
mem::swap(&mut empty_map, &mut collection.sled_agents);
let mut sled_agents = empty_map.into_values().collect::<Vec<_>>();

// Now reinsert them with IDs pulled from the omicron zones. This
// assumes we have more fake sled agents than omicron zones, which is
// currently true for the representative collection.
for &sled_id in collection.omicron_zones.keys() {
let some_sled_agent = sled_agents.pop().expect(
"fewer representative sled agents than \
representative omicron zones sleds",
);
collection.sled_agents.insert(sled_id, some_sled_agent);
}
// ... and replace its sled agent and Omicron zones with those from our
// example system.
mem::swap(
&mut collection.sled_agents,
&mut base_collection.sled_agents,
);
mem::swap(
&mut collection.omicron_zones,
&mut base_collection.omicron_zones,
);

let policy = policy_from_collection(&collection);
let planning_input = {
let mut builder = PlanningInputBuilder::new(
policy,
Generation::new(),
Generation::new(),
);
for (sled_id, agent) in &collection.sled_agents {
builder
.add_sled(
*sled_id,
fake_sled_details(Some(*agent.sled_agent_address.ip())),
)
.expect("failed to add sled to representative");
}
builder.build()
};
let blueprint = BlueprintBuilder::build_initial_from_collection(
&collection,
Generation::new(),
Generation::new(),
planning_input.all_sled_ids(SledFilter::All),
"test",
)
.unwrap();
// Treat this blueprint as the initial blueprint for the system.
blueprint.parent_blueprint_id = None;

(collection, planning_input, blueprint)
}
Expand All @@ -1442,17 +1409,11 @@ mod tests {
let mut db = test_setup_database(&logctx.log).await;
let (opctx, datastore) = datastore_test(&logctx, &db).await;

// Create an empty collection and a blueprint from it
let collection =
nexus_inventory::CollectionBuilder::new("test").build();
let blueprint1 = BlueprintBuilder::build_initial_from_collection(
&collection,
Generation::new(),
Generation::new(),
// Create an empty blueprint from it
let blueprint1 = BlueprintBuilder::build_empty_with_sleds(
std::iter::empty(),
"test",
)
.unwrap();
);
let authz_blueprint = authz_blueprint_from_id(blueprint1.id);

// Trying to read it from the database should fail with the relevant
Expand All @@ -1471,7 +1432,7 @@ mod tests {
let blueprint_read = datastore
.blueprint_read(&opctx, &authz_blueprint)
.await
.expect("failed to read collection back");
.expect("failed to read blueprint back");
assert_eq!(blueprint1, blueprint_read);
assert_eq!(
blueprint_list_all_ids(&opctx, &datastore).await,
Expand Down Expand Up @@ -1501,13 +1462,15 @@ mod tests {

#[tokio::test]
async fn test_representative_blueprint() {
const TEST_NAME: &str = "test_representative_blueprint";
// Setup
let logctx = dev::test_setup_log("test_representative_blueprint");
let logctx = dev::test_setup_log(TEST_NAME);
let mut db = test_setup_database(&logctx.log).await;
let (opctx, datastore) = datastore_test(&logctx, &db).await;

// Create a cohesive representative collection/policy/blueprint
let (collection, planning_input, blueprint1) = representative();
let (collection, planning_input, blueprint1) =
representative(&logctx.log, TEST_NAME);
let authz_blueprint1 = authz_blueprint_from_id(blueprint1.id);

// Write it to the database and read it back.
Expand Down Expand Up @@ -1632,10 +1595,23 @@ mod tests {
let blueprint2 = builder.build();
let authz_blueprint2 = authz_blueprint_from_id(blueprint2.id);

let diff = blueprint2.diff_since_blueprint(&blueprint1).unwrap();
println!("b1 -> b2: {}", diff.display());
println!("b1 disks: {:?}", blueprint1.blueprint_disks);
println!("b2 disks: {:?}", blueprint2.blueprint_disks);
// Check that we added the new sled, as well as its disks and zones.
assert_eq!(
blueprint1.blueprint_disks.len() + new_sled_zpools.len(),
blueprint2.blueprint_disks.len(),
blueprint1
.blueprint_disks
.values()
.map(|c| c.disks.len())
.sum::<usize>()
+ new_sled_zpools.len(),
blueprint2
.blueprint_disks
.values()
.map(|c| c.disks.len())
.sum::<usize>()
);
assert_eq!(
blueprint1.blueprint_zones.len() + 1,
Expand Down Expand Up @@ -1757,16 +1733,10 @@ mod tests {
// Create three blueprints:
// * `blueprint1` has no parent
// * `blueprint2` and `blueprint3` both have `blueprint1` as parent
let collection =
nexus_inventory::CollectionBuilder::new("test").build();
let blueprint1 = BlueprintBuilder::build_initial_from_collection(
&collection,
Generation::new(),
Generation::new(),
let blueprint1 = BlueprintBuilder::build_empty_with_sleds(
std::iter::empty(),
"test1",
)
.unwrap();
);
let blueprint2 = BlueprintBuilder::new_based_on(
&logctx.log,
&blueprint1,
Expand Down Expand Up @@ -1911,16 +1881,10 @@ mod tests {
let (opctx, datastore) = datastore_test(&logctx, &db).await;

// Create an initial blueprint and a child.
let collection =
nexus_inventory::CollectionBuilder::new("test").build();
let blueprint1 = BlueprintBuilder::build_initial_from_collection(
&collection,
Generation::new(),
Generation::new(),
let blueprint1 = BlueprintBuilder::build_empty_with_sleds(
std::iter::empty(),
"test1",
)
.unwrap();
);
let blueprint2 = BlueprintBuilder::new_based_on(
&logctx.log,
&blueprint1,
Expand Down
Loading

0 comments on commit 84e9c27

Please sign in to comment.