BlueprintBuilder: require caller to provide internal DNS subnet (#9250)

This is part of #9238: we'd like to pull `BlueprintResourceAllocator`'s
weird construction out of `BlueprintBuilder`, and this removes _one_ use
of it. When adding an internal DNS zone, the caller must now specify the
`DnsSubet` instead of `BlueprintBuilder` choosing internally.

To assist with this, adds
`BlueprintBuilder::available_internal_dns_subnets()`, which returns an
iterator of available subnets. I'm not sure this is the right interface;
will leave a comment inline with some other ideas.

Author: jgallagher

nexus/reconfigurator/planning/src/blueprint_builder/builder.rs

@@ -11,7 +11,6 @@ use crate::blueprint_editor::EditedSled;
 use crate::blueprint_editor::ExternalNetworkingChoice;
 use crate::blueprint_editor::ExternalNetworkingError;
 use crate::blueprint_editor::ExternalSnatNetworkingChoice;
-use crate::blueprint_editor::NoAvailableDnsSubnets;
 use crate::blueprint_editor::SledEditError;
 use crate::blueprint_editor::SledEditor;
 use crate::mgs_updates::PendingHostPhase2Changes;
@@ -65,6 +64,7 @@ use nexus_types::inventory::Collection;
 use omicron_common::address::CLICKHOUSE_HTTP_PORT;
 use omicron_common::address::DNS_HTTP_PORT;
 use omicron_common::address::DNS_PORT;
+use omicron_common::address::DnsSubnet;
 use omicron_common::address::NTP_PORT;
 use omicron_common::address::ReservedRackSubnet;
 use omicron_common::api::external::Generation;
@@ -137,8 +137,10 @@ pub enum Error {
     },
     #[error("error constructing resource allocator")]
     AllocatorInput(#[from] BlueprintResourceAllocatorInputError),
-    #[error("error allocating internal DNS subnet")]
-    AllocateInternalDnsSubnet(#[from] NoAvailableDnsSubnets),
+    #[error("no commissioned sleds - rack subnet is unknown")]
+    RackSubnetUnknownNoSleds,
+    #[error("no reserved subnets available for internal DNS")]
+    NoAvailableDnsSubnets,
     #[error("error allocating external networking resources")]
     AllocateExternalNetworking(#[from] ExternalNetworkingError),
     #[error("zone is already up-to-date and should not be updated")]
@@ -693,6 +695,40 @@ impl<'a> BlueprintBuilder<'a> {
         self.new_blueprint_id
     }
 
+    pub fn available_internal_dns_subnets(
+        &self,
+    ) -> Result<impl Iterator<Item = DnsSubnet>, Error> {
+        // TODO-multirack We need the rack subnet to know what the reserved
+        // internal DNS subnets are. Pick any sled; this isn't right in
+        // multirack (either DNS will be on a wider subnet or we need to pick a
+        // particular rack subnet here?).
+        let any_sled_subnet = self
+            .input
+            .all_sled_resources(SledFilter::Commissioned)
+            .map(|(_sled_id, resources)| resources.subnet)
+            .next()
+            .ok_or(Error::RackSubnetUnknownNoSleds)?;
+        let rack_subnet = ReservedRackSubnet::from_subnet(any_sled_subnet);
+
+        // Compute the "in use" subnets; this includes all in-service internal
+        // DNS zones _and_ any "expunged but not yet confirmed to be gone"
+        // zones, so we use the somewhat unusual `could_be_running` filter
+        // instead of the more typical `is_in_service`.
+        let internal_dns_subnets_in_use = self
+            .current_zones(BlueprintZoneDisposition::could_be_running)
+            .filter_map(|(_sled_id, zone)| match &zone.zone_type {
+                BlueprintZoneType::InternalDns(internal_dns) => {
+                    Some(DnsSubnet::from_addr(*internal_dns.dns_address.ip()))
+                }
+                _ => None,
+            })
+            .collect::<BTreeSet<_>>();
+
+        Ok(rack_subnet.get_dns_subnets().into_iter().filter(move |subnet| {
+            !internal_dns_subnets_in_use.contains(&subnet)
+        }))
+    }
+
     pub fn planning_input(&self) -> &PlanningInput {
         &self.input
     }
@@ -1381,12 +1417,9 @@ impl<'a> BlueprintBuilder<'a> {
         &mut self,
         sled_id: SledUuid,
         image_source: BlueprintZoneImageSource,
+        dns_subnet: DnsSubnet,
     ) -> Result<(), Error> {
         let gz_address_index = self.next_internal_dns_gz_address_index(sled_id);
-        let sled_subnet = self.sled_resources(sled_id)?.subnet;
-        let rack_subnet = ReservedRackSubnet::from_subnet(sled_subnet);
-        let dns_subnet =
-            self.resource_allocator()?.next_internal_dns_subnet(rack_subnet)?;
         let address = dns_subnet.dns_address();
         let zpool = self.sled_select_zpool(sled_id, ZoneKind::InternalDns)?;
         let zone_type =

nexus/reconfigurator/planning/src/blueprint_editor.rs

@@ -11,7 +11,6 @@ mod sled_editor;
 
 pub use allocators::BlueprintResourceAllocatorInputError;
 pub use allocators::ExternalNetworkingError;
-pub use allocators::NoAvailableDnsSubnets;
 pub use sled_editor::DatasetsEditError;
 pub use sled_editor::DisksEditError;
 pub use sled_editor::MultipleDatasetsOfKind;

nexus/reconfigurator/planning/src/blueprint_editor/allocators.rs

@@ -8,23 +8,16 @@ use std::net::IpAddr;
 
 use super::SledEditor;
 use nexus_types::deployment::BlueprintZoneDisposition;
-use nexus_types::deployment::BlueprintZoneType;
-use nexus_types::deployment::blueprint_zone_type::InternalDns;
-use omicron_common::address::DnsSubnet;
 use omicron_common::address::IpRange;
-use omicron_common::address::ReservedRackSubnet;
 
 mod external_networking;
-mod internal_dns;
 
 pub use self::external_networking::ExternalNetworkingError;
-pub use self::internal_dns::NoAvailableDnsSubnets;
 
 pub(crate) use self::external_networking::ExternalNetworkingChoice;
 pub(crate) use self::external_networking::ExternalSnatNetworkingChoice;
 
 use self::external_networking::ExternalNetworkingAllocator;
-use self::internal_dns::InternalDnsSubnetAllocator;
 
 #[derive(Debug, thiserror::Error)]
 pub enum BlueprintResourceAllocatorInputError {
@@ -35,7 +28,6 @@ pub enum BlueprintResourceAllocatorInputError {
 #[derive(Debug)]
 pub(crate) struct BlueprintResourceAllocator {
     external_networking: ExternalNetworkingAllocator,
-    internal_dns: InternalDnsSubnetAllocator,
 }
 
 impl BlueprintResourceAllocator {
@@ -46,26 +38,6 @@ impl BlueprintResourceAllocator {
     where
         I: Iterator<Item = &'a SledEditor> + Clone,
     {
-        let internal_dns_subnets_in_use = all_sleds
-            .clone()
-            .flat_map(|editor| {
-                editor
-                    // We use `could_be_running` here instead of `in_service` to
-                    // avoid reusing an internal DNS subnet from an
-                    // expunged-but-possibly-still-running zone.
-                    .zones(BlueprintZoneDisposition::could_be_running)
-                    .filter_map(|z| match z.zone_type {
-                        BlueprintZoneType::InternalDns(InternalDns {
-                            dns_address,
-                            ..
-                        }) => Some(DnsSubnet::from_addr(*dns_address.ip())),
-                        _ => None,
-                    })
-            })
-            .collect();
-        let internal_dns =
-            InternalDnsSubnetAllocator::new(internal_dns_subnets_in_use);
-
         let external_networking = ExternalNetworkingAllocator::new(
             all_sleds.clone().flat_map(|editor| {
                 editor.zones(BlueprintZoneDisposition::is_in_service)
@@ -77,14 +49,7 @@ impl BlueprintResourceAllocator {
         )
         .map_err(BlueprintResourceAllocatorInputError::ExternalNetworking)?;
 
-        Ok(Self { external_networking, internal_dns })
-    }
-
-    pub fn next_internal_dns_subnet(
-        &mut self,
-        rack_subnet: ReservedRackSubnet,
-    ) -> Result<DnsSubnet, NoAvailableDnsSubnets> {
-        self.internal_dns.alloc(rack_subnet)
+        Ok(Self { external_networking })
     }
 
     pub(crate) fn next_external_ip_nexus(

nexus/reconfigurator/planning/src/blueprint_editor/allocators/internal_dns.rs

@@ -598,6 +598,8 @@ impl ExampleSystemBuilder {
                             )
                             .unwrap();
                     }
+                    let mut internal_dns_subnets =
+                        builder.available_internal_dns_subnets().unwrap();
                     for _ in 0..self
                         .internal_dns_count
                         .on(discretionary_ix, discretionary_sled_count)
@@ -610,6 +612,9 @@ impl ExampleSystemBuilder {
                                     .expect(
                                         "obtained InternalDNS image source",
                                     ),
+                                internal_dns_subnets.next().expect(
+                                    "sufficient available internal DNS subnets",
+                                ),
                             )
                             .unwrap();
                     }