fix: Dhcp static hostname has to have domain specified with dnsmasq, also progress on okd installation automation

harmony/src/modules/dhcp.rs

@@ -130,6 +130,7 @@ impl<T: Topology + DhcpServer> Interpret<T> for DhcpInterpret {
 #[derive(Debug, new, Clone, Serialize)]
 pub struct DhcpHostBindingScore {
     pub host_binding: Vec<HostBinding>,
+    pub domain: Option<String>,
 }
 
 impl<T: Topology + DhcpServer> Score<T> for DhcpHostBindingScore {
@@ -168,8 +169,14 @@ impl DhcpHostBindingInterpret {
                     }
                 };
 
+                let name = if let Some(domain) = self.score.domain.as_ref() {
+                    format!("{}.{}", binding.logical_host.name, domain)
+                } else {
+                    binding.logical_host.name.clone()
+                };
+
                 DHCPStaticEntry {
-                    name: binding.logical_host.name.clone(),
+                    name,
                     mac: binding.physical_host.cluster_mac(),
                     ip,
                 }

harmony/src/modules/okd/installation.rs

@@ -61,8 +61,10 @@ use crate::{
     interpret::{Interpret, InterpretError, InterpretName, InterpretStatus, Outcome},
     inventory::{HostRole, Inventory},
     modules::{
-        dhcp::DhcpHostBindingScore, http::IPxeMacBootFileScore,
+        dhcp::DhcpHostBindingScore,
+        http::{IPxeMacBootFileScore, StaticFilesHttpScore},
         inventory::LaunchDiscoverInventoryAgentScore,
+        okd::dns::OKDDnsScore,
     },
     score::Score,
     topology::{HAClusterTopology, HostBinding},
@@ -280,6 +282,11 @@ impl Interpret<HAClusterTopology> for OKDSetup01InventoryInterpret {
         inventory: &Inventory,
         topology: &HAClusterTopology,
     ) -> Result<Outcome, InterpretError> {
+        info!("Setting up base DNS config for OKD");
+        OKDDnsScore::new(topology)
+            .interpret(inventory, topology)
+            .await?;
+
         info!(
             "Launching discovery agent, make sure that your nodes are successfully PXE booted and running inventory agent. They should answer on `http://<node_ip>:8080/inventory`"
         );
@@ -298,7 +305,7 @@ impl Interpret<HAClusterTopology> for OKDSetup01InventoryInterpret {
             if all_hosts.is_empty() {
                 warn!("No discovered hosts found yet. Waiting for hosts to appear...");
                 // Sleep to avoid spamming the user and logs while waiting for nodes.
-                tokio::time::sleep(std::time::Duration::from_secs(5));
+                tokio::time::sleep(std::time::Duration::from_secs(3)).await;
                 continue;
             }
 
@@ -339,37 +346,6 @@ impl Interpret<HAClusterTopology> for OKDSetup01InventoryInterpret {
                 bootstrap_host.summary()
             ),
         ))
-        // info!(
-        //     "Launching discovery agent, make sure that your nodes are successfully PXE booted and running inventory agent. They should answer on `http://<node_ip>:8080/inventory`"
-        // );
-        // LaunchDiscoverInventoryAgentScore {
-        //     discovery_timeout: Some(60),
-        // }
-        // .interpret(inventory, topology)
-        // .await?;
-        //
-        // // TODO write a loop
-        // let bootstrap_host: PhysicalHost;
-        // let mut found_bootstrap_host = false;
-        // let host_repo = InventoryRepositoryFactory::build().await?;
-        // while !found_bootstrap_host {
-        //     let all_hosts = host_repo.get_all_hosts().await?;
-        //     // TODO use inquire to select among the current hosts, tell the user to cancel if he
-        //     // wants to update the list. I believe inquire::Select is the correct option here
-        //     //
-        //     // The options are all_hosts, all_hosts is of type Vec<PhysicalHost> and PhysicalHost
-        //     // has a human friendly `summary() -> String` method that is perfect to have the user
-        //     // choose
-        //     //
-        //     // once the user has chosen one, call host_repo.save_role_mapping(Role::Bootstrap,
-        //     // host.id).await?;
-        //     bootstrap_host = todo!();
-        // }
-        //
-        // Ok(Outcome::new(
-        //     InterpretStatus::SUCCESS,
-        //     format!("Found bootstrap node : {}", bootstrap_host.summary()),
-        // ))
     }
 }
 
