Skip to content

[35/n] [reconfigurator] add add_zones_with_mupdate_override chicken switch #8648

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.

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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.

4 changes: 4 additions & 0 deletions clients/nexus-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ progenitor::generate_api!(
OximeterReadMode = nexus_types::deployment::OximeterReadMode,
OximeterReadPolicy = nexus_types::deployment::OximeterReadPolicy,
PendingMgsUpdate = nexus_types::deployment::PendingMgsUpdate,
PlannerChickenSwitches = nexus_types::deployment::PlannerChickenSwitches,
ReconfiguratorChickenSwitches = nexus_types::deployment::ReconfiguratorChickenSwitches,
ReconfiguratorChickenSwitchesParam = nexus_types::deployment::ReconfiguratorChickenSwitchesParam,
ReconfiguratorChickenSwitchesView = nexus_types::deployment::ReconfiguratorChickenSwitchesView,
RecoverySiloConfig = nexus_sled_agent_shared::recovery_silo::RecoverySiloConfig,
Srv = nexus_types::internal_api::params::Srv,
TypedUuidForBlueprintKind = omicron_uuid_kinds::BlueprintUuid,
Expand Down
1 change: 1 addition & 0 deletions dev-tools/omdb/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ gateway-types.workspace = true
http.workspace = true
humantime.workspace = true
iddqd.workspace = true
indent_write.workspace = true
internal-dns-resolver.workspace = true
internal-dns-types.workspace = true
itertools.workspace = true
Expand Down
107 changes: 75 additions & 32 deletions dev-tools/omdb/src/bin/omdb/nexus/chicken_switches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ use clap::ArgAction;
use clap::Args;
use clap::Subcommand;
use http::StatusCode;
use nexus_client::types::{
ReconfiguratorChickenSwitches, ReconfiguratorChickenSwitchesParam,
};
use indent_write::io::IndentWriter;
use nexus_types::deployment::PlannerChickenSwitches;
use nexus_types::deployment::ReconfiguratorChickenSwitches;
use nexus_types::deployment::ReconfiguratorChickenSwitchesParam;
use std::io;
use std::io::Write;
use std::num::ParseIntError;
use std::str::FromStr;

Expand All @@ -34,8 +37,58 @@ pub enum ChickenSwitchesCommands {

#[derive(Debug, Clone, Args)]
pub struct ChickenSwitchesSetArgs {
#[clap(long, action=ArgAction::Set)]
planner_enabled: bool,
#[clap(flatten)]
switches: ChickenSwitchesOpts,
}

// Define the switches separately so we can use `group(required = true, multiple
// = true).`
#[derive(Debug, Clone, Args)]
#[group(required = true, multiple = true)]
pub struct ChickenSwitchesOpts {
#[clap(long, action = ArgAction::Set)]
planner_enabled: Option<bool>,

#[clap(long, action = ArgAction::Set)]
add_zones_with_mupdate_override: Option<bool>,
}

impl ChickenSwitchesOpts {
/// Returns an updated `ReconfiguratorChickenSwitchesParam` regardless of
/// whether any switches were modified.
fn update(
&self,
current: &ReconfiguratorChickenSwitches,
) -> ReconfiguratorChickenSwitches {
ReconfiguratorChickenSwitches {
planner_enabled: self
.planner_enabled
.unwrap_or(current.planner_enabled),
planner_switches: PlannerChickenSwitches {
add_zones_with_mupdate_override: self
.add_zones_with_mupdate_override
.unwrap_or(
current
.planner_switches
.add_zones_with_mupdate_override,
),
},
}
}

/// Returns an updated `ReconfiguratorChickenSwitchesParam` if any
/// switches were modified, or `None` if no changes were made.
fn update_if_modified(
&self,
current: &ReconfiguratorChickenSwitches,
next_version: u32,
) -> Option<ReconfiguratorChickenSwitchesParam> {
let new = self.update(current);
(&new != current).then(|| ReconfiguratorChickenSwitchesParam {
version: next_version,
switches: new,
})
}
}

#[derive(Debug, Clone, Copy, Args)]
Expand Down Expand Up @@ -92,15 +145,12 @@ async fn chicken_switches_show(

match res {
Ok(switches) => {
let ReconfiguratorChickenSwitches {
version,
planner_enabled,
time_modified,
} = switches.into_inner();
println!("Reconfigurator Chicken Switches: ");
println!(" version: {version}");
println!(" modified time: {time_modified}");
println!(" planner enabled: {planner_enabled}");
println!("Reconfigurator chicken switches:");
let stdout = io::stdout();
let mut indented = IndentWriter::new(" ", stdout.lock());
// No need for writeln! here because .display() adds its own
// newlines.
write!(indented, "{}", switches.display()).unwrap();
}
Err(err) => {
if err.status() == Some(StatusCode::NOT_FOUND) {
Expand All @@ -124,7 +174,7 @@ async fn chicken_switches_set(
.await
{
Ok(switches) => {
let Some(version) = switches.version.checked_add(1) else {
let Some(next_version) = switches.version.checked_add(1) else {
eprintln!(
"ERROR: Failed to update chicken switches. Max version reached."
);
Expand All @@ -133,30 +183,23 @@ async fn chicken_switches_set(
let switches = switches.into_inner();
// Future switches should use the following pattern, and only update
// the current switch values if a setting changed.
//
// We may want to use `Options` in `args` to allow defaulting to
// the current setting rather than forcing the user to update all
// settings if the number of switches grows significantly. However,
// this will not play nice with the `NOT_FOUND` case below.
let mut modified = false;
if args.planner_enabled != switches.planner_enabled {
modified = true;
}
if modified {
ReconfiguratorChickenSwitchesParam {
version,
planner_enabled: args.planner_enabled,
}
} else {
let Some(switches) = args
.switches
.update_if_modified(&switches.switches, next_version)
else {
println!("No modifications made to current switch values");
return Ok(());
}
};
switches
}
Err(err) => {
if err.status() == Some(StatusCode::NOT_FOUND) {
let default_switches = ReconfiguratorChickenSwitches::default();
// In this initial case, the operator expects that we always set
// switches.
ReconfiguratorChickenSwitchesParam {
version: 1,
planner_enabled: args.planner_enabled,
switches: args.switches.update(&default_switches),
}
} else {
eprintln!("error: {:#}", err);
Expand Down
16 changes: 14 additions & 2 deletions dev-tools/omdb/src/bin/omdb/reconfigurator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ use nexus_db_queries::db::datastore::SQL_BATCH_SIZE;
use nexus_db_queries::db::pagination::Paginator;
use nexus_types::deployment::Blueprint;
use nexus_types::deployment::BlueprintMetadata;
use nexus_types::deployment::PlannerChickenSwitches;
use nexus_types::deployment::ReconfiguratorChickenSwitches;
use nexus_types::deployment::ReconfiguratorChickenSwitchesView;
use nexus_types::deployment::UnstableReconfiguratorState;
use omicron_common::api::external::Error;
use omicron_common::api::external::LookupType;
Expand Down Expand Up @@ -403,20 +405,30 @@ async fn cmd_reconfigurator_chicken_switches_history(
struct SwitchesRow {
version: String,
planner_enabled: String,
add_zones_with_mupdate_override: String,
time_modified: String,
}

let rows: Vec<_> = history
.into_iter()
.map(|s| {
let ReconfiguratorChickenSwitches {
let ReconfiguratorChickenSwitchesView {
version,
planner_enabled,
switches:
ReconfiguratorChickenSwitches {
planner_enabled,
planner_switches:
PlannerChickenSwitches {
add_zones_with_mupdate_override,
},
},
time_modified,
} = s;
SwitchesRow {
version: version.to_string(),
planner_enabled: planner_enabled.to_string(),
add_zones_with_mupdate_override:
add_zones_with_mupdate_override.to_string(),
time_modified: time_modified.to_string(),
}
})
Expand Down
41 changes: 41 additions & 0 deletions dev-tools/omdb/tests/successes.out
Original file line number Diff line number Diff line change
Expand Up @@ -1398,6 +1398,47 @@ task: "webhook_deliverator"
stderr:
note: using Nexus URL http://127.0.0.1:REDACTED_PORT/
=============================================
EXECUTING COMMAND: omdb ["nexus", "chicken-switches", "show", "current"]
termination: Exited(0)
---------------------------------------------
stdout:
No chicken switches enabled
---------------------------------------------
stderr:
note: using Nexus URL http://127.0.0.1:REDACTED_PORT/
=============================================
EXECUTING COMMAND: omdb ["-w", "nexus", "chicken-switches", "set", "--planner-enabled", "true"]
termination: Exited(0)
---------------------------------------------
stdout:
Chicken switches updated at version 1
---------------------------------------------
stderr:
note: using Nexus URL http://127.0.0.1:REDACTED_PORT/
=============================================
EXECUTING COMMAND: omdb ["-w", "nexus", "chicken-switches", "set", "--add-zones-with-mupdate-override", "false"]
termination: Exited(0)
---------------------------------------------
stdout:
Chicken switches updated at version 2
---------------------------------------------
stderr:
note: using Nexus URL http://127.0.0.1:REDACTED_PORT/
=============================================
EXECUTING COMMAND: omdb ["nexus", "chicken-switches", "show", "current"]
termination: Exited(0)
---------------------------------------------
stdout:
Reconfigurator chicken switches:
version: 2
modified time: <REDACTED_TIMESTAMP>
planner enabled: true
planner switches:
add zones with mupdate override: false
---------------------------------------------
stderr:
note: using Nexus URL http://127.0.0.1:REDACTED_PORT/
=============================================
EXECUTING COMMAND: omdb ["nexus", "sagas", "list"]
termination: Exited(0)
---------------------------------------------
Expand Down
21 changes: 21 additions & 0 deletions dev-tools/omdb/tests/test_all_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,27 @@ async fn test_omdb_success_cases(cptestctx: &ControlPlaneTestContext) {
&["nexus", "background-tasks", "show", "dns_internal"],
&["nexus", "background-tasks", "show", "dns_external"],
&["nexus", "background-tasks", "show", "all"],
// chicken switches: show and set
&["nexus", "chicken-switches", "show", "current"],
&[
"-w",
"nexus",
"chicken-switches",
"set",
"--planner-enabled",
"true",
],
&[
"-w",
"nexus",
"chicken-switches",
"set",
"--add-zones-with-mupdate-override",
"false",
],
// After the set commands above, we should see chicken switches
// populated.
&["nexus", "chicken-switches", "show", "current"],
&["nexus", "sagas", "list"],
&["--destructive", "nexus", "sagas", "demo-create"],
&["nexus", "sagas", "list"],
Expand Down
1 change: 1 addition & 0 deletions dev-tools/reconfigurator-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ camino.workspace = true
chrono.workspace = true
clap.workspace = true
colored.workspace = true
daft.workspace = true
humantime.workspace = true
iddqd.workspace = true
indent_write.workspace = true
Expand Down
Loading
Loading