Skip to content

Commit

Permalink
[7/n] [reconfigurator] combine inventory and omicron zones (#6739)
Browse files Browse the repository at this point in the history
Currently, when the collection process makes two separate queries to
sled-agent: one to obtain basic inventory, and the other to obtain
Omicron zones. With this approach, it is possible for one of the two
queries to fail -- in that case, the `sled_agents` and `omicron_zones`
maps will not have the same set of keys.

In general, consumers of collections must be able to handle these kinds
of failing cases, because collections contain information from a variety
of sources, not all of which are guaranteed to work all the time. But
this specific case (sled-agent inventory vs Omicron zones) is not
intrinsic to the system -- it is an artifact of our APIs.

Change the API so that the `/inventory` endpoint also returns Omicron
zones. The format remains the same, so that `PUT /omicron-zones` mirrors
`GET /inventory | jq .omicron_zones`.

I had to make a few changes to our tests, since we now have a unified
notion of Omicron zones and sled agents. But this exposed what feels to
me like a modeling deficiency in the way our tests work. I hope to fix
that up at some point soon.

There is also one behavior change to the reconfigurator CLI -- now,
`inventory-generate` preserves zone information passed into it from the
system. This matches the behavior of inventory generation within tests
themselves.
  • Loading branch information
sunshowers authored Oct 12, 2024
1 parent e634215 commit c9fc674
Show file tree
Hide file tree
Showing 28 changed files with 339 additions and 391 deletions.
38 changes: 15 additions & 23 deletions dev-tools/omdb/src/bin/omdb/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5180,31 +5180,23 @@ fn inv_collection_print_sleds(collection: &Collection) {
println!(" reservation: {reservation:?}, quota: {quota:?}");
}

if let Some(zones) = collection.omicron_zones.get(&sled.sled_id) {
println!(
" zones collected from {} at {}",
zones.source, zones.time_collected,
);
println!(
" zones generation: {} (count: {})",
zones.zones.generation,
zones.zones.zones.len()
);
println!(
" zones generation: {} (count: {})",
sled.omicron_zones.generation,
sled.omicron_zones.zones.len(),
);

if zones.zones.zones.is_empty() {
continue;
}
if sled.omicron_zones.zones.is_empty() {
continue;
}

println!(" ZONES FOUND");
for z in &zones.zones.zones {
println!(
" zone {} (type {})",
z.id,
z.zone_type.kind().report_str()
);
}
} else {
println!(" warning: no zone information found");
println!(" ZONES FOUND");
for z in &sled.omicron_zones.zones {
println!(
" zone {} (type {})",
z.id,
z.zone_type.kind().report_str()
);
}
}
}
Expand Down
21 changes: 3 additions & 18 deletions dev-tools/reconfigurator-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use nexus_reconfigurator_planning::planner::Planner;
use nexus_reconfigurator_planning::system::{
SledBuilder, SledHwInventory, SystemDescription,
};
use nexus_sled_agent_shared::inventory::OmicronZonesConfig;
use nexus_sled_agent_shared::inventory::ZoneKind;
use nexus_types::deployment::execution;
use nexus_types::deployment::execution::blueprint_external_dns_config;
Expand Down Expand Up @@ -739,25 +738,11 @@ fn cmd_inventory_list(
fn cmd_inventory_generate(
sim: &mut ReconfiguratorSim,
) -> anyhow::Result<Option<String>> {
let mut builder =
let builder =
sim.system.to_collection_builder().context("generating inventory")?;
// For an inventory we just generated from thin air, pretend like each sled
// has no zones on it.
let planning_input =
sim.system.to_planning_input_builder().unwrap().build();
for sled_id in planning_input.all_sled_ids(SledFilter::Commissioned) {
builder
.found_sled_omicron_zones(
"fake sled agent",
sled_id,
OmicronZonesConfig {
generation: Generation::new(),
zones: vec![],
},
)
.context("recording Omicron zones")?;
}

// sim.system carries around Omicron zones, which will make their way into
// the inventory.
let mut inventory = builder.build();
// Assign collection IDs from the RNG. This enables consistent results when
// callers have explicitly seeded the RNG (e.g., in tests).
Expand Down
88 changes: 44 additions & 44 deletions dev-tools/reconfigurator-cli/tests/output/cmd-example-stdout
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ generated inventory collection b32394d8-7d79-486f-8657-fd5219508181 from configu
> blueprint-diff-inventory b32394d8-7d79-486f-8657-fd5219508181 ade5749d-bdf3-4fab-a8ae-00bea01b3a5a
from: collection b32394d8-7d79-486f-8657-fd5219508181
to: blueprint ade5749d-bdf3-4fab-a8ae-00bea01b3a5a
MODIFIED SLEDS:
UNCHANGED SLEDS:

sled 2eb69596-f081-4e2d-9425-9994926e0832 (active):

Expand All @@ -348,24 +348,24 @@ to: blueprint ade5749d-bdf3-4fab-a8ae-00bea01b3a5a
fake-vendor fake-model serial-d792c8cb-7490-40cb-bb1c-d4917242edf4


omicron zones generation 1 -> 2:
omicron zones at generation 2:
------------------------------------------------------------------------------------------
zone type zone id disposition underlay IP
------------------------------------------------------------------------------------------
+ clickhouse fe79023f-c5d5-4be5-ad2c-da4e9e9237e4 in service fd00:1122:3344:102::23
+ crucible 054f64a5-182c-4c28-8994-d2e082550201 in service fd00:1122:3344:102::26
+ crucible 3b5bffea-e5ed-44df-8468-fd4fa69757d8 in service fd00:1122:3344:102::27
+ crucible 53dd7fa4-899e-49ed-9fc2-48222db3e20d in service fd00:1122:3344:102::2a
+ crucible 7db307d4-a6ed-4c47-bddf-6759161bf64a in service fd00:1122:3344:102::2c
+ crucible 95ad9a1d-4063-4874-974c-2fc92830be27 in service fd00:1122:3344:102::29
+ crucible bc095417-e2f0-4e95-b390-9cc3fc6e3c6d in service fd00:1122:3344:102::28
+ crucible d90401f1-fbc2-42cb-bf17-309ee0f922fe in service fd00:1122:3344:102::2b
+ crucible e8f994c0-0a1b-40e6-8db1-40a8ca89e503 in service fd00:1122:3344:102::2d
+ crucible eaec16c0-0d44-4847-b2d6-31a5151bae52 in service fd00:1122:3344:102::24
+ crucible f97aa057-6485-45d0-9cb4-4af5b0831d48 in service fd00:1122:3344:102::25
+ internal_dns 8b8f7c02-7a18-4268-b045-2e286b464c5d in service fd00:1122:3344:1::1
+ internal_ntp c67dd9a4-0d6c-4e9f-b28d-20003f211f7d in service fd00:1122:3344:102::21
+ nexus 94b45ce9-d3d8-413a-a76b-865da1f67930 in service fd00:1122:3344:102::22
clickhouse fe79023f-c5d5-4be5-ad2c-da4e9e9237e4 in service fd00:1122:3344:102::23
crucible 054f64a5-182c-4c28-8994-d2e082550201 in service fd00:1122:3344:102::26
crucible 3b5bffea-e5ed-44df-8468-fd4fa69757d8 in service fd00:1122:3344:102::27
crucible 53dd7fa4-899e-49ed-9fc2-48222db3e20d in service fd00:1122:3344:102::2a
crucible 7db307d4-a6ed-4c47-bddf-6759161bf64a in service fd00:1122:3344:102::2c
crucible 95ad9a1d-4063-4874-974c-2fc92830be27 in service fd00:1122:3344:102::29
crucible bc095417-e2f0-4e95-b390-9cc3fc6e3c6d in service fd00:1122:3344:102::28
crucible d90401f1-fbc2-42cb-bf17-309ee0f922fe in service fd00:1122:3344:102::2b
crucible e8f994c0-0a1b-40e6-8db1-40a8ca89e503 in service fd00:1122:3344:102::2d
crucible eaec16c0-0d44-4847-b2d6-31a5151bae52 in service fd00:1122:3344:102::24
crucible f97aa057-6485-45d0-9cb4-4af5b0831d48 in service fd00:1122:3344:102::25
internal_dns 8b8f7c02-7a18-4268-b045-2e286b464c5d in service fd00:1122:3344:1::1
internal_ntp c67dd9a4-0d6c-4e9f-b28d-20003f211f7d in service fd00:1122:3344:102::21
nexus 94b45ce9-d3d8-413a-a76b-865da1f67930 in service fd00:1122:3344:102::22


sled 32d8d836-4d8a-4e54-8fa9-f31d79c42646 (active):
Expand All @@ -386,23 +386,23 @@ to: blueprint ade5749d-bdf3-4fab-a8ae-00bea01b3a5a
fake-vendor fake-model serial-d7410a1c-e01d-49a4-be9c-f861f086760a


omicron zones generation 1 -> 2:
omicron zones at generation 2:
------------------------------------------------------------------------------------------
zone type zone id disposition underlay IP
------------------------------------------------------------------------------------------
+ crucible 728db429-8621-4e1e-9915-282aadfa27d1 in service fd00:1122:3344:103::24
+ crucible a999e5fa-3edc-4dac-919a-d7b554cdae58 in service fd00:1122:3344:103::27
+ crucible b416f299-c23c-46c8-9820-be2b66ffea0a in service fd00:1122:3344:103::28
+ crucible b5d5491d-b3aa-4727-8b55-f66e0581ea4f in service fd00:1122:3344:103::2c
+ crucible cc1dc86d-bd6f-4929-aa4a-9619012e9393 in service fd00:1122:3344:103::25
+ crucible cd3bb540-e605-465f-8c62-177ac482d850 in service fd00:1122:3344:103::2a
+ crucible e7dd3e98-7fe7-4827-be7f-395ff9a5f542 in service fd00:1122:3344:103::23
+ crucible e8971ab3-fb7d-4ad8-aae3-7f2fe87c51f3 in service fd00:1122:3344:103::26
+ crucible f52aa245-7e1b-46c0-8a31-e09725f02caf in service fd00:1122:3344:103::2b
+ crucible fae49024-6cec-444d-a6c4-83658ab015a4 in service fd00:1122:3344:103::29
+ internal_dns c8aa84a5-a802-46c9-adcd-d61e9c8393c9 in service fd00:1122:3344:2::1
+ internal_ntp e9bf481e-323e-466e-842f-8107078c7137 in service fd00:1122:3344:103::21
+ nexus 4f2eb088-7d28-4c4e-a27c-746400ec65ba in service fd00:1122:3344:103::22
crucible 728db429-8621-4e1e-9915-282aadfa27d1 in service fd00:1122:3344:103::24
crucible a999e5fa-3edc-4dac-919a-d7b554cdae58 in service fd00:1122:3344:103::27
crucible b416f299-c23c-46c8-9820-be2b66ffea0a in service fd00:1122:3344:103::28
crucible b5d5491d-b3aa-4727-8b55-f66e0581ea4f in service fd00:1122:3344:103::2c
crucible cc1dc86d-bd6f-4929-aa4a-9619012e9393 in service fd00:1122:3344:103::25
crucible cd3bb540-e605-465f-8c62-177ac482d850 in service fd00:1122:3344:103::2a
crucible e7dd3e98-7fe7-4827-be7f-395ff9a5f542 in service fd00:1122:3344:103::23
crucible e8971ab3-fb7d-4ad8-aae3-7f2fe87c51f3 in service fd00:1122:3344:103::26
crucible f52aa245-7e1b-46c0-8a31-e09725f02caf in service fd00:1122:3344:103::2b
crucible fae49024-6cec-444d-a6c4-83658ab015a4 in service fd00:1122:3344:103::29
internal_dns c8aa84a5-a802-46c9-adcd-d61e9c8393c9 in service fd00:1122:3344:2::1
internal_ntp e9bf481e-323e-466e-842f-8107078c7137 in service fd00:1122:3344:103::21
nexus 4f2eb088-7d28-4c4e-a27c-746400ec65ba in service fd00:1122:3344:103::22


sled 89d02b1b-478c-401a-8e28-7a26f74fa41b (active):
Expand All @@ -423,23 +423,23 @@ to: blueprint ade5749d-bdf3-4fab-a8ae-00bea01b3a5a
fake-vendor fake-model serial-fe1d5b9f-8db7-4e2d-bf17-c4b80e1f897c


omicron zones generation 1 -> 2:
omicron zones at generation 2:
------------------------------------------------------------------------------------------
zone type zone id disposition underlay IP
------------------------------------------------------------------------------------------
+ crucible 315a3670-d019-425c-b7a6-c9429428b671 in service fd00:1122:3344:101::25
+ crucible 413d3e02-e19f-400a-9718-a662347538f0 in service fd00:1122:3344:101::26
+ crucible 6cb330f9-4609-4d6c-98ad-b5cc34245813 in service fd00:1122:3344:101::2b
+ crucible 6d725df0-0189-4429-b270-3eeb891d39c8 in service fd00:1122:3344:101::2a
+ crucible 8b47e1e8-0396-4e44-a4a5-ea891405c9f2 in service fd00:1122:3344:101::24
+ crucible b43ce109-90d6-46f9-9df0-8c68bfe6d4a0 in service fd00:1122:3344:101::23
+ crucible b5443ebd-1f5b-448c-8edc-b4ca25c25db1 in service fd00:1122:3344:101::27
+ crucible bb55534c-1042-4af4-ad2f-9590803695ac in service fd00:1122:3344:101::29
+ crucible e135441d-637e-4de9-8023-5ea0096347f3 in service fd00:1122:3344:101::28
+ crucible fee71ee6-da42-4a7f-a00e-f56b6a3327ce in service fd00:1122:3344:101::2c
+ internal_dns cbe91cdc-cbb6-4760-aece-6ce08b67e85a in service fd00:1122:3344:3::1
+ internal_ntp 09937ebb-bb6a-495b-bc97-b58076b70a78 in service fd00:1122:3344:101::21
+ nexus f3628f0a-2301-4fc8-bcbf-961199771731 in service fd00:1122:3344:101::22
crucible 315a3670-d019-425c-b7a6-c9429428b671 in service fd00:1122:3344:101::25
crucible 413d3e02-e19f-400a-9718-a662347538f0 in service fd00:1122:3344:101::26
crucible 6cb330f9-4609-4d6c-98ad-b5cc34245813 in service fd00:1122:3344:101::2b
crucible 6d725df0-0189-4429-b270-3eeb891d39c8 in service fd00:1122:3344:101::2a
crucible 8b47e1e8-0396-4e44-a4a5-ea891405c9f2 in service fd00:1122:3344:101::24
crucible b43ce109-90d6-46f9-9df0-8c68bfe6d4a0 in service fd00:1122:3344:101::23
crucible b5443ebd-1f5b-448c-8edc-b4ca25c25db1 in service fd00:1122:3344:101::27
crucible bb55534c-1042-4af4-ad2f-9590803695ac in service fd00:1122:3344:101::29
crucible e135441d-637e-4de9-8023-5ea0096347f3 in service fd00:1122:3344:101::28
crucible fee71ee6-da42-4a7f-a00e-f56b6a3327ce in service fd00:1122:3344:101::2c
internal_dns cbe91cdc-cbb6-4760-aece-6ce08b67e85a in service fd00:1122:3344:3::1
internal_ntp 09937ebb-bb6a-495b-bc97-b58076b70a78 in service fd00:1122:3344:101::21
nexus f3628f0a-2301-4fc8-bcbf-961199771731 in service fd00:1122:3344:101::22


COCKROACHDB SETTINGS:
Expand Down
1 change: 1 addition & 0 deletions nexus-sled-agent-shared/src/inventory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ pub struct Inventory {
pub usable_hardware_threads: u32,
pub usable_physical_ram: ByteCount,
pub reservoir_size: ByteCount,
pub omicron_zones: OmicronZonesConfig,
pub disks: Vec<InventoryDisk>,
pub zpools: Vec<InventoryZpool>,
pub datasets: Vec<InventoryDataset>,
Expand Down
34 changes: 11 additions & 23 deletions nexus/db-model/src/inventory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ use diesel::serialize::ToSql;
use diesel::{serialize, sql_types};
use ipnetwork::IpNetwork;
use nexus_sled_agent_shared::inventory::OmicronZoneDataset;
use nexus_sled_agent_shared::inventory::{
OmicronZoneConfig, OmicronZoneType, OmicronZonesConfig,
};
use nexus_sled_agent_shared::inventory::{OmicronZoneConfig, OmicronZoneType};
use nexus_types::inventory::{
BaseboardId, Caboose, Collection, NvmeFirmware, PowerState, RotPage,
RotSlot,
Expand Down Expand Up @@ -1178,7 +1176,11 @@ impl From<InvDataset> for nexus_types::inventory::Dataset {
}
}

/// See [`nexus_types::inventory::OmicronZonesFound`].
/// Information about a sled's Omicron zones, part of
/// [`nexus_types::inventory::SledAgent`].
///
/// TODO: This table is vestigial and can be combined with `InvSledAgent`. See
/// [issue #6770](https://github.com/oxidecomputer/omicron/issues/6770).
#[derive(Queryable, Clone, Debug, Selectable, Insertable)]
#[diesel(table_name = inv_sled_omicron_zones)]
pub struct InvSledOmicronZones {
Expand All @@ -1192,28 +1194,14 @@ pub struct InvSledOmicronZones {
impl InvSledOmicronZones {
pub fn new(
inv_collection_id: CollectionUuid,
zones_found: &nexus_types::inventory::OmicronZonesFound,
sled_agent: &nexus_types::inventory::SledAgent,
) -> InvSledOmicronZones {
InvSledOmicronZones {
inv_collection_id: inv_collection_id.into(),
time_collected: zones_found.time_collected,
source: zones_found.source.clone(),
sled_id: zones_found.sled_id.into(),
generation: Generation(zones_found.zones.generation),
}
}

pub fn into_uninit_zones_found(
self,
) -> nexus_types::inventory::OmicronZonesFound {
nexus_types::inventory::OmicronZonesFound {
time_collected: self.time_collected,
source: self.source,
sled_id: self.sled_id.into(),
zones: OmicronZonesConfig {
generation: *self.generation,
zones: Vec::new(),
},
time_collected: sled_agent.time_collected,
source: sled_agent.source.clone(),
sled_id: sled_agent.sled_id.into(),
generation: Generation(sled_agent.omicron_zones.generation),
}
}
}
Expand Down
6 changes: 1 addition & 5 deletions nexus/db-queries/src/db/datastore/deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1882,10 +1882,6 @@ mod tests {
&mut collection.sled_agents,
&mut base_collection.sled_agents,
);
mem::swap(
&mut collection.omicron_zones,
&mut base_collection.omicron_zones,
);

// Treat this blueprint as the initial blueprint for the system.
blueprint.parent_blueprint_id = None;
Expand Down Expand Up @@ -1999,7 +1995,7 @@ mod tests {
);
assert_eq!(
blueprint1.blueprint_zones.len(),
collection.omicron_zones.len()
collection.sled_agents.len()
);
assert_eq!(
blueprint1.all_omicron_zones(BlueprintZoneFilter::All).count(),
Expand Down
Loading

0 comments on commit c9fc674

Please sign in to comment.