Skip to content

Commit

Permalink
Use strongly-typed BlueprintUuid (#7328)
Browse files Browse the repository at this point in the history
A few yaks deep, I found myself wanting to do some diesel stuff that
applied to blueprint IDs, but then realized we were still modeling them
as plain `Uuid`. This converts most of them to `BlueprintUuid` (while
leaving the Nexus external API untouched, which does affect a few Nexus
internal endpoints too, hence the occasional `.as_untyped_uuid()` that
shows up in this PR).
  • Loading branch information
jgallagher authored Jan 13, 2025
1 parent d9a6a8b commit 58a8886
Show file tree
Hide file tree
Showing 32 changed files with 299 additions and 229 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions clients/nexus-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ progenitor::generate_api!(
OmicronPhysicalDisksConfig = omicron_common::disk::OmicronPhysicalDisksConfig,
RecoverySiloConfig = nexus_sled_agent_shared::recovery_silo::RecoverySiloConfig,
Srv = nexus_types::internal_api::params::Srv,
TypedUuidForBlueprintKind = omicron_uuid_kinds::BlueprintUuid,
TypedUuidForCollectionKind = omicron_uuid_kinds::CollectionUuid,
TypedUuidForDatasetKind = omicron_uuid_kinds::TypedUuid<omicron_uuid_kinds::DatasetKind>,
TypedUuidForDemoSagaKind = omicron_uuid_kinds::DemoSagaUuid,
Expand Down
45 changes: 26 additions & 19 deletions dev-tools/omdb/src/bin/omdb/nexus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,11 @@ use nexus_types::internal_api::background::RegionSnapshotReplacementGarbageColle
use nexus_types::internal_api::background::RegionSnapshotReplacementStartStatus;
use nexus_types::internal_api::background::RegionSnapshotReplacementStepStatus;
use nexus_types::inventory::BaseboardId;
use omicron_uuid_kinds::BlueprintUuid;
use omicron_uuid_kinds::CollectionUuid;
use omicron_uuid_kinds::DemoSagaUuid;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::ParseError;
use omicron_uuid_kinds::PhysicalDiskUuid;
use omicron_uuid_kinds::SledUuid;
use reedline::DefaultPrompt;
Expand Down Expand Up @@ -186,11 +188,11 @@ enum BlueprintsCommands {
#[derive(Debug, Clone, Copy)]
enum BlueprintIdOrCurrentTarget {
CurrentTarget,
BlueprintId(Uuid),
BlueprintId(BlueprintUuid),
}

impl FromStr for BlueprintIdOrCurrentTarget {
type Err = uuid::Error;
type Err = ParseError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
if matches!(s, "current-target" | "current" | "target") {
Expand All @@ -206,7 +208,7 @@ impl BlueprintIdOrCurrentTarget {
async fn resolve_to_id(
&self,
client: &nexus_client::Client,
) -> anyhow::Result<Uuid> {
) -> anyhow::Result<BlueprintUuid> {
match self {
Self::CurrentTarget => {
let target = client
Expand All @@ -224,15 +226,18 @@ impl BlueprintIdOrCurrentTarget {
client: &nexus_client::Client,
) -> anyhow::Result<Blueprint> {
let id = self.resolve_to_id(client).await?;
let response = client.blueprint_view(&id).await.with_context(|| {
let suffix = match self {
BlueprintIdOrCurrentTarget::CurrentTarget => {
" (current target)"
}
BlueprintIdOrCurrentTarget::BlueprintId(_) => "",
};
format!("fetching blueprint {id}{suffix}")
})?;
let response = client
.blueprint_view(id.as_untyped_uuid())
.await
.with_context(|| {
let suffix = match self {
BlueprintIdOrCurrentTarget::CurrentTarget => {
" (current target)"
}
BlueprintIdOrCurrentTarget::BlueprintId(_) => "",
};
format!("fetching blueprint {id}{suffix}")
})?;
Ok(response.into_inner())
}
}
Expand Down Expand Up @@ -282,7 +287,7 @@ enum BlueprintTargetCommands {
#[derive(Debug, Args)]
struct BlueprintTargetSetArgs {
/// id of blueprint to make target
blueprint_id: Uuid,
blueprint_id: BlueprintUuid,
/// whether this blueprint should be enabled
enabled: BlueprintTargetSetEnabled,
/// if specified, diff against the current target and wait for confirmation
Expand Down Expand Up @@ -2311,7 +2316,7 @@ async fn cmd_nexus_blueprints_delete(
) -> Result<(), anyhow::Error> {
let blueprint_id = args.blueprint_id.resolve_to_id(client).await?;
let _ = client
.blueprint_delete(&blueprint_id)
.blueprint_delete(blueprint_id.as_untyped_uuid())
.await
.with_context(|| format!("deleting blueprint {blueprint_id}"))?;
println!("blueprint {blueprint_id} deleted");
Expand Down Expand Up @@ -2350,14 +2355,16 @@ async fn cmd_nexus_blueprints_target_set(
if args.diff {
let current_target = get_current_target().await?;
let blueprint1 = client
.blueprint_view(&current_target.target_id)
.blueprint_view(current_target.target_id.as_untyped_uuid())
.await
.context("failed to fetch target blueprint")?
.into_inner();
let blueprint2 =
client.blueprint_view(&args.blueprint_id).await.with_context(
|| format!("fetching blueprint {}", args.blueprint_id),
)?;
let blueprint2 = client
.blueprint_view(args.blueprint_id.as_untyped_uuid())
.await
.with_context(|| {
format!("fetching blueprint {}", args.blueprint_id)
})?;
let diff = blueprint2.diff_since_blueprint(&blueprint1);
println!("{}", diff.display());
println!(
Expand Down
20 changes: 10 additions & 10 deletions dev-tools/reconfigurator-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use nexus_types::deployment::{Blueprint, UnstableReconfiguratorState};
use omicron_common::api::external::Generation;
use omicron_common::api::external::Name;
use omicron_common::policy::NEXUS_REDUNDANCY;
use omicron_uuid_kinds::BlueprintUuid;
use omicron_uuid_kinds::CollectionUuid;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::OmicronZoneUuid;
Expand All @@ -47,7 +48,6 @@ use std::io::BufRead;
use std::io::IsTerminal;
use swrite::{swriteln, SWrite};
use tabled::Tabled;
use uuid::Uuid;

mod log_capture;

Expand Down Expand Up @@ -403,7 +403,7 @@ struct InventoryArgs {
#[derive(Debug, Args)]
struct BlueprintPlanArgs {
/// id of the blueprint on which this one will be based
parent_blueprint_id: Uuid,
parent_blueprint_id: BlueprintUuid,
/// id of the inventory collection to use in planning
///
/// Must be provided unless there is only one collection in the loaded
Expand All @@ -414,7 +414,7 @@ struct BlueprintPlanArgs {
#[derive(Debug, Args)]
struct BlueprintEditArgs {
/// id of the blueprint to edit
blueprint_id: Uuid,
blueprint_id: BlueprintUuid,
/// "creator" field for the new blueprint
#[arg(long)]
creator: Option<String>,
Expand All @@ -441,7 +441,7 @@ enum BlueprintEditCommands {
#[derive(Debug, Args)]
struct BlueprintArgs {
/// id of the blueprint
blueprint_id: Uuid,
blueprint_id: BlueprintUuid,
}

#[derive(Debug, Args)]
Expand All @@ -451,7 +451,7 @@ struct BlueprintDiffDnsArgs {
/// DNS version to diff against
dns_version: u32,
/// id of the blueprint
blueprint_id: Uuid,
blueprint_id: BlueprintUuid,
}

#[derive(Clone, Copy, Debug, ValueEnum)]
Expand All @@ -465,23 +465,23 @@ struct BlueprintDiffInventoryArgs {
/// id of the inventory collection
collection_id: CollectionUuid,
/// id of the blueprint
blueprint_id: Uuid,
blueprint_id: BlueprintUuid,
}

#[derive(Debug, Args)]
struct BlueprintSaveArgs {
/// id of the blueprint
blueprint_id: Uuid,
blueprint_id: BlueprintUuid,
/// output file
filename: Utf8PathBuf,
}

#[derive(Debug, Args)]
struct BlueprintDiffArgs {
/// id of the first blueprint
blueprint1_id: Uuid,
blueprint1_id: BlueprintUuid,
/// id of the second blueprint
blueprint2_id: Uuid,
blueprint2_id: BlueprintUuid,
}

#[derive(Debug, Subcommand)]
Expand Down Expand Up @@ -728,7 +728,7 @@ fn cmd_blueprint_list(
#[derive(Tabled)]
#[tabled(rename_all = "SCREAMING_SNAKE_CASE")]
struct BlueprintRow {
id: Uuid,
id: BlueprintUuid,
parent: Cow<'static, str>,
time_created: String,
}
Expand Down
3 changes: 2 additions & 1 deletion dev-tools/reconfigurator-cli/tests/test_basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use omicron_test_utils::dev::test_cmds::path_to_executable;
use omicron_test_utils::dev::test_cmds::run_command;
use omicron_test_utils::dev::test_cmds::Redactor;
use omicron_test_utils::dev::test_cmds::EXIT_SUCCESS;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::SledUuid;
use slog::debug;
use std::io::BufReader;
Expand Down Expand Up @@ -270,7 +271,7 @@ async fn test_blueprint_edit(cptestctx: &ControlPlaneTestContext) {
.expect("failed to import new blueprint");

let found_blueprint = nexus_client
.blueprint_view(&new_blueprint.id)
.blueprint_view(new_blueprint.id.as_untyped_uuid())
.await
.expect("failed to find imported blueprint in Nexus")
.into_inner();
Expand Down
1 change: 1 addition & 0 deletions end-to-end-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ internal-dns-types.workspace = true
nexus-client.workspace = true
omicron-sled-agent.workspace = true
omicron-test-utils.workspace = true
omicron-uuid-kinds.workspace = true
oxide-client.workspace = true
rand.workspace = true
reqwest = { workspace = true, features = ["cookies"] }
Expand Down
3 changes: 2 additions & 1 deletion end-to-end-tests/src/noop_blueprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use internal_dns_types::names::ServiceName;
use nexus_client::Client as NexusClient;
use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError};
use omicron_test_utils::dev::test_setup_log;
use omicron_uuid_kinds::GenericUuid;
use slog::{debug, info};
use slog_error_chain::InlineErrorChain;
use std::time::Duration;
Expand Down Expand Up @@ -68,7 +69,7 @@ async fn new_blueprint_noop() {

// Fetch its parent.
let parent_blueprint = nexus_client
.blueprint_view(&parent_blueprint_id)
.blueprint_view(parent_blueprint_id.as_untyped_uuid())
.await
.expect("failed to fetch parent blueprint")
.into_inner();
Expand Down
1 change: 1 addition & 0 deletions live-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ nexus-sled-agent-shared.workspace = true
nexus-types.workspace = true
omicron-common.workspace = true
omicron-test-utils.workspace = true
omicron-uuid-kinds.workspace = true
reqwest.workspace = true
serde.workspace = true
slog.workspace = true
Expand Down
3 changes: 2 additions & 1 deletion live-tests/tests/common/reconfigurator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use nexus_client::types::BlueprintTargetSet;
use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder;
use nexus_types::deployment::{Blueprint, PlanningInput};
use nexus_types::inventory::Collection;
use omicron_uuid_kinds::GenericUuid;
use slog::{debug, info};

/// Modify the system by editing the current target blueprint
Expand Down Expand Up @@ -57,7 +58,7 @@ pub async fn blueprint_edit_current_target(

// Fetch the actual blueprint.
let blueprint1 = nexus
.blueprint_view(&target_blueprint.target_id)
.blueprint_view(target_blueprint.target_id.as_untyped_uuid())
.await
.context("fetch current target blueprint")?
.into_inner();
Expand Down
Loading

0 comments on commit 58a8886

Please sign in to comment.