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]