[omdb] Show physical disks in blueprints and inventory #5745
Conversation
| @@ -39,7 +39,7 @@ | |||
| } | |||
| ], | |||
| "datum_type": "cumulative_u64", | |||
| "created": "2024-05-03T22:37:51.326086935Z" | |||
| "created": "2024-05-11T09:41:23.361298682Z" | |||
There was a problem hiding this comment.
Was this supposed to be committed?
There was a problem hiding this comment.
Hmm, it changed. But maybe I shouldn't have committed it?
There was a problem hiding this comment.
I think I would vote "don't commit it" along with possibly file an issue? I'm surprised this isn't an issue on main...
There was a problem hiding this comment.
This works because we explicitly ignore timestamps when comparing the schema on disk with the current one. It's fine to commit, or not, whatever you'd like.
There was a problem hiding this comment.
That makes sense. Thanks @bnaecker. I believe it changed when I ran all the tests with cargo nextest run. I'll leave it out of sheer laziness.
This PR changes the construction and display of blueprint diffs significantly. The need for the changes arose out of the tight coupling between zones and sleds. When adding disks to the mix we needed to also take them into account when determininig when a sled was modified. We also want to display physical disks as tables along with the zone tables under the sleds. This turned out to be somewhat trickier than necesary, and so I changed how the tables were constructed and rendered. Hopefully this will also make it easier to add new tables in the future. One possibly controversial change is that I changed the way zone modifications are rendered. They are no longer three lines long. Currently only `disposition` is allowed to change, and so I used an arrow from "old" to "new" versions. Additionally, I removed the (added/ modified/removed) suffixes as they seem redundant to me, make the lines longer, and make things harder to read IMO. I'm open to discussion about both of these changes. My guess is that eventually, we'll want to be able to do row filtering, and I plan to also add some colored output, also in this PR most likely. It should be noted that the output mechanism is decoupled from the representation of the tables, `BpSledSubtable`. This allows output in other formats if desired in the future. As for the inventory collection output, I added a requirement when filtering on sled-id, that we also filter on collection-id, because filtering on sled-id does a full table scan and requires a new index. I can add the index instead if we feel it's important. Fixes #5624
andrewjstone
force-pushed
the
omdb-physical-disks-experimental-tables-2
branch
from
009bc2c
to
e076d97
Compare
|
There's actually a bug here where we're generating new output on every test run because UUIDs for physical disks aren't fixed. There's another one, where we want to sort by zone type rather than uuid, but we don't. I'll fix both of these up. |
These are now fixed. I think this PR is good to go. I haven't yet changed back the diffs to be multiline, but I can do that in a follow up immediately. There are some changes that make writing tests slightly different which I'd like to get in before the blueprint stuff gets refactored. There are also a bunch of other changes to make the test system more deterministic by using seeded TypedUuids everywhere I noticed an issue. |
This PR changes the construction and display of blueprint diffs significantly. The need for the changes arose out of the tight coupling between zones and sleds. When adding disks to the mix we needed to also take them into account when determininig when a sled was modified. We also want to display physical disks as tables along with the zone tables under the sleds. This turned out to be somewhat trickier than necesary, and so I changed how the tables were constructed and rendered. Hopefully this will also make it easier to add new tables in the future.
One possibly controversial change is that I changed the way zone modifications are rendered. They are no longer three lines long. Currently only
dispositionis allowed to change, and so I used an arrow from "old" to "new" versions. Additionally, I removed the (added/ modified/removed) suffixes as they seem redundant to me, make the lines longer, and make things harder to read IMO. I'm open to discussion about both of these changes. My guess is that eventually, we'll want to be able to do row filtering, and I plan to also add some colored output, also in this PR most likely.It should be noted that the output mechanism is decoupled from the representation of the tables,
BpSledSubtable. This allows output in other formats if desired in the future.As for the inventory collection output, I added a requirement when filtering on sled-id, that we also filter on collection-id, because filtering on sled-id does a full table scan and requires a new index. I can add the index instead if we feel it's important.
Fixes #5624