Skip to content

Commit

Permalink
blueprint builder: check for unknown external networkign in input
Browse files Browse the repository at this point in the history
  • Loading branch information
jgallagher committed May 15, 2024
1 parent d39774f commit 047e16b
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 8 deletions.
61 changes: 59 additions & 2 deletions nexus/reconfigurator/planning/src/blueprint_builder/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1044,10 +1044,12 @@ pub mod test {
use crate::system::SledBuilder;
use expectorate::assert_contents;
use nexus_types::deployment::BlueprintZoneFilter;
use nexus_types::deployment::OmicronZoneNetworkResources;
use nexus_types::external_api::views::SledPolicy;
use omicron_common::address::IpRange;
use omicron_test_utils::dev::test_setup_log;
use std::collections::BTreeSet;
use std::mem;

pub const DEFAULT_N_SLEDS: usize = 3;

Expand Down Expand Up @@ -1332,6 +1334,14 @@ pub mod test {
static TEST_NAME: &str = "blueprint_builder_test_add_physical_disks";
let logctx = test_setup_log(TEST_NAME);
let (_, input, _) = example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS);
let input = {
// Clear out the external networking records from `input`, since
// we're building an empty blueprint.
let mut builder = input.into_builder();
*builder.network_resources_mut() =
OmicronZoneNetworkResources::new();
builder.build()
};

// Start with an empty blueprint (sleds with no zones).
let parent = BlueprintBuilder::build_empty_with_sleds_seeded(
Expand Down Expand Up @@ -1380,6 +1390,14 @@ pub mod test {
// Discard the example blueprint and start with an empty one.
let (collection, input, _) =
example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS);
let input = {
// Clear out the external networking records from `input`, since
// we're building an empty blueprint.
let mut builder = input.into_builder();
*builder.network_resources_mut() =
OmicronZoneNetworkResources::new();
builder.build()
};
let parent = BlueprintBuilder::build_empty_with_sleds_seeded(
input.all_sled_ids(SledFilter::Commissioned),
"test",
Expand Down Expand Up @@ -1421,7 +1439,7 @@ pub mod test {
fn test_add_nexus_error_cases() {
static TEST_NAME: &str = "blueprint_builder_test_add_nexus_error_cases";
let logctx = test_setup_log(TEST_NAME);
let (mut collection, input, mut parent) =
let (mut collection, mut input, mut parent) =
example(&logctx.log, TEST_NAME, DEFAULT_N_SLEDS);

// Remove the Nexus zone from one of the sleds so that
Expand All @@ -1435,12 +1453,51 @@ pub mod test {
if zones.zones.zones.len() < nzones_before_retain {
selected_sled_id = Some(*sled_id);
// Also remove this zone from the blueprint.
let mut removed_nexus = None;
parent
.blueprint_zones
.get_mut(sled_id)
.expect("missing sled")
.zones
.retain(|z| !z.zone_type.is_nexus());
.retain(|z| match &z.zone_type {
BlueprintZoneType::Nexus(z) => {
removed_nexus = Some(z.clone());
false
}
_ => true,
});
let removed_nexus =
removed_nexus.expect("removed Nexus from blueprint");

// Also remove this Nexus's external networking resources
// from `input`.
let mut builder = input.into_builder();
let mut new_network_resources =
OmicronZoneNetworkResources::new();
let old_network_resources = builder.network_resources_mut();
for ip in old_network_resources.omicron_zone_external_ips()
{
if ip.ip.id() != removed_nexus.external_ip.id {
new_network_resources
.add_external_ip(ip.zone_id, ip.ip)
.expect("copied IP to new input");
}
}
for nic in old_network_resources.omicron_zone_nics() {
if nic.nic.id.into_untyped_uuid()
!= removed_nexus.nic.id
{
new_network_resources
.add_nic(nic.zone_id, nic.nic)
.expect("copied NIC to new input");
}
}
mem::swap(
old_network_resources,
&mut new_network_resources,
);
input = builder.build();

break;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ use nexus_types::deployment::Blueprint;
use nexus_types::deployment::BlueprintZoneFilter;
use nexus_types::deployment::BlueprintZoneType;
use nexus_types::deployment::PlanningInput;
use omicron_common::address::DNS_OPTE_IPV4_SUBNET;
use omicron_common::address::DNS_OPTE_IPV6_SUBNET;
use omicron_common::address::NEXUS_OPTE_IPV4_SUBNET;
use omicron_common::address::NEXUS_OPTE_IPV6_SUBNET;
use omicron_common::address::NTP_OPTE_IPV4_SUBNET;
use omicron_common::address::NTP_OPTE_IPV6_SUBNET;
use omicron_common::api::external::IpNet;
use omicron_common::api::external::MacAddr;
use std::collections::HashSet;
Expand Down Expand Up @@ -117,6 +121,64 @@ impl<'a> BuilderExternalNetworking<'a> {
}
}

// Check the planning input: there shouldn't be any external networking
// resources in the database (the source of `input`) that we don't know
// about from the parent blueprint.
for external_ip_entry in
input.network_resources().omicron_zone_external_ips()
{
if !used_external_ips.contains(&external_ip_entry.ip.ip()) {
bail!(
"planning input contains unexpected external IP \
(IP not found in parent blueprint): {external_ip_entry:?}"
);
}
}
for nic_entry in input.network_resources().omicron_zone_nics() {
if !used_macs.contains(&nic_entry.nic.mac) {
bail!(
"planning input contains unexpected NIC \
(MAC not found in parent blueprint): {nic_entry:?}"
);
}
match nic_entry.nic.ip {
IpAddr::V4(ip) if NEXUS_OPTE_IPV4_SUBNET.contains(ip) => {
if !existing_nexus_v4_ips.contains(&ip) {
bail!(
"planning input contains unexpected NIC \
(IP not found in parent blueprint): {nic_entry:?}"
);
}
}
IpAddr::V4(ip) if NTP_OPTE_IPV4_SUBNET.contains(ip) => {
// TODO check existing_ntp_v4_ips, once it exists
}
IpAddr::V4(ip) if DNS_OPTE_IPV4_SUBNET.contains(ip) => {
// TODO check existing_dns_v4_ips, once it exists
}
IpAddr::V6(ip) if NEXUS_OPTE_IPV6_SUBNET.contains(ip) => {
if !existing_nexus_v6_ips.contains(&ip) {
bail!(
"planning input contains unexpected NIC \
(IP not found in parent blueprint): {nic_entry:?}"
);
}
}
IpAddr::V6(ip) if NTP_OPTE_IPV6_SUBNET.contains(ip) => {
// TODO check existing_ntp_v6_ips, once it exists
}
IpAddr::V6(ip) if DNS_OPTE_IPV6_SUBNET.contains(ip) => {
// TODO check existing_dns_v6_ips, once it exists
}
_ => {
bail!(
"planning input contains unexpected NIC \
(IP not contained in known OPTE subnet): {nic_entry:?}"
)
}
}
}

// TODO-performance Building these iterators as "walk through the list
// and skip anything we've used already" is fine as long as we're
// talking about a small number of resources (e.g., single-digit number
Expand Down
39 changes: 39 additions & 0 deletions nexus/reconfigurator/planning/src/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,16 +504,19 @@ mod test {
use nexus_types::deployment::BlueprintZoneFilter;
use nexus_types::deployment::BlueprintZoneType;
use nexus_types::deployment::DiffSledModified;
use nexus_types::deployment::OmicronZoneNetworkResources;
use nexus_types::external_api::views::SledPolicy;
use nexus_types::external_api::views::SledProvisionPolicy;
use nexus_types::external_api::views::SledState;
use nexus_types::inventory::OmicronZonesFound;
use omicron_common::api::external::Generation;
use omicron_common::disk::DiskIdentity;
use omicron_test_utils::dev::test_setup_log;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::PhysicalDiskUuid;
use omicron_uuid_kinds::ZpoolUuid;
use std::collections::HashMap;
use std::mem;

/// Runs through a basic sequence of blueprints for adding a sled
#[test]
Expand Down Expand Up @@ -723,6 +726,42 @@ mod test {
assert_eq!(collection.omicron_zones.len(), 1);
blueprint.blueprint_zones.retain(|k, _v| keep_sled_id == *k);

// Also remove all the networking resources for the zones we just
// stripped out; i.e., only keep those for `keep_sled_id`.
let mut new_network_resources = OmicronZoneNetworkResources::new();
let old_network_resources = builder.network_resources_mut();
for old_ip in old_network_resources.omicron_zone_external_ips() {
if blueprint.all_omicron_zones(BlueprintZoneFilter::All).any(
|(_, zone)| {
zone.zone_type
.external_networking()
.map(|(ip, _nic)| ip.id() == old_ip.ip.id())
.unwrap_or(false)
},
) {
new_network_resources
.add_external_ip(old_ip.zone_id, old_ip.ip)
.expect("copied IP to new input");
}
}
for old_nic in old_network_resources.omicron_zone_nics() {
if blueprint.all_omicron_zones(BlueprintZoneFilter::All).any(
|(_, zone)| {
zone.zone_type
.external_networking()
.map(|(_ip, nic)| {
nic.id == old_nic.nic.id.into_untyped_uuid()
})
.unwrap_or(false)
},
) {
new_network_resources
.add_nic(old_nic.zone_id, old_nic.nic)
.expect("copied NIC to new input");
}
}
mem::swap(old_network_resources, &mut &mut new_network_resources);

(keep_sled_id, blueprint, collection, builder.build())
};

Expand Down
24 changes: 18 additions & 6 deletions nexus/types/src/deployment/network_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,18 @@ impl OmicronZoneNetworkResources {
}
}

pub fn omicron_zone_external_ips(
&self,
) -> impl Iterator<Item = OmicronZoneExternalIpEntry> + '_ {
self.omicron_zone_external_ips.iter().copied()
}

pub fn omicron_zone_nics(
&self,
) -> impl Iterator<Item = OmicronZoneNicEntry> + '_ {
self.omicron_zone_nics.iter().copied()
}

pub fn add_external_ip(
&mut self,
zone_id: OmicronZoneUuid,
Expand All @@ -79,7 +91,7 @@ impl OmicronZoneNetworkResources {
zone_id: OmicronZoneUuid,
nic: OmicronZoneNic,
) -> Result<(), AddNetworkResourceError> {
let entry = OmicronZoneNicEntry { zone_id, nic: nic.clone() };
let entry = OmicronZoneNicEntry { zone_id, nic };
self.omicron_zone_nics.insert_no_dups(entry).map_err(|err| {
AddNetworkResourceError::DuplicateOmicronZoneNic {
zone_id,
Expand Down Expand Up @@ -221,7 +233,7 @@ pub struct OmicronZoneExternalSnatIp {
///
/// This is a slimmer `nexus_db_model::ServiceNetworkInterface` that only stores
/// the fields necessary for blueprint planning.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
pub struct OmicronZoneNic {
pub id: VnicUuid,
pub mac: MacAddr,
Expand All @@ -233,7 +245,7 @@ pub struct OmicronZoneNic {
/// A pair of an Omicron zone ID and an external IP.
///
/// Part of [`OmicronZoneNetworkResources`].
#[derive(Clone, Debug, Deserialize, Serialize)]
#[derive(Clone, Copy, Debug, Deserialize, Serialize)]
pub struct OmicronZoneExternalIpEntry {
pub zone_id: OmicronZoneUuid,
pub ip: OmicronZoneExternalIp,
Expand Down Expand Up @@ -264,10 +276,10 @@ impl TriMapEntry for OmicronZoneExternalIpEntry {
/// A pair of an Omicron zone ID and a network interface.
///
/// Part of [`OmicronZoneNetworkResources`].
#[derive(Clone, Debug, Deserialize, Serialize)]
#[derive(Clone, Copy, Debug, Deserialize, Serialize)]
pub struct OmicronZoneNicEntry {
zone_id: OmicronZoneUuid,
nic: OmicronZoneNic,
pub zone_id: OmicronZoneUuid,
pub nic: OmicronZoneNic,
}

impl TriMapEntry for OmicronZoneNicEntry {
Expand Down
6 changes: 6 additions & 0 deletions nexus/types/src/deployment/planning_input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,12 @@ impl PlanningInputBuilder {
Ok(self.network_resources.add_nic(zone_id, nic)?)
}

pub fn network_resources_mut(
&mut self,
) -> &mut OmicronZoneNetworkResources {
&mut self.network_resources
}

pub fn policy_mut(&mut self) -> &mut Policy {
&mut self.policy
}
Expand Down
4 changes: 4 additions & 0 deletions nexus/types/src/deployment/tri_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ impl<T: TriMapEntry> TriMap<T> {
}
}

pub(crate) fn iter(&self) -> impl Iterator<Item = &T> {
self.entries.iter()
}

/// Checks general invariants of the map.
///
/// The code below always upholds these invariants, but it's useful to have
Expand Down

0 comments on commit 047e16b

Please sign in to comment.