Skip to content

Commit

Permalink
omdb nexus blueprints target set: take an enabled setting (#5224)
Browse files Browse the repository at this point in the history
I made this a required argument because I'm nervous to default it to
`Enabled` when we've talked so much about manually setting a disabled
blueprint. If folks feel strongly this should be an optional
`--enabled={true,false,inherit}` defaulting to `true` instead, I
wouldn't put up a fight.

Addresses the first part of #5210.
  • Loading branch information
jgallagher authored Mar 7, 2024
1 parent 358eec4 commit 19ad111
Showing 1 changed file with 40 additions and 9 deletions.
49 changes: 40 additions & 9 deletions dev-tools/omdb/src/bin/omdb/nexus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use chrono::SecondsFormat;
use chrono::Utc;
use clap::Args;
use clap::Subcommand;
use clap::ValueEnum;
use futures::TryStreamExt;
use nexus_client::types::ActivationReason;
use nexus_client::types::BackgroundTask;
Expand Down Expand Up @@ -77,7 +78,7 @@ enum BlueprintsCommands {
Diff(BlueprintIdsArgs),
/// Delete a blueprint
Delete(BlueprintIdArgs),
/// Set the current target blueprint
/// Interact with the current target blueprint
Target(BlueprintsTargetArgs),
/// Generate an initial blueprint from a specific inventory collection
GenerateFromCollection(CollectionIdArgs),
Expand Down Expand Up @@ -116,7 +117,25 @@ enum BlueprintTargetCommands {
/// Show the current target blueprint
Show,
/// Change the current target blueprint
Set(BlueprintIdArgs),
Set(BlueprintTargetSetArgs),
}

#[derive(Debug, Args)]
struct BlueprintTargetSetArgs {
/// id of blueprint to make target
blueprint_id: Uuid,
/// whether this blueprint should be enabled
enabled: BlueprintTargetSetEnabled,
}

#[derive(Debug, Clone, Copy, ValueEnum)]
enum BlueprintTargetSetEnabled {
/// set the new current target as enabled
Enabled,
/// set the new current target as disabled
Disabled,
/// use the enabled setting from the parent blueprint
Inherit,
}

#[derive(Debug, Args)]
Expand Down Expand Up @@ -914,14 +933,26 @@ async fn cmd_nexus_blueprints_target_show(

async fn cmd_nexus_blueprints_target_set(
client: &nexus_client::Client,
args: &BlueprintIdArgs,
args: &BlueprintTargetSetArgs,
) -> Result<(), anyhow::Error> {
// Try to preserve the value of "enabled", if possible.
let enabled = client
.blueprint_target_view()
.await
.map(|current| current.into_inner().enabled)
.unwrap_or(true);
let enabled = match args.enabled {
BlueprintTargetSetEnabled::Enabled => true,
BlueprintTargetSetEnabled::Disabled => false,
// There's a small TOCTOU race with "inherit": What if the user wants to
// inherit the parent blueprint enabled bit but the current target
// blueprint enabled bit is flipped or the current target blueprint is
// changed? We expect neither of these to be problematic in practice:
// the only way for the `enable` bit to be set to anything at all is via
// `omdb`, so the user would have to be racing with another `omdb`
// operator. (In the case of the current target blueprint being changed
// entirely, that will result in a failure to set the current target
// below, because its parent will no longer be the current target.)
BlueprintTargetSetEnabled::Inherit => client
.blueprint_target_view()
.await
.map(|current| current.into_inner().enabled)
.context("failed to fetch current target blueprint")?,
};
client
.blueprint_target_set(&nexus_client::types::BlueprintTargetSet {
target_id: args.blueprint_id,
Expand Down

0 comments on commit 19ad111

Please sign in to comment.