Failures in `request_sled_agent` may leave future requests doomed

Open
#3,818 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
30/100
Issue type
Bug
Clarity
Needs clarification
Activity status
Stale
Tech stack
rust
Domain
backend, networking

Research direction

Start in sled-agent/src/bootstrap/agent.rs at BootstrapAgent::request_sled_agent(), then read the callers in agent.rs and server.rs and the LRTQ initialization in secret_retriever.rs. Trace each fallible step and the subnet advertisement and server shutdown paths; done means failed requests cannot leave stale trust-quorum state, subnet advertisements, or server bindings that break later requests.

Written by the indexing model from the issue text.

Description

BootstrapAgent::request_sled_agent() is called from two places:

  1. In BootstrapAgent::new(), if we're just starting and there's a persistent request stored in the ledger (i.e., cold boot)
  2. If we receive a request on the bootstrap server

The first thing request_sled_agent does, if trust quorum is enabled is infallibly, is initialize LRTQ using a hash of the request's rack_id as a salt. This is already potentially problematic: if we have previously entered request_sled_agent() with a different rack ID, this will result in a panic.

If we're in the SledAgentState::Before case, we then proceed to attempt to set up the sled agent, which may fail at multiple steps:

  1. We could fail to restart the hardware monitor after we've taken it out.
  2. We could fail to start the sled-agent server.
  3. We could fail to find at least one M.2 where the ledger should be stored (outright failure here is unlikely, but we could only see one drive here - see #3815).
  4. We could fail to commit the ledger.

In between fallible points 1 and 2, we start advertising the subnet prefix from the request. If we fail at 2/3/4, I don't think anything unadvertises that prefix, so if we get a future request with a different request.subnet, we could end up advertising two subnets (only one of which we're actually using).

If we fail at points 3 or 4, we will drop the sled-agent server, which will attempt to close it, but we have no way (without async Drop) of waiting for that close to actually happen, so it's possible future request_sled_agent calls could fail because the old sled-agent server is still running and bound to our port.

To be clear, I think any of these causing problems in practice are pretty unlikely (or possibly impossible) given the way we currently run RSS / initialize sled-agents; I just wanted to write them down while I'm in this part of the code looking to clean things up.

Dominant language
Rust
Stars
572
Forks
97
Avg merge
2d 12h
Merged PRs (30d)
96

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from oxidecomputer/omicron

All issues in oxidecomputer/omicron

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.