diff --git a/wicketd/src/http_entrypoints.rs b/wicketd/src/http_entrypoints.rs index 55b4d61c9a..3f460f1e37 100644 --- a/wicketd/src/http_entrypoints.rs +++ b/wicketd/src/http_entrypoints.rs @@ -82,6 +82,7 @@ impl WicketdApi for WicketdApiImpl { config.update_with_inventory_and_bootstrap_peers( &inventory, &ctx.bootstrap_peers, + &ctx.log, ); Ok(HttpResponseOk((&*config).into())) @@ -101,6 +102,7 @@ impl WicketdApi for WicketdApiImpl { config.update_with_inventory_and_bootstrap_peers( &inventory, &ctx.bootstrap_peers, + &ctx.log, ); config .update(body.into_inner(), ctx.baseboard.as_ref()) diff --git a/wicketd/src/rss_config.rs b/wicketd/src/rss_config.rs index 56e83fcd41..46ede25eaa 100644 --- a/wicketd/src/rss_config.rs +++ b/wicketd/src/rss_config.rs @@ -26,6 +26,7 @@ use omicron_common::api::external::AllowedSourceIps; use omicron_common::api::external::SwitchLocation; use once_cell::sync::Lazy; use sled_hardware_types::Baseboard; +use slog::debug; use slog::warn; use std::collections::btree_map; use std::collections::BTreeMap; @@ -115,6 +116,7 @@ impl CurrentRssConfig { &mut self, inventory: &RackV1Inventory, bootstrap_peers: &BootstrapPeers, + log: &slog::Logger, ) { let bootstrap_sleds = bootstrap_peers.sleds(); @@ -126,7 +128,15 @@ impl CurrentRssConfig { return None; } - let state = sp.state.as_ref()?; + let Some(state) = sp.state.as_ref() else { + debug!( + log, + "in update_with_inventory_and_bootstrap_peers, \ + filtering out SP with no state"; + "sp" => ?sp, + ); + return None; + }; let baseboard = Baseboard::new_gimlet( state.serial_number.clone(), state.model.clone(), diff --git a/wicketd/tests/integration_tests/inventory.rs b/wicketd/tests/integration_tests/inventory.rs index ed5ad22d5d..c7057e3adc 100644 --- a/wicketd/tests/integration_tests/inventory.rs +++ b/wicketd/tests/integration_tests/inventory.rs @@ -10,6 +10,7 @@ use super::setup::WicketdTestContext; use gateway_messages::SpPort; use gateway_test_utils::setup as gateway_setup; use sled_hardware_types::Baseboard; +use slog::{info, warn}; use wicket::OutputKind; use wicket_common::inventory::{SpIdentifier, SpType}; use wicket_common::rack_setup::BootstrapSledDescription; @@ -32,13 +33,29 @@ async fn test_inventory() { .into_inner(); match response { GetInventoryResponse::Response { inventory, .. } => { - break inventory - } - GetInventoryResponse::Unavailable => { - // Keep polling wicketd until it receives its first results from MGS. - tokio::time::sleep(Duration::from_millis(100)).await; + // Ensure that the SP state is populated -- if it's not, + // then the `configured-bootstrap-sleds` command below + // might return an empty list. + let sp_state_none: Vec<_> = inventory + .sps + .iter() + .filter(|sp| sp.state.is_none()) + .collect(); + if sp_state_none.is_empty() { + break inventory; + } + + warn!( + wicketd_testctx.log(), + "SP state not yet populated for some SPs, retrying"; + "sps" => ?sp_state_none + ) } + GetInventoryResponse::Unavailable => {} } + + // Keep polling wicketd until it receives its first results from MGS. + tokio::time::sleep(Duration::from_millis(100)).await; } }; let inventory = @@ -46,6 +63,8 @@ async fn test_inventory() { .await .expect("get_inventory completed within 10 seconds"); + info!(wicketd_testctx.log(), "inventory returned"; "inventory" => ?inventory); + // 4 SPs attached to the inventory. assert_eq!(inventory.sps.len(), 4); @@ -70,17 +89,17 @@ async fn test_inventory() { serde_json::from_slice(&stdout).expect("stdout is valid JSON"); // This only tests the case that we get sleds back with no current - // bootstrap IP. This does provide svalue: it check that the command - // exists, accesses data within wicket, and returns it in the schema we - // expect. But it does not test the case where a sled does have a - // bootstrap IP. + // bootstrap IP. This does provide some value: it checks that the + // command exists, accesses data within wicket, and returns it in the + // schema we expect. But it does not test the case where a sled does + // have a bootstrap IP. // // Unfortunately, that's a difficult thing to test today. Wicket gets // that information by enumerating the IPs on the bootstrap network and // reaching out to the bootstrap_agent on them directly to ask them who // they are. Our testing setup does not have a way to provide such an // IP, or run a bootstrap_agent on an IP to respond. We should update - // this test when we do have that capabilitiy. + // this test when we do have that capability. assert_eq!( response, vec![