From adc14c052d05b2ba6dd4a70d96f206f57ab9661a Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Tue, 4 Nov 2025 13:38:28 -0500 Subject: [PATCH 01/11] fix(host_network): retrieve proper hostname and next available bond id --- harmony/src/domain/topology/ha_cluster.rs | 105 ++++++--- harmony/src/domain/topology/k8s.rs | 59 +++++- harmony/src/modules/inventory/discovery.rs | 11 +- harmony/src/modules/okd/crd/nmstate.rs | 234 +++++++++++++++++++-- opnsense-config-xml/src/data/interfaces.rs | 2 +- 5 files changed, 356 insertions(+), 55 deletions(-) diff --git a/harmony/src/domain/topology/ha_cluster.rs b/harmony/src/domain/topology/ha_cluster.rs index d65d9aa..88045ae 100644 --- a/harmony/src/domain/topology/ha_cluster.rs +++ b/harmony/src/domain/topology/ha_cluster.rs @@ -1,14 +1,19 @@ use async_trait::async_trait; use harmony_macros::ip; use harmony_types::{ + id::Id, net::{MacAddress, Url}, switch::PortLocation, }; -use kube::api::ObjectMeta; +use k8s_openapi::api::core::v1::Node; +use kube::{ + ResourceExt, + api::{ObjectList, ObjectMeta}, +}; use log::debug; use log::info; -use crate::modules::okd::crd::nmstate::{self, NodeNetworkConfigurationPolicy}; +use crate::modules::okd::crd::nmstate::{self, NodeNetworkConfigurationPolicy, NodeNetworkState}; use crate::topology::PxeOptions; use crate::{data::FileContent, modules::okd::crd::nmstate::NMState}; use crate::{ @@ -63,7 +68,7 @@ impl K8sclient for HAClusterTopology { K8sClient::try_default().await.map_err(|e| e.to_string())?, )), Some(kubeconfig) => { - let Some(client) = K8sClient::from_kubeconfig(&kubeconfig).await else { + let Some(client) = K8sClient::from_kubeconfig(kubeconfig).await else { return Err("Failed to create k8s client".to_string()); }; Ok(Arc::new(client)) @@ -152,8 +157,19 @@ impl HAClusterTopology { Ok(()) } - fn get_next_bond_id(&self) -> u8 { - 42 // FIXME: Find a better way to declare the bond id + async fn get_next_bond_id(&self, hostname: &str) -> Result { + let network_state: Option = self + .k8s_client() + .await + .unwrap() + .get_resource(hostname, None) + .await + .map_err(|e| format!("Failed to list nodes: {e}"))?; + + println!("HELLLOOOO NETWORK STATE: {network_state:#?}"); + + let bond_id = 42; // FIXME: Find a better way to declare the bond id + Ok(format!("bond{bond_id}")) } async fn configure_bond(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { @@ -165,7 +181,20 @@ impl HAClusterTopology { )) })?; - let bond_config = self.create_bond_configuration(config); + let hostname = self.get_hostname(&config.host_id).await.map_err(|e| { + SwitchError::new(format!( + "Can't configure bond, can't get hostname for host '{}': {e}", + config.host_id + )) + })?; + let bond_id = self.get_next_bond_id(&hostname).await.map_err(|e| { + SwitchError::new(format!( + "Can't configure bond, can't get an available bond id for host '{}': {e}", + config.host_id + )) + })?; + let bond_config = self.create_bond_configuration(&hostname, &bond_id, config); + debug!( "Applying NMState bond config for host {}: {bond_config:#?}", config.host_id @@ -182,23 +211,21 @@ impl HAClusterTopology { fn create_bond_configuration( &self, + host: &str, + bond_name: &str, config: &HostNetworkConfig, ) -> NodeNetworkConfigurationPolicy { - let host_name = &config.host_id; - let bond_id = self.get_next_bond_id(); - let bond_name = format!("bond{bond_id}"); - - info!("Configuring bond '{bond_name}' for host '{host_name}'..."); + info!("Configuring bond '{bond_name}' for host '{host}'..."); let mut bond_mtu: Option = None; let mut copy_mac_from: Option = None; let mut bond_ports = Vec::new(); - let mut interfaces: Vec = Vec::new(); + let mut interfaces: Vec = Vec::new(); for switch_port in &config.switch_ports { let interface_name = switch_port.interface.name.clone(); - interfaces.push(nmstate::InterfaceSpec { + interfaces.push(nmstate::Interface { name: interface_name.clone(), description: Some(format!("Member of bond {bond_name}")), r#type: "ethernet".to_string(), @@ -228,9 +255,9 @@ impl HAClusterTopology { } } - interfaces.push(nmstate::InterfaceSpec { - name: bond_name.clone(), - description: Some(format!("Network bond for host {host_name}")), + interfaces.push(nmstate::Interface { + name: bond_name.to_string(), + description: Some(format!("Network bond for host {host}")), r#type: "bond".to_string(), state: "up".to_string(), copy_mac_from, @@ -255,19 +282,49 @@ impl HAClusterTopology { NodeNetworkConfigurationPolicy { metadata: ObjectMeta { - name: Some(format!("{host_name}-bond-config")), + name: Some(format!("{host}-bond-config")), ..Default::default() }, spec: NodeNetworkConfigurationPolicySpec { node_selector: Some(BTreeMap::from([( "kubernetes.io/hostname".to_string(), - host_name.to_string(), + host.to_string(), )])), - desired_state: nmstate::DesiredStateSpec { interfaces }, + desired_state: nmstate::NetworkState { + interfaces, + ..Default::default() + }, }, } } + async fn get_hostname(&self, host_id: &Id) -> Result { + let nodes: ObjectList = self + .k8s_client() + .await + .unwrap() + .list_resources(None, None) + .await + .map_err(|e| format!("Failed to list nodes: {e}"))?; + + let Some(node) = nodes.iter().find(|n| { + n.status + .as_ref() + .and_then(|s| s.node_info.as_ref()) + .map(|i| i.system_uuid == host_id.to_string()) + .unwrap_or(false) + }) else { + return Err(format!("No node found for host '{host_id}'")); + }; + + node.labels() + .get("kubernetes.io/hostname") + .ok_or(format!( + "Node '{host_id}' has no kubernetes.io/hostname label" + )) + .cloned() + } + async fn configure_port_channel(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { debug!("Configuring port channel: {config:#?}"); let switch_ports = config.switch_ports.iter().map(|s| s.port.clone()).collect(); @@ -458,21 +515,19 @@ impl HttpServer for HAClusterTopology { #[async_trait] impl Switch for HAClusterTopology { async fn setup_switch(&self) -> Result<(), SwitchError> { - self.switch_client.setup().await?; - Ok(()) + self.switch_client.setup().await.map(|_| ()) } async fn get_port_for_mac_address( &self, mac_address: &MacAddress, ) -> Result, SwitchError> { - let port = self.switch_client.find_port(mac_address).await?; - Ok(port) + self.switch_client.find_port(mac_address).await } async fn configure_host_network(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { - self.configure_bond(config).await?; - self.configure_port_channel(config).await + self.configure_bond(config).await + // self.configure_port_channel(config).await } } diff --git a/harmony/src/domain/topology/k8s.rs b/harmony/src/domain/topology/k8s.rs index 71129e1..03c59e1 100644 --- a/harmony/src/domain/topology/k8s.rs +++ b/harmony/src/domain/topology/k8s.rs @@ -5,13 +5,15 @@ use k8s_openapi::{ ClusterResourceScope, NamespaceResourceScope, api::{ apps::v1::Deployment, - core::v1::{Pod, ServiceAccount}, + core::v1::{Node, Pod, ServiceAccount}, }, apimachinery::pkg::version::Info, }; use kube::{ Client, Config, Discovery, Error, Resource, - api::{Api, AttachParams, DeleteParams, ListParams, Patch, PatchParams, ResourceExt}, + api::{ + Api, AttachParams, DeleteParams, ListParams, ObjectList, Patch, PatchParams, ResourceExt, + }, config::{KubeConfigOptions, Kubeconfig}, core::ErrorResponse, discovery::{ApiCapabilities, Scope}, @@ -564,7 +566,58 @@ impl K8sClient { Ok(()) } - pub(crate) async fn from_kubeconfig(path: &str) -> Option { + /// Gets a single named resource of a specific type `K`. + /// + /// This function uses the `ApplyStrategy` trait to correctly determine + /// whether to look in a specific namespace or in the entire cluster. + /// + /// Returns `Ok(None)` if the resource is not found (404). + pub async fn get_resource( + &self, + name: &str, + namespace: Option<&str>, + ) -> Result, Error> + where + K: Resource + Clone + std::fmt::Debug + DeserializeOwned, + ::Scope: ApplyStrategy, + ::DynamicType: Default, + { + let api: Api = + <::Scope as ApplyStrategy>::get_api(&self.client, namespace); + + api.get_opt(name).await + } + + /// Lists all resources of a specific type `K`. + /// + /// This function uses the `ApplyStrategy` trait to correctly determine + /// whether to list from a specific namespace or from the entire cluster. + pub async fn list_resources( + &self, + namespace: Option<&str>, + list_params: Option, + ) -> Result, Error> + where + K: Resource + Clone + std::fmt::Debug + DeserializeOwned, + ::Scope: ApplyStrategy, + ::DynamicType: Default, + { + let api: Api = + <::Scope as ApplyStrategy>::get_api(&self.client, namespace); + + let list_params = list_params.unwrap_or_default(); + api.list(&list_params).await + } + + /// Fetches a list of all Nodes in the cluster. + pub async fn get_nodes( + &self, + list_params: Option, + ) -> Result, Error> { + self.list_resources(None, list_params).await + } + + pub async fn from_kubeconfig(path: &str) -> Option { let k = match Kubeconfig::read_from(path) { Ok(k) => k, Err(e) => { diff --git a/harmony/src/modules/inventory/discovery.rs b/harmony/src/modules/inventory/discovery.rs index 143c56a..b02078b 100644 --- a/harmony/src/modules/inventory/discovery.rs +++ b/harmony/src/modules/inventory/discovery.rs @@ -74,7 +74,11 @@ impl Interpret for DiscoverHostForRoleInterpret { match ans { Ok(choice) => { - info!("Selected {} as the bootstrap node.", choice.summary()); + info!( + "Selected {} as the {:?} node.", + choice.summary(), + self.score.role + ); host_repo .save_role_mapping(&self.score.role, &choice) .await?; @@ -90,10 +94,7 @@ impl Interpret for DiscoverHostForRoleInterpret { "Failed to select node for role {:?} : {}", self.score.role, e ); - return Err(InterpretError::new(format!( - "Could not select host : {}", - e.to_string() - ))); + return Err(InterpretError::new(format!("Could not select host : {e}"))); } } } diff --git a/harmony/src/modules/okd/crd/nmstate.rs b/harmony/src/modules/okd/crd/nmstate.rs index 9f986e5..3c4955e 100644 --- a/harmony/src/modules/okd/crd/nmstate.rs +++ b/harmony/src/modules/okd/crd/nmstate.rs @@ -53,18 +53,193 @@ pub struct ProbeDns { pub struct NodeNetworkConfigurationPolicySpec { #[serde(skip_serializing_if = "Option::is_none")] pub node_selector: Option>, - pub desired_state: DesiredStateSpec, + pub desired_state: NetworkState, +} + +#[derive(CustomResource, Deserialize, Serialize, Clone, Debug, JsonSchema)] +#[kube( + group = "nmstate.io", + version = "v1beta1", + kind = "NodeNetworkState", + plural = "nodenetworkstates", + namespaced = false, + status = "NodeNetworkStateStatus" +)] +#[serde(rename_all = "camelCase")] +pub struct NodeNetworkStateSpec { + // This resource is read-only and has no spec. } #[derive(Deserialize, Serialize, Clone, Debug, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct NodeNetworkStateStatus { + #[serde(skip_serializing_if = "Option::is_none")] + pub current_state: Option, + + #[serde(skip_serializing_if = "Option::is_none")] + pub handler_nmstate_version: Option, + + #[serde(skip_serializing_if = "Option::is_none")] + pub host_network_manager_version: Option, + + #[serde(skip_serializing_if = "Option::is_none")] + pub last_successful_update_time: Option, +} + +/// The NetworkState is the top-level struct, representing the entire +/// desired or current network state. +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] #[serde(rename_all = "kebab-case")] -pub struct DesiredStateSpec { - pub interfaces: Vec, +#[serde(deny_unknown_fields)] +pub struct NetworkState { + #[serde(skip_serializing_if = "Option::is_none")] + pub hostname: Option, + #[serde(rename = "dns-resolver", skip_serializing_if = "Option::is_none")] + pub dns: Option, + #[serde(rename = "route-rules", skip_serializing_if = "Option::is_none")] + pub rules: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub routes: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub interfaces: Vec, + #[serde(rename = "ovs-db", skip_serializing_if = "Option::is_none")] + pub ovsdb: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub ovn: Option, } #[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] #[serde(rename_all = "kebab-case")] -pub struct InterfaceSpec { +pub struct HostNameState { + #[serde(skip_serializing_if = "Option::is_none")] + pub running: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub config: Option, +} + +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct DnsState { + #[serde(skip_serializing_if = "Option::is_none")] + pub running: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub config: Option, +} + +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct DnsResolverConfig { + #[serde(skip_serializing_if = "Option::is_none")] + pub search: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub server: Option>, +} + +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct RouteRuleState { + #[serde(skip_serializing_if = "Option::is_none")] + pub config: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub running: Option>, +} + +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct RouteState { + #[serde(skip_serializing_if = "Option::is_none")] + pub config: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub running: Option>, +} + +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct RouteRule { + #[serde(rename = "ip-from", skip_serializing_if = "Option::is_none")] + pub ip_from: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub priority: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub route_table: Option, +} + +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct Route { + #[serde(skip_serializing_if = "Option::is_none")] + pub destination: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub metric: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub next_hop_address: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub next_hop_interface: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub table_id: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub mtu: Option, +} + +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct OvsDbGlobalConfig { + #[serde(skip_serializing_if = "Option::is_none")] + pub external_ids: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub other_config: Option>, +} + +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct OvnConfiguration { + #[serde(skip_serializing_if = "Option::is_none")] + pub bridge_mappings: Option>, +} + +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct OvnBridgeMapping { + #[serde(skip_serializing_if = "Option::is_none")] + pub localnet: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub bridge: Option, +} + +#[derive(Deserialize, Serialize, Clone, Debug, JsonSchema)] +#[serde(untagged)] +#[serde(rename_all = "kebab-case")] +pub enum StpSpec { + Bool(bool), + Options(StpOptions), +} + +#[derive(Deserialize, Serialize, Clone, Debug, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct LldpState { + #[serde(skip_serializing_if = "Option::is_none")] + pub enabled: Option, +} + +#[derive(Deserialize, Serialize, Clone, Debug, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct OvsDb { + #[serde(skip_serializing_if = "Option::is_none")] + pub external_ids: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub other_config: Option>, +} + +#[derive(Deserialize, Serialize, Clone, Debug, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct PatchState { + #[serde(skip_serializing_if = "Option::is_none")] + pub peer: Option, +} + +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub struct Interface { pub name: String, #[serde(skip_serializing_if = "Option::is_none")] pub description: Option, @@ -98,10 +273,38 @@ pub struct InterfaceSpec { pub infiniband: Option, #[serde(skip_serializing_if = "Option::is_none")] pub linux_bridge: Option, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(alias = "bridge")] pub ovs_bridge: Option, + #[serde(skip_serializing_if = "Option::is_none")] - pub ethtool: Option, + pub ethtool: Option, + + #[serde(skip_serializing_if = "Option::is_none")] + pub accept_all_mac_addresses: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub identifier: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub lldp: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub permanent_mac_address: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub max_mtu: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub min_mtu: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub mptcp: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub profile_name: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub wait_ip: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub ovs_db: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub driver: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub patch: Option, } #[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] @@ -287,11 +490,15 @@ pub struct OvsBridgeSpec { #[serde(rename_all = "kebab-case")] pub struct OvsBridgeOptions { #[serde(skip_serializing_if = "Option::is_none")] - pub stp: Option, + pub stp: Option, #[serde(skip_serializing_if = "Option::is_none")] pub rstp: Option, #[serde(skip_serializing_if = "Option::is_none")] pub mcast_snooping_enable: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub datapath: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub fail_mode: Option, } #[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] @@ -305,18 +512,3 @@ pub struct OvsPortSpec { #[serde(skip_serializing_if = "Option::is_none")] pub r#type: Option, } - -#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] -#[serde(rename_all = "kebab-case")] -pub struct EthtoolSpec { - // TODO: Properly describe this spec (https://nmstate.io/devel/yaml_api.html#ethtool) -} - -#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] -#[serde(rename_all = "kebab-case")] -pub struct EthtoolFecSpec { - #[serde(skip_serializing_if = "Option::is_none")] - pub auto: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub mode: Option, -} diff --git a/opnsense-config-xml/src/data/interfaces.rs b/opnsense-config-xml/src/data/interfaces.rs index b06f392..fb49a4d 100644 --- a/opnsense-config-xml/src/data/interfaces.rs +++ b/opnsense-config-xml/src/data/interfaces.rs @@ -9,7 +9,7 @@ pub struct Interface { pub physical_interface_name: String, pub descr: Option, pub mtu: Option, - pub enable: MaybeString, + pub enable: Option, pub lock: Option, #[yaserde(rename = "spoofmac")] pub spoof_mac: Option, -- 2.39.5 From 4f7b0541f4d119bb31ea558371ff419882dff506 Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Tue, 4 Nov 2025 15:03:20 -0500 Subject: [PATCH 02/11] find lowest available bond id --- harmony/src/domain/topology/ha_cluster.rs | 54 ++++++++----- harmony/src/modules/okd/crd/nmstate.rs | 99 +++++++++++++++++++---- 2 files changed, 117 insertions(+), 36 deletions(-) diff --git a/harmony/src/domain/topology/ha_cluster.rs b/harmony/src/domain/topology/ha_cluster.rs index 88045ae..f7cab57 100644 --- a/harmony/src/domain/topology/ha_cluster.rs +++ b/harmony/src/domain/topology/ha_cluster.rs @@ -13,7 +13,7 @@ use kube::{ use log::debug; use log::info; -use crate::modules::okd::crd::nmstate::{self, NodeNetworkConfigurationPolicy, NodeNetworkState}; +use crate::modules::okd::crd::nmstate::{self, NodeNetworkConfigurationPolicy}; use crate::topology::PxeOptions; use crate::{data::FileContent, modules::okd::crd::nmstate::NMState}; use crate::{ @@ -27,7 +27,7 @@ use super::{ Topology, k8s::K8sClient, }; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::sync::Arc; #[derive(Debug, Clone)] @@ -157,21 +157,6 @@ impl HAClusterTopology { Ok(()) } - async fn get_next_bond_id(&self, hostname: &str) -> Result { - let network_state: Option = self - .k8s_client() - .await - .unwrap() - .get_resource(hostname, None) - .await - .map_err(|e| format!("Failed to list nodes: {e}"))?; - - println!("HELLLOOOO NETWORK STATE: {network_state:#?}"); - - let bond_id = 42; // FIXME: Find a better way to declare the bond id - Ok(format!("bond{bond_id}")) - } - async fn configure_bond(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { self.ensure_nmstate_operator_installed() .await @@ -228,7 +213,7 @@ impl HAClusterTopology { interfaces.push(nmstate::Interface { name: interface_name.clone(), description: Some(format!("Member of bond {bond_name}")), - r#type: "ethernet".to_string(), + r#type: nmstate::InterfaceType::Ethernet, state: "up".to_string(), mtu: Some(switch_port.interface.mtu), mac_address: Some(switch_port.interface.mac_address.to_string()), @@ -258,7 +243,7 @@ impl HAClusterTopology { interfaces.push(nmstate::Interface { name: bond_name.to_string(), description: Some(format!("Network bond for host {host}")), - r#type: "bond".to_string(), + r#type: nmstate::InterfaceType::Bond, state: "up".to_string(), copy_mac_from, ipv4: Some(nmstate::IpStackSpec { @@ -325,6 +310,37 @@ impl HAClusterTopology { .cloned() } + async fn get_next_bond_id(&self, hostname: &str) -> Result { + let network_state: Option = self + .k8s_client() + .await + .unwrap() + .get_resource(hostname, None) + .await + .map_err(|e| format!("Failed to list nodes: {e}"))?; + + let interfaces = vec![]; + let existing_bonds: Vec<&nmstate::Interface> = network_state + .as_ref() + .and_then(|network_state| network_state.status.current_state.as_ref()) + .map_or(&interfaces, |current_state| ¤t_state.interfaces) + .iter() + .filter(|i| i.r#type == nmstate::InterfaceType::Bond && i.link_aggregation.is_some()) + .collect(); + + let used_ids: HashSet = existing_bonds + .iter() + .filter_map(|i| { + i.name + .strip_prefix("bond") + .and_then(|id| id.parse::().ok()) + }) + .collect(); + + let next_id = (0..).find(|id| !used_ids.contains(id)).unwrap(); + Ok(format!("bond{next_id}")) + } + async fn configure_port_channel(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { debug!("Configuring port channel: {config:#?}"); let switch_ports = config.switch_ports.iter().map(|s| s.port.clone()).collect(); diff --git a/harmony/src/modules/okd/crd/nmstate.rs b/harmony/src/modules/okd/crd/nmstate.rs index 3c4955e..f0eb4ae 100644 --- a/harmony/src/modules/okd/crd/nmstate.rs +++ b/harmony/src/modules/okd/crd/nmstate.rs @@ -1,6 +1,7 @@ use std::collections::BTreeMap; -use kube::CustomResource; +use k8s_openapi::{ClusterResourceScope, Resource}; +use kube::{CustomResource, api::ObjectMeta}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -47,7 +48,7 @@ pub struct ProbeDns { group = "nmstate.io", version = "v1", kind = "NodeNetworkConfigurationPolicy", - namespaced + namespaced = false )] #[serde(rename_all = "camelCase")] pub struct NodeNetworkConfigurationPolicySpec { @@ -56,18 +57,38 @@ pub struct NodeNetworkConfigurationPolicySpec { pub desired_state: NetworkState, } -#[derive(CustomResource, Deserialize, Serialize, Clone, Debug, JsonSchema)] -#[kube( - group = "nmstate.io", - version = "v1beta1", - kind = "NodeNetworkState", - plural = "nodenetworkstates", - namespaced = false, - status = "NodeNetworkStateStatus" -)] +// Currently, kube-rs derive doesn't support resources without a `spec` field, so we have +// to implement it ourselves. +// +// Ref: +// - https://github.com/kube-rs/kube/issues/1763 +// - https://github.com/kube-rs/kube/discussions/1762 +#[derive(Deserialize, Serialize, Clone, Debug)] #[serde(rename_all = "camelCase")] -pub struct NodeNetworkStateSpec { - // This resource is read-only and has no spec. +pub struct NodeNetworkState { + metadata: ObjectMeta, + pub status: NodeNetworkStateStatus, +} + +impl Resource for NodeNetworkState { + const API_VERSION: &'static str = "nmstate.io/v1beta1"; + const GROUP: &'static str = "nmstate.io"; + const VERSION: &'static str = "v1beta1"; + const KIND: &'static str = "NodeNetworkState"; + const URL_PATH_SEGMENT: &'static str = "nodenetworkstates"; + type Scope = ClusterResourceScope; +} + +impl k8s_openapi::Metadata for NodeNetworkState { + type Ty = ObjectMeta; + + fn metadata(&self) -> &Self::Ty { + &self.metadata + } + + fn metadata_mut(&mut self) -> &mut Self::Ty { + &mut self.metadata + } } #[derive(Deserialize, Serialize, Clone, Debug, JsonSchema)] @@ -243,7 +264,7 @@ pub struct Interface { pub name: String, #[serde(skip_serializing_if = "Option::is_none")] pub description: Option, - pub r#type: String, + pub r#type: InterfaceType, pub state: String, #[serde(skip_serializing_if = "Option::is_none")] pub mac_address: Option, @@ -273,14 +294,11 @@ pub struct Interface { pub infiniband: Option, #[serde(skip_serializing_if = "Option::is_none")] pub linux_bridge: Option, - #[serde(skip_serializing_if = "Option::is_none")] #[serde(alias = "bridge")] pub ovs_bridge: Option, - #[serde(skip_serializing_if = "Option::is_none")] pub ethtool: Option, - #[serde(skip_serializing_if = "Option::is_none")] pub accept_all_mac_addresses: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -307,6 +325,53 @@ pub struct Interface { pub patch: Option, } +#[derive(Deserialize, Serialize, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub enum InterfaceType { + #[serde(rename = "unknown")] + Unknown, + #[serde(rename = "dummy")] + Dummy, + #[serde(rename = "loopback")] + Loopback, + #[serde(rename = "linux-bridge")] + LinuxBridge, + #[serde(rename = "ovs-bridge")] + OvsBridge, + #[serde(rename = "ovs-interface")] + OvsInterface, + #[serde(rename = "bond")] + Bond, + #[serde(rename = "ipvlan")] + IpVlan, + #[serde(rename = "vlan")] + Vlan, + #[serde(rename = "vxlan")] + Vxlan, + #[serde(rename = "mac-vlan")] + Macvlan, + #[serde(rename = "mac-vtap")] + Macvtap, + #[serde(rename = "ethernet")] + Ethernet, + #[serde(rename = "infiniband")] + Infiniband, + #[serde(rename = "vrf")] + Vrf, + #[serde(rename = "veth")] + Veth, + #[serde(rename = "ipsec")] + Ipsec, + #[serde(rename = "hsr")] + Hrs, +} + +impl Default for InterfaceType { + fn default() -> Self { + Self::Loopback + } +} + #[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] #[serde(rename_all = "kebab-case")] pub struct IpStackSpec { -- 2.39.5 From b5beda8efe6821278b75fd499302605f68ae008e Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Tue, 4 Nov 2025 15:27:17 -0500 Subject: [PATCH 03/11] better debug log --- harmony/src/domain/topology/ha_cluster.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/harmony/src/domain/topology/ha_cluster.rs b/harmony/src/domain/topology/ha_cluster.rs index f7cab57..9a0640e 100644 --- a/harmony/src/domain/topology/ha_cluster.rs +++ b/harmony/src/domain/topology/ha_cluster.rs @@ -148,7 +148,10 @@ impl HAClusterTopology { }, ..Default::default() }; - debug!("Creating NMState: {nmstate:#?}"); + debug!( + "Creating NMState:\n{}", + serde_yaml::to_string(&nmstate).unwrap() + ); k8s_client .apply(&nmstate, None) .await @@ -181,7 +184,8 @@ impl HAClusterTopology { let bond_config = self.create_bond_configuration(&hostname, &bond_id, config); debug!( - "Applying NMState bond config for host {}: {bond_config:#?}", + "Applying NMState bond config for host {}:\n{}", + serde_yaml::to_string(&bond_config).unwrap(), config.host_id ); self.k8s_client() -- 2.39.5 From cab4eb19edf47b83eb7fc5340297d710f81d8ca4 Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Tue, 4 Nov 2025 15:28:53 -0500 Subject: [PATCH 04/11] uncomment --- harmony/src/domain/topology/ha_cluster.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/harmony/src/domain/topology/ha_cluster.rs b/harmony/src/domain/topology/ha_cluster.rs index 9a0640e..2d5e568 100644 --- a/harmony/src/domain/topology/ha_cluster.rs +++ b/harmony/src/domain/topology/ha_cluster.rs @@ -546,8 +546,8 @@ impl Switch for HAClusterTopology { } async fn configure_host_network(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { - self.configure_bond(config).await - // self.configure_port_channel(config).await + self.configure_bond(config).await?; + self.configure_port_channel(config).await } } -- 2.39.5 From 4ea1af8d7295a97fc6010788320092b3b00063dd Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Tue, 4 Nov 2025 17:18:25 -0500 Subject: [PATCH 05/11] refactor(host_network): extract NetworkManager as a reusable component --- examples/nanodc/src/main.rs | 3 +- examples/okd_installation/src/topology.rs | 6 +- examples/okd_pxe/src/topology.rs | 6 +- examples/opnsense/src/main.rs | 3 +- harmony/src/domain/topology/ha_cluster.rs | 310 +++------------------- harmony/src/domain/topology/k8s.rs | 2 +- harmony/src/domain/topology/network.rs | 35 ++- harmony/src/infra/mod.rs | 1 + harmony/src/infra/network_manager.rs | 259 ++++++++++++++++++ harmony/src/modules/okd/host_network.rs | 186 +++++++++++-- 10 files changed, 506 insertions(+), 305 deletions(-) create mode 100644 harmony/src/infra/network_manager.rs diff --git a/examples/nanodc/src/main.rs b/examples/nanodc/src/main.rs index 629a8dc..e487fab 100644 --- a/examples/nanodc/src/main.rs +++ b/examples/nanodc/src/main.rs @@ -1,6 +1,6 @@ use std::{ net::{IpAddr, Ipv4Addr}, - sync::Arc, + sync::{Arc, OnceLock}, }; use brocade::BrocadeOptions; @@ -107,6 +107,7 @@ async fn main() { }, ], switch_client: switch_client.clone(), + network_manager: OnceLock::new(), }; let inventory = Inventory { diff --git a/examples/okd_installation/src/topology.rs b/examples/okd_installation/src/topology.rs index ce89402..2bc9fd2 100644 --- a/examples/okd_installation/src/topology.rs +++ b/examples/okd_installation/src/topology.rs @@ -9,7 +9,10 @@ use harmony::{ use harmony_macros::{ip, ipv4}; use harmony_secret::{Secret, SecretManager}; use serde::{Deserialize, Serialize}; -use std::{net::IpAddr, sync::Arc}; +use std::{ + net::IpAddr, + sync::{Arc, OnceLock}, +}; #[derive(Secret, Serialize, Deserialize, Debug, PartialEq)] struct OPNSenseFirewallConfig { @@ -81,6 +84,7 @@ pub async fn get_topology() -> HAClusterTopology { }, workers: vec![], switch_client: switch_client.clone(), + network_manager: OnceLock::new(), } } diff --git a/examples/okd_pxe/src/topology.rs b/examples/okd_pxe/src/topology.rs index 7c1244c..72695fa 100644 --- a/examples/okd_pxe/src/topology.rs +++ b/examples/okd_pxe/src/topology.rs @@ -10,7 +10,10 @@ use harmony::{ use harmony_macros::{ip, ipv4}; use harmony_secret::{Secret, SecretManager}; use serde::{Deserialize, Serialize}; -use std::{net::IpAddr, sync::Arc}; +use std::{ + net::IpAddr, + sync::{Arc, OnceLock}, +}; pub async fn get_topology() -> HAClusterTopology { let firewall = harmony::topology::LogicalHost { @@ -76,6 +79,7 @@ pub async fn get_topology() -> HAClusterTopology { }, workers: vec![], switch_client: switch_client.clone(), + network_manager: OnceLock::new(), } } diff --git a/examples/opnsense/src/main.rs b/examples/opnsense/src/main.rs index 4422f65..e74f5da 100644 --- a/examples/opnsense/src/main.rs +++ b/examples/opnsense/src/main.rs @@ -1,6 +1,6 @@ use std::{ net::{IpAddr, Ipv4Addr}, - sync::Arc, + sync::{Arc, OnceLock}, }; use brocade::BrocadeOptions; @@ -79,6 +79,7 @@ async fn main() { }, workers: vec![], switch_client: switch_client.clone(), + network_manager: OnceLock::new(), }; let inventory = Inventory { diff --git a/harmony/src/domain/topology/ha_cluster.rs b/harmony/src/domain/topology/ha_cluster.rs index 2d5e568..0edbc37 100644 --- a/harmony/src/domain/topology/ha_cluster.rs +++ b/harmony/src/domain/topology/ha_cluster.rs @@ -1,34 +1,24 @@ use async_trait::async_trait; use harmony_macros::ip; use harmony_types::{ - id::Id, net::{MacAddress, Url}, switch::PortLocation, }; -use k8s_openapi::api::core::v1::Node; -use kube::{ - ResourceExt, - api::{ObjectList, ObjectMeta}, -}; use log::debug; use log::info; -use crate::modules::okd::crd::nmstate::{self, NodeNetworkConfigurationPolicy}; +use crate::infra::network_manager::OpenShiftNmStateNetworkManager; use crate::topology::PxeOptions; -use crate::{data::FileContent, modules::okd::crd::nmstate::NMState}; -use crate::{ - executors::ExecutorError, modules::okd::crd::nmstate::NodeNetworkConfigurationPolicySpec, -}; +use crate::{data::FileContent, executors::ExecutorError}; use super::{ DHCPStaticEntry, DhcpServer, DnsRecord, DnsRecordType, DnsServer, Firewall, HostNetworkConfig, - HttpServer, IpAddress, K8sclient, LoadBalancer, LoadBalancerService, LogicalHost, - PreparationError, PreparationOutcome, Router, Switch, SwitchClient, SwitchError, TftpServer, - Topology, k8s::K8sClient, + HttpServer, IpAddress, K8sclient, LoadBalancer, LoadBalancerService, LogicalHost, NetworkError, + NetworkManager, PreparationError, PreparationOutcome, Router, Switch, SwitchClient, + SwitchError, TftpServer, Topology, k8s::K8sClient, }; -use std::collections::{BTreeMap, HashSet}; -use std::sync::Arc; +use std::sync::{Arc, OnceLock}; #[derive(Debug, Clone)] pub struct HAClusterTopology { @@ -45,6 +35,7 @@ pub struct HAClusterTopology { pub control_plane: Vec, pub workers: Vec, pub kubeconfig: Option, + pub network_manager: OnceLock>, } #[async_trait] @@ -98,263 +89,12 @@ impl HAClusterTopology { .to_string() } - async fn ensure_nmstate_operator_installed(&self) -> Result<(), String> { - let k8s_client = self.k8s_client().await?; + pub async fn network_manager(&self) -> &dyn NetworkManager { + let k8s_client = self.k8s_client().await.unwrap(); - debug!("Installing NMState controller..."); - k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/nmstate.io_nmstates.yaml -").unwrap(), Some("nmstate")) - .await - .map_err(|e| e.to_string())?; - - debug!("Creating NMState namespace..."); - k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/namespace.yaml -").unwrap(), Some("nmstate")) - .await - .map_err(|e| e.to_string())?; - - debug!("Creating NMState service account..."); - k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/service_account.yaml -").unwrap(), Some("nmstate")) - .await - .map_err(|e| e.to_string())?; - - debug!("Creating NMState role..."); - k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/role.yaml -").unwrap(), Some("nmstate")) - .await - .map_err(|e| e.to_string())?; - - debug!("Creating NMState role binding..."); - k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/role_binding.yaml -").unwrap(), Some("nmstate")) - .await - .map_err(|e| e.to_string())?; - - debug!("Creating NMState operator..."); - k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/operator.yaml -").unwrap(), Some("nmstate")) - .await - .map_err(|e| e.to_string())?; - - k8s_client - .wait_until_deployment_ready("nmstate-operator", Some("nmstate"), None) - .await?; - - let nmstate = NMState { - metadata: ObjectMeta { - name: Some("nmstate".to_string()), - ..Default::default() - }, - ..Default::default() - }; - debug!( - "Creating NMState:\n{}", - serde_yaml::to_string(&nmstate).unwrap() - ); - k8s_client - .apply(&nmstate, None) - .await - .map_err(|e| e.to_string())?; - - Ok(()) - } - - async fn configure_bond(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { - self.ensure_nmstate_operator_installed() - .await - .map_err(|e| { - SwitchError::new(format!( - "Can't configure bond, NMState operator not available: {e}" - )) - })?; - - let hostname = self.get_hostname(&config.host_id).await.map_err(|e| { - SwitchError::new(format!( - "Can't configure bond, can't get hostname for host '{}': {e}", - config.host_id - )) - })?; - let bond_id = self.get_next_bond_id(&hostname).await.map_err(|e| { - SwitchError::new(format!( - "Can't configure bond, can't get an available bond id for host '{}': {e}", - config.host_id - )) - })?; - let bond_config = self.create_bond_configuration(&hostname, &bond_id, config); - - debug!( - "Applying NMState bond config for host {}:\n{}", - serde_yaml::to_string(&bond_config).unwrap(), - config.host_id - ); - self.k8s_client() - .await - .unwrap() - .apply(&bond_config, None) - .await - .map_err(|e| SwitchError::new(format!("Failed to configure bond: {e}")))?; - - Ok(()) - } - - fn create_bond_configuration( - &self, - host: &str, - bond_name: &str, - config: &HostNetworkConfig, - ) -> NodeNetworkConfigurationPolicy { - info!("Configuring bond '{bond_name}' for host '{host}'..."); - - let mut bond_mtu: Option = None; - let mut copy_mac_from: Option = None; - let mut bond_ports = Vec::new(); - let mut interfaces: Vec = Vec::new(); - - for switch_port in &config.switch_ports { - let interface_name = switch_port.interface.name.clone(); - - interfaces.push(nmstate::Interface { - name: interface_name.clone(), - description: Some(format!("Member of bond {bond_name}")), - r#type: nmstate::InterfaceType::Ethernet, - state: "up".to_string(), - mtu: Some(switch_port.interface.mtu), - mac_address: Some(switch_port.interface.mac_address.to_string()), - ipv4: Some(nmstate::IpStackSpec { - enabled: Some(false), - ..Default::default() - }), - ipv6: Some(nmstate::IpStackSpec { - enabled: Some(false), - ..Default::default() - }), - link_aggregation: None, - ..Default::default() - }); - - bond_ports.push(interface_name.clone()); - - // Use the first port's details for the bond mtu and mac address - if bond_mtu.is_none() { - bond_mtu = Some(switch_port.interface.mtu); - } - if copy_mac_from.is_none() { - copy_mac_from = Some(interface_name); - } - } - - interfaces.push(nmstate::Interface { - name: bond_name.to_string(), - description: Some(format!("Network bond for host {host}")), - r#type: nmstate::InterfaceType::Bond, - state: "up".to_string(), - copy_mac_from, - ipv4: Some(nmstate::IpStackSpec { - dhcp: Some(true), - enabled: Some(true), - ..Default::default() - }), - ipv6: Some(nmstate::IpStackSpec { - dhcp: Some(true), - autoconf: Some(true), - enabled: Some(true), - ..Default::default() - }), - link_aggregation: Some(nmstate::BondSpec { - mode: "802.3ad".to_string(), - ports: bond_ports, - ..Default::default() - }), - ..Default::default() - }); - - NodeNetworkConfigurationPolicy { - metadata: ObjectMeta { - name: Some(format!("{host}-bond-config")), - ..Default::default() - }, - spec: NodeNetworkConfigurationPolicySpec { - node_selector: Some(BTreeMap::from([( - "kubernetes.io/hostname".to_string(), - host.to_string(), - )])), - desired_state: nmstate::NetworkState { - interfaces, - ..Default::default() - }, - }, - } - } - - async fn get_hostname(&self, host_id: &Id) -> Result { - let nodes: ObjectList = self - .k8s_client() - .await - .unwrap() - .list_resources(None, None) - .await - .map_err(|e| format!("Failed to list nodes: {e}"))?; - - let Some(node) = nodes.iter().find(|n| { - n.status - .as_ref() - .and_then(|s| s.node_info.as_ref()) - .map(|i| i.system_uuid == host_id.to_string()) - .unwrap_or(false) - }) else { - return Err(format!("No node found for host '{host_id}'")); - }; - - node.labels() - .get("kubernetes.io/hostname") - .ok_or(format!( - "Node '{host_id}' has no kubernetes.io/hostname label" - )) - .cloned() - } - - async fn get_next_bond_id(&self, hostname: &str) -> Result { - let network_state: Option = self - .k8s_client() - .await - .unwrap() - .get_resource(hostname, None) - .await - .map_err(|e| format!("Failed to list nodes: {e}"))?; - - let interfaces = vec![]; - let existing_bonds: Vec<&nmstate::Interface> = network_state + self.network_manager + .get_or_init(|| Arc::new(OpenShiftNmStateNetworkManager::new(k8s_client.clone()))) .as_ref() - .and_then(|network_state| network_state.status.current_state.as_ref()) - .map_or(&interfaces, |current_state| ¤t_state.interfaces) - .iter() - .filter(|i| i.r#type == nmstate::InterfaceType::Bond && i.link_aggregation.is_some()) - .collect(); - - let used_ids: HashSet = existing_bonds - .iter() - .filter_map(|i| { - i.name - .strip_prefix("bond") - .and_then(|id| id.parse::().ok()) - }) - .collect(); - - let next_id = (0..).find(|id| !used_ids.contains(id)).unwrap(); - Ok(format!("bond{next_id}")) - } - - async fn configure_port_channel(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { - debug!("Configuring port channel: {config:#?}"); - let switch_ports = config.switch_ports.iter().map(|s| s.port.clone()).collect(); - - self.switch_client - .configure_port_channel(&format!("Harmony_{}", config.host_id), switch_ports) - .await - .map_err(|e| SwitchError::new(format!("Failed to configure switch: {e}")))?; - - Ok(()) } pub fn autoload() -> Self { @@ -378,6 +118,7 @@ impl HAClusterTopology { bootstrap_host: dummy_host, control_plane: vec![], workers: vec![], + network_manager: OnceLock::new(), } } } @@ -545,9 +286,30 @@ impl Switch for HAClusterTopology { self.switch_client.find_port(mac_address).await } - async fn configure_host_network(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { - self.configure_bond(config).await?; - self.configure_port_channel(config).await + async fn configure_port_channel(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { + debug!("Configuring port channel: {config:#?}"); + let switch_ports = config.switch_ports.iter().map(|s| s.port.clone()).collect(); + + self.switch_client + .configure_port_channel(&format!("Harmony_{}", config.host_id), switch_ports) + .await + .map_err(|e| SwitchError::new(format!("Failed to configure port-channel: {e}")))?; + + Ok(()) + } +} + +#[async_trait] +impl NetworkManager for HAClusterTopology { + async fn ensure_network_manager_installed(&self) -> Result<(), NetworkError> { + self.network_manager() + .await + .ensure_network_manager_installed() + .await + } + + async fn configure_bond(&self, config: &HostNetworkConfig) -> Result<(), NetworkError> { + self.network_manager().await.configure_bond(config).await } } diff --git a/harmony/src/domain/topology/k8s.rs b/harmony/src/domain/topology/k8s.rs index 03c59e1..fccf0d1 100644 --- a/harmony/src/domain/topology/k8s.rs +++ b/harmony/src/domain/topology/k8s.rs @@ -25,7 +25,7 @@ use kube::{ api::{ApiResource, GroupVersionKind}, runtime::wait::await_condition, }; -use log::{debug, error, info, trace, warn}; +use log::{debug, error, trace, warn}; use serde::{Serialize, de::DeserializeOwned}; use serde_json::json; use similar::TextDiff; diff --git a/harmony/src/domain/topology/network.rs b/harmony/src/domain/topology/network.rs index cf172ee..d9e4f72 100644 --- a/harmony/src/domain/topology/network.rs +++ b/harmony/src/domain/topology/network.rs @@ -15,7 +15,7 @@ use harmony_types::{ }; use serde::Serialize; -use crate::{executors::ExecutorError, hardware::PhysicalHost}; +use crate::executors::ExecutorError; use super::{LogicalHost, k8s::K8sClient}; @@ -183,6 +183,37 @@ impl FromStr for DnsRecordType { } } +#[async_trait] +pub trait NetworkManager: Debug + Send + Sync { + async fn ensure_network_manager_installed(&self) -> Result<(), NetworkError>; + async fn configure_bond(&self, config: &HostNetworkConfig) -> Result<(), NetworkError>; +} + +#[derive(Debug, Clone, new)] +pub struct NetworkError { + msg: String, +} + +impl fmt::Display for NetworkError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.msg) + } +} + +impl Error for NetworkError {} + +impl From for NetworkError { + fn from(value: kube::Error) -> Self { + NetworkError::new(value.to_string()) + } +} + +impl From for NetworkError { + fn from(value: String) -> Self { + NetworkError::new(value) + } +} + #[async_trait] pub trait Switch: Send + Sync { async fn setup_switch(&self) -> Result<(), SwitchError>; @@ -192,7 +223,7 @@ pub trait Switch: Send + Sync { mac_address: &MacAddress, ) -> Result, SwitchError>; - async fn configure_host_network(&self, config: &HostNetworkConfig) -> Result<(), SwitchError>; + async fn configure_port_channel(&self, config: &HostNetworkConfig) -> Result<(), SwitchError>; } #[derive(Clone, Debug, PartialEq)] diff --git a/harmony/src/infra/mod.rs b/harmony/src/infra/mod.rs index 203cf90..c82a118 100644 --- a/harmony/src/infra/mod.rs +++ b/harmony/src/infra/mod.rs @@ -3,5 +3,6 @@ pub mod executors; pub mod hp_ilo; pub mod intel_amt; pub mod inventory; +pub mod network_manager; pub mod opnsense; mod sqlx; diff --git a/harmony/src/infra/network_manager.rs b/harmony/src/infra/network_manager.rs new file mode 100644 index 0000000..6518b3d --- /dev/null +++ b/harmony/src/infra/network_manager.rs @@ -0,0 +1,259 @@ +use std::{ + collections::{BTreeMap, HashSet}, + sync::Arc, +}; + +use async_trait::async_trait; +use harmony_types::id::Id; +use k8s_openapi::api::core::v1::Node; +use kube::{ + ResourceExt, + api::{ObjectList, ObjectMeta}, +}; +use log::{debug, info}; + +use crate::{ + modules::okd::crd::nmstate, + topology::{HostNetworkConfig, NetworkError, NetworkManager, k8s::K8sClient}, +}; + +pub struct OpenShiftNmStateNetworkManager { + k8s_client: Arc, +} + +impl std::fmt::Debug for OpenShiftNmStateNetworkManager { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("OpenShiftNmStateNetworkManager").finish() + } +} + +#[async_trait] +impl NetworkManager for OpenShiftNmStateNetworkManager { + async fn ensure_network_manager_installed(&self) -> Result<(), NetworkError> { + debug!("Installing NMState controller..."); + self.k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/nmstate.io_nmstates.yaml +").unwrap(), Some("nmstate")) + .await?; + + debug!("Creating NMState namespace..."); + self.k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/namespace.yaml +").unwrap(), Some("nmstate")) + .await?; + + debug!("Creating NMState service account..."); + self.k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/service_account.yaml +").unwrap(), Some("nmstate")) + .await?; + + debug!("Creating NMState role..."); + self.k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/role.yaml +").unwrap(), Some("nmstate")) + .await?; + + debug!("Creating NMState role binding..."); + self.k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/role_binding.yaml +").unwrap(), Some("nmstate")) + .await?; + + debug!("Creating NMState operator..."); + self.k8s_client.apply_url(url::Url::parse("https://github.com/nmstate/kubernetes-nmstate/releases/download/v0.84.0/operator.yaml +").unwrap(), Some("nmstate")) + .await?; + + self.k8s_client + .wait_until_deployment_ready("nmstate-operator", Some("nmstate"), None) + .await?; + + let nmstate = nmstate::NMState { + metadata: ObjectMeta { + name: Some("nmstate".to_string()), + ..Default::default() + }, + ..Default::default() + }; + debug!( + "Creating NMState:\n{}", + serde_yaml::to_string(&nmstate).unwrap() + ); + self.k8s_client.apply(&nmstate, None).await?; + + Ok(()) + } + + async fn configure_bond(&self, config: &HostNetworkConfig) -> Result<(), NetworkError> { + let hostname = self.get_hostname(&config.host_id).await.map_err(|e| { + NetworkError::new(format!( + "Can't configure bond, can't get hostname for host '{}': {e}", + config.host_id + )) + })?; + let bond_id = self.get_next_bond_id(&hostname).await.map_err(|e| { + NetworkError::new(format!( + "Can't configure bond, can't get an available bond id for host '{}': {e}", + config.host_id + )) + })?; + let bond_config = self.create_bond_configuration(&hostname, &bond_id, config); + + debug!( + "Applying NMState bond config for host {}:\n{}", + serde_yaml::to_string(&bond_config).unwrap(), + config.host_id + ); + self.k8s_client + .apply(&bond_config, None) + .await + .map_err(|e| NetworkError::new(format!("Failed to configure bond: {e}")))?; + + Ok(()) + } +} + +impl OpenShiftNmStateNetworkManager { + pub fn new(k8s_client: Arc) -> Self { + Self { k8s_client } + } + + fn create_bond_configuration( + &self, + host: &str, + bond_name: &str, + config: &HostNetworkConfig, + ) -> nmstate::NodeNetworkConfigurationPolicy { + info!("Configuring bond '{bond_name}' for host '{host}'..."); + + let mut bond_mtu: Option = None; + let mut copy_mac_from: Option = None; + let mut bond_ports = Vec::new(); + let mut interfaces: Vec = Vec::new(); + + for switch_port in &config.switch_ports { + let interface_name = switch_port.interface.name.clone(); + + interfaces.push(nmstate::Interface { + name: interface_name.clone(), + description: Some(format!("Member of bond {bond_name}")), + r#type: nmstate::InterfaceType::Ethernet, + state: "up".to_string(), + mtu: Some(switch_port.interface.mtu), + mac_address: Some(switch_port.interface.mac_address.to_string()), + ipv4: Some(nmstate::IpStackSpec { + enabled: Some(false), + ..Default::default() + }), + ipv6: Some(nmstate::IpStackSpec { + enabled: Some(false), + ..Default::default() + }), + link_aggregation: None, + ..Default::default() + }); + + bond_ports.push(interface_name.clone()); + + // Use the first port's details for the bond mtu and mac address + if bond_mtu.is_none() { + bond_mtu = Some(switch_port.interface.mtu); + } + if copy_mac_from.is_none() { + copy_mac_from = Some(interface_name); + } + } + + interfaces.push(nmstate::Interface { + name: bond_name.to_string(), + description: Some(format!("Network bond for host {host}")), + r#type: nmstate::InterfaceType::Bond, + state: "up".to_string(), + copy_mac_from, + ipv4: Some(nmstate::IpStackSpec { + dhcp: Some(true), + enabled: Some(true), + ..Default::default() + }), + ipv6: Some(nmstate::IpStackSpec { + dhcp: Some(true), + autoconf: Some(true), + enabled: Some(true), + ..Default::default() + }), + link_aggregation: Some(nmstate::BondSpec { + mode: "802.3ad".to_string(), + ports: bond_ports, + ..Default::default() + }), + ..Default::default() + }); + + nmstate::NodeNetworkConfigurationPolicy { + metadata: ObjectMeta { + name: Some(format!("{host}-bond-config")), + ..Default::default() + }, + spec: nmstate::NodeNetworkConfigurationPolicySpec { + node_selector: Some(BTreeMap::from([( + "kubernetes.io/hostname".to_string(), + host.to_string(), + )])), + desired_state: nmstate::NetworkState { + interfaces, + ..Default::default() + }, + }, + } + } + + async fn get_hostname(&self, host_id: &Id) -> Result { + let nodes: ObjectList = self + .k8s_client + .list_resources(None, None) + .await + .map_err(|e| format!("Failed to list nodes: {e}"))?; + + let Some(node) = nodes.iter().find(|n| { + n.status + .as_ref() + .and_then(|s| s.node_info.as_ref()) + .map(|i| i.system_uuid == host_id.to_string()) + .unwrap_or(false) + }) else { + return Err(format!("No node found for host '{host_id}'")); + }; + + node.labels() + .get("kubernetes.io/hostname") + .ok_or(format!( + "Node '{host_id}' has no kubernetes.io/hostname label" + )) + .cloned() + } + + async fn get_next_bond_id(&self, hostname: &str) -> Result { + let network_state: Option = self + .k8s_client + .get_resource(hostname, None) + .await + .map_err(|e| format!("Failed to list nodes: {e}"))?; + + let interfaces = vec![]; + let existing_bonds: Vec<&nmstate::Interface> = network_state + .as_ref() + .and_then(|network_state| network_state.status.current_state.as_ref()) + .map_or(&interfaces, |current_state| ¤t_state.interfaces) + .iter() + .filter(|i| i.r#type == nmstate::InterfaceType::Bond && i.link_aggregation.is_some()) + .collect(); + + let used_ids: HashSet = existing_bonds + .iter() + .filter_map(|i| { + i.name + .strip_prefix("bond") + .and_then(|id| id.parse::().ok()) + }) + .collect(); + + let next_id = (0..).find(|id| !used_ids.contains(id)).unwrap(); + Ok(format!("bond{next_id}")) + } +} diff --git a/harmony/src/modules/okd/host_network.rs b/harmony/src/modules/okd/host_network.rs index 39fb027..d9a0063 100644 --- a/harmony/src/modules/okd/host_network.rs +++ b/harmony/src/modules/okd/host_network.rs @@ -9,7 +9,7 @@ use crate::{ interpret::{Interpret, InterpretError, InterpretName, InterpretStatus, Outcome}, inventory::Inventory, score::Score, - topology::{HostNetworkConfig, NetworkInterface, Switch, SwitchPort, Topology}, + topology::{HostNetworkConfig, NetworkInterface, NetworkManager, Switch, SwitchPort, Topology}, }; #[derive(Debug, Clone, Serialize)] @@ -17,7 +17,7 @@ pub struct HostNetworkConfigurationScore { pub hosts: Vec, } -impl Score for HostNetworkConfigurationScore { +impl Score for HostNetworkConfigurationScore { fn name(&self) -> String { "HostNetworkConfigurationScore".into() } @@ -35,7 +35,7 @@ pub struct HostNetworkConfigurationInterpret { } impl HostNetworkConfigurationInterpret { - async fn configure_network_for_host( + async fn configure_network_for_host( &self, topology: &T, host: &PhysicalHost, @@ -67,10 +67,15 @@ impl HostNetworkConfigurationInterpret { ); info!("[Host {current_host}/{total_hosts}] Configuring host network..."); + topology.configure_bond(&config).await.map_err(|e| { + InterpretError::new(format!("Failed to configure host network: {e}")) + })?; topology - .configure_host_network(&config) + .configure_port_channel(&config) .await - .map_err(|e| InterpretError::new(format!("Failed to configure host: {e}")))?; + .map_err(|e| { + InterpretError::new(format!("Failed to configure host network: {e}")) + })?; } else { info!( "[Host {current_host}/{total_hosts}] No ports found for {} interfaces, skipping", @@ -169,7 +174,7 @@ impl HostNetworkConfigurationInterpret { } #[async_trait] -impl Interpret for HostNetworkConfigurationInterpret { +impl Interpret for HostNetworkConfigurationInterpret { fn get_name(&self) -> InterpretName { InterpretName::Custom("HostNetworkConfigurationInterpret") } @@ -198,6 +203,12 @@ impl Interpret for HostNetworkConfigurationInterpret { let host_count = self.score.hosts.len(); info!("Started network configuration for {host_count} host(s)...",); + info!("Setting up NetworkManager...",); + topology + .ensure_network_manager_installed() + .await + .map_err(|e| InterpretError::new(format!("NetworkManager setup failed: {e}")))?; + info!("Setting up switch with sane defaults..."); topology .setup_switch() @@ -242,7 +253,8 @@ mod tests { use crate::{ hardware::HostCategory, topology::{ - HostNetworkConfig, PreparationError, PreparationOutcome, SwitchError, SwitchPort, + HostNetworkConfig, NetworkError, PreparationError, PreparationOutcome, SwitchError, + SwitchPort, }, }; use std::{ @@ -290,15 +302,27 @@ mod tests { } #[tokio::test] - async fn host_with_one_mac_address_should_create_bond_with_one_interface() { + async fn should_setup_network_manager() { let host = given_host(&HOST_ID, vec![EXISTING_INTERFACE.clone()]); let score = given_score(vec![host]); let topology = TopologyWithSwitch::new(); let _ = score.interpret(&Inventory::empty(), &topology).await; - let configured_host_networks = topology.configured_host_networks.lock().unwrap(); - assert_that!(*configured_host_networks).contains_exactly(vec![( + let network_manager_setup = topology.network_manager_setup.lock().unwrap(); + assert_that!(*network_manager_setup).is_true(); + } + + #[tokio::test] + async fn host_with_one_mac_address_should_configure_bond_with_one_interface() { + let host = given_host(&HOST_ID, vec![EXISTING_INTERFACE.clone()]); + let score = given_score(vec![host]); + let topology = TopologyWithSwitch::new(); + + let _ = score.interpret(&Inventory::empty(), &topology).await; + + let config = topology.configured_bonds.lock().unwrap(); + assert_that!(*config).contains_exactly(vec![( HOST_ID.clone(), HostNetworkConfig { host_id: HOST_ID.clone(), @@ -311,7 +335,28 @@ mod tests { } #[tokio::test] - async fn host_with_multiple_mac_addresses_should_create_one_bond_with_all_interfaces() { + async fn host_with_one_mac_address_should_configure_port_channel_with_one_interface() { + let host = given_host(&HOST_ID, vec![EXISTING_INTERFACE.clone()]); + let score = given_score(vec![host]); + let topology = TopologyWithSwitch::new(); + + let _ = score.interpret(&Inventory::empty(), &topology).await; + + let config = topology.configured_port_channels.lock().unwrap(); + assert_that!(*config).contains_exactly(vec![( + HOST_ID.clone(), + HostNetworkConfig { + host_id: HOST_ID.clone(), + switch_ports: vec![SwitchPort { + interface: EXISTING_INTERFACE.clone(), + port: PORT.clone(), + }], + }, + )]); + } + + #[tokio::test] + async fn host_with_multiple_mac_addresses_should_configure_one_bond_with_all_interfaces() { let score = given_score(vec![given_host( &HOST_ID, vec![ @@ -323,8 +368,8 @@ mod tests { let _ = score.interpret(&Inventory::empty(), &topology).await; - let configured_host_networks = topology.configured_host_networks.lock().unwrap(); - assert_that!(*configured_host_networks).contains_exactly(vec![( + let config = topology.configured_bonds.lock().unwrap(); + assert_that!(*config).contains_exactly(vec![( HOST_ID.clone(), HostNetworkConfig { host_id: HOST_ID.clone(), @@ -343,7 +388,40 @@ mod tests { } #[tokio::test] - async fn multiple_hosts_should_create_one_bond_per_host() { + async fn host_with_multiple_mac_addresses_should_configure_one_port_channel_with_all_interfaces() + { + let score = given_score(vec![given_host( + &HOST_ID, + vec![ + EXISTING_INTERFACE.clone(), + ANOTHER_EXISTING_INTERFACE.clone(), + ], + )]); + let topology = TopologyWithSwitch::new(); + + let _ = score.interpret(&Inventory::empty(), &topology).await; + + let config = topology.configured_port_channels.lock().unwrap(); + assert_that!(*config).contains_exactly(vec![( + HOST_ID.clone(), + HostNetworkConfig { + host_id: HOST_ID.clone(), + switch_ports: vec![ + SwitchPort { + interface: EXISTING_INTERFACE.clone(), + port: PORT.clone(), + }, + SwitchPort { + interface: ANOTHER_EXISTING_INTERFACE.clone(), + port: ANOTHER_PORT.clone(), + }, + ], + }, + )]); + } + + #[tokio::test] + async fn multiple_hosts_should_configure_one_bond_per_host() { let score = given_score(vec![ given_host(&HOST_ID, vec![EXISTING_INTERFACE.clone()]), given_host(&ANOTHER_HOST_ID, vec![ANOTHER_EXISTING_INTERFACE.clone()]), @@ -352,8 +430,43 @@ mod tests { let _ = score.interpret(&Inventory::empty(), &topology).await; - let configured_host_networks = topology.configured_host_networks.lock().unwrap(); - assert_that!(*configured_host_networks).contains_exactly(vec![ + let config = topology.configured_bonds.lock().unwrap(); + assert_that!(*config).contains_exactly(vec![ + ( + HOST_ID.clone(), + HostNetworkConfig { + host_id: HOST_ID.clone(), + switch_ports: vec![SwitchPort { + interface: EXISTING_INTERFACE.clone(), + port: PORT.clone(), + }], + }, + ), + ( + ANOTHER_HOST_ID.clone(), + HostNetworkConfig { + host_id: ANOTHER_HOST_ID.clone(), + switch_ports: vec![SwitchPort { + interface: ANOTHER_EXISTING_INTERFACE.clone(), + port: ANOTHER_PORT.clone(), + }], + }, + ), + ]); + } + + #[tokio::test] + async fn multiple_hosts_should_configure_one_port_channel_per_host() { + let score = given_score(vec![ + given_host(&HOST_ID, vec![EXISTING_INTERFACE.clone()]), + given_host(&ANOTHER_HOST_ID, vec![ANOTHER_EXISTING_INTERFACE.clone()]), + ]); + let topology = TopologyWithSwitch::new(); + + let _ = score.interpret(&Inventory::empty(), &topology).await; + + let config = topology.configured_port_channels.lock().unwrap(); + assert_that!(*config).contains_exactly(vec![ ( HOST_ID.clone(), HostNetworkConfig { @@ -384,8 +497,10 @@ mod tests { let _ = score.interpret(&Inventory::empty(), &topology).await; - let configured_host_networks = topology.configured_host_networks.lock().unwrap(); - assert_that!(*configured_host_networks).is_empty(); + let config = topology.configured_port_channels.lock().unwrap(); + assert_that!(*config).is_empty(); + let config = topology.configured_bonds.lock().unwrap(); + assert_that!(*config).is_empty(); } fn given_score(hosts: Vec) -> HostNetworkConfigurationScore { @@ -422,26 +537,33 @@ mod tests { } } + #[derive(Debug)] struct TopologyWithSwitch { available_ports: Arc>>, - configured_host_networks: Arc>>, + configured_port_channels: Arc>>, switch_setup: Arc>, + network_manager_setup: Arc>, + configured_bonds: Arc>>, } impl TopologyWithSwitch { fn new() -> Self { Self { available_ports: Arc::new(Mutex::new(vec![PORT.clone(), ANOTHER_PORT.clone()])), - configured_host_networks: Arc::new(Mutex::new(vec![])), + configured_port_channels: Arc::new(Mutex::new(vec![])), switch_setup: Arc::new(Mutex::new(false)), + network_manager_setup: Arc::new(Mutex::new(false)), + configured_bonds: Arc::new(Mutex::new(vec![])), } } fn new_port_not_found() -> Self { Self { available_ports: Arc::new(Mutex::new(vec![])), - configured_host_networks: Arc::new(Mutex::new(vec![])), + configured_port_channels: Arc::new(Mutex::new(vec![])), switch_setup: Arc::new(Mutex::new(false)), + network_manager_setup: Arc::new(Mutex::new(false)), + configured_bonds: Arc::new(Mutex::new(vec![])), } } } @@ -457,6 +579,22 @@ mod tests { } } + #[async_trait] + impl NetworkManager for TopologyWithSwitch { + async fn ensure_network_manager_installed(&self) -> Result<(), NetworkError> { + let mut network_manager_installed = self.network_manager_setup.lock().unwrap(); + *network_manager_installed = true; + Ok(()) + } + + async fn configure_bond(&self, config: &HostNetworkConfig) -> Result<(), NetworkError> { + let mut configured_bonds = self.configured_bonds.lock().unwrap(); + configured_bonds.push((config.host_id.clone(), config.clone())); + + Ok(()) + } + } + #[async_trait] impl Switch for TopologyWithSwitch { async fn setup_switch(&self) -> Result<(), SwitchError> { @@ -476,12 +614,12 @@ mod tests { Ok(Some(ports.remove(0))) } - async fn configure_host_network( + async fn configure_port_channel( &self, config: &HostNetworkConfig, ) -> Result<(), SwitchError> { - let mut configured_host_networks = self.configured_host_networks.lock().unwrap(); - configured_host_networks.push((config.host_id.clone(), config.clone())); + let mut configured_port_channels = self.configured_port_channels.lock().unwrap(); + configured_port_channels.push((config.host_id.clone(), config.clone())); Ok(()) } -- 2.39.5 From 325d7891be95e8cbc4baa22badd933d48d3907a9 Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Tue, 4 Nov 2025 17:48:12 -0500 Subject: [PATCH 06/11] adjust logs --- harmony/src/infra/network_manager.rs | 2 +- harmony/src/modules/okd/host_network.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/harmony/src/infra/network_manager.rs b/harmony/src/infra/network_manager.rs index 6518b3d..5b1b825 100644 --- a/harmony/src/infra/network_manager.rs +++ b/harmony/src/infra/network_manager.rs @@ -97,8 +97,8 @@ impl NetworkManager for OpenShiftNmStateNetworkManager { debug!( "Applying NMState bond config for host {}:\n{}", + config.host_id, serde_yaml::to_string(&bond_config).unwrap(), - config.host_id ); self.k8s_client .apply(&bond_config, None) diff --git a/harmony/src/modules/okd/host_network.rs b/harmony/src/modules/okd/host_network.rs index d9a0063..c79b372 100644 --- a/harmony/src/modules/okd/host_network.rs +++ b/harmony/src/modules/okd/host_network.rs @@ -118,7 +118,7 @@ impl HostNetworkConfigurationInterpret { port, }); } - Ok(None) => debug!("No port found for '{mac_address}', skipping"), + Ok(None) => {} Err(e) => { return Err(InterpretError::new(format!( "Failed to get port for host '{}': {}", -- 2.39.5 From c89c30e8f275e738577450e15c661605b6a4ec50 Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Wed, 5 Nov 2025 12:47:41 -0500 Subject: [PATCH 07/11] fix(host_network): skip configuration for host with only 1 interface/port --- harmony/src/modules/okd/host_network.rs | 190 +++++++++++++++++------- 1 file changed, 134 insertions(+), 56 deletions(-) diff --git a/harmony/src/modules/okd/host_network.rs b/harmony/src/modules/okd/host_network.rs index c79b372..5bef8fb 100644 --- a/harmony/src/modules/okd/host_network.rs +++ b/harmony/src/modules/okd/host_network.rs @@ -1,6 +1,6 @@ use async_trait::async_trait; use harmony_types::id::Id; -use log::{debug, info}; +use log::{debug, info, warn}; use serde::Serialize; use crate::{ @@ -49,6 +49,13 @@ impl HostNetworkConfigurationInterpret { switch_ports: vec![], }); } + if host.network.len() == 1 { + info!("[Host {current_host}/{total_hosts}] Only one interface to configure, skipping"); + return Ok(HostNetworkConfig { + host_id: host.id.clone(), + switch_ports: vec![], + }); + } let switch_ports = self .collect_switch_ports_for_host(topology, host, current_host, total_hosts) @@ -59,7 +66,7 @@ impl HostNetworkConfigurationInterpret { switch_ports, }; - if !config.switch_ports.is_empty() { + if config.switch_ports.len() > 1 { info!( "[Host {current_host}/{total_hosts}] Found {} ports for {} interfaces", config.switch_ports.len(), @@ -76,11 +83,16 @@ impl HostNetworkConfigurationInterpret { .map_err(|e| { InterpretError::new(format!("Failed to configure host network: {e}")) })?; - } else { + } else if config.switch_ports.is_empty() { info!( "[Host {current_host}/{total_hosts}] No ports found for {} interfaces, skipping", host.network.len() ); + } else { + warn!( + "[Host {current_host}/{total_hosts}] Found a single port for {} interfaces, skipping", + host.network.len() + ); } Ok(config) @@ -227,6 +239,7 @@ impl Interpret for HostNetworkConfigur host_configurations.push(host_configuration); current_host += 1; } + if current_host > 1 { let details = self.format_host_configuration(host_configurations); @@ -279,6 +292,18 @@ mod tests { speed_mbps: None, mtu: 1, }; + pub static ref YET_ANOTHER_EXISTING_INTERFACE: NetworkInterface = NetworkInterface { + mac_address: MacAddress::try_from("AA:BB:CC:DD:EE:F3".to_string()).unwrap(), + name: "interface-3".into(), + speed_mbps: None, + mtu: 1, + }; + pub static ref LAST_EXISTING_INTERFACE: NetworkInterface = NetworkInterface { + mac_address: MacAddress::try_from("AA:BB:CC:DD:EE:F4".to_string()).unwrap(), + name: "interface-4".into(), + speed_mbps: None, + mtu: 1, + }; pub static ref UNKNOWN_INTERFACE: NetworkInterface = NetworkInterface { mac_address: MacAddress::try_from("11:22:33:44:55:61".to_string()).unwrap(), name: "unknown-interface".into(), @@ -287,6 +312,8 @@ mod tests { }; pub static ref PORT: PortLocation = PortLocation(1, 0, 42); pub static ref ANOTHER_PORT: PortLocation = PortLocation(2, 0, 42); + pub static ref YET_ANOTHER_PORT: PortLocation = PortLocation(1, 0, 45); + pub static ref LAST_PORT: PortLocation = PortLocation(2, 0, 45); } #[tokio::test] @@ -314,7 +341,7 @@ mod tests { } #[tokio::test] - async fn host_with_one_mac_address_should_configure_bond_with_one_interface() { + async fn host_with_one_mac_address_should_skip_host_configuration() { let host = given_host(&HOST_ID, vec![EXISTING_INTERFACE.clone()]); let score = given_score(vec![host]); let topology = TopologyWithSwitch::new(); @@ -322,37 +349,9 @@ mod tests { let _ = score.interpret(&Inventory::empty(), &topology).await; let config = topology.configured_bonds.lock().unwrap(); - assert_that!(*config).contains_exactly(vec![( - HOST_ID.clone(), - HostNetworkConfig { - host_id: HOST_ID.clone(), - switch_ports: vec![SwitchPort { - interface: EXISTING_INTERFACE.clone(), - port: PORT.clone(), - }], - }, - )]); - } - - #[tokio::test] - async fn host_with_one_mac_address_should_configure_port_channel_with_one_interface() { - let host = given_host(&HOST_ID, vec![EXISTING_INTERFACE.clone()]); - let score = given_score(vec![host]); - let topology = TopologyWithSwitch::new(); - - let _ = score.interpret(&Inventory::empty(), &topology).await; - + assert_that!(*config).is_empty(); let config = topology.configured_port_channels.lock().unwrap(); - assert_that!(*config).contains_exactly(vec![( - HOST_ID.clone(), - HostNetworkConfig { - host_id: HOST_ID.clone(), - switch_ports: vec![SwitchPort { - interface: EXISTING_INTERFACE.clone(), - port: PORT.clone(), - }], - }, - )]); + assert_that!(*config).is_empty(); } #[tokio::test] @@ -423,8 +422,20 @@ mod tests { #[tokio::test] async fn multiple_hosts_should_configure_one_bond_per_host() { let score = given_score(vec![ - given_host(&HOST_ID, vec![EXISTING_INTERFACE.clone()]), - given_host(&ANOTHER_HOST_ID, vec![ANOTHER_EXISTING_INTERFACE.clone()]), + given_host( + &HOST_ID, + vec![ + EXISTING_INTERFACE.clone(), + ANOTHER_EXISTING_INTERFACE.clone(), + ], + ), + given_host( + &ANOTHER_HOST_ID, + vec![ + YET_ANOTHER_EXISTING_INTERFACE.clone(), + LAST_EXISTING_INTERFACE.clone(), + ], + ), ]); let topology = TopologyWithSwitch::new(); @@ -436,20 +447,32 @@ mod tests { HOST_ID.clone(), HostNetworkConfig { host_id: HOST_ID.clone(), - switch_ports: vec![SwitchPort { - interface: EXISTING_INTERFACE.clone(), - port: PORT.clone(), - }], + switch_ports: vec![ + SwitchPort { + interface: EXISTING_INTERFACE.clone(), + port: PORT.clone(), + }, + SwitchPort { + interface: ANOTHER_EXISTING_INTERFACE.clone(), + port: ANOTHER_PORT.clone(), + }, + ], }, ), ( ANOTHER_HOST_ID.clone(), HostNetworkConfig { host_id: ANOTHER_HOST_ID.clone(), - switch_ports: vec![SwitchPort { - interface: ANOTHER_EXISTING_INTERFACE.clone(), - port: ANOTHER_PORT.clone(), - }], + switch_ports: vec![ + SwitchPort { + interface: YET_ANOTHER_EXISTING_INTERFACE.clone(), + port: YET_ANOTHER_PORT.clone(), + }, + SwitchPort { + interface: LAST_EXISTING_INTERFACE.clone(), + port: LAST_PORT.clone(), + }, + ], }, ), ]); @@ -458,8 +481,20 @@ mod tests { #[tokio::test] async fn multiple_hosts_should_configure_one_port_channel_per_host() { let score = given_score(vec![ - given_host(&HOST_ID, vec![EXISTING_INTERFACE.clone()]), - given_host(&ANOTHER_HOST_ID, vec![ANOTHER_EXISTING_INTERFACE.clone()]), + given_host( + &HOST_ID, + vec![ + EXISTING_INTERFACE.clone(), + ANOTHER_EXISTING_INTERFACE.clone(), + ], + ), + given_host( + &ANOTHER_HOST_ID, + vec![ + YET_ANOTHER_EXISTING_INTERFACE.clone(), + LAST_EXISTING_INTERFACE.clone(), + ], + ), ]); let topology = TopologyWithSwitch::new(); @@ -471,27 +506,39 @@ mod tests { HOST_ID.clone(), HostNetworkConfig { host_id: HOST_ID.clone(), - switch_ports: vec![SwitchPort { - interface: EXISTING_INTERFACE.clone(), - port: PORT.clone(), - }], + switch_ports: vec![ + SwitchPort { + interface: EXISTING_INTERFACE.clone(), + port: PORT.clone(), + }, + SwitchPort { + interface: ANOTHER_EXISTING_INTERFACE.clone(), + port: ANOTHER_PORT.clone(), + }, + ], }, ), ( ANOTHER_HOST_ID.clone(), HostNetworkConfig { host_id: ANOTHER_HOST_ID.clone(), - switch_ports: vec![SwitchPort { - interface: ANOTHER_EXISTING_INTERFACE.clone(), - port: ANOTHER_PORT.clone(), - }], + switch_ports: vec![ + SwitchPort { + interface: YET_ANOTHER_EXISTING_INTERFACE.clone(), + port: YET_ANOTHER_PORT.clone(), + }, + SwitchPort { + interface: LAST_EXISTING_INTERFACE.clone(), + port: LAST_PORT.clone(), + }, + ], }, ), ]); } #[tokio::test] - async fn port_not_found_for_mac_address_should_not_configure_interface() { + async fn port_not_found_for_mac_address_should_not_configure_host() { let score = given_score(vec![given_host(&HOST_ID, vec![UNKNOWN_INTERFACE.clone()])]); let topology = TopologyWithSwitch::new_port_not_found(); @@ -503,6 +550,22 @@ mod tests { assert_that!(*config).is_empty(); } + #[tokio::test] + async fn only_one_port_found_for_multiple_mac_addresses_should_not_configure_host() { + let score = given_score(vec![given_host( + &HOST_ID, + vec![EXISTING_INTERFACE.clone(), UNKNOWN_INTERFACE.clone()], + )]); + let topology = TopologyWithSwitch::new_single_port_found(); + + let _ = score.interpret(&Inventory::empty(), &topology).await; + + let config = topology.configured_port_channels.lock().unwrap(); + assert_that!(*config).is_empty(); + let config = topology.configured_bonds.lock().unwrap(); + assert_that!(*config).is_empty(); + } + fn given_score(hosts: Vec) -> HostNetworkConfigurationScore { HostNetworkConfigurationScore { hosts } } @@ -549,7 +612,12 @@ mod tests { impl TopologyWithSwitch { fn new() -> Self { Self { - available_ports: Arc::new(Mutex::new(vec![PORT.clone(), ANOTHER_PORT.clone()])), + available_ports: Arc::new(Mutex::new(vec![ + PORT.clone(), + ANOTHER_PORT.clone(), + YET_ANOTHER_PORT.clone(), + LAST_PORT.clone(), + ])), configured_port_channels: Arc::new(Mutex::new(vec![])), switch_setup: Arc::new(Mutex::new(false)), network_manager_setup: Arc::new(Mutex::new(false)), @@ -566,6 +634,16 @@ mod tests { configured_bonds: Arc::new(Mutex::new(vec![])), } } + + fn new_single_port_found() -> Self { + Self { + available_ports: Arc::new(Mutex::new(vec![PORT.clone()])), + configured_port_channels: Arc::new(Mutex::new(vec![])), + switch_setup: Arc::new(Mutex::new(false)), + network_manager_setup: Arc::new(Mutex::new(false)), + configured_bonds: Arc::new(Mutex::new(vec![])), + } + } } #[async_trait] -- 2.39.5 From 7fe15ffa2404c78dad25c9d90f48979d6a2ca1f1 Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Wed, 5 Nov 2025 13:20:51 -0500 Subject: [PATCH 08/11] remove unnecessary println --- harmony/src/modules/okd/host_network.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/harmony/src/modules/okd/host_network.rs b/harmony/src/modules/okd/host_network.rs index 5bef8fb..9d48035 100644 --- a/harmony/src/modules/okd/host_network.rs +++ b/harmony/src/modules/okd/host_network.rs @@ -150,15 +150,6 @@ impl HostNetworkConfigurationInterpret { ]; for config in configs { - let host = self - .score - .hosts - .iter() - .find(|h| h.id == config.host_id) - .unwrap(); - - println!("[Host] {host}"); - if config.switch_ports.is_empty() { report.push(format!( "⏭️ Host {}: SKIPPED (No matching switch ports found)", -- 2.39.5 From c166351d8b30de3948605b3d78ebeb0d96a39d87 Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Wed, 5 Nov 2025 14:53:42 -0500 Subject: [PATCH 09/11] fix nmstate attribute --- harmony/src/infra/network_manager.rs | 2 +- harmony/src/modules/okd/crd/nmstate.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/harmony/src/infra/network_manager.rs b/harmony/src/infra/network_manager.rs index 5b1b825..f52e57b 100644 --- a/harmony/src/infra/network_manager.rs +++ b/harmony/src/infra/network_manager.rs @@ -179,7 +179,7 @@ impl OpenShiftNmStateNetworkManager { }), link_aggregation: Some(nmstate::BondSpec { mode: "802.3ad".to_string(), - ports: bond_ports, + port: bond_ports, ..Default::default() }), ..Default::default() diff --git a/harmony/src/modules/okd/crd/nmstate.rs b/harmony/src/modules/okd/crd/nmstate.rs index f0eb4ae..5bb839c 100644 --- a/harmony/src/modules/okd/crd/nmstate.rs +++ b/harmony/src/modules/okd/crd/nmstate.rs @@ -417,7 +417,7 @@ pub struct EthernetSpec { #[serde(rename_all = "kebab-case")] pub struct BondSpec { pub mode: String, - pub ports: Vec, + pub port: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub options: Option>, } @@ -477,7 +477,7 @@ pub struct LinuxBridgeSpec { #[serde(skip_serializing_if = "Option::is_none")] pub options: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub ports: Option>, + pub port: Option>, } #[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] @@ -548,7 +548,7 @@ pub struct OvsBridgeSpec { #[serde(skip_serializing_if = "Option::is_none")] pub options: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub ports: Option>, + pub port: Option>, } #[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] -- 2.39.5 From 1d5ef24844f5ccab9f023ad64ef40cc7381cf3bd Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Wed, 5 Nov 2025 15:02:32 -0500 Subject: [PATCH 10/11] revert last changes (was unnecessary) --- harmony/src/infra/network_manager.rs | 2 +- harmony/src/modules/okd/crd/nmstate.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/harmony/src/infra/network_manager.rs b/harmony/src/infra/network_manager.rs index f52e57b..5b1b825 100644 --- a/harmony/src/infra/network_manager.rs +++ b/harmony/src/infra/network_manager.rs @@ -179,7 +179,7 @@ impl OpenShiftNmStateNetworkManager { }), link_aggregation: Some(nmstate::BondSpec { mode: "802.3ad".to_string(), - port: bond_ports, + ports: bond_ports, ..Default::default() }), ..Default::default() diff --git a/harmony/src/modules/okd/crd/nmstate.rs b/harmony/src/modules/okd/crd/nmstate.rs index 5bb839c..f0eb4ae 100644 --- a/harmony/src/modules/okd/crd/nmstate.rs +++ b/harmony/src/modules/okd/crd/nmstate.rs @@ -417,7 +417,7 @@ pub struct EthernetSpec { #[serde(rename_all = "kebab-case")] pub struct BondSpec { pub mode: String, - pub port: Vec, + pub ports: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub options: Option>, } @@ -477,7 +477,7 @@ pub struct LinuxBridgeSpec { #[serde(skip_serializing_if = "Option::is_none")] pub options: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub port: Option>, + pub ports: Option>, } #[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] @@ -548,7 +548,7 @@ pub struct OvsBridgeSpec { #[serde(skip_serializing_if = "Option::is_none")] pub options: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub port: Option>, + pub ports: Option>, } #[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] -- 2.39.5 From 77dae13cba03b6c6692fd2debeae2fc224931b71 Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Wed, 5 Nov 2025 17:46:07 -0500 Subject: [PATCH 11/11] fix(host_network): remove extra fields from bond config to prevent clashes --- harmony/src/infra/network_manager.rs | 4 +--- harmony/src/modules/okd/crd/nmstate.rs | 1 + harmony/src/modules/okd/host_network.rs | 2 +- harmony_types/src/net.rs | 10 +++++++++- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/harmony/src/infra/network_manager.rs b/harmony/src/infra/network_manager.rs index 5b1b825..484d6b0 100644 --- a/harmony/src/infra/network_manager.rs +++ b/harmony/src/infra/network_manager.rs @@ -135,8 +135,6 @@ impl OpenShiftNmStateNetworkManager { description: Some(format!("Member of bond {bond_name}")), r#type: nmstate::InterfaceType::Ethernet, state: "up".to_string(), - mtu: Some(switch_port.interface.mtu), - mac_address: Some(switch_port.interface.mac_address.to_string()), ipv4: Some(nmstate::IpStackSpec { enabled: Some(false), ..Default::default() @@ -162,7 +160,7 @@ impl OpenShiftNmStateNetworkManager { interfaces.push(nmstate::Interface { name: bond_name.to_string(), - description: Some(format!("Network bond for host {host}")), + description: Some(format!("HARMONY - Network bond for host {host}")), r#type: nmstate::InterfaceType::Bond, state: "up".to_string(), copy_mac_from, diff --git a/harmony/src/modules/okd/crd/nmstate.rs b/harmony/src/modules/okd/crd/nmstate.rs index f0eb4ae..3055766 100644 --- a/harmony/src/modules/okd/crd/nmstate.rs +++ b/harmony/src/modules/okd/crd/nmstate.rs @@ -417,6 +417,7 @@ pub struct EthernetSpec { #[serde(rename_all = "kebab-case")] pub struct BondSpec { pub mode: String, + #[serde(alias = "port")] pub ports: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub options: Option>, diff --git a/harmony/src/modules/okd/host_network.rs b/harmony/src/modules/okd/host_network.rs index 9d48035..33d1ef5 100644 --- a/harmony/src/modules/okd/host_network.rs +++ b/harmony/src/modules/okd/host_network.rs @@ -1,5 +1,5 @@ use async_trait::async_trait; -use harmony_types::id::Id; +use harmony_types::{id::Id, net::MacAddress, switch::PortLocation}; use log::{debug, info, warn}; use serde::Serialize; diff --git a/harmony_types/src/net.rs b/harmony_types/src/net.rs index 51de86e..6086e54 100644 --- a/harmony_types/src/net.rs +++ b/harmony_types/src/net.rs @@ -1,6 +1,6 @@ use serde::{Deserialize, Serialize}; -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, PartialOrd, Ord)] +#[derive(Copy, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, PartialOrd, Ord)] pub struct MacAddress(pub [u8; 6]); impl MacAddress { @@ -19,6 +19,14 @@ impl From<&MacAddress> for String { } } +impl std::fmt::Debug for MacAddress { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_tuple("MacAddress") + .field(&String::from(self)) + .finish() + } +} + impl std::fmt::Display for MacAddress { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(&String::from(self)) -- 2.39.5