Skip to content

Commit 33d4f82

Browse files
authored
[sled-agent] Remove chicken switch and always destroy orphaned Omicron datasets (#8895)
The main point of this PR is that the sled-agent config reconciler now always attempts to destroy datasets it believes have been expunged instead of defaulting to reporting them unless an operator has toggled a chicken switch. (See the comments on `datasets_destroy_orphans()` for details; this is pretty specific to only destroy datasets whose names we understand.) We wanted to ship "report only" in R16 and then confirm during upgrades that this wasn't showing any surprises; now that R16 is out the door we can turn this on all the time. This also (starts to? attempts to?) removes the sled-agent API endpoints for interacting with the chicken switches. The openapi xtask yelled at me for this, so instead I followed the guide for removing endpoints. This was almost entirely painless, except that I'd already (locally) committed the change to remove the endpoints mixed in with other things, so I had to dig back out the endpoints and the types they used. Kudos to all involved for the tooling and instructions here! Closes #6177. Closes #7312 (no longer needed). Closes #7313.
1 parent 683a29a commit 33d4f82

File tree

15 files changed

+8429
-214
lines changed

15 files changed

+8429
-214
lines changed

clients/sled-agent-client/src/lib.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,19 @@ impl From<omicron_common::api::internal::shared::NetworkInterfaceKind>
331331
}
332332
}
333333

