Add Blippy::check_against_planning_input()

Author: jgallagher

nexus/reconfigurator/blippy/src/blippy.rs

@@ -10,6 +10,9 @@ use nexus_types::deployment::Blueprint;
 use nexus_types::deployment::BlueprintArtifactVersion;
 use nexus_types::deployment::BlueprintDatasetConfig;
 use nexus_types::deployment::BlueprintZoneConfig;
+use nexus_types::deployment::OmicronZoneExternalIp;
+use nexus_types::deployment::OmicronZoneNicEntry;
+use nexus_types::deployment::PlanningInput;
 use nexus_types::inventory::ZpoolName;
 use omicron_common::address::DnsSubnet;
 use omicron_common::address::Ipv6Subnet;
@@ -55,48 +58,55 @@ impl fmt::Display for Severity {
 pub enum Kind {
     Blueprint(BlueprintKind),
     Sled { sled_id: SledUuid, kind: Box<SledKind> },
+    PlanningInput(PlanningInputKind),
 }
 
 impl Kind {
     pub fn display_component(&self) -> impl fmt::Display + '_ {
         enum Component<'a> {
             Blueprint,
             Sled(&'a SledUuid),
+            PlanningInput,
         }
 
         impl fmt::Display for Component<'_> {
             fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
                 match self {
                     Component::Blueprint => write!(f, "blueprint"),
                     Component::Sled(id) => write!(f, "sled {id}"),
+                    Component::PlanningInput => write!(f, "planning input"),
                 }
             }
         }
 
         match self {
-            Kind::Blueprint { .. } => Component::Blueprint,
+            Kind::Blueprint(_) => Component::Blueprint,
             Kind::Sled { sled_id, .. } => Component::Sled(sled_id),
+            Kind::PlanningInput(_) => Component::PlanningInput,
         }
     }
 
     pub fn display_subkind(&self) -> impl fmt::Display + '_ {
         enum Subkind<'a> {
             Blueprint(&'a BlueprintKind),
             Sled(&'a SledKind),
+            PlanningInput(&'a PlanningInputKind),
         }
 
         impl fmt::Display for Subkind<'_> {
             fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
                 match self {
                     Subkind::Blueprint(kind) => write!(f, "{kind}"),
                     Subkind::Sled(kind) => write!(f, "{kind}"),
+                    Subkind::PlanningInput(kind) => write!(f, "{kind}"),
                 }
             }
         }
 
         match self {
             Kind::Blueprint(kind) => Subkind::Blueprint(kind),
             Kind::Sled { kind, .. } => Subkind::Sled(kind),
+            Kind::PlanningInput(kind) => Subkind::PlanningInput(kind),
         }
     }
 }
@@ -480,6 +490,54 @@ impl fmt::Display for SledKind {
     }
 }
 
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub enum PlanningInputKind {
+    IpNotInBlueprint(OmicronZoneExternalIp),
+    NicMacNotInBluperint(OmicronZoneNicEntry),
+    NicIpNotInBlueprint(OmicronZoneNicEntry),
+    NicWithUnknownOpteSubnet(OmicronZoneNicEntry),
+}
+
+impl fmt::Display for PlanningInputKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            PlanningInputKind::IpNotInBlueprint(ip) => {
+                write!(
+                    f,
+                    "planning input contains an external IP \
+                     not described by the blueprint: {} ({})",
+                    ip.ip(),
+                    ip.id()
+                )
+            }
+            PlanningInputKind::NicMacNotInBluperint(nic) => {
+                write!(
+                    f,
+                    "planning input contains a NIC with a MAC address \
+                     not described by the blueprint: {} (NIC {} in zone {})",
+                    nic.nic.mac, nic.nic.id, nic.zone_id,
+                )
+            }
+            PlanningInputKind::NicIpNotInBlueprint(nic) => {
+                write!(
+                    f,
+                    "planning input contains a NIC with an IP address \
+                     not described by the blueprint: {} (NIC {} in zone {})",
+                    nic.nic.ip, nic.nic.id, nic.zone_id,
+                )
+            }
+            PlanningInputKind::NicWithUnknownOpteSubnet(nic) => {
+                write!(
+                    f,
+                    "planning input contains a NIC with an IP not in a known
+                     OPTE subnet: {} (NIC {} in zone {})",
+                    nic.nic.ip, nic.nic.id, nic.zone_id,
+                )
+            }
+        }
+    }
+}
+
 impl Note {
     pub fn display(&self, sort_key: BlippyReportSortKey) -> NoteDisplay<'_> {
         NoteDisplay { note: self, sort_key }
@@ -530,6 +588,14 @@ impl<'a> Blippy<'a> {
         slf
     }
 
+    pub fn check_against_planning_input(
+        mut self,
+        input: &PlanningInput,
+    ) -> Self {
+        checks::perform_planning_input_checks(&mut self, input);
+        self
+    }
+
     pub fn blueprint(&self) -> &'a Blueprint {
         self.blueprint
     }
