[nexus] Add zpool to inventory #5249
Conversation
|
This PR basically implements the following: In which Nexus can "PUT" zpools to the sled before knowing their size, and relies on the inventory (+ this background task) to update zpool information via scanning. But I'm wondering if we'd prefer to just do the following instead: In which we avoid the background task altogether, and simply await "all relevant zpool info" in the response to |
|
Chatted with @davepacheco a little about this PR, here's our consensus:
|
smklein
changed the title
| @@ -316,7 +316,11 @@ impl CandidateZpools { | |||
| // into a BigInt, not a Numeric. I welcome a better solution. | |||
| let it_will_fit = (old_zpool_usage::dsl::size_used | |||
| + diesel::dsl::sql(&zpool_size_delta.to_string())) | |||
| .le(diesel::dsl::sql(zpool_dsl::total_size::NAME)); | |||
| .le(diesel::dsl::sql( | |||
There was a problem hiding this comment.
Hours spent trying to make this work with Diesel, getting pushed away from subqueries, fighting aliases, and then fumbling with full-table scans as I tried to split this into a separate query:
~4
I am still motivated to make #5063 work after getting through this disk expungement stuff.
| @@ -351,6 +353,17 @@ impl nexus_test_interface::NexusServer for Server { | |||
| .unwrap(); | |||
| } | |||
|
|
|||
| async fn inventory_collect_and_get_latest_collection( | |||
There was a problem hiding this comment.
This API is part of the nexus_test_interface API, and is basically punched through so tests which create synthetic disks can also "cause inventory collection to happen". They need it, to get the inv_zpool table populated.
|
I need to take a lap through the instructions here for fixing the reconfigurator "test_complex" test (see: #5145 (comment)), but this is otherwise ready for review |
|
Also, for completeness, we ended up with an implementation like this, rather than the sequence diagrams above:
|
| @@ -1156,6 +1198,103 @@ mod test { | |||
| logctx.cleanup_successful(); | |||
| } | |||
|
|
|||
| #[tokio::test] | |||
This PR adds zpools to the inventory, and updates the existing
omicron.public.zpooltable to only include aspects of the zpool which are fully controlled by Nexus.This PR is intended to solve a problem introduced by #5172 . In that PR, physical disk and zpool provisioning will be decided by Nexus, and sent to the Sled Agent as a request. As a result, the
total_sizevalue of zpools is not known when a pool is provisioned. Here, we add zpools to the inventory, and JOIN with that table to access the "total_size" of the zpool if it is known.Additionally, this PR adds a test to validate that the database (appropriately) throws "Insufficient capacity" errors if the Zpool exists, but has not appeared in the inventory.