From 9d3381ca8aae2aa5b5fca9d2ce030d2b806bd60f Mon Sep 17 00:00:00 2001 From: Drew Bloechl Date: Fri, 14 Aug 2026 13:22:47 -0700 Subject: [PATCH 1/7] feat(nvue-client): Add types::bgp module --- crates/nvue-client/src/types/bgp.rs | 329 ++++++++++++++++++++++++++++ crates/nvue-client/src/types/mod.rs | 1 + 2 files changed, 330 insertions(+) create mode 100644 crates/nvue-client/src/types/bgp.rs diff --git a/crates/nvue-client/src/types/bgp.rs b/crates/nvue-client/src/types/bgp.rs new file mode 100644 index 0000000000..28242453fa --- /dev/null +++ b/crates/nvue-client/src/types/bgp.rs @@ -0,0 +1,329 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use std::collections::BTreeMap; + +use serde::Deserialize; +use serde_json::Value as JsonValue; + +/// BGP VRF response data. +/// Corresponds to `#/x-defs/cue-show-schema-bgp-vrf-bgp`. +#[derive(Debug, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub struct BgpVrfInfo { + // #/x-defs/schema-feature-isa-feature-default-off-config + pub enable: Option, + + // #/x-defs/schema-bgp-vrf-bgp-config + pub autonomous_system: Option, + pub router_id: Option, + pub rd: Option, + + // #/x-defs/cue-show-schema-bgp-vrf-bgp-config-children + pub address_family: Option, + pub neighbor: Option>, + pub peer_group: Option, + pub path_selection: Option, + pub route_reflection: Option, + pub route_export: Option, + pub route_import: Option, + pub timers: Option, + pub confederation: Option, + pub dynamic_neighbor: Option, + + // #/x-defs/schema-bgp-vrf-bgp-show + pub configured_neighbors: Option, + pub established_neighbors: Option, + + // #/x-defs/cue-show-schema-bgp-vrf-bgp-show-children + pub nexthop: Option, + + // #/x-defs/schema-bgp-bgp-action-children + #[serde(rename = "@clear")] + pub clear: Option, + #[serde(rename = "in")] + pub in_: Option, + pub out: Option, + pub soft: Option, +} + +/// BGP peer response data. +/// Corresponds to `#/x-defs/cue-show-schema-bgp-peer-peer`. +#[derive(Debug, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub struct BgpPeerInfo { + // #/x-defs/schema-bgp-peer-peer-common-config + pub password: Option, + pub enforce_first_as: Option, + pub passive_mode: Option, + pub nexthop_connected_check: Option, + pub multihop_ttl: Option, + pub description: Option, + pub shutdown: Option, + pub update_source: Option, + + // #/x-defs/cue-show-schema-bgp-peer-peer-common-config-children + pub bfd: Option, + pub ttl_security: Option, + pub local_as: Option, + pub timers: Option, + + // #/x-defs/cue-show-schema-bgp-peer-peer-address-family-config-children + pub address_family: Option, + + // #/x-defs/schema-feature-isa-feature-default-on-config + pub enable: Option, + + // #/x-defs/schema-bgp-peer-peer-config + #[serde(rename = "type")] + pub peer_type: Option, + pub peer_group: Option, + pub remote_as: Option, + pub graceful_shutdown: Option, + + // #/x-defs/cue-show-schema-bgp-peer-peer-config-children + pub capabilities: Option, + pub graceful_restart: Option, + + // #/x-defs/schema-bgp-peer-peer-show + pub local_hostname: Option, + pub local_domain: Option, + pub remote_hostname: Option, + pub remote_domain: Option, + pub bgp_version: Option, + pub remote_router_id: Option, + pub state: Option, + pub uptime: Option, + pub uptime_msec: Option, + pub connection_type: Option, + pub connections_established: Option, + pub connections_dropped: Option, + pub last_reset_timer: Option, + pub last_reset_reason: Option, + pub last_reset_code: Option, + pub local_ip: Option, + pub remote_ip: Option, + pub local_port: Option, + pub remote_port: Option, + + // #/x-defs/cue-show-schema-bgp-peer-peer-show-children + pub nexthop: Option, + pub ebgp_policy: Option, + pub message_stats: Option, + + // #/x-defs/schema-bgp-peer-peer-action-children + #[serde(rename = "@clear")] + pub clear: Option, + #[serde(rename = "in")] + pub in_: Option, + pub out: Option, + pub soft: Option, +} + +/// BGP peer address-family response data. +/// Corresponds to `#/x-defs/cue-show-schema-bgp-peer-address-family-address-family`. +#[derive(Debug, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub struct BgpPeerAddressFamilyInfo { + // #/x-defs/cue-show-schema-bgp-peer-address-family-address-family-config-children + pub ipv4_unicast: Option, + pub ipv6_unicast: Option, + pub l2vpn_evpn: Option, +} + +/// BGP peer operational state. +/// Corresponds to the `state` field from `#/x-defs/schema-bgp-peer-peer-show`. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum BgpPeerState { + Idle, + Connect, + Active, + OpenSent, + OpenConfirm, + Established, + Clearing, + Deleted, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn deserializes_dynamic_neighbors_with_strongly_typed_fields() { + let bgp: BgpVrfInfo = serde_json::from_str( + r#" + { + "autonomous-system": 65100, + "configured-neighbors": 1, + "neighbor": { + "192.0.2.10": { + "state": "established", + "peer-group": "underlay-peers", + "address-family": { + "ipv4-unicast": { "enable": "on" }, + "ipv6-unicast": null, + "l2vpn-evpn": { "enable": "off" } + }, + "enforce-first-as": "off", + "connections-established": 4 + } + } + } + "#, + ) + .expect("BGP VRF info should deserialize"); + + assert_eq!(bgp.autonomous_system, Some(JsonValue::from(65100))); + assert_eq!(bgp.configured_neighbors, Some(JsonValue::from(1))); + + let neighbors = bgp.neighbor.expect("neighbor map should deserialize"); + let peer = neighbors + .get("192.0.2.10") + .expect("dynamic peer ID should be preserved"); + assert_eq!(peer.state, Some(BgpPeerState::Established)); + assert_eq!(peer.peer_group.as_deref(), Some("underlay-peers")); + assert_eq!(peer.enforce_first_as, Some(JsonValue::from("off"))); + assert_eq!(peer.connections_established, Some(JsonValue::from(4))); + + let address_family = peer + .address_family + .as_ref() + .expect("peer address-family should deserialize"); + assert!(address_family.ipv4_unicast.is_some()); + assert!(address_family.ipv6_unicast.is_none()); + assert!(address_family.l2vpn_evpn.is_some()); + } + + #[test] + fn deserializes_special_field_names() { + let bgp: BgpVrfInfo = serde_json::from_str( + r#" + { + "@clear": { "state": "running" }, + "in": { "@clear": { "state": "queued" } }, + "out": {}, + "soft": {}, + "neighbor": { + "swp1": { + "@clear": { "state": "complete" }, + "in": {}, + "out": {}, + "soft": {}, + "type": "unnumbered" + } + } + } + "#, + ) + .expect("BGP VRF info should deserialize"); + + assert_eq!( + bgp.clear + .as_ref() + .and_then(|clear| clear.get("state")) + .and_then(JsonValue::as_str), + Some("running") + ); + assert!(bgp.in_.is_some()); + assert!(bgp.out.is_some()); + assert!(bgp.soft.is_some()); + + let peer = bgp + .neighbor + .as_ref() + .and_then(|neighbors| neighbors.get("swp1")) + .expect("peer should deserialize"); + assert!(peer.clear.is_some()); + assert!(peer.in_.is_some()); + assert!(peer.out.is_some()); + assert!(peer.soft.is_some()); + assert_eq!(peer.peer_type, Some(JsonValue::from("unnumbered"))); + } + + #[test] + fn omitted_fields_deserialize_as_none() { + let bgp: BgpVrfInfo = serde_json::from_str(r#"{}"#).expect("empty BGP info should parse"); + + assert!(bgp.neighbor.is_none()); + assert!(bgp.autonomous_system.is_none()); + assert!(bgp.clear.is_none()); + } + + #[test] + fn deserializes_known_peer_states() { + struct Case { + raw: &'static str, + expected: BgpPeerState, + } + + let cases = [ + Case { + raw: "idle", + expected: BgpPeerState::Idle, + }, + Case { + raw: "connect", + expected: BgpPeerState::Connect, + }, + Case { + raw: "active", + expected: BgpPeerState::Active, + }, + Case { + raw: "opensent", + expected: BgpPeerState::OpenSent, + }, + Case { + raw: "openconfirm", + expected: BgpPeerState::OpenConfirm, + }, + Case { + raw: "established", + expected: BgpPeerState::Established, + }, + Case { + raw: "clearing", + expected: BgpPeerState::Clearing, + }, + Case { + raw: "deleted", + expected: BgpPeerState::Deleted, + }, + ]; + + for case in cases { + let peer: BgpPeerInfo = + serde_json::from_str(&format!(r#"{{ "state": "{}" }}"#, case.raw)) + .expect("peer should deserialize"); + + assert_eq!(peer.state, Some(case.expected)); + } + } + + #[test] + fn rejects_unknown_peer_state() { + let error = serde_json::from_str::(r#"{ "state": "future-state" }"#) + .expect_err("unknown peer state should fail deserialization"); + + assert!( + error.to_string().contains("unknown variant"), + "unexpected error: {error}" + ); + } +} diff --git a/crates/nvue-client/src/types/mod.rs b/crates/nvue-client/src/types/mod.rs index 73f015d266..9ade4e997a 100644 --- a/crates/nvue-client/src/types/mod.rs +++ b/crates/nvue-client/src/types/mod.rs @@ -1,3 +1,4 @@ +pub mod bgp; pub mod revision; #[derive(Debug, serde::Deserialize)] From a04a8387a1f9c241aaa4e68b3104cf351f0df8c9 Mon Sep 17 00:00:00 2001 From: Drew Bloechl Date: Fri, 14 Aug 2026 15:00:44 -0700 Subject: [PATCH 2/7] feat(nvue-client): Add client methods to get BGP VRF info --- Cargo.lock | 1 + crates/nvue-client/Cargo.toml | 1 + crates/nvue-client/src/client.rs | 186 +++++++++++++++++++++++++++++++ crates/nvue-client/src/lib.rs | 2 +- 4 files changed, 189 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 9f442533de..7aee50f69e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7981,6 +7981,7 @@ dependencies = [ "serde_yaml", "thiserror 2.0.18", "tokio", + "urlencoding", ] [[package]] diff --git a/crates/nvue-client/Cargo.toml b/crates/nvue-client/Cargo.toml index f5c11f00dc..6ab095a463 100644 --- a/crates/nvue-client/Cargo.toml +++ b/crates/nvue-client/Cargo.toml @@ -36,6 +36,7 @@ serde_json = { workspace = true } serde_yaml = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } +urlencoding = { workspace = true } [dev-dependencies] glob = { workspace = true } diff --git a/crates/nvue-client/src/client.rs b/crates/nvue-client/src/client.rs index 96ff70ab79..5867a90647 100644 --- a/crates/nvue-client/src/client.rs +++ b/crates/nvue-client/src/client.rs @@ -25,8 +25,76 @@ use reqwest::{Client, ClientBuilder, Method, Response, Url}; pub use serde_json::Value as JsonValue; use crate::config::{NvueConfig, NvueConfigWithHeader, NvueRevision}; +use crate::types::bgp::BgpVrfInfo; use crate::types::revision::{RevisionApplyStatus, RevisionData, RevisionIssueSummary}; +/// Repeated NVUE field-selection query parameters. +/// +/// Filter values are JSON Pointer-style paths that start with `/` and may +/// use Unix shell-style wildcards to match dynamic object keys. For example, +/// `/neighbor/*/state` matches the `state` field for every BGP neighbor. Values +/// are passed to NVUE without local validation. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct FieldFilter { + include: Vec, + omit: Vec, +} + +impl FieldFilter { + /// Create an empty filter. + pub fn new() -> Self { + Self::default() + } + + /// Create a filter from `include` field patterns. + pub fn with_includes(fields: I) -> Self + where + I: IntoIterator, + S: Into, + { + Self { + include: fields.into_iter().map(Into::into).collect(), + omit: Vec::new(), + } + } + + /// Create a filter from `omit` field patterns. + pub fn with_omits(fields: I) -> Self + where + I: IntoIterator, + S: Into, + { + Self { + include: Vec::new(), + omit: fields.into_iter().map(Into::into).collect(), + } + } + + /// Add an `include` field pattern. + pub fn include(mut self, field: impl Into) -> Self { + self.include.push(field.into()); + self + } + + /// Add an `omit` field pattern. + pub fn omit(mut self, field: impl Into) -> Self { + self.omit.push(field.into()); + self + } + + fn is_empty(&self) -> bool { + self.include.is_empty() && self.omit.is_empty() + } + + fn query_pairs(&self) -> Vec<(&str, &str)> { + self.include + .iter() + .map(|field| ("include", field.as_str())) + .chain(self.omit.iter().map(|field| ("omit", field.as_str()))) + .collect() + } +} + #[derive(Debug)] pub struct NvueClient { server_address: NvueServerAddress, @@ -129,6 +197,42 @@ impl NvueClient { Ok(nvue_config) } + /// Return BGP data for a VRF. + /// + /// This calls `GET /nvue_v1/vrf/{vrf-id}/router/bgp` without field filters. + /// The `vrf_id` path segment is URL-encoded before the request is built. + pub async fn get_bgp_vrf_info(&self, vrf_id: &str) -> Result { + self.get_bgp_vrf_info_filtered(vrf_id, FieldFilter::new()) + .await + } + + /// Return BGP data for a VRF, applying NVUE field-selection filters. + /// + /// Non-empty filters are encoded as repeated `include` and `omit` query + /// parameters. Field patterns are passed through without local validation. + pub async fn get_bgp_vrf_info_filtered( + &self, + vrf_id: &str, + filter: FieldFilter, + ) -> Result { + let path = format!( + "/nvue_v1/vrf/{encoded_vrf_id}/router/bgp", + encoded_vrf_id = urlencoding::encode(vrf_id), + ); + let mut request = self.request(Method::GET, &path)?.build()?; + + if !filter.is_empty() { + let mut query_pairs = request.url_mut().query_pairs_mut(); + for (key, value) in filter.query_pairs() { + query_pairs.append_pair(key, value); + } + } + + let response = self.execute("get_bgp_vrf_info", request).await?; + let bgp_vrf_info = response.json().await?; + Ok(bgp_vrf_info) + } + /// Create a new NVUE config revision, returning the revision ID. pub async fn create_config_revision(&self) -> Result { const PATH: &str = "/nvue_v1/revision"; @@ -435,3 +539,85 @@ pub struct RequestFailed { #[source] pub source: reqwest::Error, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn field_filter_empty_has_no_query_pairs() { + let filter = FieldFilter::new(); + + assert!(filter.is_empty()); + assert!(filter.query_pairs().is_empty()); + } + + #[test] + fn field_filter_preserves_include_pairs() { + let filter = FieldFilter::new() + .include("/neighbor/*/state") + .include("/neighbor/*/peer-group"); + + assert_eq!( + filter.query_pairs(), + vec![ + ("include", "/neighbor/*/state"), + ("include", "/neighbor/*/peer-group"), + ] + ); + } + + #[test] + fn field_filter_with_includes_builds_include_pairs() { + let filter = FieldFilter::with_includes(["/neighbor/*/state", "/neighbor/*/peer-group"]); + + assert_eq!( + filter.query_pairs(), + vec![ + ("include", "/neighbor/*/state"), + ("include", "/neighbor/*/peer-group"), + ] + ); + } + + #[test] + fn field_filter_preserves_omit_pairs() { + let filter = FieldFilter::new() + .omit("/peer-group") + .omit("/address-family"); + + assert_eq!( + filter.query_pairs(), + vec![("omit", "/peer-group"), ("omit", "/address-family")] + ); + } + + #[test] + fn field_filter_with_omits_builds_omit_pairs() { + let filter = FieldFilter::with_omits(["/peer-group", "/address-family"]); + + assert_eq!( + filter.query_pairs(), + vec![("omit", "/peer-group"), ("omit", "/address-family")] + ); + } + + #[test] + fn field_filter_combines_include_and_omit_pairs() { + let filter = FieldFilter::new() + .include("/neighbor/*/state") + .omit("/peer-group") + .include("/configured-neighbors") + .omit("/address-family"); + + assert_eq!( + filter.query_pairs(), + vec![ + ("include", "/neighbor/*/state"), + ("include", "/configured-neighbors"), + ("omit", "/peer-group"), + ("omit", "/address-family"), + ] + ); + } +} diff --git a/crates/nvue-client/src/lib.rs b/crates/nvue-client/src/lib.rs index 3408925edf..9f59eadede 100644 --- a/crates/nvue-client/src/lib.rs +++ b/crates/nvue-client/src/lib.rs @@ -19,5 +19,5 @@ pub mod client; pub mod config; pub mod types; -pub use client::NvueClient; +pub use client::{FieldFilter, NvueClient}; pub use config::NvueConfig; From 184dcc5ae8fa4f273b7181098d86531affa9366d Mon Sep 17 00:00:00 2001 From: Drew Bloechl Date: Fri, 14 Aug 2026 16:25:07 -0700 Subject: [PATCH 3/7] feat(agent): Add health::nvue::check_bgp_uplink_sessions --- crates/agent/src/health.rs | 1 + crates/agent/src/health/nvue.rs | 352 ++++++++++++++++++++++++++++++++ 2 files changed, 353 insertions(+) create mode 100644 crates/agent/src/health/nvue.rs diff --git a/crates/agent/src/health.rs b/crates/agent/src/health.rs index b5b5ff8ff7..ca489760b3 100644 --- a/crates/agent/src/health.rs +++ b/crates/agent/src/health.rs @@ -28,6 +28,7 @@ use tokio::time::timeout; use crate::{HBNDeviceNames, hbn}; mod bgp; +mod nvue; mod probe_ids; const HBN_DAEMONS_FILE: &str = "etc/frr/daemons"; diff --git a/crates/agent/src/health/nvue.rs b/crates/agent/src/health/nvue.rs new file mode 100644 index 0000000000..057659b71a --- /dev/null +++ b/crates/agent/src/health/nvue.rs @@ -0,0 +1,352 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use std::collections::BTreeMap; + +use health_report::HealthReport; +use nvue_client::types::bgp::{BgpPeerInfo, BgpPeerState, BgpVrfInfo}; + +use super::{failed, make_alert, probe_ids}; +use crate::HBNDeviceNames; + +/// Checks configured ToR BGP sessions from an already-fetched NVUE BGP VRF response. +/// +/// All configured HBN uplinks are evaluated, and the check passes when at least +/// `min_healthy_links` of them are present and established. If too few uplinks +/// are healthy, each missing peer, missing state, or non-established state emits +/// a `BgpPeeringTor` alert targeted at that uplink. A `BgpPeeringTor` alert is +/// emitted when `min_healthy_links` asks for more uplinks than the configured +/// device names provide. The helper does not emit success entries. +#[cfg_attr( + not(test), + expect( + dead_code, + reason = "Reusable NVUE BGP health helper is intentionally not wired yet" + ) +)] +pub(super) fn check_bgp_uplink_sessions( + report: &mut HealthReport, + bgp: &BgpVrfInfo, + min_healthy_links: usize, + hbn_device_names: &HBNDeviceNames, +) { + let mut unhealthy_uplink_names = Vec::new(); + let mut healthy_uplink_count = 0; + let expected_hbn_uplinks = hbn_device_names.uplinks.iter().copied(); + + for expected_uplink in expected_hbn_uplinks { + match check_expected_peer_established(bgp.neighbor.as_ref(), expected_uplink) { + Ok(()) => healthy_uplink_count += 1, + Err(message) => unhealthy_uplink_names.push((expected_uplink.to_string(), message)), + } + } + + if healthy_uplink_count >= min_healthy_links { + return; + } + + if min_healthy_links > hbn_device_names.uplinks.len() { + failed( + report, + probe_ids::BgpPeeringTor.clone(), + None, + format!( + "Site configuration requires a minimum of {min_healthy_links} \ + healthy uplinks, but this is greater than the number of \ + expected uplink interface names ({hbn_uplink_names})", + hbn_uplink_names = hbn_device_names.uplinks.join(",") + ), + ); + } + + // This behavior differs from what the non-NVUE code path does (in + // BgpHealthData::into_health_report()), in that we always treat this + // case as critical. I couldn't make sense of why the other path computes + // criticality like this: + // + // let num_unhealthy_tors = self.unhealthy_tor_peers.len(); + // // TODO: This is correct for environments with both DPU ports connected + // let unhealthy_tors_critical = num_unhealthy_tors > 1; + // + // This looks like a duplication of what I think min_healthy_links is + // concerned with, and also seems to hardcode an assumption about uplink + // count. + // + // It was all added before The Big Squash so I couldn't learn anything from + // the git history of the file. + // - drew + let is_critical = true; + for (uplink, message) in unhealthy_uplink_names { + report.alerts.push(make_alert( + probe_ids::BgpPeeringTor.clone(), + Some(uplink), + message, + is_critical, + )); + } +} + +/// Checks whether an expected peer has an established NVUE BGP session. +/// +/// Returns `Ok(())` when the peer exists in the NVUE neighbor map and reports +/// `BgpPeerState::Established`. A missing NVUE neighbor map, missing peer, +/// missing state, or non-established state returns a descriptive error message +/// for conversion to a health alert by the caller. +#[cfg_attr( + not(test), + expect( + dead_code, + reason = "Reusable NVUE BGP health helper is intentionally not wired yet" + ) +)] +fn check_expected_peer_established( + neighbors: Option<&BTreeMap>, + peer_name: &str, +) -> Result<(), String> { + let Some(neighbors) = neighbors else { + return Err(format!( + "BGP neighbor data was not reported; expected session for {peer_name}" + )); + }; + let Some(peer) = neighbors.get(peer_name) else { + return Err(format!( + "expected session for {peer_name} was not found in BGP peer data" + )); + }; + let Some(peer_state) = peer.state.as_ref() else { + return Err(format!("state field for {peer_name} peer is not present")); + }; + match peer_state { + BgpPeerState::Established => Ok(()), + state => Err(format!( + "BGP session {peer_name} is not Established, but in state {state:?}" + )), + } +} + +#[cfg(test)] +mod tests { + use carbide_test_support::{Check, check_values}; + + use super::*; + + /// One `check_bgp_uplink_sessions` scenario and its exact expected alerts. + struct Row { + scenario: &'static str, + bgp_json: &'static str, + min_healthy_links: usize, + expected_alerts: Vec, + } + + /// Builds a NVUE BGP VRF response from a compact scenario JSON fixture. + fn bgp_vrf_info(bgp_json: &str) -> BgpVrfInfo { + serde_json::from_str(bgp_json).expect("BGP VRF info should deserialize") + } + + /// Builds a ToR peering alert with the same target and criticality rules as production. + fn tor_alert(port: &str, message: &str, critical: bool) -> health_report::HealthProbeAlert { + make_alert( + probe_ids::BgpPeeringTor.clone(), + Some(port.to_string()), + message.to_string(), + critical, + ) + } + + /// Builds the site-configuration alert for impossible uplink thresholds. + fn config_mismatch_alert(min_healthy_links: usize) -> health_report::HealthProbeAlert { + make_alert( + probe_ids::BgpPeeringTor.clone(), + None, + format!( + "Site configuration requires a minimum of {min_healthy_links} healthy uplinks, but this is greater than the number of expected uplink interface names (p0_if,p1_if)" + ), + true, + ) + } + + /// Orders alerts by `(id, target, message)` so table rows can stay readable. + fn sort_alerts(alerts: &mut [health_report::HealthProbeAlert]) { + alerts.sort_by(|a, b| (&a.id, &a.target, &a.message).cmp(&(&b.id, &b.target, &b.message))); + } + + #[test] + fn check_bgp_tor_sessions_emits_expected_alerts() { + check_values( + [ + Row { + scenario: "all configured uplinks established", + bgp_json: r#" + { + "neighbor": { + "p0_if": { "state": "established" }, + "p1_if": { "state": "established" } + } + } + "#, + min_healthy_links: 2, + expected_alerts: vec![], + }, + Row { + scenario: "missing neighbor map alerts for each configured uplink", + bgp_json: r#"{}"#, + min_healthy_links: 2, + expected_alerts: vec![ + tor_alert( + "p0_if", + "BGP neighbor data was not reported; expected session for p0_if", + true, + ), + tor_alert( + "p1_if", + "BGP neighbor data was not reported; expected session for p1_if", + true, + ), + ], + }, + Row { + scenario: "missing configured uplink targets that uplink", + bgp_json: r#" + { + "neighbor": { + "p0_if": { "state": "established" } + } + } + "#, + min_healthy_links: 2, + expected_alerts: vec![tor_alert( + "p1_if", + "expected session for p1_if was not found in BGP peer data", + true, + )], + }, + Row { + scenario: "idle configured uplink alerts when below threshold", + bgp_json: r#" + { + "neighbor": { + "p0_if": { "state": "idle" }, + "p1_if": { "state": "established" } + } + } + "#, + min_healthy_links: 2, + expected_alerts: vec![tor_alert( + "p0_if", + "BGP session p0_if is not Established, but in state Idle", + true, + )], + }, + Row { + scenario: "another non-established state alerts", + bgp_json: r#" + { + "neighbor": { + "p0_if": { "state": "active" }, + "p1_if": { "state": "established" } + } + } + "#, + min_healthy_links: 2, + expected_alerts: vec![tor_alert( + "p0_if", + "BGP session p0_if is not Established, but in state Active", + true, + )], + }, + Row { + scenario: "missing state alerts", + bgp_json: r#" + { + "neighbor": { + "p0_if": {}, + "p1_if": { "state": "established" } + } + } + "#, + min_healthy_links: 2, + expected_alerts: vec![tor_alert( + "p0_if", + "state field for p0_if peer is not present", + true, + )], + }, + Row { + scenario: "extra non-ToR and route-server neighbors ignored", + bgp_json: r#" + { + "neighbor": { + "p0_if": { "state": "established" }, + "p1_if": { "state": "established" }, + "tenant-vrf-peer": { "state": "idle" }, + "10.217.126.67": { "state": "active" } + } + } + "#, + min_healthy_links: 2, + expected_alerts: vec![], + }, + Row { + scenario: "one healthy uplink may be the second configured uplink", + bgp_json: r#" + { + "neighbor": { + "p0_if": { "state": "idle" }, + "p1_if": { "state": "established" } + } + } + "#, + min_healthy_links: 1, + expected_alerts: vec![], + }, + Row { + scenario: "min healthy links greater than configured uplinks alerts", + bgp_json: r#" + { + "neighbor": { + "p0_if": { "state": "established" }, + "p1_if": { "state": "established" } + } + } + "#, + min_healthy_links: 3, + expected_alerts: vec![config_mismatch_alert(3)], + }, + ] + .map(|mut row| { + sort_alerts(&mut row.expected_alerts); + let expect = row.expected_alerts.clone(); + Check { + scenario: row.scenario, + input: row, + expect, + } + }), + |row| { + let mut hr = HealthReport::empty("forge-dpu-agent".to_string()); + check_bgp_uplink_sessions( + &mut hr, + &bgp_vrf_info(row.bgp_json), + row.min_healthy_links, + &HBNDeviceNames::hbn_23(), + ); + sort_alerts(&mut hr.alerts); + hr.alerts + }, + ); + } +} From bc978413a5926f2c18cebeccd116b37d2aa85774 Mon Sep 17 00:00:00 2001 From: Drew Bloechl Date: Mon, 17 Aug 2026 13:26:31 -0700 Subject: [PATCH 4/7] feat(agent): Wire up NVUE BGP uplink health checks into main loop --- crates/agent/src/health.rs | 23 +------ crates/agent/src/health/nvue.rs | 93 +++++++++++++++++++++++----- crates/agent/src/health/probe_ids.rs | 2 +- crates/agent/src/main_loop.rs | 10 ++- 4 files changed, 91 insertions(+), 37 deletions(-) diff --git a/crates/agent/src/health.rs b/crates/agent/src/health.rs index ca489760b3..87c61917b1 100644 --- a/crates/agent/src/health.rs +++ b/crates/agent/src/health.rs @@ -21,14 +21,13 @@ use std::path::Path; use std::str::FromStr; use std::time::Duration; -use health_report::{HealthProbeId, HealthReport}; -use nvue_client::NvueClient; +use health_report::HealthProbeId; use tokio::process::Command as TokioCommand; use tokio::time::timeout; use crate::{HBNDeviceNames, hbn}; mod bgp; -mod nvue; +pub(crate) mod nvue; mod probe_ids; const HBN_DAEMONS_FILE: &str = "etc/frr/daemons"; @@ -111,7 +110,7 @@ pub(super) fn is_up(health_report: &health_report::HealthReport) -> bool { let nvue_api_up = health_report .successes .iter() - .any(|s| s.id == *probe_ids::NvueApiRunning); + .any(|s| s.id == *probe_ids::NvueApi); let hbn_healthy = (container_exists && supervisord_up) || nvue_api_up; hbn_healthy && !has_failed_services } @@ -728,22 +727,6 @@ enum SctlState { Fatal, } -pub(super) async fn nvue_api_health(nvue_client: &NvueClient) -> HealthReport { - // All we can really do here is check that the API is alive. The HBN flavor of NVUE - // doesn't seem to expose much of anything that we can look at for node health. - let mut report = HealthReport::empty("forge-dpu-agent".into()); - match nvue_client.system_info().await { - Ok(_) => passed(&mut report, probe_ids::NvueApiRunning.clone(), None), - Err(e) => failed( - &mut report, - probe_ids::NvueApiRunning.clone(), - None, - format!("Error communicating with NVUE API: {e}"), - ), - } - report -} - #[cfg(test)] mod tests { use carbide_test_support::Outcome::*; diff --git a/crates/agent/src/health/nvue.rs b/crates/agent/src/health/nvue.rs index 057659b71a..7bf43942e8 100644 --- a/crates/agent/src/health/nvue.rs +++ b/crates/agent/src/health/nvue.rs @@ -17,12 +17,89 @@ use std::collections::BTreeMap; -use health_report::HealthReport; +use health_report::{HealthProbeAlert, HealthProbeSuccess, HealthReport}; use nvue_client::types::bgp::{BgpPeerInfo, BgpPeerState, BgpVrfInfo}; +use nvue_client::{FieldFilter, NvueClient}; use super::{failed, make_alert, probe_ids}; use crate::HBNDeviceNames; +/// The VRF we'll look for our BGP uplinks in. +const BGP_VRF_UPLINKS: &str = "default"; + +/// Health check configuration for NVUE API targets. +pub(crate) struct NvueHealthCheck<'a> { + /// NVUE client used for API availability checks and BGP queries. + pub(crate) nvue_client: &'a NvueClient, + /// Minimum number of configured ToR uplink sessions that must be established. + pub(crate) min_healthy_links: usize, + /// HBN interface names used to identify expected ToR uplink sessions. + pub(crate) hbn_device_names: &'a HBNDeviceNames, +} + +impl NvueHealthCheck<'_> { + /// Performs all health checks against the NVUE API. + pub(crate) async fn health_check(&self) -> HealthReport { + let mut report = HealthReport::empty("forge-dpu-agent".into()); + + match self.nvue_api_health().await { + Ok(success) => report.successes.push(success), + Err(alert) => { + // If the NVUE API wasn't healthy, we can't use it to check + // anything else. + report.alerts.push(alert); + return report; + } + } + + self.check_bgp_uplinks(&mut report).await; + report + } + + /// Checks BGP uplink session health through the configured NVUE API target. + async fn check_bgp_uplinks(&self, report: &mut HealthReport) { + const BGP_NEIGHBOR_STATE_FIELD: &str = "/neighbor/*/state"; + + let bgp_vrf_info = self + .nvue_client + .get_bgp_vrf_info_filtered( + BGP_VRF_UPLINKS, + FieldFilter::with_includes([BGP_NEIGHBOR_STATE_FIELD]), + ) + .await; + match bgp_vrf_info.as_ref() { + Ok(bgp_vrf_info) => check_bgp_uplink_sessions( + report, + bgp_vrf_info, + self.min_healthy_links, + self.hbn_device_names, + ), + Err(error) => failed( + report, + probe_ids::BgpPeeringTor.clone(), + None, + format!("Error fetching NVUE BGP data for VRF {BGP_VRF_UPLINKS}: {error}"), + ), + } + } + + /// Checks whether the NVUE API can answer a basic system-information request. + async fn nvue_api_health(&self) -> Result { + match self.nvue_client.system_info().await { + Ok(_) => Ok(HealthProbeSuccess { + id: probe_ids::NvueApi.clone(), + target: None, + }), + Err(e) => Err(make_alert( + probe_ids::NvueApi.clone(), + None, + format!("Error communicating with NVUE API: {e}"), + true, + )), + } + } +} + /// Checks configured ToR BGP sessions from an already-fetched NVUE BGP VRF response. /// /// All configured HBN uplinks are evaluated, and the check passes when at least @@ -31,13 +108,6 @@ use crate::HBNDeviceNames; /// a `BgpPeeringTor` alert targeted at that uplink. A `BgpPeeringTor` alert is /// emitted when `min_healthy_links` asks for more uplinks than the configured /// device names provide. The helper does not emit success entries. -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Reusable NVUE BGP health helper is intentionally not wired yet" - ) -)] pub(super) fn check_bgp_uplink_sessions( report: &mut HealthReport, bgp: &BgpVrfInfo, @@ -106,13 +176,6 @@ pub(super) fn check_bgp_uplink_sessions( /// `BgpPeerState::Established`. A missing NVUE neighbor map, missing peer, /// missing state, or non-established state returns a descriptive error message /// for conversion to a health alert by the caller. -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Reusable NVUE BGP health helper is intentionally not wired yet" - ) -)] fn check_expected_peer_established( neighbors: Option<&BTreeMap>, peer_name: &str, diff --git a/crates/agent/src/health/probe_ids.rs b/crates/agent/src/health/probe_ids.rs index 5fbd15069a..acf4afff65 100644 --- a/crates/agent/src/health/probe_ids.rs +++ b/crates/agent/src/health/probe_ids.rs @@ -35,5 +35,5 @@ lazy_static::lazy_static! { pub static ref PostConfigCheckWait: HealthProbeId = "PostConfigCheckWait".parse().unwrap(); pub static ref DpuDiskUtilizationCheck: HealthProbeId = "DpuDiskUtilizationCheck".parse().unwrap(); pub static ref DpuDiskUtilizationCritical: HealthProbeId = "DpuDiskUtilizationCritical".parse().unwrap(); - pub static ref NvueApiRunning: HealthProbeId = "NvueApiRunning".parse().unwrap(); + pub static ref NvueApi: HealthProbeId = "NvueApi".parse().unwrap(); } diff --git a/crates/agent/src/main_loop.rs b/crates/agent/src/main_loop.rs index 79c6b2a4c1..23885b90cf 100644 --- a/crates/agent/src/main_loop.rs +++ b/crates/agent/src/main_loop.rs @@ -1145,7 +1145,15 @@ impl MainLoop { }) .await } - Some(nvue_context) => health::nvue_api_health(&nvue_context.nvue_client).await, + Some(nvue_context) => { + health::nvue::NvueHealthCheck { + nvue_client: &nvue_context.nvue_client, + min_healthy_links: conf.min_dpu_functioning_links.unwrap_or(2) as usize, + hbn_device_names: &self.hbn_device_names, + } + .health_check() + .await + } }; is_healthy = !health_report.successes.is_empty() && health_report.alerts.is_empty(); self.is_hbn_up = health::is_up(&health_report); From 7b06170b97296418fd14297b0de6d22bd7ca68ef Mon Sep 17 00:00:00 2001 From: Drew Bloechl Date: Mon, 17 Aug 2026 16:37:40 -0700 Subject: [PATCH 5/7] fix(nvue-client): Give requests a 60s timeout --- crates/nvue-client/src/client.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/nvue-client/src/client.rs b/crates/nvue-client/src/client.rs index 5867a90647..bc045de4d7 100644 --- a/crates/nvue-client/src/client.rs +++ b/crates/nvue-client/src/client.rs @@ -146,6 +146,8 @@ impl NvueClient { ) -> Result { let url = self.construct_url_string(path); let builder = self.client.request(method, url); + // TODO: Make this timeout configurable. + let builder = builder.timeout(std::time::Duration::from_secs(60)); let builder = match self.auth_creds() { Some(creds) => builder.basic_auth(&creds.username, Some(&creds.password)), None => builder, From 98eacf2451c0c1a27227d33c2a71271d84c9ea38 Mon Sep 17 00:00:00 2001 From: Drew Bloechl Date: Mon, 17 Aug 2026 16:40:13 -0700 Subject: [PATCH 6/7] fix(nvue-client): Implement Display for BgpPeerState --- crates/agent/src/health/nvue.rs | 7 ++++--- crates/nvue-client/src/types/bgp.rs | 25 ++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/crates/agent/src/health/nvue.rs b/crates/agent/src/health/nvue.rs index 7bf43942e8..a25cd92b58 100644 --- a/crates/agent/src/health/nvue.rs +++ b/crates/agent/src/health/nvue.rs @@ -193,10 +193,11 @@ fn check_expected_peer_established( let Some(peer_state) = peer.state.as_ref() else { return Err(format!("state field for {peer_name} peer is not present")); }; + let expected_state = BgpPeerState::Established; match peer_state { BgpPeerState::Established => Ok(()), state => Err(format!( - "BGP session {peer_name} is not Established, but in state {state:?}" + "BGP session {peer_name} is not {expected_state}, but in state {state}" )), } } @@ -310,7 +311,7 @@ mod tests { min_healthy_links: 2, expected_alerts: vec![tor_alert( "p0_if", - "BGP session p0_if is not Established, but in state Idle", + "BGP session p0_if is not established, but in state idle", true, )], }, @@ -327,7 +328,7 @@ mod tests { min_healthy_links: 2, expected_alerts: vec![tor_alert( "p0_if", - "BGP session p0_if is not Established, but in state Active", + "BGP session p0_if is not established, but in state active", true, )], }, diff --git a/crates/nvue-client/src/types/bgp.rs b/crates/nvue-client/src/types/bgp.rs index 28242453fa..33566406e2 100644 --- a/crates/nvue-client/src/types/bgp.rs +++ b/crates/nvue-client/src/types/bgp.rs @@ -160,6 +160,27 @@ pub enum BgpPeerState { Deleted, } +impl BgpPeerState { + const fn as_str(&self) -> &'static str { + match self { + Self::Idle => "idle", + Self::Connect => "connect", + Self::Active => "active", + Self::OpenSent => "opensent", + Self::OpenConfirm => "openconfirm", + Self::Established => "established", + Self::Clearing => "clearing", + Self::Deleted => "deleted", + } + } +} + +impl std::fmt::Display for BgpPeerState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + #[cfg(test)] mod tests { use super::*; @@ -312,7 +333,9 @@ mod tests { serde_json::from_str(&format!(r#"{{ "state": "{}" }}"#, case.raw)) .expect("peer should deserialize"); - assert_eq!(peer.state, Some(case.expected)); + let state = peer.state.expect("state should deserialize"); + assert_eq!(state, case.expected); + assert_eq!(state.to_string(), case.raw); } } From f28c68a906d698b218886cb66f132f82c88f8dd1 Mon Sep 17 00:00:00 2001 From: Drew Bloechl Date: Mon, 17 Aug 2026 16:53:05 -0700 Subject: [PATCH 7/7] fixup! feat(agent): Wire up NVUE BGP uplink health checks into main loop --- crates/agent/src/health.rs | 2 +- crates/agent/src/health/nvue.rs | 4 ++-- crates/agent/src/health/probe_ids.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/agent/src/health.rs b/crates/agent/src/health.rs index 87c61917b1..e0b2b65fdc 100644 --- a/crates/agent/src/health.rs +++ b/crates/agent/src/health.rs @@ -110,7 +110,7 @@ pub(super) fn is_up(health_report: &health_report::HealthReport) -> bool { let nvue_api_up = health_report .successes .iter() - .any(|s| s.id == *probe_ids::NvueApi); + .any(|s| s.id == *probe_ids::NvueApiRunning); let hbn_healthy = (container_exists && supervisord_up) || nvue_api_up; hbn_healthy && !has_failed_services } diff --git a/crates/agent/src/health/nvue.rs b/crates/agent/src/health/nvue.rs index a25cd92b58..9d8c64fc52 100644 --- a/crates/agent/src/health/nvue.rs +++ b/crates/agent/src/health/nvue.rs @@ -87,11 +87,11 @@ impl NvueHealthCheck<'_> { async fn nvue_api_health(&self) -> Result { match self.nvue_client.system_info().await { Ok(_) => Ok(HealthProbeSuccess { - id: probe_ids::NvueApi.clone(), + id: probe_ids::NvueApiRunning.clone(), target: None, }), Err(e) => Err(make_alert( - probe_ids::NvueApi.clone(), + probe_ids::NvueApiRunning.clone(), None, format!("Error communicating with NVUE API: {e}"), true, diff --git a/crates/agent/src/health/probe_ids.rs b/crates/agent/src/health/probe_ids.rs index acf4afff65..5fbd15069a 100644 --- a/crates/agent/src/health/probe_ids.rs +++ b/crates/agent/src/health/probe_ids.rs @@ -35,5 +35,5 @@ lazy_static::lazy_static! { pub static ref PostConfigCheckWait: HealthProbeId = "PostConfigCheckWait".parse().unwrap(); pub static ref DpuDiskUtilizationCheck: HealthProbeId = "DpuDiskUtilizationCheck".parse().unwrap(); pub static ref DpuDiskUtilizationCritical: HealthProbeId = "DpuDiskUtilizationCritical".parse().unwrap(); - pub static ref NvueApi: HealthProbeId = "NvueApi".parse().unwrap(); + pub static ref NvueApiRunning: HealthProbeId = "NvueApiRunning".parse().unwrap(); }