Repository navigation
Gate sled-agent startup on trust quorum unlock - #11684
Open
andrewjstone wants to merge 1 commit into
Open
andrewjstone wants to merge 1 commit into
andrewjstone wants to merge 1 commit into
Conversation
We discovered in #11671 that sled-agent will start even if the sled has been expunged from the trust quorum. A sled learns that it is expunged from the trust quorum when it tries to load the rack secret. Therefore before starting the sled-agent we wait until we can load the rack secret. Since the sled-agent has not been started at this point, we also need a way to inspect the state of the rack. A lockstep bootstrap server endpoint was added for this. We could potentially add this to the commissioning API as well, or instead of this bootstrap endpoint. Also note that this assumes that LRTQ is no longer in use. I think that is true across all customers, and if so we should implement #9311 to remove it.
Contributor
Author
|
I plan to test the scenario in #11671 on voxel. Alternatively, if someone wants to test on hardware while I do something else that would be appreciated. |
Contributor
Author
|
I tested this on voxel. I started with a 4 node cluster, then removed Then I logged into I then logged back in and did the same thing. The gate held and eventually the state transitioned to |
Contributor
Author
|
I'm going to run another test in voxel to ensure that we can still add sleds. |
Contributor
Author
|
Adding a sled works fine |
jgallagher
reviewed
Nov 5, 2026
jgallagher
left a comment
jgallagher
left a comment
Contributor
There was a problem hiding this comment.
claude found a previously-unreachable panic that's now reachable: the HardwareMonitor has a select! loop over the sled_agent_started_rx oneshot channel. This was fine because we always held the sending side until sled-agent started, but now if we're expunged we'll drop the sending side. If the select! loop fires multiple times (e.g., because disks are changing), it'll try to poll the oneshot multiple times (which will panic). I assume you didn't see this in voxel because there wouldn't be any hardware changes.
This should fix it, I think, but please double-check:
--- a/sled-agent/src/hardware_monitor.rs
+++ b/sled-agent/src/hardware_monitor.rs
@@ -133,14 +133,34 @@ impl HardwareMonitor {
// its monitor.
self.check_latest_hardware_snapshot().await;
+ // If `sled_agent_started_rx` is dropped without ever sending us the
+ // "sled agent started" message, the bootstrap server has decided it
+ // will never happen. This should only happen if we've been expunged.
+ let mut is_sled_agent_started_rx_alive = true;
+
loop {
tokio::select! {
- Ok(sled_agent) = &mut self.sled_agent_started_rx,
- if self.sled_agent.is_none() =>
+ result = &mut self.sled_agent_started_rx,
+ if is_sled_agent_started_rx_alive =>
{
- info!(self.log, "Sled Agent Started");
- self.sled_agent = Some(sled_agent);
- self.check_latest_hardware_snapshot().await;
+ match result {
+ Ok(sled_agent) => {
+ info!(self.log, "Sled Agent Started");
+ self.sled_agent = Some(sled_agent);
+ self.check_latest_hardware_snapshot().await;
+ }
+ Err(_) => {
+ error!(
+ self.log,
+ "Sled Agent will never start - start channel \
+ dropped. This should only happen if the sled \
+ has been expunged.",
+ );
+ }
+ }
+ // This is a oneshot channel: whatever result we got, it's
+ // done, and we must not poll it again.
+ is_sled_agent_started_rx_alive = false;
}
Ok(service_manager) = &mut self.service_manager_ready_rx,
if self.service_manager.is_none() =>| info!(log, "rack unlocked"); | ||
| return RackUnlockOutcome::Unlocked; | ||
| } | ||
| Ok(None) => (), |
Contributor
There was a problem hiding this comment.
Just making sure I understand the happy path - POLL_INTERVAL is pretty aggressive, but this is the branch we expect to loop in (which doesn't log anything, which seems correct) if we're in a normal "waiting for rack unlock" case, right? All the other logs in this function are either "we're done" or "something unexpected has happened"?
| "error" => InlineErrorChain::new(&err), | ||
| ); | ||
| sleep(POLL_INTERVAL).await; | ||
| continue; |
Contributor
There was a problem hiding this comment.
Is the continue here because we can't / shouldn't call load_latest_rack_secret() if calling status() failed?
| "epoch" => %expunged.epoch, | ||
| "learned_from" => %expunged.from, | ||
| ); | ||
| return RackUnlockOutcome::Expunged(expunged); |
Contributor
There was a problem hiding this comment.
Is there any TOCTOU issue with calling status() separately from load_latest_rack_secret()? E.g., if this sled were expunged in between the two calls, could it still conceivably load the (old) rack secret and report Unlocked below even if it would have simultaneously reported "expunged" if we'd checked status at the same time?
This might not matter much - expunging a sled as its sled-agent is starting up is inherently racy, I guess.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We discovered in #11671 that sled-agent will start even if the sled has been expunged from the trust quorum. A sled learns that it is expunged from the trust quorum when it tries to load the rack secret. Therefore before starting the sled-agent we wait until we can load the rack secret.
Since the sled-agent has not been started at this point, we also need a way to inspect the state of the rack. A lockstep bootstrap server endpoint was added for this. We could potentially add this to the commissioning API as well, or instead of this bootstrap endpoint.
Also note that this assumes that LRTQ is no longer in use. I think that is true across all customers, and if so we should implement #9311 to remove it.