From a54ee9da32f9dc8616b6a160a4318a239aee9607 Mon Sep 17 00:00:00 2001 From: Levon Tarver Date: Fri, 28 Aug 2026 22:12:26 +0000 Subject: [PATCH 1/3] Use sled inventory to build sled agent clients Instead of deriving sled agent addresses from switch zone service addresses (which does not work in the test context), pull the sled agent information from the inventory records in the database. --- nexus/src/app/background/init.rs | 1 - .../tasks/sync_switch_configuration.rs | 91 +++++++++---------- nexus/tests/integration_tests/switch_port.rs | 57 +++++++++--- 3 files changed, 84 insertions(+), 65 deletions(-) diff --git a/nexus/src/app/background/init.rs b/nexus/src/app/background/init.rs index d5659dac257..5da116e60a6 100644 --- a/nexus/src/app/background/init.rs +++ b/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()), diff --git a/nexus/src/app/background/tasks/sync_switch_configuration.rs b/nexus/src/app/background/tasks/sync_switch_configuration.rs index e29a075a66d..e3f884d5477 100644 --- a/nexus/src/app/background/tasks/sync_switch_configuration.rs +++ b/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, - resolver: Resolver, rx_blueprint: watch::Receiver>, } impl SwitchPortSettingsManager { pub fn new( datastore: Arc, - resolver: Resolver, rx_blueprint: watch::Receiver>, ) -> 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,30 @@ 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) + ) + }); + } }; + let sled_agent_addrs: Vec<_> = sleds.into_iter().map(|s| s.address()).collect(); + // 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 sled_agent_clients = build_sled_agent_clients(&sled_agent_addrs, &log); // // calculate and apply bootstore changes @@ -335,20 +330,21 @@ 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 can move on once we have updated a + // single sled agent. let mut one_succeeded = false; - for (switch_slot, client) in &scrimlet_sled_agent_clients { + for client in &sled_agent_clients { if let Err(e) = client.write_network_bootstore_config(&write_request).await { error!( log, "error updating bootstore"; - "switch_slot" => ?switch_slot, + "switch_slot" => ?client, "request" => ?write_request, "error" => %e, ) } else { one_succeeded = true; + break; } } @@ -408,23 +404,18 @@ where } fn build_sled_agent_clients( - mappings: &HashMap, + addrs: &[std::net::SocketAddrV6], log: &slog::Logger, -) -> HashMap { - let sled_agent_clients: HashMap = - 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 +) -> Vec { + addrs + .iter() + .map(|addr| { + sled_agent_client::Client::new( + &format!("http://{}", addr), + log.clone(), + ) + }) + .collect() } // Helper to decide whether we should update the replicated bootstore. diff --git a/nexus/tests/integration_tests/switch_port.rs b/nexus/tests/integration_tests/switch_port.rs index 1957d46f9ad..f79fd8885c8 100644 --- a/nexus/tests/integration_tests/switch_port.rs +++ b/nexus/tests/integration_tests/switch_port.rs @@ -669,20 +669,49 @@ 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: - // . - // - // 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 + // ). + // 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-agent's in-memory bootstore was actually updated, + // confirming it was successfully contacted. + let bootstore_generation = ctx + .first_sled_agent() + .bootstore_network_config + .lock() + .unwrap() + .generation; + assert!( + bootstore_generation > 0, + "sync_switch_configuration should have written to the sled-agent \ + bootstore (generation was still 0, indicating it was never contacted)", + ); } #[nexus_test] From f1b6243c38bb7e459ff58e89858a74b8373d0b46 Mon Sep 17 00:00:00 2001 From: Levon Tarver Date: Mon, 31 Aug 2026 22:15:12 +0000 Subject: [PATCH 2/3] pr review fixes --- .../tasks/sync_switch_configuration.rs | 36 ++++------ nexus/tests/integration_tests/switch_port.rs | 69 +++++++++++++++---- 2 files changed, 68 insertions(+), 37 deletions(-) diff --git a/nexus/src/app/background/tasks/sync_switch_configuration.rs b/nexus/src/app/background/tasks/sync_switch_configuration.rs index e3f884d5477..cb83ed49c16 100644 --- a/nexus/src/app/background/tasks/sync_switch_configuration.rs +++ b/nexus/src/app/background/tasks/sync_switch_configuration.rs @@ -98,10 +98,15 @@ impl BackgroundTask for SwitchPortSettingsManager { } }; - let sled_agent_addrs: Vec<_> = sleds.into_iter().map(|s| s.address()).collect(); - - // TODO https://github.com/oxidecomputer/omicron/issues/5201 - let sled_agent_clients = build_sled_agent_clients(&sled_agent_addrs, &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 @@ -330,21 +335,19 @@ impl BackgroundTask for SwitchPortSettingsManager { } }; - // Update the bootstore. We can move on once we have updated a - // single sled agent. + // Update the bootstore. We eagerly push updates to both scrimlets. let mut one_succeeded = false; - for client in &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" => ?client, + "scrimlet_client" => ?client, "request" => ?write_request, "error" => %e, ) } else { one_succeeded = true; - break; } } @@ -403,21 +406,6 @@ where left == right } -fn build_sled_agent_clients( - addrs: &[std::net::SocketAddrV6], - log: &slog::Logger, -) -> Vec { - addrs - .iter() - .map(|addr| { - sled_agent_client::Client::new( - &format!("http://{}", addr), - log.clone(), - ) - }) - .collect() -} - // Helper to decide whether we should update the replicated bootstore. // // `current_contents` are the most-recently-written bootstore contents; it diff --git a/nexus/tests/integration_tests/switch_port.rs b/nexus/tests/integration_tests/switch_port.rs index f79fd8885c8..0b0ab7c5be3 100644 --- a/nexus/tests/integration_tests/switch_port.rs +++ b/nexus/tests/integration_tests/switch_port.rs @@ -21,6 +21,8 @@ use omicron_common::api::external::Name; use omicron_common::api::external::{ IdentityMetadataCreateParams, IdentityMetadataUpdateParams, NameOrId, }; +use omicron_test_utils::dev::poll::CondCheckError; +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 +32,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; @@ -655,6 +658,30 @@ async fn test_port_settings_basic_v6_crud(ctx: &ControlPlaneTestContext) { let rack_id = racks[0].identity.id; + for (i, sled_agent) in ctx.sled_agents.iter().enumerate() { + let sled_agent = sled_agent.sled_agent().clone(); + wait_for_condition( + || async { + let generation = sled_agent + .bootstore_network_config + .lock() + .unwrap() + .generation; + if generation == 3 { + Ok(()) + } else { + Err(CondCheckError::<()>::NotYet { status: None }) + } + }, + &Duration::from_millis(50), + &Duration::from_secs(60), + ) + .await + .unwrap_or_else(|_| { + panic!("sled-agent {i}'s bootstore should be 3 prior to update") + }); + } + NexusRequest::new( RequestBuilder::new( client, @@ -699,19 +726,35 @@ async fn test_port_settings_basic_v6_crud(ctx: &ControlPlaneTestContext) { ); // The task only writes to the sled-agent if it can build a valid config. - // Check the sim sled-agent's in-memory bootstore was actually updated, - // confirming it was successfully contacted. - let bootstore_generation = ctx - .first_sled_agent() - .bootstore_network_config - .lock() - .unwrap() - .generation; - assert!( - bootstore_generation > 0, - "sync_switch_configuration should have written to the sled-agent \ - bootstore (generation was still 0, indicating it was never contacted)", - ); + // Check the sim sled-agents' in-memory bootstores were actually updated, + // confirming both scrimlets were successfully contacted. + for (i, sled_agent) in ctx.sled_agents.iter().enumerate() { + let sled_agent = sled_agent.sled_agent().clone(); + wait_for_condition( + || async { + let generation = sled_agent + .bootstore_network_config + .lock() + .unwrap() + .generation; + if generation == 4 { + Ok(()) + } else { + Err(CondCheckError::<()>::NotYet { status: None }) + } + }, + &Duration::from_millis(50), + &Duration::from_secs(60), + ) + .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)", + ) + }); + } } #[nexus_test] From 6a91150d25c83cea28db1db84c623ee5ada4bb6f Mon Sep 17 00:00:00 2001 From: Levon Tarver Date: Tue, 1 Sep 2026 21:07:31 +0000 Subject: [PATCH 3/3] refactor wait_for_condition --- nexus/tests/integration_tests/switch_port.rs | 70 +++++++++----------- 1 file changed, 30 insertions(+), 40 deletions(-) diff --git a/nexus/tests/integration_tests/switch_port.rs b/nexus/tests/integration_tests/switch_port.rs index 0b0ab7c5be3..56af57038a7 100644 --- a/nexus/tests/integration_tests/switch_port.rs +++ b/nexus/tests/integration_tests/switch_port.rs @@ -21,7 +21,9 @@ 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; @@ -658,28 +660,12 @@ async fn test_port_settings_basic_v6_crud(ctx: &ControlPlaneTestContext) { let rack_id = racks[0].identity.id; - for (i, sled_agent) in ctx.sled_agents.iter().enumerate() { - let sled_agent = sled_agent.sled_agent().clone(); - wait_for_condition( - || async { - let generation = sled_agent - .bootstore_network_config - .lock() - .unwrap() - .generation; - if generation == 3 { - Ok(()) - } else { - Err(CondCheckError::<()>::NotYet { status: None }) - } - }, - &Duration::from_millis(50), - &Duration::from_secs(60), - ) - .await - .unwrap_or_else(|_| { - panic!("sled-agent {i}'s bootstore should be 3 prior to update") - }); + 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( @@ -728,24 +714,8 @@ async fn test_port_settings_basic_v6_crud(ctx: &ControlPlaneTestContext) { // 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, sled_agent) in ctx.sled_agents.iter().enumerate() { - let sled_agent = sled_agent.sled_agent().clone(); - wait_for_condition( - || async { - let generation = sled_agent - .bootstore_network_config - .lock() - .unwrap() - .generation; - if generation == 4 { - Ok(()) - } else { - Err(CondCheckError::<()>::NotYet { status: None }) - } - }, - &Duration::from_millis(50), - &Duration::from_secs(60), - ) + for (i, s) in ctx.sled_agents.iter().enumerate() { + wait_for_sled_agent_bootstore_gen(&s.sled_agent(), 4) .await .unwrap_or_else(|_| { panic!( @@ -757,6 +727,26 @@ async fn test_port_settings_basic_v6_crud(ctx: &ControlPlaneTestContext) { } } +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] async fn test_bgp_config_update(ctx: &ControlPlaneTestContext) { let client = &ctx.external_client;