oxidecomputer / oxidecomputer/omicron
`Nexus::select_runtime_change_action`'s final `match` needs tuning
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 572
- Forks
- 97
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 96
Description
Specifically this bit: https://github.com/oxidecomputer/omicron/blob/6dd980251a26430466bcd5aff1edad5416cf94e5/nexus/src/app/instance.rs#L781-L813
This match is a little hard to parse. At this point, Nexus already believes the instance of interest has a running Propolis and is just deciding whether to send a state change request there for disposition. I think the general idea should be to say something like
- If this is a request to start, reboot, or stop, and the Propolis is in a non-terminal state (i.e. it's not Failed or Destroyed), and it doesn't look like we're migrating, forward the request to Propolis. (We don't currently transmit the state change request queue from source to target during a migration, so allowing a reboot/stop request on a migrating instance might result in a request being queued to the source that won't be picked up by the target.)
- If this is a request to start Propolis via migration in, and the target Propolis hasn't reported that it's started migrating yet, permit the request; otherwise the VMM has already started and there's nothing left to do
For start/stop/reboot, this is pretty close to what we have today, but the handling for requests to migrate could stand to be tightened up a bit.
It's also worth noting that this match is in a path where we already know that we've got an active Propolis. We should consider whether it'd be clearer to look at the VMM's state directly instead of looking at it as interpreted through InstanceAndActiveVmm::determine_effective_state.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in nexus/src/app/instance.rs around lines 781-813, reading Nexus::select_runtime_change_action and InstanceAndActiveVmm::determine_effective_state. Trace the existing handling of start, reboot, stop, and migration requests; done means the match clearly enforces the stated terminal-state and migration conditions, with the relevant tests passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, distributed-systems
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100