Skip to content

Commit

Permalink
Use SledUuid keys in Blueprint::blueprint_zones (#5623)
Browse files Browse the repository at this point in the history
  • Loading branch information
jgallagher authored Apr 25, 2024
1 parent 4fda855 commit d80cd29
Show file tree
Hide file tree
Showing 13 changed files with 83 additions and 143 deletions.
31 changes: 11 additions & 20 deletions nexus/db-queries/src/db/datastore/deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,24 +137,16 @@ impl DataStore {
.blueprint_zones
.iter()
.map(|(sled_id, zones_config)| {
BpSledOmicronZones::new(
blueprint_id,
SledUuid::from_untyped_uuid(*sled_id),
zones_config,
)
BpSledOmicronZones::new(blueprint_id, *sled_id, zones_config)
})
.collect::<Vec<_>>();
let omicron_zones = blueprint
.blueprint_zones
.iter()
.flat_map(|(sled_id, zones_config)| {
zones_config.zones.iter().map(move |zone| {
BpOmicronZone::new(
blueprint_id,
SledUuid::from_untyped_uuid(*sled_id),
zone,
)
.map_err(|e| Error::internal_error(&format!("{:#}", e)))
BpOmicronZone::new(blueprint_id, *sled_id, zone)
.map_err(|e| Error::internal_error(&format!("{:#}", e)))
})
})
.collect::<Result<Vec<_>, Error>>()?;
Expand Down Expand Up @@ -302,7 +294,7 @@ impl DataStore {
// the `OmicronZonesConfig` generation number for each sled that is a
// part of this blueprint. Construct the BTreeMap we ultimately need,
// but all the `zones` vecs will be empty until our next query below.
let mut blueprint_zones: BTreeMap<Uuid, BlueprintZonesConfig> = {
let mut blueprint_zones: BTreeMap<SledUuid, BlueprintZonesConfig> = {
use db::schema::bp_sled_omicron_zones::dsl;

let mut blueprint_zones = BTreeMap::new();
Expand All @@ -325,7 +317,7 @@ impl DataStore {

for s in batch {
let old = blueprint_zones.insert(
s.sled_id.into_untyped_uuid(),
s.sled_id.into(),
BlueprintZonesConfig {
generation: *s.generation,
zones: Vec::new(),
Expand Down Expand Up @@ -467,24 +459,23 @@ impl DataStore {
})
})
.transpose()?;
let sled_zones = blueprint_zones
.get_mut(z.sled_id.as_untyped_uuid())
.ok_or_else(|| {
let sled_id = SledUuid::from(z.sled_id);
let zone_id = z.id;
let sled_zones =
blueprint_zones.get_mut(&sled_id).ok_or_else(|| {
// This error means that we found a row in
// bp_omicron_zone with no associated record in
// bp_sled_omicron_zones. This should be
// impossible and reflects either a bug or database
// corruption.
Error::internal_error(&format!(
"zone {}: unknown sled: {}",
z.id, z.sled_id
"zone {zone_id}: unknown sled: {sled_id}",
))
})?;
let zone_id = z.id;
let zone = z
.into_blueprint_zone_config(nic_row)
.with_context(|| {
format!("zone {:?}: parse from database", zone_id)
format!("zone {zone_id}: parse from database")
})
.map_err(|e| {
Error::internal_error(&format!(
Expand Down
14 changes: 7 additions & 7 deletions nexus/db-queries/src/db/datastore/rack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,9 +977,9 @@ mod test {
};
use omicron_common::api::internal::shared::SourceNatConfig;
use omicron_test_utils::dev;
use omicron_uuid_kinds::TypedUuid;
use omicron_uuid_kinds::{ExternalIpUuid, OmicronZoneUuid};
use omicron_uuid_kinds::{GenericUuid, ZpoolUuid};
use omicron_uuid_kinds::{SledUuid, TypedUuid};
use sled_agent_client::types::OmicronZoneDataset;
use std::collections::{BTreeMap, HashMap};
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV6};
Expand Down Expand Up @@ -1292,7 +1292,7 @@ mod test {

let mut blueprint_zones = BTreeMap::new();
blueprint_zones.insert(
sled1.id(),
SledUuid::from_untyped_uuid(sled1.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![
Expand Down Expand Up @@ -1367,7 +1367,7 @@ mod test {
},
);
blueprint_zones.insert(
sled2.id(),
SledUuid::from_untyped_uuid(sled2.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![
Expand Down Expand Up @@ -1443,7 +1443,7 @@ mod test {
},
);
blueprint_zones.insert(
sled3.id(),
SledUuid::from_untyped_uuid(sled3.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![BlueprintZoneConfig {
Expand Down Expand Up @@ -1620,7 +1620,7 @@ mod test {

let mut blueprint_zones = BTreeMap::new();
blueprint_zones.insert(
sled.id(),
SledUuid::from_untyped_uuid(sled.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![
Expand Down Expand Up @@ -1890,7 +1890,7 @@ mod test {
let mut macs = MacAddr::iter_system();
let mut blueprint_zones = BTreeMap::new();
blueprint_zones.insert(
sled.id(),
SledUuid::from_untyped_uuid(sled.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![BlueprintZoneConfig {
Expand Down Expand Up @@ -1994,7 +1994,7 @@ mod test {

let mut blueprint_zones = BTreeMap::new();
blueprint_zones.insert(
sled.id(),
SledUuid::from_untyped_uuid(sled.id()),
BlueprintZonesConfig {
generation: Generation::new().next(),
zones: vec![
Expand Down
5 changes: 3 additions & 2 deletions nexus/db-queries/src/db/datastore/vpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1567,7 +1567,8 @@ mod tests {

fn blueprint_zone_configs(
&self,
) -> impl Iterator<Item = (Uuid, BlueprintZoneConfig)> + '_ {
) -> impl Iterator<Item = (SledUuid, BlueprintZoneConfig)> + '_
{
self.nexuses.iter().zip(self.db_nics()).map(|(nexus, nic)| {
let config = BlueprintZoneConfig {
disposition: BlueprintZoneDisposition::InService,
Expand Down Expand Up @@ -1597,7 +1598,7 @@ mod tests {
},
),
};
(nexus.sled_id.into_untyped_uuid(), config)
(nexus.sled_id, config)
})
}
}
Expand Down
8 changes: 3 additions & 5 deletions nexus/reconfigurator/execution/src/dns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,6 @@ mod test {
use omicron_common::api::external::IdentityMetadataCreateParams;
use omicron_test_utils::dev::test_setup_log;
use omicron_uuid_kinds::ExternalIpUuid;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::OmicronZoneUuid;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
Expand Down Expand Up @@ -560,7 +559,7 @@ mod test {
let mut blueprint_zones = BTreeMap::new();
for (sled_id, zones_config) in collection.omicron_zones {
blueprint_zones.insert(
sled_id.into_untyped_uuid(),
sled_id,
BlueprintZonesConfig {
generation: zones_config.zones.generation,
zones: zones_config
Expand Down Expand Up @@ -628,17 +627,16 @@ mod test {
.zip(possible_sled_subnets)
.enumerate()
.map(|(i, (sled_id, subnet))| {
let sled_id = SledUuid::from_untyped_uuid(*sled_id);
let sled_info = Sled {
id: sled_id,
id: *sled_id,
sled_agent_address: get_sled_address(Ipv6Subnet::new(
subnet.network(),
)),
// The first two of these (arbitrarily) will be marked
// Scrimlets.
is_scrimlet: i < 2,
};
(sled_id, sled_info)
(*sled_id, sled_info)
})
.collect();

Expand Down
15 changes: 4 additions & 11 deletions nexus/reconfigurator/execution/src/omicron_zones.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,17 @@ use omicron_uuid_kinds::SledUuid;
use slog::info;
use slog::warn;
use std::collections::BTreeMap;
use uuid::Uuid;

/// Idempotently ensure that the specified Omicron zones are deployed to the
/// corresponding sleds
pub(crate) async fn deploy_zones(
opctx: &OpContext,
sleds_by_id: &BTreeMap<SledUuid, Sled>,
zones: &BTreeMap<Uuid, BlueprintZonesConfig>,
zones: &BTreeMap<SledUuid, BlueprintZonesConfig>,
) -> Result<(), Vec<anyhow::Error>> {
let errors: Vec<_> = stream::iter(zones)
.filter_map(|(sled_id, config)| async move {
let db_sled = match sleds_by_id
.get(&SledUuid::from_untyped_uuid(*sled_id))
{
let db_sled = match sleds_by_id.get(sled_id) {
Some(sled) => sled,
None => {
if config.are_all_zones_expunged() {
Expand All @@ -41,7 +38,7 @@ pub(crate) async fn deploy_zones(
);
return None;
}
let err = anyhow!("sled not found in db list: {}", sled_id);
let err = anyhow!("sled not found in db list: {sled_id}");
warn!(opctx.log, "{err:#}");
return Some(err);
}
Expand Down Expand Up @@ -106,7 +103,6 @@ mod test {
};
use nexus_types::inventory::OmicronZoneDataset;
use omicron_common::api::external::Generation;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::OmicronZoneUuid;
use omicron_uuid_kinds::SledUuid;
use std::collections::BTreeMap;
Expand All @@ -128,10 +124,7 @@ mod test {
},
Blueprint {
id,
blueprint_zones: blueprint_zones
.into_iter()
.map(|(typed_id, z)| (typed_id.into_untyped_uuid(), z))
.collect(),
blueprint_zones,
blueprint_disks: BTreeMap::new(),
parent_blueprint_id: None,
internal_dns_version: Generation::new(),
Expand Down
30 changes: 13 additions & 17 deletions nexus/reconfigurator/planning/src/blueprint_builder/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ use slog::error;
use slog::info;
use slog::o;
use slog::Logger;
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::collections::HashSet;
Expand All @@ -70,7 +69,6 @@ use std::net::SocketAddrV6;
use thiserror::Error;
use typed_rng::TypedUuidRng;
use typed_rng::UuidRng;
use uuid::Uuid;

use super::zones::is_already_expunged;
use super::zones::BuilderZoneState;
Expand Down Expand Up @@ -198,7 +196,7 @@ impl<'a> BlueprintBuilder<'a> {
generation: Generation::new(),
zones: Vec::new(),
};
(sled_id.into_untyped_uuid(), config)
(sled_id, config)
})
.collect::<BTreeMap<_, _>>();
let num_sleds = blueprint_zones.len();
Expand Down Expand Up @@ -961,17 +959,14 @@ impl BlueprintBuilderRng {
/// struct makes it easy for callers iterate over the right set of zones.
pub(super) struct BlueprintZonesBuilder<'a> {
changed_zones: BTreeMap<SledUuid, BuilderZonesConfig>,
// Temporarily make a clone of the parent blueprint's zones so we can use
// typed UUIDs everywhere. Once we're done migrating, this `Cow` can be
// removed.
parent_zones: Cow<'a, BTreeMap<SledUuid, BlueprintZonesConfig>>,
parent_zones: &'a BTreeMap<SledUuid, BlueprintZonesConfig>,
}

impl<'a> BlueprintZonesBuilder<'a> {
pub fn new(parent_blueprint: &'a Blueprint) -> BlueprintZonesBuilder {
BlueprintZonesBuilder {
changed_zones: BTreeMap::new(),
parent_zones: Cow::Owned(parent_blueprint.typed_blueprint_zones()),
parent_zones: &parent_blueprint.blueprint_zones,
}
}

Expand Down Expand Up @@ -1018,25 +1013,26 @@ impl<'a> BlueprintZonesBuilder<'a> {
pub fn into_zones_map(
mut self,
sled_ids: impl Iterator<Item = SledUuid>,
) -> BTreeMap<Uuid, BlueprintZonesConfig> {
) -> BTreeMap<SledUuid, BlueprintZonesConfig> {
sled_ids
.map(|sled_id| {
// Start with self.changed_zones, which contains entries for any
// sled whose zones config is changing in this blueprint.
if let Some(zones) = self.changed_zones.remove(&sled_id) {
(sled_id.into_untyped_uuid(), zones.build())
(sled_id, zones.build())
}
// Next, check self.parent_zones, to represent an unchanged sled.
// Next, check self.parent_zones, to represent an unchanged
// sled.
else if let Some(parent_zones) =
self.parent_zones.get(&sled_id)
{
(sled_id.into_untyped_uuid(), parent_zones.clone())
(sled_id, parent_zones.clone())
} else {
// If the sled is not in self.parent_zones, then it must be a
// new sled and we haven't added any zones to it yet. Use the
// standard initial config.
// If the sled is not in self.parent_zones, then it must be
// a new sled and we haven't added any zones to it yet. Use
// the standard initial config.
(
sled_id.into_untyped_uuid(),
sled_id,
BlueprintZonesConfig {
generation: Generation::new(),
zones: vec![],
Expand Down Expand Up @@ -1465,7 +1461,7 @@ pub mod test {
// Also remove this zone from the blueprint.
parent
.blueprint_zones
.get_mut(sled_id.as_untyped_uuid())
.get_mut(sled_id)
.expect("missing sled")
.zones
.retain(|z| !z.zone_type.is_nexus());
Expand Down
6 changes: 1 addition & 5 deletions nexus/reconfigurator/planning/src/example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use nexus_types::deployment::OmicronZoneNic;
use nexus_types::deployment::PlanningInput;
use nexus_types::deployment::SledFilter;
use nexus_types::inventory::Collection;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::SledKind;
use typed_rng::TypedUuidRng;

Expand Down Expand Up @@ -91,10 +90,7 @@ impl ExampleSystem {
builder.set_rng_seed((test_name, "ExampleSystem collection"));

for sled_id in blueprint.sleds() {
// TODO-cleanup use `TypedUuid` everywhere
let Some(zones) =
blueprint.blueprint_zones.get(sled_id.as_untyped_uuid())
else {
let Some(zones) = blueprint.blueprint_zones.get(&sled_id) else {
continue;
};
for zone in zones.zones.iter() {
Expand Down
Loading

0 comments on commit d80cd29

Please sign in to comment.