oxidecomputer / oxidecomputer/omicron
physical disks should know which slot they're in
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 572
- Forks
- 97
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 96
Description
In the external API, there is an endpoint for listing all physical disks, and another one for listing all physical disks in a given sled. These indicate whether the physical disk is a M.2 or U.2 device, and include its FRU identity (manufacturer, model number, and serial number). However, they do not indicate which physical slot on the sled the disk is located in.
For fault management purposes, it will be important to be able to say which slot a disk is located in, especially when we need to indicate to the operator that the disk needs to be replaced. We should be able to include location information in these API models. I think this may also be helpful for the physical disk adoption flow, so that the operator can check that the disk being adopted is indeed in the location where a new disk was inserted.
Poking around a bit, I see that the physical location of a disk is not included in the omicron.public.physical_disk table:
https://github.com/oxidecomputer/omicron/blob/906f6804f3c4393d2f15f97835ba9797b41cc75b/nexus/db-model/src/physical_disk.rs#L23-L41
...but it is already collected when inventorying physical disks from sled-agent and is part of the omicron.public.inv_physical_disk table:
One way we could populate this in the API by looking up the slot that the disk was observed to be in in the most recent inventory collection, at the time that the disk is fetched/listed. This would work similar to what we are now doing for sled slots in #11077. This would involve looking up the sled-agent's inventory entry and then finding the disk with the same (vendor, model, serial) as the disk record. Unfortunately, these are currently stored in a Vec<PhysicalDisk>, rather than a map indexed by the disk's identity:
https://github.com/oxidecomputer/omicron/blob/906f6804f3c4393d2f15f97835ba9797b41cc75b/nexus/types/src/inventory.rs#L716
...so we'd have to either do a scan over all the disks on that sled, or change this to an IdOrdMap<DiskIdentity, PhysicalDisk>.
There's also a note here suggesting that maybe the inventory and non-inventory types could be unified, though: https://github.com/oxidecomputer/omicron/blob/906f6804f3c4393d2f15f97835ba9797b41cc75b/nexus/types/src/inventory.rs#L607-L610
Regarding that, IIUC, once a physical disk is adopted, the physical_disk db record is tied not to the entire lifetime of the disk but specifically to its presence in the sled whose UUID is included in the physical_disk row, so if a disk moves from one sled to another, I think this would result in the creation of a new physical_disk record, rather than changing the existing one. I'm not sure what happens (from the control plane's perspective) if a disk that has been adopted moves between slots in the same sled. If that would also result in re-adoption and the creation of a new physical_disk record, we could probably do the simpler thing and just copy the slot number from the inventory collection when creating a new physical_disk row, so we don't have to do a complex lookup against the inventory every time it's accessed. That would be nicer, if we knew that a disk being removed and inserted into a different slot in the same sled would result in a new physical_disk record being created. I'd need to trace the lifecycle of the physical_disk table a bit more closely (or ask someone who already knows how this works) to be sure if this is viable.
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 with nexus/db-model/src/physical_disk.rs and nexus/db-model/src/inventory.rs, then trace the physical-disk API models and adoption lifecycle. Read nexus/types/src/inventory.rs, including the PhysicalDisk collection and the note about unifying inventory and non-inventory types, and compare the sled-slot lookup work in #11077. Done means the physical-disk listing and sled-listing APIs expose the disk's physical slot using a behavior consistent with inventory and adoption.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, backend, databases
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100