oxidecomputer / oxidecomputer/omicron

use inventory instead of `physical_disk` when reporting physical device information

Open
#5,183 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

tl;dr: After #5150, I think we want to use the inventory system for some of the uses where we're currently using the physical_disk table. We might also want to make it harder to accidentally use physical_disk when inventory makes more sense. Why does this matter? The inventory system reflects ground truth about what physical devices are where. It cleanly represents a hardware device having moved between sleds or even the same device appearing in multiple sleds around the same time. If you want to know what's currently in a sled (or, was, during the inventory collection), it's more accurate than physical_disk.

Relatedly, there's currently a database constraint that the same vendor/model/serial does not appear in physical_disk more than once. I think that's not right. If nothing else, in RFD 457 we're saying that a disk can be expunged and re-added, in which case it will have multiple physical_disk records (some dead).

Should the constraint remain for "live" records? I'm less sure. All things being equal it'd be nice if we didn't depend on uniqueness of device-reported data for correctness -- I believe we don't depend on this for sleds. But it also seems fine if we want to impose that constraint.

Relatedly: by having vendor/model/serial in the physical_disk table, I'm a little worried that people will think it's correct to use the physical_disk table to get the devices (make/model/serial) that are physically in the sled. That might be what the caller wants, but quite possibly not: that'll be missing disks that haven't been formally adopted and it will contain disks that are physically missing. I'm not sure what it will say for disks that get moved. This might not really be a problem or anything that we can do anything about.

Similarly, I'm a little worried people will think it's correct to figure out which sled currently contains a disk by looking up the physical_disk and seeing the corresponding sled_id. That might work but it depends on what they want. Really, that's just the last sled that adopted the disk. The disk could be missing. Or there could be other disks in the system reporting the same serial (presumably unlikely). Or it could have been moved.

Concretely, I think this boils down to:

  • Find current users of physical_disk and see if they should use inventory instead. I think this would include the API to list physical disks.
  • Determine what constraints we want to impose on physical_disk in terms of vendor/model/serial uniqueness.

A related problem is dealing with disks that have moved. One idea I discussed with @smklein is: during planning, Nexus can check if any of the physical_disks in a sled are in some other sled. If so, it could:

  • update the physical_disk table to point at the new sled_id
  • tell the first sled to stop using the disk
  • tell the second sled to start using the disk

I think this fits into the Reconfigurator model. Presumably we'd have an API for telling sled agent what disks it's managing and invoke it for every blueprint execution similar to what we do for Omicron zones.

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.

Research direction

Start by finding current users of the physical_disk table, especially the API that lists physical disks, and compare their needs with the inventory system. Inspect the physical_disk vendor/model/serial uniqueness constraint and the Nexus, Reconfigurator, and sled-agent flows; done means the affected callers and the desired constraint and disk-movement behavior are clearly decided.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, backend, databases
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.