[blueprints] Add external IP IDs to BlueprintZoneType (#5608)

This converts the `BlueprintZoneType`s for Nexus, External DNS, and
Boundary NTP to hold `OmicronZoneExternalIp`s of various flavors, which
contain the database ID for this IP in addition to the other information
they already held (IP address, SNAT config, etc., as applicable). The
motivating case here is to _delete_ external IPs (by ID) for expunged
zones, which will come in a subsequent PR.

There is some other immediate payoff here. For example,
`reconfigurator-cli` can now drop its goofy `external_ip` ID map;
previously, it had to conjure up IDs for external IPs at various points,
but now it can read them directly from the blueprint. We also get to
remove a couple `TODO-cleanup`s where we were assuming a particular kind
of IP (floating vs snat), where now we know exactly what the kind is.

dev-tools/reconfigurator-cli/src/main.rs

@@ -21,8 +21,6 @@ use nexus_reconfigurator_planning::system::{
     SledBuilder, SledHwInventory, SystemDescription,
 };
 use nexus_types::deployment::BlueprintZoneFilter;
-use nexus_types::deployment::OmicronZoneExternalIp;
-use nexus_types::deployment::OmicronZoneExternalIpKind;
 use nexus_types::deployment::OmicronZoneNic;
 use nexus_types::deployment::PlanningInput;
 use nexus_types::deployment::SledFilter;
@@ -34,13 +32,10 @@ use nexus_types::inventory::SledRole;
 use omicron_common::api::external::Generation;
 use omicron_common::api::external::Name;
 use omicron_uuid_kinds::CollectionUuid;
-use omicron_uuid_kinds::ExternalIpUuid;
 use omicron_uuid_kinds::SledUuid;
 use reedline::{Reedline, Signal};
-use std::cell::RefCell;
 use std::collections::BTreeMap;
 use std::io::BufRead;
-use std::net::IpAddr;
 use swrite::{swriteln, SWrite};
 use tabled::Tabled;
 use uuid::Uuid;
@@ -61,14 +56,6 @@ struct ReconfiguratorSim {
     /// blueprints created by the user
     blueprints: IndexMap<Uuid, Blueprint>,
 
-    /// external IPs allocated to services
-    ///
-    /// In the real system, external IPs have IDs, but those IDs only live in
-    /// CRDB - they're not part of the zone config sent from Reconfigurator to
-    /// sled-agent. This mimics the minimal bit of the CRDB `external_ip` table
-    /// we need.
-    external_ips: RefCell<IndexMap<IpAddr, ExternalIpUuid>>,
-
     /// internal DNS configurations
     internal_dns: BTreeMap<Generation, DnsConfigParams>,
     /// external DNS configurations
@@ -152,17 +139,7 @@ impl ReconfiguratorSim {
         for (_, zone) in
             parent_blueprint.all_omicron_zones(BlueprintZoneFilter::All)
         {
-            if let Some(ip) = zone.zone_type.external_ip() {
-                let external_ip = OmicronZoneExternalIp {
-                    id: *self
-                        .external_ips
-                        .borrow_mut()
-                        .entry(ip)
-                        .or_insert_with(ExternalIpUuid::new_v4),
-                    // TODO-cleanup This is potentially wrong;
-                    // zone_type should tell us the IP kind.
-                    kind: OmicronZoneExternalIpKind::Floating(ip),
-                };
+            if let Some(external_ip) = zone.zone_type.external_ip() {
                 builder
                     .add_omicron_zone_external_ip(zone.id, external_ip)
                     .context("adding omicron zone external IP")?;
@@ -205,7 +182,6 @@ fn main() -> anyhow::Result<()> {
         system: SystemDescription::new(),
         collections: IndexMap::new(),
         blueprints: IndexMap::new(),
-        external_ips: RefCell::new(IndexMap::new()),
         internal_dns: BTreeMap::new(),
         external_dns: BTreeMap::new(),
         log,

nexus/db-model/src/deployment.rs

@@ -15,7 +15,6 @@ use crate::typed_uuid::DbTypedUuid;
 use crate::{
     impl_enum_type, ipv6, Generation, MacAddr, Name, SqlU16, SqlU32, SqlU8,
 };
-use anyhow::Context;
 use chrono::{DateTime, Utc};
 use ipnetwork::IpNetwork;
 use nexus_types::deployment::BlueprintPhysicalDiskConfig;
@@ -27,9 +26,9 @@ use nexus_types::deployment::BlueprintZonesConfig;
 use omicron_common::api::internal::shared::NetworkInterface;
 use omicron_common::disk::DiskIdentity;
 use omicron_uuid_kinds::GenericUuid;
-use omicron_uuid_kinds::SledKind;
 use omicron_uuid_kinds::SledUuid;
 use omicron_uuid_kinds::ZpoolUuid;
+use omicron_uuid_kinds::{ExternalIpKind, SledKind};
 use uuid::Uuid;
 
 /// See [`nexus_types::deployment::Blueprint`].
@@ -223,6 +222,8 @@ pub struct BpOmicronZone {
     pub snat_last_port: Option<SqlU16>,
 
     disposition: DbBpZoneDisposition,
+
+    pub external_ip_id: Option<DbTypedUuid<ExternalIpKind>>,
 }
 
 impl BpOmicronZone {
@@ -231,11 +232,14 @@ impl BpOmicronZone {
         sled_id: SledUuid,
         blueprint_zone: &BlueprintZoneConfig,
     ) -> Result<Self, anyhow::Error> {
+        let external_ip_id =
+            blueprint_zone.zone_type.external_ip().map(|ip| ip.id());
         let zone = OmicronZone::new(
             sled_id,
             blueprint_zone.id.into_untyped_uuid(),
             blueprint_zone.underlay_address,
             &blueprint_zone.zone_type.clone().into(),
+            external_ip_id,
         )?;
         Ok(Self {
             blueprint_id,
@@ -260,6 +264,7 @@ impl BpOmicronZone {
             snat_first_port: zone.snat_first_port,
             snat_last_port: zone.snat_last_port,
             disposition: to_db_bp_zone_disposition(blueprint_zone.disposition),
+            external_ip_id: zone.external_ip_id.map(From::from),
         })
     }
 
@@ -288,14 +293,12 @@ impl BpOmicronZone {
             snat_ip: self.snat_ip,
             snat_first_port: self.snat_first_port,
             snat_last_port: self.snat_last_port,
+            external_ip_id: self.external_ip_id.map(From::from),
         };
-        let config =
-            zone.into_omicron_zone_config(nic_row.map(OmicronZoneNic::from))?;
-        BlueprintZoneConfig::from_omicron_zone_config(
-            config,
+        zone.into_blueprint_zone_config(
             self.disposition.into(),
+            nic_row.map(OmicronZoneNic::from),
         )
-        .context("failed to convert OmicronZoneConfig")
     }
 }
 

nexus/db-model/src/external_ip.rs

@@ -17,8 +17,9 @@ use db_macros::Resource;
 use diesel::Queryable;
 use diesel::Selectable;
 use ipnetwork::IpNetwork;
+use nexus_types::deployment::OmicronZoneExternalFloatingIp;
 use nexus_types::deployment::OmicronZoneExternalIp;
-use nexus_types::deployment::OmicronZoneExternalIpKind;
+use nexus_types::deployment::OmicronZoneExternalSnatIp;
 use nexus_types::external_api::params;
 use nexus_types::external_api::shared;
 use nexus_types::external_api::views;
@@ -145,6 +146,10 @@ pub enum OmicronZoneExternalIpError {
     IpIsForInstance,
     #[error("invalid SNAT configuration")]
     InvalidSnatConfig(#[from] SourceNatConfigError),
+    #[error(
+        "Omicron zone has a range of IPs ({0}); only a single IP is supported"
+    )]
+    NotSingleIp(IpNetwork),
     #[error(
         "database IP is ephemeral; currently unsupported for Omicron zones"
     )]
@@ -158,24 +163,31 @@ impl TryFrom<&'_ ExternalIp> for OmicronZoneExternalIp {
         if !row.is_service {
             return Err(OmicronZoneExternalIpError::IpIsForInstance);
         }
+        let size = match row.ip.size() {
+            ipnetwork::NetworkSize::V4(n) => u128::from(n),
+            ipnetwork::NetworkSize::V6(n) => n,
+        };
+        if size != 1 {
+            return Err(OmicronZoneExternalIpError::NotSingleIp(row.ip));
+        }
 
-        let kind = match row.kind {
-            IpKind::SNat => {
-                OmicronZoneExternalIpKind::Snat(SourceNatConfig::new(
+        match row.kind {
+            IpKind::SNat => Ok(Self::Snat(OmicronZoneExternalSnatIp {
+                id: ExternalIpUuid::from_untyped_uuid(row.id),
+                snat_cfg: SourceNatConfig::new(
                     row.ip.ip(),
                     row.first_port.0,
                     row.last_port.0,
-                )?)
-            }
+                )?,
+            })),
             IpKind::Floating => {
-                OmicronZoneExternalIpKind::Floating(row.ip.ip())
+                Ok(Self::Floating(OmicronZoneExternalFloatingIp {
+                    id: ExternalIpUuid::from_untyped_uuid(row.id),
+                    ip: row.ip.ip(),
+                }))
             }
-            IpKind::Ephemeral => {
-                return Err(OmicronZoneExternalIpError::EphemeralIp)
-            }
-        };
-
-        Ok(Self { id: ExternalIpUuid::from_untyped_uuid(row.id), kind })
+            IpKind::Ephemeral => Err(OmicronZoneExternalIpError::EphemeralIp),
+        }
     }
 }
 
@@ -355,10 +367,8 @@ impl IncompleteExternalIp {
         zone_id: OmicronZoneUuid,
         zone_kind: ZoneKind,
     ) -> Self {
-        let (kind, ip, port_range, name, description, state) = match external_ip
-            .kind
-        {
-            OmicronZoneExternalIpKind::Floating(ip) => {
+        let (kind, port_range, name, description, state) = match external_ip {
+            OmicronZoneExternalIp::Floating(_) => {
                 // We'll name this external IP the same as we'll name the NIC
                 // associated with this zone.
                 let name = ServiceNetworkInterface::name(zone_id, zone_kind);
@@ -371,19 +381,20 @@ impl IncompleteExternalIp {
 
                 (
                     IpKind::Floating,
-                    ip,
                     None,
                     Some(name),
                     Some(zone_kind.to_string()),
                     state,
                 )
             }
-            OmicronZoneExternalIpKind::Snat(snat_cfg) => {
+            OmicronZoneExternalIp::Snat(OmicronZoneExternalSnatIp {
+                snat_cfg,
+                ..
+            }) => {
                 let (first_port, last_port) = snat_cfg.port_range_raw();
                 let kind = IpKind::SNat;
                 (
                     kind,
-                    snat_cfg.ip,
                     Some((first_port.into(), last_port.into())),
                     // Only floating IPs are allowed to have names and
                     // descriptions.
@@ -395,7 +406,7 @@ impl IncompleteExternalIp {
         };
 
         Self {
-            id: external_ip.id.into_untyped_uuid(),
+            id: external_ip.id().into_untyped_uuid(),
             name,
             description,
             time_created: Utc::now(),
@@ -405,7 +416,7 @@ impl IncompleteExternalIp {
             parent_id: Some(zone_id.into_untyped_uuid()),
             pool_id,
             project_id: None,
-            explicit_ip: Some(IpNetwork::from(ip)),
+            explicit_ip: Some(IpNetwork::from(external_ip.ip())),
             explicit_port_range: port_range,
             state,
         }

nexus/db-model/src/inventory.rs

@@ -882,11 +882,14 @@ impl InvOmicronZone {
         sled_id: SledUuid,
         zone: &nexus_types::inventory::OmicronZoneConfig,
     ) -> Result<InvOmicronZone, anyhow::Error> {
+        // Inventory zones do not know the external IP ID.
+        let external_ip_id = None;
         let zone = OmicronZone::new(
             sled_id,
             zone.id,
             zone.underlay_address,
             &zone.zone_type,
+            external_ip_id,
         )?;
         Ok(Self {
             inv_collection_id: inv_collection_id.into(),
@@ -938,6 +941,9 @@ impl InvOmicronZone {
             snat_ip: self.snat_ip,
             snat_first_port: self.snat_first_port,
             snat_last_port: self.snat_last_port,
+            // Inventory zones don't know an external IP ID, and Omicron zone
+            // configs don't need it.
+            external_ip_id: None,
         };
         zone.into_omicron_zone_config(nic_row.map(OmicronZoneNic::from))
     }