fix(host_network): skip configuration for host with only 1 interface/port

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<T: Topology + NetworkManager + Switch> Interpret<T> 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![(
+        assert_that!(*config).is_empty();
-            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;
-
         let config = topology.configured_port_channels.lock().unwrap();
-        assert_that!(*config).contains_exactly(vec![(
+        assert_that!(*config).is_empty();
-            HOST_ID.clone(),
-            HostNetworkConfig {
-                host_id: HOST_ID.clone(),
-                switch_ports: vec![SwitchPort {
-                    interface: EXISTING_INTERFACE.clone(),
-                    port: PORT.clone(),
-                }],
-            },
-        )]);
     }
 
     #[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(
-            given_host(&ANOTHER_HOST_ID, vec![ANOTHER_EXISTING_INTERFACE.clone()]),
+                &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 {
+                    switch_ports: vec![
-                        interface: EXISTING_INTERFACE.clone(),
+                        SwitchPort {
-                        port: PORT.clone(),
+                            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 {
+                    switch_ports: vec![
-                        interface: ANOTHER_EXISTING_INTERFACE.clone(),
+                        SwitchPort {
-                        port: ANOTHER_PORT.clone(),
+                            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(
-            given_host(&ANOTHER_HOST_ID, vec![ANOTHER_EXISTING_INTERFACE.clone()]),
+                &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 {
+                    switch_ports: vec![
-                        interface: EXISTING_INTERFACE.clone(),
+                        SwitchPort {
-                        port: PORT.clone(),
+                            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 {
+                    switch_ports: vec![
-                        interface: ANOTHER_EXISTING_INTERFACE.clone(),
+                        SwitchPort {
-                        port: ANOTHER_PORT.clone(),
+                            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<PhysicalHost>) -> 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]