Repository navigation
Conversation
Sled-agent finds disks through the hardware topology, so every disk it reports has a chassis location. Add sled-agent API version MAKE_DISK_LOCATION_REQUIRED, where InventoryDisk::location is a String, and make nexus_types::inventory::PhysicalDisk::location required. The simulated sled-agent now labels disks the way a real sled does (N0.., "M.2 East", "M.2 West"), falling back to N10 or "M.2 2" past the real bay counts. inv_physical_disk.location stays nullable for now: rows written by older Nexus versions have no location. Reading a collection skips those disks; a later release will delete them and make the column NOT NULL.
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // Rows written by Nexus versions that predate the | ||
| // `location` column are NULL. These disks are left out | ||
| // of the collection. | ||
| let Some(location) = disk.location else { | ||
| nskipped += 1; | ||
| continue; | ||
| }; |
There was a problem hiding this comment.
🟡 (optional) Right after an upgrade, omdb nexus sleds expunge-disk can skip its "disk is PRESENT" safety prompt for a disk that is still installed. Reading a collection now silently drops every disk row whose location is NULL. Every collection written before schema version 313 (inv-physical-disk-location) has NULL there. Fix: keep NULL-location disks visible to readers that only check presence, or have the omdb expunge check fall back to inv_physical_disk rows. Another option is to warn when the latest collection skipped disks.
Why this was flagged
The trigger is an operator expunging a disk after the schema migration and before new Nexus finishes its first collection. In that window the latest collection was written by pre-313 Nexus, so location is NULL on every inv_physical_disk row. The read at inventory.rs:3488-3491 hits let Some(location) = disk.location else { nskipped += 1; continue; } and returns sleds with empty disks. dev-tools/omdb/src/bin/omdb/nexus.rs:5734-5740 then finds no match in sleds_containing_disk. Its 0 => {} arm skips the "WARNING: physical disk ... is PRESENT" prompt, so an installed disk is expunged without confirmation. On the base branch the row read back with location: None and the prompt fired. The only signal is a Nexus log warn!, which omdb never sees. The window lasts until the first new collection; inventory.period_secs_collect = 600, but one also runs at startup.
Verification: After schema 313, the latest collection can still have NULL location rows. The diff adds let Some(location) = disk.location else { nskipped += 1; continue; }; in inventory_collection_read, so those disks never reach sled_agents[..].disks. In dev-tools/omdb/src/bin/omdb/nexus.rs:5720-5740, sleds_containing_disk.len() is 0, so the "PRESENT" y/N prompt at 5744-5753 never shows. On the base branch, the disk stayed visible.
Stacked on #11590. Groundwork for #11625.
Since #11590, sled-agent finds disks through the hardware topology, so every disk it reports has a chassis location. This makes that location required beyond sled-agent.
MAKE_DISK_LOCATION_REQUIRED, makesInventoryDisk::locationaString. Clients ofADD_DISK_LOCATION_TO_INVENTORYstill getSome(location).N0,N1, … for U.2s, andM.2 East,M.2 Westfor M.2s. Past the real counts it falls back toN10orM.2 2.pcie_slotnumbering is unchanged, because it's still part of the inventory primary key.PhysicalDisk::locationis required. Reconfigurator state files without alocationno longer load.inv_physical_disk.locationstays nullable for now, because rows written by Nexus versions that predate the column have no location. Every new row gets one, and reading a collection skips disks without one. A later release will delete those rows and make the columnNOT NULL, ahead of the primary-key change in pcie_slot should not be used as a DB primary key #11625.Testing: a new simulated sled-agent test covers disk labels, and a new datastore test covers reading a collection that has a NULL location. Existing nexus-types, nexus-inventory, nexus-db-queries (inventory and physical disk), reconfigurator, rack-setup and omdb tests pass.