Skip to content

Commit 496b854

Browse files
authored
[bootstrap-agent] Refactor for clarity (#3827)
The main goals here are, in roughly decreasing order: 1. Make the startup flow of bootstrap agent more obvious/clear 2. Simplify the handoff from bootstrap agent to sled agent 3. Use fewer tokio `Mutex`es 4. In general, use less shared memory objects and more message passing The two big changes I made to work towards those: 1. Removed the `bootstrap::Agent` type entirely. The bootstrap server now spawns a tokio task that handles messages and holds the sled agent state in a local, non-mutex variable that it is free to mutate as needed. 2. Removed `HardwareMonitor`. This was the source of a lot of confusion for me personally, and also made the handoff to sled-agent quite complex. Now the bootstrap agent is solely responsible for monitoring for hardware changes. (This is the place where I am most likely to have introduced issues, hence needing to test this more.) There are many smaller changes, including a few things that now happen in a slightly different order (but I believe to be correct either way); for example, prior to this PR, on a cold boot the sled-agent dropshot server would start before the bootstrap agent dropshot server. Now, the bootstrap server always starts first.
1 parent 0b42d7c commit 496b854

18 files changed

+1702
-1634
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

sled-agent/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ tofino.workspace = true
7373
tokio = { workspace = true, features = [ "full" ] }
7474
tokio-tungstenite.workspace = true
7575
toml.workspace = true
76+
usdt.workspace = true
7677
uuid.workspace = true
7778
zeroize.workspace = true
7879
zone.workspace = true

sled-agent/src/bin/sled-agent.rs

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,10 @@ use camino::Utf8PathBuf;
88
use clap::{Parser, Subcommand};
99
use omicron_common::cmd::fatal;
1010
use omicron_common::cmd::CmdError;
11+
use omicron_sled_agent::bootstrap::server as bootstrap_server;
1112
use omicron_sled_agent::bootstrap::RssAccessError;
12-
use omicron_sled_agent::bootstrap::{
13-
config::Config as BootstrapConfig, server as bootstrap_server,
14-
};
1513
use omicron_sled_agent::rack_setup::config::SetupServiceConfig as RssConfig;
1614
use omicron_sled_agent::{config::Config as SledConfig, server as sled_server};
17-
use uuid::Uuid;
1815

1916
#[derive(Subcommand, Debug)]
2017
enum OpenapiFlavor {
@@ -92,33 +89,16 @@ async fn do_run() -> Result<(), CmdError> {
9289
None
9390
};
9491

95-
// Derive the bootstrap addresses from the data link's MAC address.
96-
let link = config
97-
.get_link()
98-
.map_err(|e| CmdError::Failure(e.to_string()))?;
99-
100-
// Configure and run the Bootstrap server.
101-
let bootstrap_config = BootstrapConfig {
102-
id: Uuid::new_v4(),
103-
link,
104-
log: config.log.clone(),
105-
updates: config.updates.clone(),
106-
};
107-
108-
// TODO: It's a little silly to pass the config this way - namely,
109-
// that we construct the bootstrap config from `config`, but then
110-
// pass it separately just so the sled agent can ingest it later on.
111-
let server =
112-
bootstrap_server::Server::start(bootstrap_config, config)
113-
.await
114-
.map_err(CmdError::Failure)?;
92+
let server = bootstrap_server::Server::start(config)
93+
.await
94+
.map_err(|err| CmdError::Failure(format!("{err:#}")))?;
11595

11696
// If requested, automatically supply the RSS configuration.
11797
//
11898
// This should remain equivalent to the HTTP request which can
11999
// be invoked by Wicket.
120100
if let Some(rss_config) = rss_config {
121-
match server.agent().start_rack_initialize(rss_config) {
101+
match server.start_rack_initialize(rss_config) {
122102
// If the rack has already been initialized, we shouldn't
123103
// abandon the server.
124104
Ok(_) | Err(RssAccessError::AlreadyInitialized) => {}

0 commit comments

Comments
 (0)