Update links to API code about hard-coded allowed transitions

[david-crespo]

Closes #2334

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
console	✅ Ready (Inspect)	Visit Preview	Aug 20, 2024 4:59pm

[david-crespo]

Hey @gjcolombo — Charlie and I are double-checking the set of instance states where reboot and stop are allowed and were wondering how to interpret this function.

https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/instance.rs#L790-L798

I see there's different logic depending on whether a VMM is found, but I don't know how to think about that from a user point of view. I see that there a bunch of states where you should never fail to find a VMM.

[gjcolombo]

That match is pretty spooky, especially in light of some recent improvements to Nexus (omicron#5749 and omicron#5854). I filed oxidecomputer/omicron#6390 to see if we can clean it up a bit.

For purposes of this PR I almost think you want to link both to Nexus and to this bit of Propolis: https://github.com/oxidecomputer/propolis/blob/b278193/bin/propolis-server/src/lib/vm/request_queue.rs

The idea underlying the code you linked is that when an instance has a running Propolis--roughly between when you successfully ask to start the instance and the time that Propolis comes back and says it's Destroyed--the Propolis process is the source of truth as to what state changes can and can't be accepted. The match is saying, "OK, I have this request to change the instance's state, and I have a Propolis to talk to; given what I know about the state that Propolis last reported, should I even bother forwarding it this request to get its disposition?"

For requests to run, stop, and reboot, the answer is basically always "let Propolis decide what to do." (The main edge case is Migrating: which Propolis gets the request then? I think the simplest answer is "neither, the request should be rejected until we know where to send it"; omicron#6390 tracks this.)

The migration-request side of the match could also use some tweaking, but the external API doesn't allow explicit requests to migrate anymore (omicron#6311), so that's probably not as much of a concern here.

On the console side, I think the logic you have for these states (right side lines 102/103) is right, though we could also allow a request to stop a rebooting instance (Propolis will queue it), or even reboot a rebooting instance (Propolis will queue the second reboot to execute when the first is done; note though that once you've queued one request to reboot, Propolis will silently drop new ones until the queued request begins to be processed).

Hope this helps a bit--LMK what else I can clarify or fill you in on.

[david-crespo]

That's perfect, thanks. Generally I'm ok with being slightly more restrictive than what's technically possible, though in the UI we sometimes say "an instance can only be rebooted in X states", and in that case I'd prefer to be accurate since we're educating the user there about the system. So I think I'll add rebooting to the lists.

[david-crespo]

On second thought, nope, nope. Too damn weird. Will leave rebooting out, but thanks for the link, and I'm linking to this comment as well.

[gjcolombo]

Technically correct: the best kind of correct!

(Yeah, that looks pretty silly. Leaving rebooting off the lists is the right move.)