diff --git a/examples/opnsense_vm_integration/src/main.rs b/examples/opnsense_vm_integration/src/main.rs index 9e8f180..ed43581 100644 --- a/examples/opnsense_vm_integration/src/main.rs +++ b/examples/opnsense_vm_integration/src/main.rs @@ -600,7 +600,6 @@ fn build_all_scores() -> Result>>, Box LoadBalancerScore { lb_service.clone(), lb_service.clone(), ], - wan_firewall_ports: vec![], } } diff --git a/harmony/src/domain/topology/firewall_pair.rs b/harmony/src/domain/topology/firewall_pair.rs index 3954816..1039cf4 100644 --- a/harmony/src/domain/topology/firewall_pair.rs +++ b/harmony/src/domain/topology/firewall_pair.rs @@ -15,7 +15,7 @@ use async_trait::async_trait; use harmony_types::firewall::VipMode; use harmony_types::id::Id; use harmony_types::net::{IpAddress, MacAddress}; -use log::info; +use log::{info, warn}; use serde::Serialize; use crate::config::secret::{OPNSenseApiCredentials, OPNSenseFirewallCredentials}; @@ -176,8 +176,24 @@ impl DhcpServer for FirewallPairTopology { } async fn list_static_mappings(&self) -> Vec<(MacAddress, IpAddress)> { - // Return primary's view — both should be identical - self.primary.list_static_mappings().await + let primary_mappings = self.primary.list_static_mappings().await; + let backup_mappings = self.backup.list_static_mappings().await; + + let primary_set: std::collections::HashSet<_> = primary_mappings.iter().collect(); + let backup_set: std::collections::HashSet<_> = backup_mappings.iter().collect(); + + let only_primary: Vec<_> = primary_set.difference(&backup_set).collect(); + let only_backup: Vec<_> = backup_set.difference(&primary_set).collect(); + + if !only_primary.is_empty() || !only_backup.is_empty() { + warn!( + "DHCP static mapping mismatch between primary and backup firewalls! \ + Only on primary: {:?}, Only on backup: {:?}", + only_primary, only_backup + ); + } + + primary_mappings } /// Returns the primary firewall's IP. In a CARP setup, callers diff --git a/harmony/src/domain/topology/ha_cluster.rs b/harmony/src/domain/topology/ha_cluster.rs index b556dfc..87e7f29 100644 --- a/harmony/src/domain/topology/ha_cluster.rs +++ b/harmony/src/domain/topology/ha_cluster.rs @@ -489,6 +489,9 @@ impl LoadBalancer for DummyInfra { async fn reload_restart(&self) -> Result<(), ExecutorError> { unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA) } + async fn ensure_wan_access(&self, _port: u16) -> Result<(), ExecutorError> { + unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA) + } } #[async_trait] diff --git a/harmony/src/domain/topology/load_balancer.rs b/harmony/src/domain/topology/load_balancer.rs index 3bb36c5..8d70716 100644 --- a/harmony/src/domain/topology/load_balancer.rs +++ b/harmony/src/domain/topology/load_balancer.rs @@ -37,11 +37,9 @@ pub trait LoadBalancer: Send + Sync { /// from the WAN interface. Used by load balancers that need to receive /// external traffic (e.g., OKD ingress on ports 80/443). /// - /// Default implementation is a no-op for topologies that don't manage - /// firewall rules (e.g., cloud environments with security groups). - async fn ensure_wan_access(&self, _port: u16) -> Result<(), ExecutorError> { - Ok(()) - } + /// Topologies that don't manage firewall rules (e.g., cloud environments + /// with security groups) should return `Ok(())`. + async fn ensure_wan_access(&self, port: u16) -> Result<(), ExecutorError>; } #[derive(Debug, PartialEq, Clone, Serialize)] diff --git a/harmony/src/infra/opnsense/load_balancer.rs b/harmony/src/infra/opnsense/load_balancer.rs index bb97b25..4e496be 100644 --- a/harmony/src/infra/opnsense/load_balancer.rs +++ b/harmony/src/infra/opnsense/load_balancer.rs @@ -111,12 +111,16 @@ impl LoadBalancer for OPNSenseFirewall { } fn haproxy_service_to_harmony(svc: &HaproxyService) -> Option { - let listening_port = svc.bind.parse().unwrap_or_else(|_| { - panic!( - "HAProxy frontend address should be a valid SocketAddr, got {}", - svc.bind - ) - }); + let listening_port = match svc.bind.parse() { + Ok(addr) => addr, + Err(e) => { + warn!( + "Skipping HAProxy service: bind address '{}' is not a valid SocketAddr: {e}", + svc.bind + ); + return None; + } + }; let backend_servers: Vec = svc .servers diff --git a/harmony/src/modules/kvm/executor.rs b/harmony/src/modules/kvm/executor.rs index d79cb24..155be5b 100644 --- a/harmony/src/modules/kvm/executor.rs +++ b/harmony/src/modules/kvm/executor.rs @@ -10,6 +10,7 @@ use virt::sys; use super::error::KvmError; use super::types::{CdromConfig, NetworkConfig, VmConfig, VmInterface, VmStatus}; use super::xml; +use harmony_types::net::MacAddress; /// A handle to a libvirt hypervisor. /// @@ -374,14 +375,15 @@ impl KvmExecutor { pub async fn set_interface_link( &self, vm_name: &str, - mac: &str, + mac: &MacAddress, up: bool, ) -> Result<(), KvmError> { let state = if up { "up" } else { "down" }; - info!("Setting {vm_name} interface {mac} link {state}"); + let mac_str = mac.to_string(); + info!("Setting {vm_name} interface {mac_str} link {state}"); let output = tokio::process::Command::new("virsh") - .args(["-c", &self.uri, "domif-setlink", vm_name, mac, state]) + .args(["-c", &self.uri, "domif-setlink", vm_name, &mac_str, state]) .output() .await?; @@ -420,11 +422,18 @@ impl KvmExecutor { // virsh domiflist columns: Interface, Type, Source, Model, MAC let parts: Vec<&str> = line.split_whitespace().collect(); if parts.len() >= 5 { + let mac = match MacAddress::try_from(parts[4].to_string()) { + Ok(m) => m, + Err(e) => { + warn!("Skipping interface with invalid MAC '{}': {e}", parts[4]); + continue; + } + }; interfaces.push(VmInterface { interface_type: parts[1].to_string(), source: parts[2].to_string(), model: parts[3].to_string(), - mac: parts[4].to_string(), + mac, }); } } diff --git a/harmony/src/modules/kvm/types.rs b/harmony/src/modules/kvm/types.rs index 2fc9829..590bfe6 100644 --- a/harmony/src/modules/kvm/types.rs +++ b/harmony/src/modules/kvm/types.rs @@ -1,3 +1,4 @@ +use harmony_types::net::MacAddress; use serde::{Deserialize, Serialize}; /// Information about a VM's network interface, as reported by `virsh domiflist`. @@ -10,7 +11,7 @@ pub struct VmInterface { /// Device model (e.g. "virtio") pub model: String, /// MAC address - pub mac: String, + pub mac: MacAddress, } /// Specifies how a KVM host is accessed. @@ -95,7 +96,7 @@ pub struct NetworkRef { pub name: String, /// Optional fixed MAC address for this interface. When `None`, libvirt /// assigns one automatically. - pub mac: Option, + pub mac: Option, } impl NetworkRef { @@ -106,8 +107,8 @@ impl NetworkRef { } } - pub fn with_mac(mut self, mac: impl Into) -> Self { - self.mac = Some(mac.into()); + pub fn with_mac(mut self, mac: MacAddress) -> Self { + self.mac = Some(mac); self } } @@ -260,7 +261,7 @@ impl VmConfigBuilder { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct DhcpHost { /// MAC address (e.g. `"52:54:00:00:50:01"`). - pub mac: String, + pub mac: MacAddress, /// IP to assign (e.g. `"10.50.0.2"`). pub ip: String, /// Optional hostname. @@ -356,12 +357,12 @@ impl NetworkConfigBuilder { /// Add a static DHCP host entry (MAC → fixed IP). pub fn dhcp_host( mut self, - mac: impl Into, + mac: MacAddress, ip: impl Into, name: Option, ) -> Self { self.dhcp_hosts.push(DhcpHost { - mac: mac.into(), + mac, ip: ip.into(), name, }); diff --git a/harmony/src/modules/kvm/xml.rs b/harmony/src/modules/kvm/xml.rs index ec3f141..cc4db9b 100644 --- a/harmony/src/modules/kvm/xml.rs +++ b/harmony/src/modules/kvm/xml.rs @@ -136,7 +136,7 @@ fn nic_devices(vm: &VmConfig) -> String { .map(|net| { let mac_line = net .mac - .as_deref() + .as_ref() .map(|m| format!("\n ")) .unwrap_or_default(); format!( @@ -221,6 +221,7 @@ mod tests { use crate::modules::kvm::types::{ BootDevice, ForwardMode, NetworkConfig, NetworkRef, VmConfig, }; + use harmony_types::net::MacAddress; // ── Domain XML ────────────────────────────────────────────────────── @@ -284,12 +285,13 @@ mod tests { #[test] fn domain_xml_nic_with_mac_address() { + let mac: MacAddress = "52:54:00:aa:bb:cc".to_string().try_into().unwrap(); let vm = VmConfig::builder("mac-test") - .network(NetworkRef::named("mynet").with_mac("52:54:00:AA:BB:CC")) + .network(NetworkRef::named("mynet").with_mac(mac)) .build(); let xml = domain_xml(&vm, "/tmp"); - assert!(xml.contains("mac address='52:54:00:AA:BB:CC'")); + assert!(xml.contains("mac address='52:54:00:aa:bb:cc'")); } #[test] @@ -454,14 +456,11 @@ mod tests { #[test] fn network_xml_with_dhcp_host() { + let mac: MacAddress = "52:54:00:00:50:01".to_string().try_into().unwrap(); let cfg = NetworkConfig::builder("hostnet") .subnet("10.50.0.1", 24) .dhcp_range("10.50.0.100", "10.50.0.200") - .dhcp_host( - "52:54:00:00:50:01", - "10.50.0.2", - Some("opnsense".to_string()), - ) + .dhcp_host(mac, "10.50.0.2", Some("opnsense".to_string())) .build(); let xml = network_xml(&cfg); diff --git a/harmony/src/modules/load_balancer.rs b/harmony/src/modules/load_balancer.rs index 912aeb2..033ba2e 100644 --- a/harmony/src/modules/load_balancer.rs +++ b/harmony/src/modules/load_balancer.rs @@ -19,12 +19,6 @@ pub struct LoadBalancerScore { // (listen_interface, LoadBalancerService) tuples or something like that // I am not sure what to use as listen_interface, should it be interface name, ip address, // uuid? - /// TCP ports that must be open for inbound WAN traffic. - /// - /// The load balancer interpret will call `ensure_wan_access` for each port - /// before configuring services, so that the load balancer is reachable - /// from outside the LAN. - pub wan_firewall_ports: Vec, } impl Score for LoadBalancerScore { @@ -66,11 +60,6 @@ impl Interpret for LoadBalancerInterpret { load_balancer.ensure_initialized().await? ); - for port in &self.score.wan_firewall_ports { - info!("Ensuring WAN access for port {port}"); - load_balancer.ensure_wan_access(*port).await?; - } - for service in self.score.public_services.iter() { info!("Ensuring service exists {service:?}"); diff --git a/harmony/src/modules/okd/bootstrap_02_bootstrap.rs b/harmony/src/modules/okd/bootstrap_02_bootstrap.rs index 2f5ec3c..b1f7185 100644 --- a/harmony/src/modules/okd/bootstrap_02_bootstrap.rs +++ b/harmony/src/modules/okd/bootstrap_02_bootstrap.rs @@ -350,13 +350,20 @@ impl OKDSetup02BootstrapInterpret { &self, inventory: &Inventory, ) -> Result<(), InterpretError> { - info!("[Stage 02/Bootstrap] Waiting for bootstrap to complete..."); + let timeout_minutes: u64 = std::env::var("HARMONY_OKD_BOOTSTRAP_TIMEOUT_MINUTES") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(90); + + info!( + "[Stage 02/Bootstrap] Waiting for bootstrap to complete (timeout: {timeout_minutes}m)..." + ); info!("[Stage 02/Bootstrap] Running: openshift-install wait-for bootstrap-complete"); let okd_installation_path = format!("./data/okd/installation_files_{}", inventory.location.name); - let output = Command::new("./data/okd/bin/openshift-install") + let child = Command::new("./data/okd/bin/openshift-install") .args([ "wait-for", "bootstrap-complete", @@ -364,8 +371,17 @@ impl OKDSetup02BootstrapInterpret { &okd_installation_path, "--log-level=info", ]) - .output() + .output(); + + let timeout = std::time::Duration::from_secs(timeout_minutes * 60); + let output = tokio::time::timeout(timeout, child) .await + .map_err(|_| { + InterpretError::new(format!( + "[Stage 02/Bootstrap] bootstrap-complete timed out after {timeout_minutes} minutes. \ + Set HARMONY_OKD_BOOTSTRAP_TIMEOUT_MINUTES to increase the timeout and retry." + )) + })? .map_err(|e| { InterpretError::new(format!( "[Stage 02/Bootstrap] Failed to run openshift-install wait-for bootstrap-complete: {e}" diff --git a/harmony/src/modules/okd/bootstrap_load_balancer.rs b/harmony/src/modules/okd/bootstrap_load_balancer.rs index cd408ef..814bba3 100644 --- a/harmony/src/modules/okd/bootstrap_load_balancer.rs +++ b/harmony/src/modules/okd/bootstrap_load_balancer.rs @@ -56,7 +56,6 @@ impl OKDBootstrapLoadBalancerScore { load_balancer_score: LoadBalancerScore { public_services: vec![], private_services, - wan_firewall_ports: vec![80, 443], }, } } diff --git a/harmony/src/modules/okd/load_balancer.rs b/harmony/src/modules/okd/load_balancer.rs index fb21790..0f9dfa6 100644 --- a/harmony/src/modules/okd/load_balancer.rs +++ b/harmony/src/modules/okd/load_balancer.rs @@ -114,7 +114,6 @@ impl OKDLoadBalancerScore { load_balancer_score: LoadBalancerScore { public_services, private_services, - wan_firewall_ports: vec![80, 443], }, } } @@ -339,13 +338,6 @@ mod tests { assert_eq!(private_service_22623.backend_servers.len(), 3); } - #[test] - fn test_wan_firewall_ports_include_http_and_https() { - let topology = create_test_topology(); - let score = OKDLoadBalancerScore::new(&topology); - assert_eq!(score.load_balancer_score.wan_firewall_ports, vec![80, 443]); - } - #[test] fn test_all_backend_servers_have_correct_port() { let topology = create_test_topology();