Skip to content

Commit

Permalink
Put BlueprintZoneConfig into a map (#7315)
Browse files Browse the repository at this point in the history 
This is a largely mechanical change to store zones in a Blueprint mapped
by their zone uuid rather than in a Vec. This is primarily to support
diffs of `BlueprintZonesConfig` via diffus. Diffus doesn't handle Vecs
in a manner where inserts, removals, and modifications are tracked based
on an id.

To minimize the PR diff, the zone id was not removed from
`BlueprintZoneConfig`. The key to the map must always match the value in
`BlueprintZoneConfig`. It's a shame a mismatch is now representable, and
we can go about trying to change that if necessary. We can also just
make it part of blippy for now. FWIW, this matches the pattern in
`BlueprintDatasetsConfig`.

Putting the `BlueprintZoneConfig` into a map means we no longer require
the sort method on `BlueprintZonesConfig` as the order is stable now.
However, we still want to ensure the same display sort order as
previously, so we make sure to sort things properly before creating the
table.

We'll want similar changes for `BlueprintPhysicalDisksConfig`. That can
come in a follow up PR.
  • Loading branch information
@andrewjstone
andrewjstone authored Jan 8, 2025
1 parent f460a3c commit 5797b8b
Show file tree
Hide file tree
Showing 21 changed files with 225 additions and 179 deletions.
20 changes: 9 additions & 11 deletions nexus/db-queries/src/db/datastore/deployment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ impl DataStore {
.blueprint_zones
.iter()
.flat_map(|(sled_id, zones_config)| {
zones_config.zones.iter().map(move |zone| {
zones_config.zones.values().map(move |zone| {
BpOmicronZone::new(blueprint_id, *sled_id, zone)
.map_err(|e| Error::internal_error(&format!("{:#}", e)))
})
Expand All @@ -276,7 +276,7 @@ impl DataStore {
.blueprint_zones
.values()
.flat_map(|zones_config| {
zones_config.zones.iter().filter_map(|zone| {
zones_config.zones.values().filter_map(|zone| {
BpOmicronZoneNic::new(blueprint_id, zone)
.with_context(|| format!("zone {}", zone.id))
.map_err(|e| Error::internal_error(&format!("{:#}", e)))
Expand Down Expand Up @@ -587,7 +587,7 @@ impl DataStore {
s.sled_id.into(),
BlueprintZonesConfig {
generation: *s.generation,
zones: Vec::new(),
zones: BTreeMap::new(),
},
);
bail_unless!(
Expand Down Expand Up @@ -794,16 +794,11 @@ impl DataStore {
e.to_string()
))
})?;
sled_zones.zones.push(zone);
sled_zones.zones.insert(zone.id, zone);
}
}
}

// Sort all zones to match what blueprint builders do.
for (_, zones_config) in blueprint_zones.iter_mut() {
zones_config.sort();
}

bail_unless!(
omicron_zone_nics.is_empty(),
"found extra Omicron zone NICs: {:?}",
Expand Down Expand Up @@ -2807,7 +2802,7 @@ mod tests {
sled_id,
BlueprintZonesConfig {
generation: omicron_common::api::external::Generation::new(),
zones: vec![BlueprintZoneConfig {
zones: [BlueprintZoneConfig {
disposition: BlueprintZoneDisposition::InService,
id: zone_id,
filesystem_pool: None,
Expand Down Expand Up @@ -2843,7 +2838,10 @@ mod tests {
external_dns_servers: vec![],
},
),
}],
}]
.into_iter()
.map(|z| (z.id, z))
.collect(),
},
);

Expand Down
54 changes: 30 additions & 24 deletions nexus/db-queries/src/db/datastore/rack.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1373,7 +1373,7 @@ mod test {
SledUuid::from_untyped_uuid(sled1.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![
zones: [
BlueprintZoneConfig {
disposition: BlueprintZoneDisposition::InService,
id: external_dns_id,
Expand Down Expand Up @@ -1437,14 +1437,17 @@ mod test {
},
),
},
],
]
.into_iter()
.map(|z| (z.id, z))
.collect(),
},
);
blueprint_zones.insert(
SledUuid::from_untyped_uuid(sled2.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![
zones: [
BlueprintZoneConfig {
disposition: BlueprintZoneDisposition::InService,
id: nexus_id,
Expand Down Expand Up @@ -1511,14 +1514,17 @@ mod test {
},
),
},
],
]
.into_iter()
.map(|z| (z.id, z))
.collect(),
},
);
blueprint_zones.insert(
SledUuid::from_untyped_uuid(sled3.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![BlueprintZoneConfig {
zones: [BlueprintZoneConfig {
disposition: BlueprintZoneDisposition::InService,
id: ntp3_id,
filesystem_pool: Some(random_zpool()),
Expand All @@ -1527,12 +1533,12 @@ mod test {
address: "[::1]:80".parse().unwrap(),
},
),
}],
}]
.into_iter()
.map(|z| (z.id, z))
.collect(),
},
);
for zone_config in blueprint_zones.values_mut() {
zone_config.sort();
}
let blueprint = Blueprint {
id: Uuid::new_v4(),
sled_state: sled_states_active(blueprint_zones.keys().copied()),
Expand Down Expand Up @@ -1698,7 +1704,7 @@ mod test {
SledUuid::from_untyped_uuid(sled.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![
zones: [
BlueprintZoneConfig {
disposition: BlueprintZoneDisposition::InService,
id: nexus_id1,
Expand Down Expand Up @@ -1763,7 +1769,10 @@ mod test {
},
),
},
],
]
.into_iter()
.map(|z| (z.id, z))
.collect(),
},
);

Expand Down Expand Up @@ -1791,9 +1800,6 @@ mod test {
HashMap::from([("api.sys".to_string(), external_records.clone())]),
);

for zone_config in blueprint_zones.values_mut() {
zone_config.sort();
}
let blueprint = Blueprint {
id: Uuid::new_v4(),
sled_state: sled_states_active(blueprint_zones.keys().copied()),
Expand Down Expand Up @@ -1971,7 +1977,7 @@ mod test {
SledUuid::from_untyped_uuid(sled.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![BlueprintZoneConfig {
zones: [BlueprintZoneConfig {
disposition: BlueprintZoneDisposition::InService,
id: nexus_id,
filesystem_pool: Some(random_zpool()),
Expand Down Expand Up @@ -2000,12 +2006,12 @@ mod test {
},
},
),
}],
}]
.into_iter()
.map(|z| (z.id, z))
.collect(),
},
);
for zone_config in blueprint_zones.values_mut() {
zone_config.sort();
}
let blueprint = Blueprint {
id: Uuid::new_v4(),
sled_state: sled_states_active(blueprint_zones.keys().copied()),
Expand Down Expand Up @@ -2080,7 +2086,7 @@ mod test {
SledUuid::from_untyped_uuid(sled.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![
zones: [
BlueprintZoneConfig {
disposition: BlueprintZoneDisposition::InService,
id: external_dns_id,
Expand Down Expand Up @@ -2142,13 +2148,13 @@ mod test {
},
),
},
],
]
.into_iter()
.map(|z| (z.id, z))
.collect(),
},
);

for zone_config in blueprint_zones.values_mut() {
zone_config.sort();
}
let blueprint = Blueprint {
id: Uuid::new_v4(),
sled_state: sled_states_active(blueprint_zones.keys().copied()),
Expand Down
4 changes: 2 additions & 2 deletions nexus/db-queries/src/db/datastore/support_bundle.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,7 @@ mod test {
.values()
.flat_map(|zones_config| {
let mut nexus_zones = vec![];
for zone in &zones_config.zones {
for (_, zone) in &zones_config.zones {
if matches!(zone.zone_type, BlueprintZoneType::Nexus(_))
&& zone.disposition.matches(filter)
{
Expand Down Expand Up @@ -957,7 +957,7 @@ mod test {

fn expunge_nexus_for_bundle(bp: &mut Blueprint, bundle: &SupportBundle) {
for zones in bp.blueprint_zones.values_mut() {
for zone in &mut zones.zones {
for (_, zone) in &mut zones.zones {
if zone.id == bundle.assigned_nexus.unwrap().into() {
zone.disposition = BlueprintZoneDisposition::Expunged;
}
Expand Down
28 changes: 23 additions & 5 deletions nexus/db-queries/src/db/datastore/vpc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3145,7 +3145,13 @@ mod tests {
let bp1_nic = datastore
.service_create_network_interface_raw(
&opctx,
db_nic_from_zone(&bp1.blueprint_zones[&sled_ids[2]].zones[0]),
db_nic_from_zone(
bp1.blueprint_zones[&sled_ids[2]]
.zones
.first_key_value()
.unwrap()
.1,
),
)
.await
.expect("failed to insert service VNIC");
Expand Down Expand Up @@ -3176,7 +3182,11 @@ mod tests {
datastore
.service_delete_network_interface(
&opctx,
bp1.blueprint_zones[&sled_ids[2]].zones[0]
bp1.blueprint_zones[&sled_ids[2]]
.zones
.first_key_value()
.unwrap()
.1
.id
.into_untyped_uuid(),
bp1_nic.id(),
Expand Down Expand Up @@ -3207,7 +3217,13 @@ mod tests {
datastore
.service_create_network_interface_raw(
&opctx,
db_nic_from_zone(&bp3.blueprint_zones[&sled_id].zones[0]),
db_nic_from_zone(
bp3.blueprint_zones[&sled_id]
.zones
.first_key_value()
.unwrap()
.1,
),
)
.await
.expect("failed to insert service VNIC");
Expand Down Expand Up @@ -3261,15 +3277,17 @@ mod tests {
.blueprint_zones
.get_mut(&sled_ids[2])
.expect("zones for sled");
sled2.zones[0].disposition = BlueprintZoneDisposition::Quiesced;
sled2.zones.values_mut().next().unwrap().disposition =
BlueprintZoneDisposition::Quiesced;
sled2.generation = sled2.generation.next();

// Sled index 3's zone is expunged (should be excluded).
let sled3 = bp4
.blueprint_zones
.get_mut(&sled_ids[3])
.expect("zones for sled");
sled3.zones[0].disposition = BlueprintZoneDisposition::Expunged;
sled3.zones.values_mut().next().unwrap().disposition =
BlueprintZoneDisposition::Expunged;
sled3.generation = sled3.generation.next();

bp4
Expand Down
22 changes: 11 additions & 11 deletions nexus/reconfigurator/blippy/src/checks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ mod tests {
// Copy the underlay IP from one Nexus to another.
let mut nexus_iter = blueprint.blueprint_zones.iter_mut().flat_map(
|(sled_id, zones_config)| {
zones_config.zones.iter_mut().filter_map(move |zone| {
zones_config.zones.values_mut().filter_map(move |zone| {
if zone.zone_type.is_nexus() {
Some((*sled_id, zone))
} else {
Expand Down Expand Up @@ -678,7 +678,7 @@ mod tests {
.get(&nexus1_sled_id)
.unwrap()
.zones
.iter();
.values();
let sled1_zone1 = sled1_zones.next().expect("at least one zone");
let sled1_zone2 = sled1_zones.next().expect("at least two zones");
if sled1_zone1.id == nexus1.id {
Expand Down Expand Up @@ -745,7 +745,7 @@ mod tests {
.blueprint_zones
.iter_mut()
.flat_map(|(sled_id, zones_config)| {
zones_config.zones.iter_mut().filter_map(move |zone| {
zones_config.zones.values_mut().filter_map(move |zone| {
if zone.zone_type.is_internal_dns() {
Some((*sled_id, zone))
} else {
Expand Down Expand Up @@ -826,7 +826,7 @@ mod tests {
// Copy the external IP from one Nexus to another.
let mut nexus_iter = blueprint.blueprint_zones.iter_mut().flat_map(
|(sled_id, zones_config)| {
zones_config.zones.iter_mut().filter_map(move |zone| {
zones_config.zones.values_mut().filter_map(move |zone| {
if zone.zone_type.is_nexus() {
Some((*sled_id, zone))
} else {
Expand Down Expand Up @@ -896,7 +896,7 @@ mod tests {
// Copy the external IP from one Nexus to another.
let mut nexus_iter = blueprint.blueprint_zones.iter_mut().flat_map(
|(sled_id, zones_config)| {
zones_config.zones.iter_mut().filter_map(move |zone| {
zones_config.zones.values_mut().filter_map(move |zone| {
if zone.zone_type.is_nexus() {
Some((*sled_id, zone))
} else {
Expand Down Expand Up @@ -961,7 +961,7 @@ mod tests {
// Copy the external IP from one Nexus to another.
let mut nexus_iter = blueprint.blueprint_zones.iter_mut().flat_map(
|(sled_id, zones_config)| {
zones_config.zones.iter_mut().filter_map(move |zone| {
zones_config.zones.values_mut().filter_map(move |zone| {
if zone.zone_type.is_nexus() {
Some((*sled_id, zone))
} else {
Expand Down Expand Up @@ -1030,7 +1030,7 @@ mod tests {
// Copy the durable zpool from one external DNS to another.
let mut dns_iter = blueprint.blueprint_zones.iter_mut().flat_map(
|(sled_id, zones_config)| {
zones_config.zones.iter_mut().filter_map(move |zone| {
zones_config.zones.values_mut().filter_map(move |zone| {
if zone.zone_type.is_external_dns() {
Some((*sled_id, zone))
} else {
Expand Down Expand Up @@ -1110,7 +1110,7 @@ mod tests {
// Copy the filesystem zpool from one external DNS to another.
let mut dns_iter = blueprint.blueprint_zones.iter_mut().flat_map(
|(sled_id, zones_config)| {
zones_config.zones.iter_mut().filter_map(move |zone| {
zones_config.zones.values_mut().filter_map(move |zone| {
if zone.zone_type.is_external_dns() {
Some((*sled_id, zone))
} else {
Expand Down Expand Up @@ -1335,7 +1335,7 @@ mod tests {
// with a filesystem_pool dataset to remove.
let mut durable_zone = None;
let mut root_zone = None;
for z in &zones_config.zones {
for (_, z) in &zones_config.zones {
if durable_zone.is_none() {
if z.zone_type.durable_zpool().is_some() {
durable_zone = Some(z.clone());
Expand Down Expand Up @@ -1501,7 +1501,7 @@ mod tests {
.expect("got zones for sled with datasets");
let mut durable_zone = None;
let mut root_zone = None;
for z in &zones_config.zones {
for (_, z) in &zones_config.zones {
if durable_zone.is_none() {
if z.zone_type.durable_zpool().is_some() {
durable_zone = Some(z.clone());
Expand All @@ -1517,7 +1517,7 @@ mod tests {
root_zone.expect("found zone with root dataset to prune");
zones_config
.zones
.retain(|z| z.id != durable_zone.id && z.id != root_zone.id);
.retain(|_, z| z.id != durable_zone.id && z.id != root_zone.id);

let durable_dataset = durable_zone.zone_type.durable_dataset().unwrap();
let root_dataset = root_zone.filesystem_dataset().unwrap();
Expand Down
Loading

0 comments on commit 5797b8b

Please sign in to comment.