update external DNS during blueprint execution (#5212)

clients/dns-service-client/src/diff.rs

@@ -59,8 +59,13 @@ impl<'a> DnsDiff<'a> {
         &self,
     ) -> impl Iterator<Item = (&str, &[DnsRecord], &[DnsRecord])> {
         self.left.iter().filter_map(|(k, v1)| match self.right.get(k) {
-            Some(v2) if v1 != v2 => {
-                Some((k.as_ref(), v1.as_ref(), v2.as_ref()))
+            Some(v2) => {
+                let mut v1_sorted = v1.clone();
+                let mut v2_sorted = v2.clone();
+                v1_sorted.sort();
+                v2_sorted.sort();
+                (v1_sorted != v2_sorted)
+                    .then(|| (k.as_ref(), v1.as_ref(), v2.as_ref()))
             }
             _ => None,
         })

clients/dns-service-client/src/lib.rs

@@ -109,3 +109,42 @@ impl types::DnsConfigParams {
         Ok(&self.zones[0])
     }
 }
+
+impl Ord for types::DnsRecord {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        use types::DnsRecord;
+        match (self, other) {
+            // Same kinds: compare the items in them
+            (DnsRecord::A(addr1), DnsRecord::A(addr2)) => addr1.cmp(addr2),
+            (DnsRecord::Aaaa(addr1), DnsRecord::Aaaa(addr2)) => {
+                addr1.cmp(addr2)
+            }
+            (DnsRecord::Srv(srv1), DnsRecord::Srv(srv2)) => srv1
+                .target
+                .cmp(&srv2.target)
+                .then_with(|| srv1.port.cmp(&srv2.port)),
+
+            // Different kinds: define an arbitrary order among the kinds.
+            // We could use std::mem::discriminant() here but it'd be nice if
+            // this were stable over time.
+            // We define (arbitrarily): A < Aaaa < Srv
+            (DnsRecord::A(_), DnsRecord::Aaaa(_) | DnsRecord::Srv(_)) => {
+                std::cmp::Ordering::Less
+            }
+            (DnsRecord::Aaaa(_), DnsRecord::Srv(_)) => std::cmp::Ordering::Less,
+
+            // Anything else will result in "Greater".  But let's be explicit.
+            (DnsRecord::Aaaa(_), DnsRecord::A(_))
+            | (DnsRecord::Srv(_), DnsRecord::A(_))
+            | (DnsRecord::Srv(_), DnsRecord::Aaaa(_)) => {
+                std::cmp::Ordering::Greater
+            }
+        }
+    }
+}
+
+impl PartialOrd for types::DnsRecord {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}

dev-tools/omdb/src/bin/omdb/nexus.rs

@@ -1073,7 +1073,10 @@ async fn cmd_nexus_blueprints_generate_from_collection(
         )
         .await
         .context("creating blueprint from collection id")?;
-    eprintln!("created blueprint {} from collection id", blueprint.id);
+    eprintln!(
+        "created blueprint {} from collection id {}",
+        blueprint.id, args.collection_id
+    );
     Ok(())
 }
 

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

@@ -454,6 +454,7 @@ fn cmd_blueprint_from_inventory(
     let blueprint = BlueprintBuilder::build_initial_from_collection(
         collection,
         dns_version,
+        dns_version,
         &policy,
         creator,
     )
@@ -487,6 +488,7 @@ fn cmd_blueprint_plan(
         sim.log.clone(),
         parent_blueprint,
         dns_version,
+        dns_version,
         &policy,
         creator,
         collection,

dev-tools/reconfigurator-cli/tests/input/complex.json

@@ -7973,6 +7973,7 @@
       ],
       "parent_blueprint_id": null,
       "internal_dns_version": 1,
+      "external_dns_version": 1,
       "time_created": "2024-03-12T19:09:41.953854Z",
       "creator": "0d459323-e414-4f32-9944-76c7331da622",
       "comment": "from collection 3f61b723-cfe7-40ec-b22c-e3e1f35325f9"
@@ -8645,9 +8646,10 @@
       ],
       "parent_blueprint_id": null,
       "internal_dns_version": 1,
+      "external_dns_version": 1,
       "time_created": "2024-03-12T19:11:49.876621Z",
       "creator": "0d459323-e414-4f32-9944-76c7331da622",
       "comment": "from collection 5e1d6ae0-c5b4-4884-ac41-680cd4f5762d"
     }
   ]
-}
+}

nexus/db-model/src/deployment.rs