@@ -411,10 +387,7 @@ impl OKDSetup02BootstrapInterpret {
         }
     }
 
-    async fn get_bootstrap_node(
+    async fn get_bootstrap_node(&self) -> Result<PhysicalHost, InterpretError> {
-        &self,
-        _inventory: &Inventory,
-    ) -> Result<PhysicalHost, InterpretError> {
         let repo = InventoryRepositoryFactory::build().await?;
         match repo
             .get_host_for_role(HostRole::Bootstrap)
@@ -429,6 +402,19 @@ impl OKDSetup02BootstrapInterpret {
         }
     }
 
+    async fn prepare_ignition_files(
+        &self,
+        inventory: &Inventory,
+        topology: &HAClusterTopology,
+    ) -> Result<(), InterpretError> {
+        StaticFilesHttpScore {
+            folder_to_serve: None,
+            files: todo!(),
+        }
+        .interpret(inventory, topology)
+        .await?;
+    }
+
     async fn configure_host_binding(
         &self,
         inventory: &Inventory,
@@ -436,12 +422,13 @@ impl OKDSetup02BootstrapInterpret {
     ) -> Result<(), InterpretError> {
         let binding = HostBinding {
             logical_host: topology.bootstrap_host.clone(),
-            physical_host: self.get_bootstrap_node(inventory).await?,
+            physical_host: self.get_bootstrap_node().await?,
         };
         info!("Configuring host binding for bootstrap node {binding:?}");
 
         DhcpHostBindingScore {
             host_binding: vec![binding],
+            domain: Some(topology.domain_name),
         }
         .interpret(inventory, topology)
         .await?;
@@ -455,7 +442,7 @@ impl OKDSetup02BootstrapInterpret {
     ) -> Result<(), InterpretError> {
         // Placeholder: use Harmony templates to emit {MAC}.ipxe selecting SCOS live + bootstrap ignition.
         info!("[Bootstrap] Rendering per-MAC PXE for bootstrap node");
-        let bootstrap_node = self.get_bootstrap_node(inventory).await?;
+        let bootstrap_node = self.get_bootstrap_node().await?;
         IPxeMacBootFileScore {
             mac_address: bootstrap_node.get_mac_address(),
             content: todo!("templace for bootstrap node"),
@@ -465,6 +452,14 @@ impl OKDSetup02BootstrapInterpret {
         Ok(())
     }
 
+    async fn setup_bootstrap_load_balancer(
+        &self,
+        inventory: &Inventory,
+        topology: &HAClusterTopology,
+    ) -> Result<(), InterpretError> {
+        todo!("OKD loadbalancer score already exists, just call it here probably? 6443 22623, 80 and 443 \n\nhttps://docs.okd.io/latest/installing/installing_bare_metal/upi/installing-bare-metal.html#installation-load-balancing-user-infra_installing-bare-metal");
+    }
+
     async fn reboot_target(&self) -> Result<(), InterpretError> {
         // Placeholder: ssh reboot using the inventory ephemeral key
         info!("[Bootstrap] Rebooting bootstrap node via SSH");
@@ -502,7 +497,13 @@ impl Interpret<HAClusterTopology> for OKDSetup02BootstrapInterpret {
         topology: &HAClusterTopology,
     ) -> Result<Outcome, InterpretError> {
         self.configure_host_binding(inventory, topology).await?;
+        self.prepare_ignition_files(inventory, topology).await?;
         self.render_per_mac_pxe(inventory, topology).await?;
+        self.setup_bootstrap_load_balancer(inventory, topology).await?;
+
+        // TODO https://docs.okd.io/latest/installing/installing_bare_metal/upi/installing-bare-metal.html#installation-user-provisioned-validating-dns_installing-bare-metal
+        // self.validate_dns_config(inventory, topology).await?;
+
         self.reboot_target().await?;
         self.wait_for_bootstrap_complete().await?;
 

opnsense-config/src/modules/dhcp.rs

@@ -1,4 +1,4 @@
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub enum DhcpError {
     InvalidMacAddress(String),
     InvalidIpAddress(String),

opnsense-config/src/modules/dnsmasq.rs

@@ -69,9 +69,9 @@ impl<'a> DhcpConfigDnsMasq<'a> {
     /// This function implements specific logic to handle existing entries:
     /// - If no host exists for the given IP or hostname, a new entry is created.
     /// - If exactly one host exists for the IP and/or hostname, the new MAC is appended to it.
-    /// - It will panic if the IP and hostname exist but point to two different host entries,
+    /// - It will error if the IP and hostname exist but point to two different host entries,
     ///   as this represents an unresolvable conflict.
-    /// - It will also panic if multiple entries are found for the IP or hostname, indicating an
+    /// - It will also error if multiple entries are found for the IP or hostname, indicating an
     ///   ambiguous state.
     pub fn add_static_mapping(
         &mut self,
@@ -79,6 +79,10 @@ impl<'a> DhcpConfigDnsMasq<'a> {
         ipaddr: Ipv4Addr,
         hostname: &str,
     ) -> Result<(), DhcpError> {
+        let mut hostname_split = hostname.split(".");
+        let hostname = hostname_split.next().expect("hostname cannot be empty");
+        let domain_name = hostname_split.collect::<Vec<&str>>().join(".");
+
         if !Self::is_valid_mac(mac) {
             return Err(DhcpError::InvalidMacAddress(mac.to_string()));
         }
@@ -107,10 +111,10 @@ impl<'a> DhcpConfigDnsMasq<'a> {
             && !hostname_indices.is_empty()
             && ip_set.intersection(&hostname_set).count() == 0
         {
-            panic!(
+            return Err(DhcpError::Configuration(format!(
                 "Configuration conflict: IP {} and hostname '{}' exist, but in different static host entries.",
                 ipaddr, hostname
-            );
+            )));
         }
 
         let mut all_indices: Vec<&usize> = ip_set.union(&hostname_set).collect();
@@ -129,6 +133,7 @@ impl<'a> DhcpConfigDnsMasq<'a> {
                     hwaddr: mac.to_string().into(),
                     local: MaybeString::from("1"),
                     ignore: Some(0),
+                    domain: domain_name.into(),
                     ..Default::default()
                 };
                 hosts.push(new_host);
@@ -162,7 +167,7 @@ impl<'a> DhcpConfigDnsMasq<'a> {
                     let mut updated_macs = host_to_modify.hwaddr.content_string().to_string();
                     updated_macs.push(',');
                     updated_macs.push_str(mac);
-                    host_to_modify.hwaddr = updated_macs.into();
+                    host_to_modify.hwaddr.content = updated_macs.into();
                 } else {
                     info!(
                         "MAC {} already present in static host entry for {} ({}). No changes made.",
@@ -171,10 +176,10 @@ impl<'a> DhcpConfigDnsMasq<'a> {
                 }
             }
             _ => {
-                panic!(
+                return Err(DhcpError::Configuration(format!(
                     "Configuration conflict: Found multiple host entries matching IP {} and/or hostname '{}'. Cannot resolve automatically.",
                     ipaddr, hostname
-                );
+                )));
             }
         }
 
@@ -202,8 +207,11 @@ impl<'a> DhcpConfigDnsMasq<'a> {
             .exec("configctl dhcpd list static")
             .await?;
 
-        let value: serde_json::Value = serde_json::from_str(&list_static_output)
+        let value: serde_json::Value = serde_json::from_str(&list_static_output).map_err(|e| {
-            .unwrap_or_else(|_| panic!("Got invalid json from configctl {list_static_output}"));
+            Error::Command(format!(
+                "Got invalid json from configctl {list_static_output} : {e}"
+            ))
+        })?;
 
         // The JSON output key might be 'dhcpd' even when dnsmasq is the backend.
         let static_maps = value["dhcpd"]
@@ -358,6 +366,28 @@ mod test {
         assert!(Uuid::parse_str(&host.uuid).is_ok());
     }
 
+    #[test]
+    fn test_hostname_split_into_host_domain() {
+        let mut dhcp_config = setup_test_env(vec![]);
+        let ip = Ipv4Addr::new(192, 168, 1, 10);
+        let mac = "00:11:22:33:44:55";
+        let hostname = "new-host";
+        let domain = "some.domain";
+
+        dhcp_config
+            .add_static_mapping(mac, ip, &format!("{hostname}.{domain}"))
+            .unwrap();
+
+        let hosts = &dhcp_config.opnsense.dnsmasq.as_ref().unwrap().hosts;
+        assert_eq!(hosts.len(), 1);
+        let host = &hosts[0];
+        assert_eq!(host.host, hostname);
+        assert_eq!(host.domain.content_string(), domain);
+        assert_eq!(host.ip, ip.to_string().into());
+        assert_eq!(host.hwaddr.content_string(), mac);
+        assert!(Uuid::parse_str(&host.uuid).is_ok());
+    }
+
     #[test]
     fn test_add_mac_to_existing_host_by_ip_and_hostname() {
         let initial_host = create_host(
@@ -462,41 +492,33 @@ mod test {
     }
 
     #[test]
-    #[should_panic(
+    fn test_error_on_conflicting_ip_and_hostname() {
-        expected = "Configuration conflict: IP 192.168.1.10 and hostname 'host-b' exist, but in different static host entries."
-    )]
-    fn test_panic_on_conflicting_ip_and_hostname() {
         let host_a = create_host("uuid-a", "host-a", "192.168.1.10", "AA:AA:AA:AA:AA:AA");
         let host_b = create_host("uuid-b", "host-b", "192.168.1.20", "BB:BB:BB:BB:BB:BB");
         let mut dhcp_config = setup_test_env(vec![host_a, host_b]);
 
+        let result = dhcp_config.add_static_mapping(
+            "CC:CC:CC:CC:CC:CC",
+            Ipv4Addr::new(192, 168, 1, 10),
+            "host-b",
+        );
         // This IP belongs to host-a, but the hostname belongs to host-b.
-        dhcp_config
+        assert_eq!(result, Err(DhcpError::Configuration("Configuration conflict: IP 192.168.1.10 and hostname 'host-b' exist, but in different static host entries.".to_string())));
-            .add_static_mapping(
-                "CC:CC:CC:CC:CC:CC",
-                Ipv4Addr::new(192, 168, 1, 10),
-                "host-b",
-            )
-            .unwrap();
     }
 
     #[test]
-    #[should_panic(
+    fn test_error_on_multiple_ip_matches() {
-        expected = "Configuration conflict: Found multiple host entries matching IP 192.168.1.30 and/or hostname 'new-host'."
-    )]
-    fn test_panic_on_multiple_ip_matches() {
         let host_a = create_host("uuid-a", "host-a", "192.168.1.30", "AA:AA:AA:AA:AA:AA");
         let host_b = create_host("uuid-b", "host-b", "192.168.1.30", "BB:BB:BB:BB:BB:BB");
         let mut dhcp_config = setup_test_env(vec![host_a, host_b]);
 
         // This IP is ambiguous.
-        dhcp_config
+        let result = dhcp_config.add_static_mapping(
-            .add_static_mapping(
+            "CC:CC:CC:CC:CC:CC",
-                "CC:CC:CC:CC:CC:CC",
+            Ipv4Addr::new(192, 168, 1, 30),
-                Ipv4Addr::new(192, 168, 1, 30),
+            "new-host",
-                "new-host",
+        );
-            )
+        assert_eq!(result, Err(DhcpError::Configuration("Configuration conflict: Found multiple host entries matching IP 192.168.1.30 and/or hostname 'new-host'. Cannot resolve automatically.".to_string())));
-            .unwrap();
     }
 
     #[test]