loop with DNS lookup when trying for SwitchLocation (#7779)

Currently, callers of `map_switch_zone_addrs()` first get the IP for
`ServiceName::Dendrite` from DNS, then loop (forever) trying to
translate that IP into a `SwitchLocation`. Under normal conditions, this
is fine. However, if a sled has been expunged, or a new sled is being
added, it's possible that what is returned in:
```
let switch_zone_addresses = match resolver
            .lookup_all_ipv6(ServiceName::Dendrite)
            .await
```
Will change. If that changes happens after we start looping in
`map_switch_zone_addrs()`, then the loop will go on forever looking for
something that is no longer correct.

To fix this we put the `lookup_all_ipv6` into the loop by using the
function `switch_zone_address_mappings()` instead.
`switch_zone_address_mappings()`'s loop includes the call to lookup
addresses in DNS will call `map_switch_zone_addrs()`. This allows us to
include the DNS lookup inside the loop.

Most places where we called `map_switch_zone_addrs()` were also using
the same `lookup_all_ipv6()` call, so transitioning them to call
`switch_zone_address_mappings()` will just drop right in.

A fix for #7739

---------

Co-authored-by: Alan Hanson <alan@oxide.computer>

Author: leftwo

nexus/src/app/background/tasks/bfd.rs

@@ -6,14 +6,14 @@
 //! (BFD) sessions.
 
 use crate::app::{
-    background::tasks::networking::build_mgd_clients, map_switch_zone_addrs,
+    background::tasks::networking::build_mgd_clients,
+    switch_zone_address_mappings,
 };
 
 use crate::app::background::BackgroundTask;
 use futures::FutureExt;
 use futures::future::BoxFuture;
 use internal_dns_resolver::Resolver;
-use internal_dns_types::names::ServiceName;
 use mg_admin_client::types::{BfdPeerConfig, SessionMode};
 use nexus_db_model::{BfdMode, BfdSession};
 use nexus_db_queries::{context::OpContext, db::DataStore};
@@ -117,12 +117,8 @@ impl BackgroundTask for BfdManager {
 
             let mut current: HashSet<BfdSessionKey> = HashSet::new();
 
-            let switch_zone_addresses = match self
-                .resolver
-                .lookup_all_ipv6(ServiceName::Dendrite)
-                .await
-            {
-                Ok(addrs) => addrs,
+            let mappings = match switch_zone_address_mappings(&self.resolver, log).await {
+                Ok(mappings) => mappings,
                 Err(e) => {
                     error!(log, "failed to resolve addresses for Dendrite services"; "error" => %e);
                     return json!({
@@ -131,13 +127,10 @@ impl BackgroundTask for BfdManager {
                                 "failed to resolve addresses for Dendrite services: {:#}",
                                 e
                             )
-                        });
+                    });
                 },
             };
 
-            let mappings =
-                map_switch_zone_addrs(log, switch_zone_addresses).await;
-
             let mgd_clients = build_mgd_clients(mappings, log);
 
             for (location, c) in &mgd_clients {

nexus/src/app/background/tasks/nat_cleanup.rs

@@ -6,15 +6,14 @@
 //! Responsible for cleaning up soft deleted entries once they
 //! have been propagated to running dpd instances.
 
-use crate::app::map_switch_zone_addrs;
+use crate::app::switch_zone_address_mappings;
 
 use super::networking::build_dpd_clients;
 use crate::app::background::BackgroundTask;
 use chrono::{Duration, Utc};
 use futures::FutureExt;
 use futures::future::BoxFuture;
 use internal_dns_resolver::Resolver;
-use internal_dns_types::names::ServiceName;
 use nexus_db_queries::context::OpContext;
 use nexus_db_queries::db::DataStore;
 use serde_json::json;
@@ -65,27 +64,25 @@ impl BackgroundTask for Ipv4NatGarbageCollector {
                 }
             };
 
-            let switch_zone_addresses = match self
-                .resolver
-                .lookup_all_ipv6(ServiceName::Dendrite)
-                .await
+            let mappings = match
+                switch_zone_address_mappings(&self.resolver, log).await
             {
-                Ok(addrs) => addrs,
+                Ok(mappings) => mappings,
                 Err(e) => {
-                    error!(log, "failed to resolve addresses for Dendrite services"; "error" => %e);
+                    error!(
+                        log,
+                        "failed to resolve addresses for Dendrite services"; "error" => %e
+                    );
                     return json!({
                         "error":
                             format!(
                                 "failed to resolve addresses for Dendrite services: {:#}",
                                 e
                             )
-                        });
-                },
+                    });
+                }
             };
 
