Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions crates/health/example/config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ port = 443
mac = "11:22:33:44:55:66"
username = "admin"
password = "secret"
switch = { id = "fsw100htjtiaehv1n5vh67tbmqq4eabcjdng40f7jupsadbedhruh6rag1l0", serial = "SN-SWITCH-BMC-001", endpoint_role = "bmc", slot_number = 7, tray_index = 3 }
# Configure the same domain UUID on every static endpoint for the switch.
# Invalid or nil values are omitted from telemetry.
switch = { id = "fsw100htjtiaehv1n5vh67tbmqq4eabcjdng40f7jupsadbedhruh6rag1l0", serial = "SN-SWITCH-BMC-001", endpoint_role = "bmc", slot_number = 7, tray_index = 3, nvlink_domain_uuid = "9f4b45ec-705a-4af4-89f7-a112bc9c8f4e" }

[[endpoint_sources.static_bmc_endpoints]]
ip = "10.0.1.2"
Expand All @@ -60,7 +62,7 @@ password = "secret"
# For static switch host endpoints, nmxc_enabled controls direct NMX-C
# Subscribe eligibility after the endpoint_role="host" and is_primary=true
# checks. If omitted, it defaults to is_primary.
switch = { id = "fsw100htjtiaehv1n5vh67tbmqq4eabcjdng40f7jupsadbedhruh6rag1l0", serial = "SN-SWITCH-HOST-001", endpoint_role = "host", is_primary = true, nmxc_enabled = true, slot_number = 7, tray_index = 3 }
switch = { id = "fsw100htjtiaehv1n5vh67tbmqq4eabcjdng40f7jupsadbedhruh6rag1l0", serial = "SN-SWITCH-HOST-001", endpoint_role = "host", is_primary = true, nmxc_enabled = true, slot_number = 7, tray_index = 3, nvlink_domain_uuid = "9f4b45ec-705a-4af4-89f7-a112bc9c8f4e" }

[[endpoint_sources.static_bmc_endpoints]]
ip = "10.0.2.1"
Expand Down
54 changes: 53 additions & 1 deletion crates/health/src/api_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use std::net::IpAddr;
use std::str::FromStr;
use std::sync::{Arc, Mutex};