@@ -554,6 +620,14 @@ impl<'a> Blippy<'a> {
         });
     }
 
+    pub(crate) fn push_planning_input_note(
+        &mut self,
+        severity: Severity,
+        kind: PlanningInputKind,
+    ) {
+        self.notes.push(Note { severity, kind: Kind::PlanningInput(kind) });
+    }
+
     pub fn into_report(
         self,
         sort_key: BlippyReportSortKey,

nexus/reconfigurator/blippy/src/checks.rs

@@ -4,6 +4,7 @@
 
 use crate::blippy::Blippy;
 use crate::blippy::BlueprintKind;
+use crate::blippy::PlanningInputKind;
 use crate::blippy::Severity;
 use crate::blippy::SledKind;
 use nexus_sled_agent_shared::inventory::ZoneKind;
@@ -17,6 +18,7 @@ use nexus_types::deployment::BlueprintZoneDisposition;
 use nexus_types::deployment::BlueprintZoneImageSource;
 use nexus_types::deployment::BlueprintZoneType;
 use nexus_types::deployment::OmicronZoneExternalIp;
+use nexus_types::deployment::PlanningInput;
 use nexus_types::deployment::SledFilter;
 use nexus_types::deployment::blueprint_zone_type;
 use omicron_common::address::DnsSubnet;
@@ -31,8 +33,16 @@ use omicron_uuid_kinds::ZpoolUuid;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::collections::btree_map::Entry;
+use std::net::IpAddr;
 use std::net::Ipv6Addr;
 
+pub(crate) fn perform_planning_input_checks(
+    blippy: &mut Blippy<'_>,
+    input: &PlanningInput,
+) {
+    check_planning_input_network_records_appear_in_blueprint(blippy, input);
+}
+
 pub(crate) fn perform_all_blueprint_only_checks(blippy: &mut Blippy<'_>) {
     check_underlay_ips(blippy);
     check_external_networking(blippy);
@@ -2037,3 +2047,163 @@ mod tests {
         logctx.cleanup_successful();
     }
 }
+
+// For a given `PlanningInput` / `Blueprint` pair that could be passed to the
+// planner, there should never be any external networking resources in the
+// planning input (which is derived from the contents of CRDB) that we don't
+// know about from the parent blueprint. It's possible a given planning
+// iteration could see such a state if there have been intermediate changes made
+// by other Nexus instances; e.g.,
+//
+// 1. Nexus A generates a `PlanningInput` by reading from CRDB
+// 2. Nexus B executes on a target blueprint that removes IPs/NICs from
+//    CRDB
+// 3. Nexus B regenerates a new blueprint and prunes the zone(s) associated
+//    with the IPs/NICs from step 2
+// 4. Nexus B makes this new blueprint the target
+// 5. Nexus A attempts to run planning with its `PlanningInput` from step 1 but
+//    the target blueprint from step 4; this will fail the following checks
+//    because the input contains records that were removed in step 3
+//
+// We do not need to handle this class of error; it's a transient failure that
+// will clear itself up when Nexus A repeats its planning loop from the top and
+// generates a new `PlanningInput`.
+//
+// There may still be database records corresponding to _expunged_ zones, but
+// that's okay: it just means we haven't yet realized a blueprint where those
+// zones are expunged. And those should should still be in the blueprint (not
+// pruned) until their database records are cleaned up.
+//
+// It's also possible that there may be networking records in the database
+// assigned to zones that have been expunged, and the blueprint uses those same
+// records for new zones. This is also fine and expected, and is a similar case
+// to the previous paragraph: a zone with networking resources was expunged, the
+// database doesn't realize it yet, but can still move forward and make planning
+// decisions that reuse those resources for new zones.
+fn check_planning_input_network_records_appear_in_blueprint(
+    blippy: &mut Blippy<'_>,
+    input: &PlanningInput,
+) {
+    use nexus_types::deployment::OmicronZoneExternalIp;
+    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::MacAddr;
+
+    let mut all_macs: BTreeSet<MacAddr> = BTreeSet::new();
+    let mut all_nexus_nic_ips: BTreeSet<IpAddr> = BTreeSet::new();
+    let mut all_boundary_ntp_nic_ips: BTreeSet<IpAddr> = BTreeSet::new();
+    let mut all_external_dns_nic_ips: BTreeSet<IpAddr> = BTreeSet::new();
+    let mut all_external_ips: BTreeSet<OmicronZoneExternalIp> = BTreeSet::new();
+
+    // Unlike the construction of the external IP allocator and existing IPs
+    // constructed above in `BuilderExternalNetworking::new()`, we do not
+    // check for duplicates here: we could very well see reuse of IPs
+    // between expunged zones or between expunged -> running zones.
+    for (_, z) in
+        blippy.blueprint().all_omicron_zones(BlueprintZoneDisposition::any)
+    {
+        let zone_type = &z.zone_type;
+        match zone_type {
+            BlueprintZoneType::BoundaryNtp(ntp) => {
+                all_boundary_ntp_nic_ips.insert(ntp.nic.ip);
+            }
+            BlueprintZoneType::Nexus(nexus) => {
+                all_nexus_nic_ips.insert(nexus.nic.ip);
+            }
+            BlueprintZoneType::ExternalDns(dns) => {
+                all_external_dns_nic_ips.insert(dns.nic.ip);
+            }
+            _ => (),
+        }
+
+        if let Some((external_ip, nic)) = zone_type.external_networking() {
+            // Ignore localhost (used by the test suite).
+            if !external_ip.ip().is_loopback() {
+                all_external_ips.insert(external_ip);
+            }
+            all_macs.insert(nic.mac);
+        }
+    }
+    for external_ip_entry in
+        input.network_resources().omicron_zone_external_ips()
+    {
+        // As above, ignore localhost (used by the test suite).
+        if external_ip_entry.ip.ip().is_loopback() {
+            continue;
+        }
+        if !all_external_ips.contains(&external_ip_entry.ip) {
+            blippy.push_planning_input_note(
+                Severity::Fatal,
+                PlanningInputKind::IpNotInBlueprint(external_ip_entry.ip),
+            );
+        }
+    }
+    for nic_entry in input.network_resources().omicron_zone_nics() {
+        if !all_macs.contains(&nic_entry.nic.mac) {
+            blippy.push_planning_input_note(
+                Severity::Fatal,
+                PlanningInputKind::NicMacNotInBluperint(nic_entry),
+            );
+        }
+        match nic_entry.nic.ip {
+            IpAddr::V4(ip) if NEXUS_OPTE_IPV4_SUBNET.contains(ip) => {
+                if !all_nexus_nic_ips.contains(&ip.into()) {
+                    blippy.push_planning_input_note(
+                        Severity::Fatal,
+                        PlanningInputKind::NicIpNotInBlueprint(nic_entry),
+                    );
+                }
+            }
+            IpAddr::V4(ip) if NTP_OPTE_IPV4_SUBNET.contains(ip) => {
+                if !all_boundary_ntp_nic_ips.contains(&ip.into()) {
+                    blippy.push_planning_input_note(
+                        Severity::Fatal,
+                        PlanningInputKind::NicIpNotInBlueprint(nic_entry),
+                    );
+                }
+            }
+            IpAddr::V4(ip) if DNS_OPTE_IPV4_SUBNET.contains(ip) => {
+                if !all_external_dns_nic_ips.contains(&ip.into()) {
+                    blippy.push_planning_input_note(
+                        Severity::Fatal,
+                        PlanningInputKind::NicIpNotInBlueprint(nic_entry),
+                    );
+                }
+            }
+            IpAddr::V6(ip) if NEXUS_OPTE_IPV6_SUBNET.contains(ip) => {
+                if !all_nexus_nic_ips.contains(&ip.into()) {
+                    blippy.push_planning_input_note(
+                        Severity::Fatal,
+                        PlanningInputKind::NicIpNotInBlueprint(nic_entry),
+                    );
+                }
+            }
+            IpAddr::V6(ip) if NTP_OPTE_IPV6_SUBNET.contains(ip) => {
+                if !all_boundary_ntp_nic_ips.contains(&ip.into()) {
+                    blippy.push_planning_input_note(
+                        Severity::Fatal,
+                        PlanningInputKind::NicIpNotInBlueprint(nic_entry),
+                    );
+                }
+            }
+            IpAddr::V6(ip) if DNS_OPTE_IPV6_SUBNET.contains(ip) => {
+                if !all_external_dns_nic_ips.contains(&ip.into()) {
+                    blippy.push_planning_input_note(
+                        Severity::Fatal,
+                        PlanningInputKind::NicIpNotInBlueprint(nic_entry),
+                    );
+                }
+            }
+            _ => {
+                blippy.push_planning_input_note(
+                    Severity::Fatal,
+                    PlanningInputKind::NicWithUnknownOpteSubnet(nic_entry),
+                );
+            }
+        }
+    }
+}

nexus/types/src/deployment/network_resources.rs

@@ -292,7 +292,9 @@ pub struct OmicronZoneExternalSnatIp {
 ///
 /// This is a slimmer `nexus_db_model::ServiceNetworkInterface` that only stores
 /// the fields necessary for blueprint planning.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(
+    Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize,
+)]
 pub struct OmicronZoneNic {
     pub id: VnicUuid,
     pub mac: MacAddr,
@@ -337,7 +339,9 @@ impl TriHashItem for OmicronZoneExternalIpEntry {
 /// A pair of an Omicron zone ID and a network interface.
 ///
 /// Part of [`OmicronZoneNetworkResources`].
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)]
+#[derive(
+    Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize,
+)]
 pub struct OmicronZoneNicEntry {
     pub zone_id: OmicronZoneUuid,
     pub nic: OmicronZoneNic,