334+
// TODO-cleanup This is icky; can we move these methods to a separate client so
335+
// we don't need to add this header by hand?
336+
// https://github.com/oxidecomputer/omicron/issues/8900
337+
trait ApiVersionHeader {
338+
fn api_version_header(self, api_version: &'static str) -> Self;
339+
}
340+
341+
impl ApiVersionHeader for reqwest::RequestBuilder {
342+
fn api_version_header(self, api_version: &'static str) -> Self {
343+
self.header("api-version", api_version)
344+
}
345+
}
346+
334347
/// Exposes additional [`Client`] interfaces for use by the test suite. These
335348
/// are bonus endpoints, not generated in the real client.
336349
#[async_trait]
@@ -353,6 +366,7 @@ impl TestInterfaces for Client {
353366
let url = format!("{}/vmms/{}/poke-single-step", baseurl, id);
354367
client
355368
.post(url)
369+
.api_version_header(self.api_version())
356370
.send()
357371
.await
358372
.expect("instance_single_step() failed unexpectedly");
@@ -364,6 +378,7 @@ impl TestInterfaces for Client {
364378
let url = format!("{}/vmms/{}/poke", baseurl, id);
365379
client
366380
.post(url)
381+
.api_version_header(self.api_version())
367382
.send()
368383
.await
369384
.expect("instance_finish_transition() failed unexpectedly");
@@ -375,6 +390,7 @@ impl TestInterfaces for Client {
375390
let url = format!("{}/disks/{}/poke", baseurl, id);
376391
client
377392
.post(url)
393+
.api_version_header(self.api_version())
378394
.send()
379395
.await
380396
.expect("disk_finish_transition() failed unexpectedly");
@@ -390,6 +406,7 @@ impl TestInterfaces for Client {
390406
let url = format!("{baseurl}/vmms/{id}/sim-migration-source");
391407
client
392408
.post(url)
409+
.api_version_header(self.api_version())
393410
.json(&params)
394411
.send()
395412
.await

dev-tools/omdb/src/bin/omdb/sled_agent.rs

Lines changed: 1 addition & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,11 @@
55
//! omdb commands that query or update specific Sleds
66
77
use crate::Omdb;
8-
use crate::check_allow_destructive::DestructiveOperationToken;
98
use crate::helpers::CONNECTION_OPTIONS_HEADING;
109
use anyhow::Context;
1110
use anyhow::bail;
1211
use clap::Args;
1312
use clap::Subcommand;
14-
use sled_agent_client::types::ChickenSwitchDestroyOrphanedDatasets;
1513

1614
/// Arguments to the "omdb sled-agent" subcommand
1715
#[derive(Debug, Args)]
@@ -39,11 +37,6 @@ enum SledAgentCommands {
3937
/// print information about the local bootstore node
4038
#[clap(subcommand)]
4139
Bootstore(BootstoreCommands),
42-
43-
/// control "chicken switches" (potentially-destructive sled-agent behavior
44-
/// that can be toggled on or off via `omdb`)
45-
#[clap(subcommand)]
46-
ChickenSwitch(ChickenSwitchCommands),
4740
}
4841

4942
#[derive(Debug, Subcommand)]
@@ -58,12 +51,6 @@ enum BootstoreCommands {
5851
Status,
5952
}
6053

61-
#[derive(Debug, Subcommand)]
62-
enum ChickenSwitchCommands {
63-
/// interact with the "destroy orphaned datasets" chicken switch
64-
DestroyOrphans(DestroyOrphansArgs),
65-
}
66-
6754
#[derive(Debug, Args)]
6855
struct DestroyOrphansArgs {
6956
#[command(subcommand)]
@@ -84,7 +71,7 @@ impl SledAgentArgs {
8471
/// Run a `omdb sled-agent` subcommand.
8572
pub(crate) async fn run_cmd(
8673
&self,
87-
omdb: &Omdb,
74+
_omdb: &Omdb,
8875
log: &slog::Logger,
8976
) -> Result<(), anyhow::Error> {
9077
// This is a little goofy. The sled URL is required, but can come
@@ -105,29 +92,6 @@ impl SledAgentArgs {
10592
SledAgentCommands::Bootstore(BootstoreCommands::Status) => {
10693
cmd_bootstore_status(&client).await
10794
}
108-
SledAgentCommands::ChickenSwitch(
109-
ChickenSwitchCommands::DestroyOrphans(DestroyOrphansArgs {
110-
command: DestroyOrphansCommands::Get,
111-
}),
112-
) => cmd_chicken_switch_destroy_orphans_get(&client).await,
113-
SledAgentCommands::ChickenSwitch(
114-
ChickenSwitchCommands::DestroyOrphans(DestroyOrphansArgs {
115-
command: DestroyOrphansCommands::Enable,
116-
}),
117-
) => {
118-
let token = omdb.check_allow_destructive()?;
119-
cmd_chicken_switch_destroy_orphans_set(&client, true, token)
120-
.await
121-
}
122-
SledAgentCommands::ChickenSwitch(
123-
ChickenSwitchCommands::DestroyOrphans(DestroyOrphansArgs {
124-
command: DestroyOrphansCommands::Disable,
125-
}),
126-
) => {
127-
let token = omdb.check_allow_destructive()?;
128-
cmd_chicken_switch_destroy_orphans_set(&client, false, token)
129-
.await
130-
}
13195
}
13296
}
13397
}
@@ -193,33 +157,3 @@ async fn cmd_bootstore_status(
193157

194158
Ok(())
195159
}
196-
197-
/// Runs `omdb sled-agent chicken-switch destroy-orphans get`
198-
async fn cmd_chicken_switch_destroy_orphans_get(
199-
client: &sled_agent_client::Client,
200-
) -> Result<(), anyhow::Error> {
201-
let ChickenSwitchDestroyOrphanedDatasets { destroy_orphans } = client
202-
.chicken_switch_destroy_orphaned_datasets_get()
203-
.await
204-
.context("get chicken switch value")?
205-
.into_inner();
206-
let status = if destroy_orphans { "enabled" } else { "disabled" };
207-
println!("destroy orphaned datasets {status}");
208-
Ok(())
209-
}
210-
211-
/// Runs `omdb sled-agent chicken-switch destroy-orphans {enable,disable}`
212-
async fn cmd_chicken_switch_destroy_orphans_set(
213-
client: &sled_agent_client::Client,
214-
destroy_orphans: bool,
215-
_token: DestructiveOperationToken,
216-
) -> Result<(), anyhow::Error> {
217-
let options = ChickenSwitchDestroyOrphanedDatasets { destroy_orphans };
218-
client
219-
.chicken_switch_destroy_orphaned_datasets_put(&options)
220-
.await
221-
.context("put chicken switch value")?;
222-
let status = if destroy_orphans { "enabled" } else { "disabled" };
223-
println!("destroy orphaned datasets {status}");
224-
Ok(())
225-
}

dev-tools/omdb/tests/usage_errors.out

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,11 +1063,9 @@ Debug a specific Sled
10631063
Usage: omdb sled-agent [OPTIONS] <COMMAND>
10641064

10651065
Commands:
1066-
zones print information about zones
1067-
bootstore print information about the local bootstore node
1068-
chicken-switch control "chicken switches" (potentially-destructive sled-agent behavior that can
1069-
be toggled on or off via `omdb`)
1070-
help Print this message or the help of the given subcommand(s)
1066+
zones print information about zones
1067+
bootstore print information about the local bootstore node
1068+
help Print this message or the help of the given subcommand(s)
10711069

10721070
Options:
10731071
--log-level <LOG_LEVEL> log level filter [env: LOG_LEVEL=] [default: warn]

nexus/mgs-updates/src/test_util/host_phase_2_test_state.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,12 @@ impl HostPhase2TestContext {
124124
bind_address: "[::1]:0".parse().unwrap(),
125125
..Default::default()
126126
})
127+
.version_policy(dropshot::VersionPolicy::Dynamic(Box::new(
128+
dropshot::ClientSpecifiesVersionInHeader::new(
129+
omicron_common::api::VERSION_HEADER,
130+
sled_agent_api::VERSION_REMOVE_DESTROY_ORPHANED_DATASETS_CHICKEN_SWITCH,
131+
),
132+
)))
127133
.start()
128134
.context("failed to create dropshot server")?
129135
};

0 commit comments

Comments
 (0)