BFD status shouldn't 404 if only one scrimlet is present

nexus/src/app/bfd.rs

@@ -11,28 +11,6 @@ use sled_agent_types::early_networking::BfdMode;
 use sled_agent_types::early_networking::SwitchLocation;
 
 impl super::Nexus {
-    async fn mg_client_for_switch_location(
-        &self,
-        switch: SwitchLocation,
-    ) -> Result<mg_admin_client::Client, Error> {
-        let mg_client: mg_admin_client::Client = self
-            .mg_clients()
-            .await
-            .map_err(|e| {
-                Error::internal_error(&format!("failed to get mg clients: {e}"))
-            })?
-            .get(&switch)
-            .ok_or_else(|| {
-                Error::not_found_by_name(
-                    omicron_common::api::external::ResourceType::Switch,
-                    &switch.to_string().parse().unwrap(),
-                )
-            })?
-            .clone();
-
-        Ok(mg_client)
-    }
-
     pub async fn bfd_enable(
         &self,
         opctx: &OpContext,
@@ -69,9 +47,19 @@ impl super::Nexus {
     ) -> Result<Vec<bfd::BfdStatus>, Error> {
         // ask each rack switch about all its BFD sessions. This will need to
         // be updated for multirack.
+        let mg_clients = self.mg_clients().await.map_err(|err| {
+            Error::internal_error(&format!("failed to get mg clients: {err}"))
+        })?;
         let mut result = Vec::new();
         for s in &[SwitchLocation::Switch0, SwitchLocation::Switch1] {
-            let mg_client = self.mg_client_for_switch_location(*s).await?;
+            // If we only have one scrimlet, we won't have an entry in
+            // `mg_clients` for one of the switch locations. Log that, but
+            // continue so we can still report status from whichever switch we
+            // do have.
+            let Some(mg_client) = mg_clients.get(s) else {
+                warn!(self.log, "no mgd client found for switch location {s}");
+                continue;
+            };
             let status = mg_client
                 .get_bfd_peers()
                 .await

nexus/src/app/mod.rs

@@ -43,6 +43,7 @@ use sled_agent_types::early_networking::SwitchLocation;
 use slog::Logger;
 use slog_error_chain::InlineErrorChain;
 use std::collections::HashMap;
+use std::net::SocketAddr;
 use std::net::SocketAddrV6;
 use std::net::{IpAddr, Ipv6Addr};
 use std::num::NonZeroU32;
@@ -1159,19 +1160,41 @@ impl Nexus {
         let resolver = self.resolver();
         let mappings =
             switch_zone_address_mappings(resolver, &self.log).await?;
-        let mut clients: Vec<(SwitchLocation, mg_admin_client::Client)> =
-            vec![];
-        for (location, addr) in &mappings {
-            let port = MGD_PORT;
-            let socketaddr =
-                std::net::SocketAddr::V6(SocketAddrV6::new(*addr, port, 0, 0));
-            let client = mg_admin_client::Client::new(
-                format!("http://{}", socketaddr).as_str(),
-                self.log.clone(),
+        let mgd_addrs = resolver
+            .lookup_all_socket_v6(ServiceName::Mgd)
+            .await
+            .map_err(|err| {
+            format!(
+                "failed to resolve mgd in DNS: {}",
+                InlineErrorChain::new(&err)
+            )
+        })?;
+        let mut clients = HashMap::new();
+        for (location, ip) in mappings {
+            let addr =
+                match mgd_addrs.iter().copied().find(|addr| *addr.ip() == ip) {
+                    Some(addr) => SocketAddr::V6(addr),
+                    None => {
+                        warn!(
+                            self.log,
+                            "no MGD DNS entry found matching switch location \
+                             IP address; assuming default port";
+                            "switch-location" => ?location,
+                            "switch-ip" => %ip,
+                            "mgd-dns-entries" => ?mgd_addrs,
+                        );
+                        SocketAddr::V6(SocketAddrV6::new(ip, MGD_PORT, 0, 0))
+                    }
+                };
+            clients.insert(
+                location,
+                mg_admin_client::Client::new(
+                    &format!("http://{addr}"),
+                    self.log.clone(),
+                ),
             );
-            clients.push((*location, client));
         }
-        Ok(clients.into_iter().collect::<HashMap<_, _>>())
+        Ok(clients)
     }
 
     pub(crate) fn demo_sagas(

nexus/tests/integration_tests/bfd.rs

@@ -0,0 +1,30 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+//! Tests BFD support in the API
+
+use nexus_test_utils::http_testing::AuthnMode;
+use nexus_test_utils::http_testing::NexusRequest;
+use nexus_test_utils_macros::nexus_test;
+use nexus_types::external_api::bfd::BfdStatus;
+
+type ControlPlaneTestContext =
+    nexus_test_utils::ControlPlaneTestContext<omicron_nexus::Server>;
+
+const STATUS_URL: &str = "/v1/system/networking/bfd-status";
+
+#[nexus_test]
+async fn test_empty_bfd_status(cptestctx: &ControlPlaneTestContext) {
+    let client = &cptestctx.external_client;
+
+    let status = NexusRequest::object_get(client, STATUS_URL)
+        .authn_as(AuthnMode::PrivilegedUser)
+        .execute_and_parse_unwrap::<Vec<BfdStatus>>()
+        .await;
+
+    // `#[nexus_test]` doesn't set up BFD, so we should have no status. But we
+    // should still be able to ask for that! (#[nexus_test] also only sets up
+    // one fake scrimlet - that used to cause this endpoint to fail.)
+    assert_eq!(status, Vec::new());
+}

nexus/tests/integration_tests/mod.rs

@@ -14,6 +14,7 @@ mod audit_log;
 mod authn_http;
 mod authz;
 mod basic;
+mod bfd;
 mod certificates;
 mod cockroach;
 mod commands;