@@ -25,6 +25,7 @@ pub struct Blueprint {
     pub id: Uuid,
     pub parent_blueprint_id: Option<Uuid>,
     pub internal_dns_version: Generation,
+    pub external_dns_version: Generation,
     pub time_created: DateTime<Utc>,
     pub creator: String,
     pub comment: String,
@@ -36,6 +37,7 @@ impl From<&'_ nexus_types::deployment::Blueprint> for Blueprint {
             id: bp.id,
             parent_blueprint_id: bp.parent_blueprint_id,
             internal_dns_version: Generation(bp.internal_dns_version),
+            external_dns_version: Generation(bp.external_dns_version),
             time_created: bp.time_created,
             creator: bp.creator.clone(),
             comment: bp.comment.clone(),
@@ -49,6 +51,7 @@ impl From<Blueprint> for nexus_types::deployment::BlueprintMetadata {
             id: value.id,
             parent_blueprint_id: value.parent_blueprint_id,
             internal_dns_version: *value.internal_dns_version,
+            external_dns_version: *value.external_dns_version,
             time_created: value.time_created,
             creator: value.creator,
             comment: value.comment,

nexus/db-model/src/schema.rs

@@ -13,7 +13,7 @@ use omicron_common::api::external::SemverVersion;
 ///
 /// This should be updated whenever the schema is changed. For more details,
 /// refer to: schema/crdb/README.adoc
-pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(41, 0, 0);
+pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(42, 0, 0);
 
 table! {
     disk (id) {
@@ -1441,6 +1441,7 @@ table! {
         comment -> Text,
 
         internal_dns_version -> Int8,
+        external_dns_version -> Int8,
     }
 }
 

nexus/db-queries/src/db/datastore/deployment.rs

@@ -229,6 +229,7 @@ impl DataStore {
         let (
             parent_blueprint_id,
             internal_dns_version,
+            external_dns_version,
             time_created,
             creator,
             comment,
@@ -251,6 +252,7 @@ impl DataStore {
             (
                 blueprint.parent_blueprint_id,
                 *blueprint.internal_dns_version,
+                *blueprint.external_dns_version,
                 blueprint.time_created,
                 blueprint.creator,
                 blueprint.comment,
@@ -480,6 +482,7 @@ impl DataStore {
             zones_in_service,
             parent_blueprint_id,
             internal_dns_version,
+            external_dns_version,
             time_created,
             creator,
             comment,
@@ -1271,6 +1274,7 @@ mod tests {
         let blueprint = BlueprintBuilder::build_initial_from_collection(
             &collection,
             Generation::new(),
+            Generation::new(),
             &policy,
             "test",
         )
@@ -1305,6 +1309,7 @@ mod tests {
         let blueprint1 = BlueprintBuilder::build_initial_from_collection(
             &collection,
             Generation::new(),
+            Generation::new(),
             &EMPTY_POLICY,
             "test",
         )
@@ -1432,11 +1437,13 @@ mod tests {
 
         // Create a builder for a child blueprint.  While we're at it, use a
         // different DNS version to test that that works.
-        let new_dns_version = blueprint1.internal_dns_version.next();
+        let new_internal_dns_version = blueprint1.internal_dns_version.next();
+        let new_external_dns_version = new_internal_dns_version.next();
         let mut builder = BlueprintBuilder::new_based_on(
             &logctx.log,
             &blueprint1,
-            new_dns_version,
+            new_internal_dns_version,
+            new_external_dns_version,
             &policy,
             "test",
         )
@@ -1488,7 +1495,8 @@ mod tests {
             .expect("failed to read collection back");
         println!("diff: {}", blueprint2.diff_sleds(&blueprint_read).display());
         assert_eq!(blueprint2, blueprint_read);
-        assert_eq!(blueprint2.internal_dns_version, new_dns_version);
+        assert_eq!(blueprint2.internal_dns_version, new_internal_dns_version);
+        assert_eq!(blueprint2.external_dns_version, new_external_dns_version);
         {
             let mut expected_ids = [blueprint1.id, blueprint2.id];
             expected_ids.sort();
@@ -1581,6 +1589,7 @@ mod tests {
         let blueprint1 = BlueprintBuilder::build_initial_from_collection(
             &collection,
             Generation::new(),
+            Generation::new(),
             &EMPTY_POLICY,
             "test1",
         )
@@ -1589,6 +1598,7 @@ mod tests {
             &logctx.log,
             &blueprint1,
             Generation::new(),
+            Generation::new(),
             &EMPTY_POLICY,
             "test2",
         )
@@ -1598,6 +1608,7 @@ mod tests {
             &logctx.log,
             &blueprint1,
             Generation::new(),
+            Generation::new(),
             &EMPTY_POLICY,
             "test3",
         )
@@ -1695,6 +1706,7 @@ mod tests {
             &logctx.log,
             &blueprint3,
             Generation::new(),
+            Generation::new(),
             &EMPTY_POLICY,
             "test3",
         )
@@ -1734,6 +1746,7 @@ mod tests {
         let blueprint1 = BlueprintBuilder::build_initial_from_collection(
             &collection,
             Generation::new(),
+            Generation::new(),
             &EMPTY_POLICY,
             "test1",
         )
@@ -1742,6 +1755,7 @@ mod tests {
             &logctx.log,
             &blueprint1,
             Generation::new(),
+            Generation::new(),
             &EMPTY_POLICY,
             "test2",
         )

nexus/db-queries/src/db/datastore/rack.rs

@@ -27,7 +27,6 @@ use crate::db::model::Rack;
 use crate::db::model::Zpool;
 use crate::db::pagination::paginated;
 use crate::db::pool::DbConnection;
-use crate::transaction_retry::OptionalError;
 use async_bb8_diesel::AsyncConnection;
 use async_bb8_diesel::AsyncRunQueryDsl;
 use chrono::Utc;
@@ -44,6 +43,8 @@ use nexus_db_model::PasswordHashString;
 use nexus_db_model::SiloUser;
 use nexus_db_model::SiloUserPasswordHash;
 use nexus_db_model::SledUnderlaySubnetAllocation;
+use nexus_types::deployment::Blueprint;
+use nexus_types::deployment::OmicronZoneType;
 use nexus_types::external_api::params as external_params;
 use nexus_types::external_api::shared;
 use nexus_types::external_api::shared::IdentityType;
@@ -54,6 +55,7 @@ use nexus_types::internal_api::params as internal_params;
 use omicron_common::api::external::DataPageParams;
 use omicron_common::api::external::Error;
 use omicron_common::api::external::IdentityMetadataCreateParams;
+use omicron_common::api::external::InternalContext;
 use omicron_common::api::external::ListResultVec;
 use omicron_common::api::external::LookupType;
 use omicron_common::api::external::ResourceType;
@@ -793,60 +795,49 @@ impl DataStore {
     pub async fn nexus_external_addresses(
         &self,
         opctx: &OpContext,
+        blueprint: Option<&Blueprint>,
     ) -> Result<(Vec<IpAddr>, Vec<DnsZone>), Error> {
         opctx.authorize(authz::Action::Read, &authz::DNS_CONFIG).await?;
 
-        use crate::db::schema::external_ip::dsl as extip_dsl;
-        use crate::db::schema::service::dsl as service_dsl;
-
-        let err = OptionalError::new();
-        let conn = self.pool_connection_authorized(opctx).await?;
-        self.transaction_retry_wrapper("nexus_external_addresses")
-            .transaction(&conn, |conn| {
-                let err = err.clone();
-                async move {
-                    let ips = extip_dsl::external_ip
-                        .inner_join(
-                            service_dsl::service.on(service_dsl::id
-                                .eq(extip_dsl::parent_id.assume_not_null())),
-                        )
-                        .filter(extip_dsl::parent_id.is_not_null())
-                        .filter(extip_dsl::time_deleted.is_null())
-                        .filter(extip_dsl::is_service)
-                        .filter(
-                            service_dsl::kind.eq(db::model::ServiceKind::Nexus),
-                        )
-                        .select(ExternalIp::as_select())
-                        .get_results_async(&conn)
-                        .await?
-                        .into_iter()
-                        .map(|external_ip| external_ip.ip.ip())
-                        .collect();
-
-                    let dns_zones = self
-                        .dns_zones_list_all_on_connection(
-                            opctx,
-                            &conn,
-                            DnsGroup::External,
-                        )
-                        .await
-                        .map_err(|e| match e.retryable() {
-                            NotRetryable(not_retryable_err) => {
-                                err.bail(not_retryable_err)
-                            }
-                            Retryable(retryable_err) => retryable_err,
-                        })?;
-
-                    Ok((ips, dns_zones))
-                }
-            })
+        let dns_zones = self
+            .dns_zones_list_all(opctx, DnsGroup::External)
             .await
-            .map_err(|e| {
-                if let Some(err) = err.take() {
-                    return err.into();
-                }
-                public_error_from_diesel(e, ErrorHandler::Server)
-            })
+            .internal_context("listing DNS zones to list external addresses")?;
+
+        let nexus_external_ips = if let Some(blueprint) = blueprint {
+            blueprint
+                .all_omicron_zones()
+                .filter_map(|(_, z)| match z.zone_type {
+                    OmicronZoneType::Nexus { external_ip, .. } => {
+                        Some(external_ip)
+                    }
+                    _ => None,
+                })
+                .collect()
+        } else {
+            use crate::db::schema::external_ip::dsl as extip_dsl;
+            use crate::db::schema::service::dsl as service_dsl;
+
+            let conn = self.pool_connection_authorized(opctx).await?;
+
+            extip_dsl::external_ip
+                .inner_join(service_dsl::service.on(
+                    service_dsl::id.eq(extip_dsl::parent_id.assume_not_null()),
+                ))
+                .filter(extip_dsl::parent_id.is_not_null())
+                .filter(extip_dsl::time_deleted.is_null())
+                .filter(extip_dsl::is_service)
+                .filter(service_dsl::kind.eq(db::model::ServiceKind::Nexus))
+                .select(ExternalIp::as_select())
+                .get_results_async(&*conn)
+                .await
+                .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?
+                .into_iter()
+                .map(|external_ip| external_ip.ip.ip())
+                .collect()
+        };
+
+        Ok((nexus_external_ips, dns_zones))
     }
 }