Use sled inventory to build sled agent clients

nexus/src/app/background/init.rs

@@ -703,7 +703,6 @@ impl BackgroundTasksInitializer {
             period: config.switch_port_settings_manager.period_secs,
             task_impl: Box::new(SwitchPortSettingsManager::new(
                 datastore.clone(),
-                resolver.clone(),
                 rx_blueprint.clone(),
             )),
             opctx: opctx.child(BTreeMap::new()),

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

@@ -5,12 +5,10 @@
 //! Background task for propagating user provided switch configurations
 //! to the bootstore via sled-agent
 
-use crate::app::{
-    background::LoadedTargetBlueprint, switch_zone_address_mappings,
-};
+use crate::app::background::LoadedTargetBlueprint;
 
-use internal_dns_resolver::Resolver;
 use nexus_db_model::{BootstoreConfig, NETWORK_KEY};
+use nexus_types::deployment::SledFilter;
 use tokio::sync::watch;
 
 use crate::app::background::BackgroundTask;
@@ -21,37 +19,27 @@ use nexus_db_queries::{context::OpContext, db::DataStore};
 use nexus_types::identity::Asset;
 use nexus_types::internal_api::background::IncompleteBootstoreConfigReport;
 use nexus_types::internal_api::background::SwitchPortSettingsManagerStatus;
-use omicron_common::{
-    address::{Ipv6Subnet, get_sled_address},
-    api::external::DataPageParams,
-};
+use omicron_common::api::external::DataPageParams;
 use serde_json::json;
 use sled_agent_types::early_networking::EarlyNetworkConfigEnvelope;
 use sled_agent_types::early_networking::RackNetworkConfig;
-use sled_agent_types::early_networking::SwitchSlot;
 use sled_agent_types::system_networking::BlueprintExternalNetworkingConfig;
 use sled_agent_types::system_networking::SystemNetworkingConfig;
 use sled_agent_types::system_networking::WriteNetworkConfigRequest;
 use slog_error_chain::InlineErrorChain;
-use std::{
-    collections::{HashMap, HashSet},
-    hash::Hash,
-    sync::Arc,
-};
+use std::{collections::HashSet, hash::Hash, sync::Arc};
 
 pub struct SwitchPortSettingsManager {
     datastore: Arc<DataStore>,
-    resolver: Resolver,
     rx_blueprint: watch::Receiver<Option<LoadedTargetBlueprint>>,
 }
 
 impl SwitchPortSettingsManager {
     pub fn new(
         datastore: Arc<DataStore>,
-        resolver: Resolver,
         rx_blueprint: watch::Receiver<Option<LoadedTargetBlueprint>>,
     ) -> Self {
-        Self { datastore, resolver, rx_blueprint }
+        Self { datastore, rx_blueprint }
     }
 }
 
@@ -79,6 +67,7 @@ impl BackgroundTask for SwitchPortSettingsManager {
                 },
             };
 
+
             let mut status = SwitchPortSettingsManagerStatus::default();
 
             // TODO: https://github.com/oxidecomputer/omicron/issues/3090
@@ -89,24 +78,35 @@ impl BackgroundTask for SwitchPortSettingsManager {
                 let rack_id = rack.id().to_string();
                 let log = log.new(slog::o!("rack_id" => rack_id));
 
-                // lookup switch zones via DNS
-                // TODO https://github.com/oxidecomputer/omicron/issues/5201
-                let mappings = match
-                    switch_zone_address_mappings(&self.resolver, &log).await
+                let sleds = match self
+                    .datastore
+                    .sled_list_all_batched(opctx, SledFilter::Commissioned)
+                    .await
                 {
-                    Ok(mappings) => mappings,
+                    Ok(sleds) => sleds,
                     Err(e) => {
-                        error!(
-                            log,
-                            "failed to resolve addresses for switch services";
-                            "error" => %e);
-                        continue;
-                    },
+                        error!(log, "failed to retrieve sleds from database";
+                               "error" => %DisplayErrorChain::new(&e)
+                        );
+                        return json!({
+                            "error":
+                            format!(
+                                "failed to retrieve sleds from database : {}",
+                                DisplayErrorChain::new(&e)
+                            )
+                        });
+                    }
                 };
 
-                // TODO https://github.com/oxidecomputer/omicron/issues/5201
-                // build sled agent clients for sleds that are connected to the switches
-                let scrimlet_sled_agent_clients = build_sled_agent_clients(&mappings, &log);
+                let scrimlet_clients = sleds
+                    .into_iter()
+                    .filter(|s| s.is_scrimlet())
+                    .map(|s| {
+                        sled_agent_client::Client::new(
+                            &format!("http://{}", s.address()),
+                            log.clone(),
+                        )
+                    });
 
                 //
                 // calculate and apply bootstore changes
@@ -335,15 +335,14 @@ impl BackgroundTask for SwitchPortSettingsManager {
                         }
                     };
 
-                    // push the updates to both scrimlets
-                    // if both scrimlets are down, bootstore updates aren't happening anyway
+                    // Update the bootstore. We eagerly push updates to both scrimlets.
                     let mut one_succeeded = false;