use carbide_uuid::nvlink::NvLinkDomainId;
use carbide_uuid::rack::RackId;
use carbide_uuid::switch::SwitchId;
use forge_tls::client_config::ClientCert;
Expand Down Expand Up @@ -275,6 +276,9 @@ fn switch_endpoint_metadata(
.placement_in_rack
.as_ref()
.and_then(|placement| placement.tray_index),
nvlink_domain_uuid: switch
.nvlink_domain_uuid
.filter(|domain_uuid| domain_uuid != &NvLinkDomainId::nil()),
endpoint_role,
is_primary: switch.is_primary,
nmxc_enabled: config.enable_nmxc,
Expand Down Expand Up @@ -755,7 +759,8 @@ impl From<rpc::forge::bmc_credentials::Type> for BmcCredentials {
mod tests {
use std::sync::atomic::{AtomicUsize, Ordering};

use carbide_test_support::value_scenarios;
use carbide_test_support::{Check, check_values, value_scenarios};
use carbide_uuid::nvlink::NvLinkDomainId;
use carbide_uuid::switch::{SwitchId, SwitchIdSource, SwitchType};
use nv_redfish::bmc_http::reqwest::ClientParams as ReqwestClientParams;

Expand Down Expand Up @@ -847,6 +852,53 @@ mod tests {
);
}

#[test]
fn switch_endpoint_metadata_uses_non_nil_api_domain() {
let domain = NvLinkDomainId::from_str("9f4b45ec-705a-4af4-89f7-a112bc9c8f4e")
.expect("valid domain UUID");

check_values(
[
Check {
scenario: "domain is missing",
input: None,
expect: None,
},
Check {
scenario: "nil domain is absent",
input: Some(NvLinkDomainId::nil()),
expect: None,
},
Check {
scenario: "non-nil API switch field",
input: Some(domain),
expect: Some(domain),
},
],
|nvlink_domain_uuid| {
let metadata = switch_endpoint_metadata(
&rpc::forge::Switch {
config: Some(rpc::forge::SwitchConfig {
name: "switch-a".to_string(),
..Default::default()
}),
nvlink_domain_uuid,
..Default::default()
},
SwitchEndpointRole::Bmc,
false,
)
.expect("switch metadata");

let EndpointMetadata::Switch(switch) = metadata else {
panic!("expected switch metadata");
};

switch.nvlink_domain_uuid
},
);
}

#[tokio::test]
async fn cache_returns_existing_client_on_matching_kind() {
let mut cache: HashMap<MacAddress, CachedBmcClient> = HashMap::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,7 @@ mod tests {
serial: "SN-SWITCH-001".to_string(),
slot_number: Some(7),
tray_index: Some(3),
nvlink_domain_uuid: None,
endpoint_role: SwitchEndpointRole::Host,
is_primary: false,
nmxc_enabled: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,7 @@ mod tests {
serial: "SN-SWITCH-001".to_string(),
slot_number: Some(7),
tray_index: Some(3),
nvlink_domain_uuid: None,
endpoint_role: SwitchEndpointRole::Host,
is_primary: false,
nmxc_enabled: false,
Expand Down
13 changes: 12 additions & 1 deletion crates/health/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,11 @@ pub struct StaticSwitchEndpoint {
pub slot_number: Option<i32>,
#[serde(alias = "compute_tray_index")]
pub tray_index: Option<i32>,

/// Optional non-nil NVLink domain UUID associated with this switch.
/// Invalid or nil values are omitted from telemetry.
pub nvlink_domain_uuid: Option<String>,

#[serde(default = "default_static_switch_endpoint_role")]
pub endpoint_role: StaticSwitchEndpointRole,
#[serde(default)]
Expand Down Expand Up @@ -1918,6 +1923,7 @@ mod tests {
serial: Some("switch-serial".to_string()),
slot_number: None,
tray_index: None,
nvlink_domain_uuid: None,
endpoint_role: StaticSwitchEndpointRole::Host,
is_primary: false,
nmxc_enabled: None,
Expand Down Expand Up @@ -3866,7 +3872,7 @@ ip = "10.0.1.2"
mac = "11:22:33:44:55:77"
username = "admin"
password = "pass"
switch = { id = "fsw100htjtiaehv1n5vh67tbmqq4eabcjdng40f7jupsadbedhruh6rag1l0", serial = "SN-SW-002", endpoint_role = "host", is_primary = false, nmxc_enabled = true, nmxt_enabled = true }
switch = { id = "fsw100htjtiaehv1n5vh67tbmqq4eabcjdng40f7jupsadbedhruh6rag1l0", serial = "SN-SW-002", endpoint_role = "host", is_primary = false, nmxc_enabled = true, nmxt_enabled = true, nvlink_domain_uuid = "9f4b45ec-705a-4af4-89f7-a112bc9c8f4e" }
"#;

let config: Config = Figment::new()
Expand All @@ -3884,6 +3890,11 @@ switch = { id = "fsw100htjtiaehv1n5vh67tbmqq4eabcjdng40f7jupsadbedhruh6rag1l0",
assert!(!switch.is_primary);
assert_eq!(switch.nmxc_enabled, Some(true));
assert_eq!(switch.nmxt_enabled, Some(true));

assert_eq!(
switch.nvlink_domain_uuid.as_deref(),
Some("9f4b45ec-705a-4af4-89f7-a112bc9c8f4e")
);
}

#[test]
Expand Down
185 changes: 180 additions & 5 deletions crates/health/src/discovery/cleanup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,102 @@

use std::borrow::Cow;
use std::collections::HashSet;
use std::sync::Arc;

use futures::future::join_all;

use super::context::{CollectorKind, DiscoveryLoopContext};
use crate::collectors::Collector;
use crate::endpoint::BmcEndpoint;

#[derive(Clone, Copy)]
enum CollectorStopReason {
EndpointRemoved,
SwitchEndpointNoLongerEligible,
SwitchDomainChanged,
}

impl std::fmt::Display for CollectorStopReason {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(match self {
Self::EndpointRemoved => "endpoint removed",
Self::SwitchEndpointNoLongerEligible => "switch endpoint is no longer eligible",
Self::SwitchDomainChanged => "switch NVLink domain changed",
})
}
}

fn stop_collectors_for_keys(
/// Restarts switch collectors when discovery reports a different NVLink domain.
///
/// Collectors retain the endpoint metadata captured at startup. Because the
/// endpoint key does not change with the domain UUID, removed-endpoint cleanup
/// cannot refresh that metadata. This function removes affected collectors and
/// waits for their shutdown before discovery respawns them.
pub(super) async fn stop_stale_switch_collectors(
ctx: &mut DiscoveryLoopContext,
endpoints: &[Arc<BmcEndpoint>],
) {
// Keep one domain observation per collector key. The active set also
// identifies which saved observations remain valid after this pass.
let mut active_switch_endpoints = HashSet::with_capacity(endpoints.len());
let mut changed_endpoints = HashSet::new();

for endpoint in endpoints {
let Some(switch) = endpoint.switch_data() else {
continue;
};

let key = Cow::Owned(endpoint.key());

// Collector spawning uses the first endpoint for a key. Apply the same
// precedence here so a later source cannot create a false domain change
// for the collector that was spawned from the first endpoint.
if active_switch_endpoints.contains(&key) {
continue;
}

if ctx
.collectors
.observe_switch_domain(&key, switch.nvlink_domain_uuid)
{
changed_endpoints.insert(key.clone());
}

active_switch_endpoints.insert(key);
}

// Forget observations for switches absent from this discovery pass. If a
// switch returns later, its current domain establishes a fresh baseline.
ctx.collectors
.retain_switch_domains(&active_switch_endpoints);

// Remove every collector kind before awaiting shutdown. Same-pass spawning
// can then create replacements with the updated endpoint metadata.
let stale_collectors = CollectorKind::ALL
.into_iter()
.flat_map(|kind| {
take_collectors_for_keys(
ctx,
kind,
&changed_endpoints,
CollectorStopReason::SwitchDomainChanged,
)
})
.collect::<Vec<_>>();

// CollectorRemoved unregisters the old Prometheus label set. Wait for that
// cleanup before replacement collectors register the new domain UUID.
join_all(stale_collectors.into_iter().map(Collector::stop)).await;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

fn take_collectors_for_keys(
ctx: &mut DiscoveryLoopContext,
kind: CollectorKind,
removed_keys: &HashSet<Cow<'static, str>>,
stop_reason: CollectorStopReason,
) {
) -> Vec<Collector> {
let collectors = ctx.collectors.map_mut(kind);
let mut removed = Vec::new();
for key in removed_keys {
if let Some(collector) = collectors.remove(key) {
tracing::info!(
Expand All @@ -51,11 +122,23 @@ fn stop_collectors_for_keys(
remaining_collector_count = collectors.len(),
"Stopping collector"
);
tokio::spawn(async move {
collector.stop().await;
});
removed.push(collector);
}
}
removed
}

fn stop_collectors_for_keys(
ctx: &mut DiscoveryLoopContext,
kind: CollectorKind,
removed_keys: &HashSet<Cow<'static, str>>,
stop_reason: CollectorStopReason,
) {
for collector in take_collectors_for_keys(ctx, kind, removed_keys, stop_reason) {
tokio::spawn(async move {
collector.stop().await;
});
}
}

pub(super) fn stop_removed_bmc_collectors(
Expand Down Expand Up @@ -134,6 +217,8 @@ mod tests {
use super::*;
use crate::collectors::Collector;
use crate::config::Config;
use crate::endpoint::test_support::{mac, test_endpoint};
use crate::endpoint::{EndpointMetadata, SwitchData, SwitchEndpointRole};
use crate::limiter::{NoopLimiter, RateLimiter};
use crate::metrics::MetricsManager;

Expand Down Expand Up @@ -220,4 +305,94 @@ mod tests {
.contains(CollectorKind::Nmxt, "ineligible-switch")
);
}

#[tokio::test]
async fn switch_domain_change_restarts_collectors_for_same_endpoint_key() {
let mut ctx = context("switch_domain_change_restarts_collectors");
let mut endpoint = test_endpoint(mac("00:11:22:33:44:55"));
endpoint.metadata = Some(EndpointMetadata::Switch(SwitchData {
id: None,
serial: "switch-1".to_string(),
slot_number: None,
tray_index: None,
nvlink_domain_uuid: None,
endpoint_role: SwitchEndpointRole::Host,
is_primary: true,
nmxc_enabled: true,
nmxt_enabled: true,
}));
let key = endpoint.key();
let mut endpoint = Arc::new(endpoint);

ctx.collectors.insert(
CollectorKind::NvueRest,
Cow::Owned(key.clone()),
noop_collector(),
);
stop_stale_switch_collectors(&mut ctx, std::slice::from_ref(&endpoint)).await;
assert!(ctx.collectors.contains(CollectorKind::NvueRest, &key));

let expected_domain = carbide_uuid::nvlink::NvLinkDomainId::new();
let Some(EndpointMetadata::Switch(switch)) = Arc::make_mut(&mut endpoint).metadata.as_mut()
else {
panic!("test endpoint should contain switch metadata");
};
switch.nvlink_domain_uuid = Some(expected_domain);

stop_stale_switch_collectors(&mut ctx, std::slice::from_ref(&endpoint)).await;

assert!(!ctx.collectors.contains(CollectorKind::NvueRest, &key));

let updated_context = crate::sink::EventContext::from_endpoint(&endpoint, "nvue_rest");
assert_eq!(updated_context.nvlink_domain_uuid(), Some(expected_domain));

ctx.collectors.insert(
CollectorKind::NvueRest,
Cow::Owned(key.clone()),
noop_collector(),
);
stop_stale_switch_collectors(&mut ctx, &[endpoint]).await;
assert!(ctx.collectors.contains(CollectorKind::NvueRest, &key));
}

#[tokio::test]
async fn duplicate_switch_domains_use_first_source_without_restarts() {
let mut ctx = context("duplicate_switch_domains_use_first_source");
let mut first = test_endpoint(mac("00:11:22:33:44:55"));

first.metadata = Some(EndpointMetadata::Switch(SwitchData {
id: None,
serial: "switch-1".to_string(),
slot_number: None,
tray_index: None,
nvlink_domain_uuid: None,
endpoint_role: SwitchEndpointRole::Host,
is_primary: true,
nmxc_enabled: true,
nmxt_enabled: true,
}));

let key = first.key();
let first = Arc::new(first);
let mut duplicate = first.as_ref().clone();

let Some(EndpointMetadata::Switch(switch)) = duplicate.metadata.as_mut() else {
panic!("test endpoint should contain switch metadata");
};

switch.nvlink_domain_uuid = Some(carbide_uuid::nvlink::NvLinkDomainId::new());

let endpoints = [first, Arc::new(duplicate)];
stop_stale_switch_collectors(&mut ctx, &endpoints).await;

ctx.collectors.insert(
CollectorKind::NvueRest,
Cow::Owned(key.clone()),
noop_collector(),
);

stop_stale_switch_collectors(&mut ctx, &endpoints).await;

assert!(ctx.collectors.contains(CollectorKind::NvueRest, &key));
}
}
Loading
Loading