-            let mappings =
-                map_switch_zone_addrs(log, switch_zone_addresses).await;
-
             let dpd_clients = build_dpd_clients(&mappings, log);
 
             for (_location, client) in dpd_clients {

nexus/src/app/background/tasks/sync_service_zone_nat.rs

@@ -5,15 +5,14 @@
 //! Background task for detecting changes to service zone locations and
 //! updating the NAT rpw table accordingly
 
-use crate::app::map_switch_zone_addrs;
+use crate::app::switch_zone_address_mappings;
 
 use super::networking::build_dpd_clients;
 use crate::app::background::BackgroundTask;
 use anyhow::Context;
 use futures::FutureExt;
 use futures::future::BoxFuture;
 use internal_dns_resolver::Resolver;
-use internal_dns_types::names::ServiceName;
 use nexus_db_model::Ipv4NatValues;
 use nexus_db_queries::context::OpContext;
 use nexus_db_queries::db::DataStore;
@@ -318,12 +317,10 @@ impl BackgroundTask for ServiceZoneNatTracker {
             // notify dpd if we've added any new records
             if result > 0 {
 
-                let switch_zone_addresses = match self
-                    .resolver
-                    .lookup_all_ipv6(ServiceName::Dendrite)
-                    .await
+                let mappings = match
+                    switch_zone_address_mappings(&self.resolver, log).await
                 {
-                    Ok(addrs) => addrs,
+                    Ok(mappings) => mappings,
                     Err(e) => {
                         error!(log, "failed to resolve addresses for Dendrite services"; "error" => %e);
                         return json!({
@@ -332,13 +329,10 @@ impl BackgroundTask for ServiceZoneNatTracker {
                                     "failed to resolve addresses for Dendrite services: {:#}",
                                     e
                                 )
-                            });
+                        });
                     },
                 };
 
-                let mappings =
-                    map_switch_zone_addrs(log, switch_zone_addresses).await;
-
                 let dpd_clients = build_dpd_clients(&mappings, log);
 
                 for (_location, client) in dpd_clients {

nexus/src/app/background/tasks/sync_switch_configuration.rs

@@ -9,13 +9,12 @@ use crate::app::{
     background::tasks::networking::{
         api_to_dpd_port_settings, build_dpd_clients, build_mgd_clients,
     },
-    map_switch_zone_addrs,
+    switch_zone_address_mappings,
 };
 use oxnet::Ipv4Net;
 use slog::o;
 
 use internal_dns_resolver::Resolver;
-use internal_dns_types::names::ServiceName;
 use ipnetwork::IpNetwork;
 use nexus_db_model::{
     AddressLotBlock, BgpConfig, BootstoreConfig, INFRA_LOT, LoopbackAddress,
@@ -322,23 +321,19 @@ impl BackgroundTask for SwitchPortSettingsManager {
 
                 // lookup switch zones via DNS
                 // TODO https://github.com/oxidecomputer/omicron/issues/5201
-                let switch_zone_addresses = match self
-                    .resolver
-                    .lookup_all_ipv6(ServiceName::Dendrite)
-                    .await
+                let mappings = match
+                    switch_zone_address_mappings(&self.resolver, &log).await
                 {
-                    Ok(addrs) => addrs,
+                    Ok(mappings) => mappings,
                     Err(e) => {
-                        error!(log, "failed to resolve addresses for Dendrite services";
-                            "error" => %DisplayErrorChain::new(&e));
+                        error!(
+                            log,
+                            "failed to resolve addresses for Dendrite services";
+                            "error" => %e);
                         continue;
                     },
                 };
 
-                // TODO https://github.com/oxidecomputer/omicron/issues/5201
-                let mappings =
-                    map_switch_zone_addrs(&log, switch_zone_addresses).await;
-
                 // TODO https://github.com/oxidecomputer/omicron/issues/5201
                 // build sled agent clients
                 let sled_agent_clients = build_sled_agent_clients(&mappings, &log);

nexus/src/app/mod.rs

@@ -1030,21 +1030,35 @@ pub(crate) async fn lldpd_clients(
     Ok(clients)
 }
 
+// Look up Dendrite addresses in DNS then determine the switch location for
+// each address DNS returns.  If we fail to lookup ServiceName::Dendrite, we
+// return error, otherwise we keep looping until we can determine the switch
+// location of all addresses returned to us from the lookup.
 async fn switch_zone_address_mappings(
     resolver: &internal_dns_resolver::Resolver,
     log: &slog::Logger,
 ) -> Result<HashMap<SwitchLocation, Ipv6Addr>, String> {
-    let switch_zone_addresses = match resolver
-        .lookup_all_ipv6(ServiceName::Dendrite)
-        .await
-    {
-        Ok(addrs) => addrs,
-        Err(e) => {
-            error!(log, "failed to resolve addresses for Dendrite services"; "error" => %e);
-            return Err(e.to_string());
+    loop {
+        let switch_zone_addresses = match resolver
+            .lookup_all_ipv6(ServiceName::Dendrite)
+            .await
+        {
+            Ok(addrs) => addrs,
+            Err(e) => {
+                error!(log, "failed to resolve addresses for Dendrite services"; "error" => %e);
+                return Err(e.to_string());
+            }
+        };
+        match map_switch_zone_addrs(&log, switch_zone_addresses).await {
+            Ok(mappings) => {
+                return Ok(mappings);
+            }
+            Err(e) => {
+                warn!(log, "Failed to map switch zone addr: {e}, retrying");
+                tokio::time::sleep(std::time::Duration::from_secs(2)).await;
+            }
         }
-    };
-    Ok(map_switch_zone_addrs(&log, switch_zone_addresses).await)
+    }
 }
 
 // TODO: #3596 Allow updating of Nexus from `handoff_to_nexus()`
@@ -1058,7 +1072,7 @@ async fn switch_zone_address_mappings(
 async fn map_switch_zone_addrs(
     log: &Logger,
     switch_zone_addresses: Vec<Ipv6Addr>,
-) -> HashMap<SwitchLocation, Ipv6Addr> {
+) -> Result<HashMap<SwitchLocation, Ipv6Addr>, String> {
     use gateway_client::Client as MgsClient;
     info!(log, "Determining switch slots managed by switch zones");
     let mut switch_zone_addrs = HashMap::new();
@@ -1069,28 +1083,25 @@ async fn map_switch_zone_addrs(
         );
 
         info!(log, "determining switch slot managed by dendrite zone"; "zone_address" => #?addr);
-        // TODO: #3599 Use retry function instead of looping on a fixed timer
-        let switch_slot = loop {
-            match mgs_client.sp_local_switch_id().await {
-                Ok(switch) => {
-                    info!(
-                        log,
-                        "identified switch slot for dendrite zone";
-                        "slot" => #?switch,
-                        "zone_address" => #?addr
-                    );
-                    break switch.slot;
-                }
-                Err(e) => {
-                    warn!(
-                        log,
-                        "failed to identify switch slot for dendrite, will retry in 2 seconds";
-                        "zone_address" => #?addr,
-                        "reason" => #?e
-                    );
-                }
+        let switch_slot = match mgs_client.sp_local_switch_id().await {
+            Ok(switch) => {
+                info!(
+                    log,
+                    "identified switch slot for dendrite zone";
+                    "slot" => #?switch,
+                    "zone_address" => #?addr
+                );
+                switch.slot
+            }
+            Err(e) => {
+                warn!(
+                    log,
+                    "failed to identify switch slot for dendrite";
+                    "zone_address" => #?addr,
+                    "reason" => #?e
+                );
+                return Err(e.to_string());
             }
-            tokio::time::sleep(std::time::Duration::from_secs(2)).await;
         };
 
         match switch_slot {
@@ -1113,5 +1124,5 @@ async fn map_switch_zone_addrs(
         "completed mapping dendrite zones to switch slots";
         "mappings" => #?switch_zone_addrs
     );
-    switch_zone_addrs
+    Ok(switch_zone_addrs)
 }