-                    for (switch_slot, client) in &scrimlet_sled_agent_clients {
+                    for client in scrimlet_clients {
                         if let Err(e) = client.write_network_bootstore_config(&write_request).await {
                             error!(
                                 log,
                                 "error updating bootstore";
-                                "switch_slot" => ?switch_slot,
+                                "scrimlet_client" => ?client,
                                 "request" => ?write_request,
                                 "error" => %e,
                             )
@@ -407,26 +406,6 @@ where
     left == right
 }
 
-fn build_sled_agent_clients(
-    mappings: &HashMap<SwitchSlot, std::net::Ipv6Addr>,
-    log: &slog::Logger,
-) -> HashMap<SwitchSlot, sled_agent_client::Client> {
-    let sled_agent_clients: HashMap<SwitchSlot, sled_agent_client::Client> =
-        mappings
-            .iter()
-            .map(|(switch_slot, addr)| {
-                // build sled agent address from switch zone address
-                let addr = get_sled_address(Ipv6Subnet::new(*addr));
-                let client = sled_agent_client::Client::new(
-                    &format!("http://{}", addr),
-                    log.clone(),
-                );
-                (*switch_slot, client)
-            })
-            .collect();
-    sled_agent_clients
-}
-
 // Helper to decide whether we should update the replicated bootstore.
 //
 // `current_contents` are the most-recently-written bootstore contents; it

nexus/tests/integration_tests/switch_port.rs

@@ -21,6 +21,10 @@ use omicron_common::api::external::Name;
 use omicron_common::api::external::{
     IdentityMetadataCreateParams, IdentityMetadataUpdateParams, NameOrId,
 };
+use omicron_sled_agent::sim;
+use omicron_test_utils::dev::poll::CondCheckError;
+use omicron_test_utils::dev::poll::Error;
+use omicron_test_utils::dev::poll::wait_for_condition;
 use oxnet::IpNet;
 use sled_agent_types::early_networking::ImportExportPolicy;
 use sled_agent_types::early_networking::LinkFec;
@@ -30,6 +34,7 @@ use sled_agent_types::early_networking::NumberedRouter;
 use sled_agent_types::early_networking::RouterLifetimeConfig;
 use sled_agent_types::early_networking::UnnumberedRouter;
 use std::str::FromStr;
+use std::time::Duration;
 
 type ControlPlaneTestContext =
     nexus_test_utils::ControlPlaneTestContext<omicron_nexus::Server>;
@@ -655,6 +660,14 @@ async fn test_port_settings_basic_v6_crud(ctx: &ControlPlaneTestContext) {
 
     let rack_id = racks[0].identity.id;
 
+    for (i, s) in ctx.sled_agents.iter().enumerate() {
+        wait_for_sled_agent_bootstore_gen(s.sled_agent(), 3)
+            .await
+            .unwrap_or_else(|_| {
+                panic!("sled-agent {i}'s bootstore should be 3 prior to update")
+            });
+    }
+
     NexusRequest::new(
         RequestBuilder::new(
             client,
@@ -669,20 +682,69 @@ async fn test_port_settings_basic_v6_crud(ctx: &ControlPlaneTestContext) {
     .await
     .unwrap();
 
-    // TODO-cleanup We'd like to confirm that the `sync_switch_configuration`
-    // background task propagates the changes requested above out to the
-    // bootstore via sled-agent, but in the test suite, that propagation fails
-    // for unrelated reasons:
-    // <https://github.com/oxidecomputer/omicron/issues/10958>.
-    //
-    // As a fallback, it'd be nice to check that `sync_switch_configuration` at
-    // least persists the new config into CRDB, but the task gates that on
-    // having successfully contacted at least one sled-agent, so this also is
-    // blocked by the above issue. For now, we've manually confirmed that the
-    // above route is present in the request `sync_switch_configuration`
-    // attempts to send by inspecting the logfile (where the request is included
-    // alongside the connection error from trying to contact a nonexistent
-    // sled-agent).
+    // Verify that `sync_switch_configuration` propagates the changes to the
+    // bootstore via sled-agent (regression test for
+    // <https://github.com/oxidecomputer/omicron/issues/10958>).
+    // Run sync_switch_configuration.
+    let task = nexus_test_utils::background::activate_background_task(
+        &ctx.lockstep_client,
+        "switch_port_config_manager",
+    )
+    .await;
+
+    let nexus_lockstep_client::types::LastResult::Completed(result) = task.last
+    else {
+        panic!(
+            "switch_port_config_manager task did not complete: {:?}",
+            task.last
+        );
+    };
+    let status = serde_json::from_value::<
+        nexus_types::internal_api::background::SwitchPortSettingsManagerStatus,
+    >(result.details)
+    .expect(
+        "task details should deserialize as SwitchPortSettingsManagerStatus",
+    );
+    assert!(
+        status.incomplete_bootstore_configs.is_empty(),
+        "sync_switch_configuration should have successfully built a bootstore \
+         config for all racks: {status:?}",
+    );
+
+    // The task only writes to the sled-agent if it can build a valid config.
+    // Check the sim sled-agents' in-memory bootstores were actually updated,
+    // confirming both scrimlets were successfully contacted.
+    for (i, s) in ctx.sled_agents.iter().enumerate() {
+        wait_for_sled_agent_bootstore_gen(&s.sled_agent(), 4)
+        .await
+        .unwrap_or_else(|_| {
+            panic!(
+                "sync_switch_configuration should have written to sled-agent \
+                 {i}'s bootstore (generation was still 3, indicating it was \
+                 never contacted)",
+            )
+        });
+    }
+}
+
+async fn wait_for_sled_agent_bootstore_gen(
+    sled_agent: &sim::SledAgent,
+    g: u64,
+) -> Result<(), Error<()>> {
+    wait_for_condition(
+        || async {
+            let generation =
+                sled_agent.bootstore_network_config.lock().unwrap().generation;
+            if generation == g {
+                Ok(())
+            } else {
+                Err(CondCheckError::<()>::NotYet { status: None })
+            }
+        },
+        &Duration::from_millis(50),
+        &Duration::from_secs(60),
+    )
+    .await
 }
 
 #[nexus_test]