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..cb83ed49c16 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,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, - 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 -} - // 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 1957d46f9ad..56af57038a7 100644 --- a/nexus/tests/integration_tests/switch_port.rs +++ b/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; @@ -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: - // . - // - // 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-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]