From 95cfc03518e90095bbc3ae594bb1f07136554a7b Mon Sep 17 00:00:00 2001 From: Jean-Gabriel Gill-Couture Date: Wed, 29 Oct 2025 17:24:35 -0400 Subject: [PATCH 1/4] feat(kube): Utility function to convert kube_openapi Resource to DynamicObject. This will allow initializing resources strongly typed and then bundle various types into a list of DynamicObject --- harmony/src/infra/kube.rs | 182 ++++++++++++++++++++++++++++++++++++++ harmony/src/infra/mod.rs | 1 + 2 files changed, 183 insertions(+) create mode 100644 harmony/src/infra/kube.rs diff --git a/harmony/src/infra/kube.rs b/harmony/src/infra/kube.rs new file mode 100644 index 0000000..9fb1247 --- /dev/null +++ b/harmony/src/infra/kube.rs @@ -0,0 +1,182 @@ +use k8s_openapi::Resource as K8sResource; +use kube::api::{ApiResource, DynamicObject, GroupVersionKind}; +use kube::core::TypeMeta; +use serde::Serialize; +use serde::de::DeserializeOwned; +use serde_json::Value; + +/// Convert a typed Kubernetes resource `K` into a `DynamicObject`. +/// +/// Requirements: +/// - `K` must be a k8s_openapi resource (provides static GVK via `Resource`). +/// - `K` must have standard Kubernetes shape (metadata + payload fields). +/// +/// Notes: +/// - We set `types` (apiVersion/kind) and copy `metadata`. +/// - We place the remaining top-level fields into `obj.data` as JSON. +/// - Scope is not encoded on the object itself; you still need the corresponding +/// `DynamicResource` (derived from K::group/version/kind) when constructing an Api. +/// +/// Example usage: +/// let dyn_obj = kube_resource_to_dynamic(secret)?; +/// let api: Api = Api::namespaced_with(client, "ns", &dr); +/// api.patch(&dyn_obj.name_any(), &PatchParams::apply("mgr"), &Patch::Apply(dyn_obj)).await?; +pub fn kube_resource_to_dynamic(res: &K) -> Result +where + K: K8sResource + Serialize + DeserializeOwned, +{ + // Serialize the typed resource to JSON so we can split metadata and payload + let mut v = serde_json::to_value(res).map_err(|e| format!("Failed to serialize : {e}"))?; + let obj = v + .as_object_mut() + .ok_or_else(|| "expected object JSON".to_string())?; + + // Extract and parse metadata into kube::core::ObjectMeta + let metadata_value = obj + .remove("metadata") + .ok_or_else(|| "missing metadata".to_string())?; + let metadata: kube::core::ObjectMeta = serde_json::from_value(metadata_value) + .map_err(|e| format!("Failed to deserialize : {e}"))?; + + // Name is required for DynamicObject::new; prefer metadata.name + let name = metadata + .name + .clone() + .ok_or_else(|| "metadata.name is required".to_string())?; + + // Remaining fields (spec/status/data/etc.) become the dynamic payload + let payload = Value::Object(obj.clone()); + + // Construct the DynamicObject + let mut dyn_obj = DynamicObject::new( + &name, + &ApiResource::from_gvk(&GroupVersionKind::gvk(K::GROUP, K::VERSION, K::KIND)), + ); + dyn_obj.types = Some(TypeMeta { + api_version: api_version_for::(), + kind: K::KIND.into(), + }); + + // Preserve namespace/labels/annotations/etc. + dyn_obj.metadata = metadata; + + // Attach payload + dyn_obj.data = payload; + + Ok(dyn_obj) +} + +/// Helper: compute apiVersion string ("group/version" or "v1" for core). +fn api_version_for() -> String +where + K: K8sResource, +{ + let group = K::GROUP; + let version = K::VERSION; + if group.is_empty() { + version.to_string() // core/v1 => "v1" + } else { + format!("{}/{}", group, version) + } +} +#[cfg(test)] +mod test { + use super::*; + use k8s_openapi::api::{ + apps::v1::{Deployment, DeploymentSpec}, + core::v1::{PodTemplateSpec, Secret}, + }; + use kube::api::ObjectMeta; + use pretty_assertions::assert_eq; + + #[test] + fn secret_to_dynamic_roundtrip() { + // Create a sample Secret resource + let mut secret = Secret { + metadata: ObjectMeta { + name: Some("my-secret".to_string()), + ..Default::default() + }, + type_: Some("kubernetes.io/service-account-token".to_string()), + ..Default::default() + }; + + // Convert to DynamicResource + let dynamic: DynamicObject = + kube_resource_to_dynamic(&secret).expect("Failed to convert Secret to DynamicResource"); + + // Serialize both the original and dynamic resources to Value + let original_value = serde_json::to_value(&secret).expect("Failed to serialize Secret"); + let dynamic_value = + serde_json::to_value(&dynamic).expect("Failed to serialize DynamicResource"); + + // Assert that they are identical + assert_eq!(original_value, dynamic_value); + + secret.metadata.namespace = Some("false".to_string()); + let modified_value = serde_json::to_value(&secret).expect("Failed to serialize Secret"); + assert_ne!(modified_value, dynamic_value); + } + + #[test] + fn deployment_to_dynamic_roundtrip() { + // Create a sample Deployment with nested structures + let mut deployment = Deployment { + metadata: ObjectMeta { + name: Some("my-deployment".to_string()), + labels: Some({ + let mut map = std::collections::BTreeMap::new(); + map.insert("app".to_string(), "nginx".to_string()); + map + }), + ..Default::default() + }, + spec: Some(DeploymentSpec { + replicas: Some(3), + selector: Default::default(), + template: PodTemplateSpec { + metadata: Some(ObjectMeta { + labels: Some({ + let mut map = std::collections::BTreeMap::new(); + map.insert("app".to_string(), "nginx".to_string()); + map + }), + ..Default::default() + }), + spec: Some(Default::default()), // PodSpec with empty containers for simplicity + }, + ..Default::default() + }), + ..Default::default() + }; + + let dynamic = kube_resource_to_dynamic(&deployment).expect("Failed to convert Deployment"); + + let original_value = serde_json::to_value(&deployment).unwrap(); + let dynamic_value = serde_json::to_value(&dynamic).unwrap(); + + assert_eq!(original_value, dynamic_value); + + assert_eq!( + dynamic.data.get("spec").unwrap().get("replicas").unwrap(), + 3 + ); + assert_eq!( + dynamic + .data + .get("spec") + .unwrap() + .get("template") + .unwrap() + .get("metadata") + .unwrap() + .get("labels") + .unwrap() + .get("app") + .unwrap() + .as_str() + .unwrap(), + "nginx".to_string() + ); + } +} diff --git a/harmony/src/infra/mod.rs b/harmony/src/infra/mod.rs index 203cf90..253176c 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 kube; pub mod opnsense; mod sqlx; From 9d4e6acac0eb1027b39ba522c680e6bbc5f23668 Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Wed, 5 Nov 2025 23:38:24 +0000 Subject: [PATCH 2/4] fix(host_network): retrieve proper hostname and next available bond id (#182) In order to query the current network state `NodeNetworkState` and to apply a `NodeNetworkConfigurationPolicy` for a given node, we first needed to find its hostname. As all we had was the UUID of a node. We had different options available (e.g. updating the Harmony Inventory Agent to retrieve it, store it in the OKD installation pipeline on assignation, etc.). But for the sake of simplicity and for better flexibility (e.g. being able to run this score on a cluster that wasn't setup with Harmony), the `hostname` was retrieved directly in the cluster by running the equivalent of `kubectl get nodes -o yaml` and matching the nodes with the system UUID. ### Other changes * Find the next available bond id for a node * Apply a network config policy for a node (configuring a bond in our case) * Adjust the CRDs for NMState Note: to see a quick demo, watch the recording in https://git.nationtech.io/NationTech/harmony/pulls/183 Reviewed-on: https://git.nationtech.io/NationTech/harmony/pulls/182 Reviewed-by: johnride --- harmony/src/domain/topology/ha_cluster.rs | 133 +++++++-- harmony/src/domain/topology/k8s.rs | 59 +++- harmony/src/modules/inventory/discovery.rs | 11 +- harmony/src/modules/okd/crd/nmstate.rs | 305 +++++++++++++++++++-- 4 files changed, 447 insertions(+), 61 deletions(-) diff --git a/harmony/src/domain/topology/ha_cluster.rs b/harmony/src/domain/topology/ha_cluster.rs index d65d9aa..8f65e53 100644 --- a/harmony/src/domain/topology/ha_cluster.rs +++ b/harmony/src/domain/topology/ha_cluster.rs @@ -1,10 +1,15 @@ 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; @@ -22,7 +27,7 @@ use super::{ Topology, k8s::K8sClient, }; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::sync::Arc; #[derive(Debug, Clone)] @@ -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)) @@ -143,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 @@ -152,10 +160,6 @@ impl HAClusterTopology { Ok(()) } - fn get_next_bond_id(&self) -> u8 { - 42 // FIXME: Find a better way to declare the bond id - } - async fn configure_bond(&self, config: &HostNetworkConfig) -> Result<(), SwitchError> { self.ensure_nmstate_operator_installed() .await @@ -165,9 +169,23 @@ 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:#?}", + "Applying NMState bond config for host {}:\n{}", + serde_yaml::to_string(&bond_config).unwrap(), config.host_id ); self.k8s_client() @@ -182,26 +200,24 @@ 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(), + 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()), @@ -228,10 +244,10 @@ impl HAClusterTopology { } } - interfaces.push(nmstate::InterfaceSpec { - name: bond_name.clone(), - description: Some(format!("Network bond for host {host_name}")), - r#type: "bond".to_string(), + 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 { @@ -255,19 +271,80 @@ 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 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) + .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(); @@ -458,16 +535,14 @@ 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> { 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..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,28 +48,223 @@ pub struct ProbeDns { group = "nmstate.io", version = "v1", kind = "NodeNetworkConfigurationPolicy", - namespaced + namespaced = false )] #[serde(rename_all = "camelCase")] pub struct NodeNetworkConfigurationPolicySpec { #[serde(skip_serializing_if = "Option::is_none")] pub node_selector: Option>, - pub desired_state: DesiredStateSpec, + pub desired_state: NetworkState, +} + +// 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 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)] +#[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, - pub r#type: String, + pub r#type: InterfaceType, pub state: String, #[serde(skip_serializing_if = "Option::is_none")] pub mac_address: Option, @@ -99,9 +295,81 @@ pub struct InterfaceSpec { #[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, 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)] @@ -287,11 +555,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 +577,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, -} From 06a004a65d7b0115e5c7c7fc4455d0252aa3244e Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Thu, 6 Nov 2025 00:02:52 +0000 Subject: [PATCH 3/4] refactor(host_network): extract NetworkManager as a reusable component (#183) The NetworkManager logic was implemented directly into the `HaClusterTopology`, which wasn't directly its concern and prevented us from being able to reuse that NetworkManaager implementations in the future for a different Topology. * Extract a `NetworkManager` trait * Implement a `OpenShiftNmStateNetworkManager` for `NetworkManager` * Dynamically instantiate the NetworkManager in the Topology to delegate calls to it Reviewed-on: https://git.nationtech.io/NationTech/harmony/pulls/183 Reviewed-by: johnride --- 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 | 309 +++------------------- 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 | 190 +++++++++++-- 10 files changed, 508 insertions(+), 306 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 8f65e53..558f97c 100644 --- a/harmony/src/domain/topology/ha_cluster.rs +++ b/harmony/src/domain/topology/ha_cluster.rs @@ -5,30 +5,21 @@ use harmony_types::{ 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 +36,7 @@ pub struct HAClusterTopology { pub control_plane: Vec, pub workers: Vec, pub kubeconfig: Option, + pub network_manager: OnceLock>, } #[async_trait] @@ -98,263 +90,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) - .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 +119,7 @@ impl HAClusterTopology { bootstrap_host: dummy_host, control_plane: vec![], workers: vec![], + network_manager: OnceLock::new(), } } } @@ -545,9 +287,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 253176c..b996afb 100644 --- a/harmony/src/infra/mod.rs +++ b/harmony/src/infra/mod.rs @@ -4,5 +4,6 @@ pub mod hp_ilo; pub mod intel_amt; pub mod inventory; pub mod kube; +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..89321fe --- /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{}", + config.host_id, + serde_yaml::to_string(&bond_config).unwrap(), + ); + 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) + .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..e68c6df 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::info; use serde::Serialize; use crate::{ @@ -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", @@ -113,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 '{}': {}", @@ -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(()) } From 66d346a10c01c8966927a1259c68e2f0e61f20a2 Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Thu, 6 Nov 2025 00:07:20 +0000 Subject: [PATCH 4/4] fix(host_network): skip configuration for host with only 1 interface/port (#185) Reviewed-on: https://git.nationtech.io/NationTech/harmony/pulls/185 Reviewed-by: johnride --- harmony/src/modules/okd/host_network.rs | 199 ++++++++++++++++-------- 1 file changed, 134 insertions(+), 65 deletions(-) diff --git a/harmony/src/modules/okd/host_network.rs b/harmony/src/modules/okd/host_network.rs index e68c6df..ee68942 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::info; +use log::{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) @@ -138,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)", @@ -227,6 +230,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 +283,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 +303,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 +332,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 +340,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 +413,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 +438,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 +472,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 +497,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 +541,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 +603,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 +625,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]