feat(tui): add panic logging and improve event handling

- Integrate `log_panics` for better error tracking in TUI.
- Enhance score interpretation result handling with async task management.
- Improve layout consistency in the UI rendering process.

harmony-rs/Cargo.lock

@@ -826,6 +826,21 @@ dependencies = [
  "url",
 ]
 
+[[package]]
+name = "example-tui"
+version = "0.1.0"
+dependencies = [
+ "cidr",
+ "env_logger",
+ "harmony",
+ "harmony_macros",
+ "harmony_tui",
+ "harmony_types",
+ "log",
+ "tokio",
+ "url",
+]
+
 [[package]]
 name = "eyre"
 version = "0.6.12"
@@ -1151,6 +1166,7 @@ dependencies = [
  "env_logger",
  "harmony",
  "log",
+ "log-panics",
  "ratatui",
  "tokio",
  "tokio-stream",
@@ -1850,6 +1866,15 @@ version = "0.4.22"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24"
 
+[[package]]
+name = "log-panics"
+version = "2.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "68f9dd8546191c1850ecf67d22f5ff00a935b890d0e84713159a55495cc2ac5f"
+dependencies = [
+ "log",
+]
+
 [[package]]
 name = "lru"
 version = "0.12.5"

harmony-rs/examples/opnsense/src/main.rs

@@ -10,9 +10,7 @@ use harmony::{
     inventory::Inventory,
     maestro::Maestro,
     modules::{
-        http::HttpScore,
-        okd::{dhcp::OKDDhcpScore, dns::OKDDnsScore},
-        tftp::TftpScore,
+        dummy::{ErrorScore, PanicScore, SuccessScore}, http::HttpScore, okd::{dhcp::OKDDhcpScore, dns::OKDDnsScore}, tftp::TftpScore
     },
     topology::{LogicalHost, UnmanagedRouter, Url},
 };
@@ -88,6 +86,9 @@ async fn main() {
         Box::new(load_balancer_score),
         Box::new(tftp_score),
         Box::new(http_score),
+        Box::new(SuccessScore {}),
+        Box::new(ErrorScore {}),
+        Box::new(PanicScore {}),
     ]);
     harmony_tui::init(maestro).await.unwrap();
 }

harmony-rs/harmony/src/domain/hardware/mod.rs

@@ -171,7 +171,6 @@ pub struct Location {
 }
 
 impl Location {
-    #[cfg(test)]
     pub fn test_building() -> Location {
         Self {
             address: String::new(),

harmony-rs/harmony/src/domain/interpret/mod.rs

@@ -16,6 +16,8 @@ pub enum InterpretName {
     LoadBalancer,
     Tftp,
     Http,
+    Dummy,
+    Panic,
 }
 
 impl std::fmt::Display for InterpretName {
@@ -26,6 +28,8 @@ impl std::fmt::Display for InterpretName {
             InterpretName::LoadBalancer => f.write_str("LoadBalancer"),
             InterpretName::Tftp => f.write_str("Tftp"),
             InterpretName::Http => f.write_str("Http"),
+            InterpretName::Dummy => f.write_str("Dummy"),
+            InterpretName::Panic => f.write_str("Panic"),
         }
     }
 }
@@ -43,7 +47,7 @@ pub trait Interpret: std::fmt::Debug + Send {
     fn get_children(&self) -> Vec<Id>;
 }
 
-#[derive(Debug, new)]
+#[derive(Debug, new, Clone)]
 pub struct Outcome {
     pub status: InterpretStatus,
     pub message: String,
@@ -89,7 +93,7 @@ impl std::fmt::Display for InterpretStatus {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone, new)]
 pub struct InterpretError {
     msg: String,
 }

harmony-rs/harmony/src/domain/inventory/mod.rs

@@ -34,7 +34,6 @@ pub struct Inventory {
 }
 
 impl Inventory {
-    #[cfg(test)]
     pub fn empty_inventory() -> Self {
         Self {
             location: Location::test_building(),

harmony-rs/harmony/src/domain/topology/ha_cluster.rs

@@ -0,0 +1,232 @@
+use async_trait::async_trait;
+use harmony_macros::ip;
+use harmony_types::net::MacAddress;
+
+use crate::executors::ExecutorError;
+
+use super::DHCPStaticEntry;
+use super::DhcpServer;
+use super::DnsRecord;
+use super::DnsRecordType;
+use super::DnsServer;
+use super::Firewall;
+use super::HttpServer;
+use super::IpAddress;
+use super::LoadBalancer;
+use super::LoadBalancerService;
+use super::LogicalHost;
+use super::Router;
+use super::TftpServer;
+
+use super::Url;
+use super::openshift::OpenshiftClient;
+use std::sync::Arc;
+
+#[derive(Debug, Clone)]
+pub struct HAClusterTopology {
+    pub domain_name: String,
+    pub router: Arc<dyn Router>,
+    pub load_balancer: Arc<dyn LoadBalancer>,
+    pub firewall: Arc<dyn Firewall>,
+    pub dhcp_server: Arc<dyn DhcpServer>,
+    pub tftp_server: Arc<dyn TftpServer>,
+    pub http_server: Arc<dyn HttpServer>,
+    pub dns_server: Arc<dyn DnsServer>,
+    pub bootstrap_host: LogicalHost,
+    pub control_plane: Vec<LogicalHost>,
+    pub workers: Vec<LogicalHost>,
+    pub switch: Vec<LogicalHost>,
+}
+
+impl HAClusterTopology {
+    pub async fn oc_client(&self) -> Result<Arc<OpenshiftClient>, kube::Error> {
+        Ok(Arc::new(OpenshiftClient::try_default().await?))
+    }
+
+    pub fn dummy() -> Self {
+        let dummy_infra = Arc::new(DummyInfra {});
+        let dummy_host = LogicalHost {
+            ip: ip!("0.0.0.0"),
+            name: "dummyhost".to_string(),
+        };
+
+        Self {
+            domain_name: "DummyTopology".to_string(),
+            router: dummy_infra.clone(),
+            load_balancer: dummy_infra.clone(),
+            firewall: dummy_infra.clone(),
+            dhcp_server: dummy_infra.clone(),
+            tftp_server: dummy_infra.clone(),
+            http_server: dummy_infra.clone(),
+            dns_server: dummy_infra.clone(),
+            bootstrap_host: dummy_host,
+            control_plane: vec![],
+            workers: vec![],
+            switch: vec![],
+        }
+    }
+}
+
+struct DummyInfra;
+
+const UNIMPLEMENTED_DUMMY_INFRA: &str = "This is a dummy infrastructure, no operation is supported";
+
+impl Router for DummyInfra {
+    fn get_gateway(&self) -> super::IpAddress {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_cidr(&self) -> cidr::Ipv4Cidr {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_host(&self) -> LogicalHost {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+}
+
+impl Firewall for DummyInfra {
+    fn add_rule(
+        &mut self,
+        _rule: super::FirewallRule,
+    ) -> Result<(), crate::executors::ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn remove_rule(&mut self, _rule_id: &str) -> Result<(), crate::executors::ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn list_rules(&self) -> Vec<super::FirewallRule> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_ip(&self) -> super::IpAddress {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_host(&self) -> LogicalHost {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+}
+
+#[async_trait]
+impl DhcpServer for DummyInfra {
+    async fn add_static_mapping(&self, _entry: &DHCPStaticEntry) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn remove_static_mapping(&self, _mac: &MacAddress) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn list_static_mappings(&self) -> Vec<(MacAddress, IpAddress)> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn set_next_server(&self, _ip: IpAddress) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn set_boot_filename(&self, _boot_filename: &str) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_ip(&self) -> IpAddress {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_host(&self) -> LogicalHost {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn commit_config(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+}
+
+#[async_trait]
+impl LoadBalancer for DummyInfra {
+    fn get_ip(&self) -> IpAddress {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_host(&self) -> LogicalHost {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn add_service(&self, _service: &LoadBalancerService) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn remove_service(&self, _service: &LoadBalancerService) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn list_services(&self) -> Vec<LoadBalancerService> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn ensure_initialized(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn commit_config(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn reload_restart(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+}
+
+#[async_trait]
+impl TftpServer for DummyInfra {
+    async fn serve_files(&self, _url: &Url) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_ip(&self) -> IpAddress {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+
+    async fn set_ip(&self, _ip: IpAddress) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn ensure_initialized(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn commit_config(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn reload_restart(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+}
+
+#[async_trait]
+impl HttpServer for DummyInfra {
+    async fn serve_files(&self, _url: &Url) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_ip(&self) -> IpAddress {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn ensure_initialized(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn commit_config(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn reload_restart(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+}
+
+#[async_trait]
+impl DnsServer for DummyInfra {
+    async fn register_dhcp_leases(&self, _register: bool) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn register_hosts(&self, _hosts: Vec<DnsRecord>) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn remove_record(
+        &mut self,
+        _name: &str,
+        _record_type: DnsRecordType,
+    ) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn list_records(&self) -> Vec<DnsRecord> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_ip(&self) -> IpAddress {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    fn get_host(&self) -> LogicalHost {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+    async fn commit_config(&self) -> Result<(), ExecutorError> {
+        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
+    }
+}

harmony-rs/harmony/src/domain/topology/mod.rs

@@ -4,8 +4,9 @@ mod load_balancer;
 pub mod openshift;
 mod router;
 mod tftp;
+mod ha_cluster;
+pub use ha_cluster::*;
 pub use load_balancer::*;
-use openshift::OpenshiftClient;
 pub use router::*;
 mod network;
 pub use host_binding::*;
@@ -15,27 +16,6 @@ pub use tftp::*;
 
 use std::{net::IpAddr, sync::Arc};
 
-#[derive(Debug, Clone)]
-pub struct HAClusterTopology {
-    pub domain_name: String,
-    pub router: Arc<dyn Router>,
-    pub load_balancer: Arc<dyn LoadBalancer>,
-    pub firewall: Arc<dyn Firewall>,
-    pub dhcp_server: Arc<dyn DhcpServer>,
-    pub tftp_server: Arc<dyn TftpServer>,
-    pub http_server: Arc<dyn HttpServer>,
-    pub dns_server: Arc<dyn DnsServer>,
-    pub bootstrap_host: LogicalHost,
-    pub control_plane: Vec<LogicalHost>,
-    pub workers: Vec<LogicalHost>,
-    pub switch: Vec<LogicalHost>,
-}
-
-impl HAClusterTopology {
-    pub async fn oc_client(&self) -> Result<Arc<OpenshiftClient>, kube::Error> {
-        Ok(Arc::new(OpenshiftClient::try_default().await?))
-    }
-}
 
 pub type IpAddress = IpAddr;
 

harmony-rs/harmony/src/modules/dummy.rs

@@ -0,0 +1,138 @@
+use async_trait::async_trait;
+
+use crate::{
+    data::Version,
+    interpret::{Interpret, InterpretError, InterpretName, InterpretStatus, Outcome},
+    inventory::Inventory,
+    score::Score,
+    topology::HAClusterTopology,
+};
+
+/// Score that always errors. This is only useful for development/testing purposes. It does nothing
+/// except returning Err(InterpretError) when interpreted.
+#[derive(Debug, Clone)]
+pub struct ErrorScore;
+
+impl Score for ErrorScore {
+    fn create_interpret(&self) -> Box<dyn crate::interpret::Interpret> {
+        Box::new(DummyInterpret {
+            result: Err(InterpretError::new("Error Score default error".to_string())),
+            status: InterpretStatus::QUEUED,
+        })
+    }
+
+    fn name(&self) -> String {
+        "ErrorScore".to_string()
+    }
+
+    fn clone_box(&self) -> Box<dyn Score> {
+        Box::new(self.clone())
+    }
+}
+
+/// Score that always succeeds. This is only useful for development/testing purposes. It does nothing
+/// except returning Ok(Outcome::success) when interpreted.
+#[derive(Debug, Clone)]
+pub struct SuccessScore;
+
+impl Score for SuccessScore {
+    fn create_interpret(&self) -> Box<dyn crate::interpret::Interpret> {
+        Box::new(DummyInterpret {
+            result: Ok(Outcome::success("SuccessScore default success".to_string())),
+            status: InterpretStatus::QUEUED,
+        })
+    }
+
+    fn name(&self) -> String {
+        "SuccessScore".to_string()
+    }
+
+    fn clone_box(&self) -> Box<dyn Score> {
+        Box::new(self.clone())
+    }
+}
+
+/// An interpret that only returns the result it is given when built. It does nothing else. Only
+/// useful for development/testing purposes.
+#[derive(Debug)]
+struct DummyInterpret {
+    status: InterpretStatus,
+    result: Result<Outcome, InterpretError>,
+}
+
+#[async_trait]
+impl Interpret for DummyInterpret {
+    fn get_name(&self) -> InterpretName {
+        InterpretName::Dummy
+    }
+
+    fn get_version(&self) -> Version {
+        Version::from("1.0.0").unwrap()
+    }
+
+    fn get_status(&self) -> InterpretStatus {
+        self.status.clone()
+    }
+
+    fn get_children(&self) -> Vec<crate::domain::data::Id> {
+        todo!()
+    }
+
+    async fn execute(
+        &self,
+        _inventory: &Inventory,
+        _topology: &HAClusterTopology,
+    ) -> Result<Outcome, InterpretError> {
+        self.result.clone()
+    }
+}
+
+/// Score that always panics. This is only useful for development/testing purposes. It does nothing
+/// except panic! with an error message when interpreted
+#[derive(Debug, Clone)]
+pub struct PanicScore;
+
+impl Score for PanicScore {
+    fn create_interpret(&self) -> Box<dyn crate::interpret::Interpret> {
+        Box::new(PanicInterpret {})
+    }
+
+    fn name(&self) -> String {
+        "PanicScore".to_string()
+    }
+
+    fn clone_box(&self) -> Box<dyn Score> {
+        Box::new(self.clone())
+    }
+}
+
+/// An interpret that always panics when executed. Useful for development/testing purposes.
+#[derive(Debug)]
+struct PanicInterpret;
+
+#[async_trait]
+impl Interpret for PanicInterpret {
+    fn get_name(&self) -> InterpretName {
+        InterpretName::Panic
+    }
+
+    fn get_version(&self) -> Version {
+        Version::from("1.0.0").unwrap()
+    }
+
+    fn get_status(&self) -> InterpretStatus {
+        InterpretStatus::QUEUED
+    }
+
+    fn get_children(&self) -> Vec<crate::domain::data::Id> {
+        todo!()
+    }
+
+    async fn execute(
+        &self,
+        _inventory: &Inventory,
+        _topology: &HAClusterTopology,
+    ) -> Result<Outcome, InterpretError> {
+        panic!("Panic interpret always panics when executed")
+    }
+}

harmony-rs/harmony/src/modules/mod.rs

@@ -5,3 +5,5 @@ pub mod k8s;
 pub mod load_balancer;
 pub mod okd;
 pub mod tftp;
+pub mod dummy;
+

harmony-rs/harmony_tui/Cargo.toml

@@ -15,3 +15,4 @@ crossterm = { version = "0.28.1", features = [ "event-stream" ] }
 color-eyre = "0.6.3"
 tokio-stream = "0.1.17"
 tui-logger = "0.14.1"
+log-panics = "2.1.0"

harmony-rs/harmony_tui/src/lib.rs

@@ -1,22 +1,21 @@
 mod ratatui_utils;
 mod widget;
 
-use log::{debug, info};
+use log::{debug, error, info};
 use tokio::sync::mpsc;
 use tokio_stream::StreamExt;
 use tui_logger::{TuiWidgetEvent, TuiWidgetState};
 use widget::{help::HelpWidget, score::ScoreListWidget};
 
-use std::{sync::Arc, time::Duration};
+use std::{panic, sync::Arc, time::Duration};
 
 use crossterm::event::{Event, EventStream, KeyCode, KeyEventKind};
 use harmony::{maestro::Maestro, score::Score};
 use ratatui::{
-    self,
+    self, Frame,
     layout::{Constraint, Layout, Position},
     style::{Color, Style},
     widgets::{Block, Borders, ListItem},
-    Frame,
 };
 
 pub mod tui {
@@ -51,7 +50,6 @@ pub async fn init(maestro: Maestro) -> Result<(), Box<dyn std::error::Error>> {
 pub struct HarmonyTUI {
     score: ScoreListWidget,
     should_quit: bool,
-    channel_handle: tokio::task::JoinHandle<()>,
     tui_state: TuiWidgetState,
 }
 
@@ -69,7 +67,6 @@ impl HarmonyTUI {
         HarmonyTUI {
             should_quit: false,
             score,
-            channel_handle: handle,
             tui_state: TuiWidgetState::new(),
         }
     }
@@ -84,15 +81,24 @@ impl HarmonyTUI {
                 info!("Received event {event:#?}");
                 match event {
                     HarmonyTuiEvent::LaunchScore(score_item) => {
-                        info!(
-                            "Interpretation result {:#?}",
-                            maestro.interpret(score_item.0).await
-                        )
+                        let maestro = maestro.clone();
+
+                        let interpretation_result =
+                            tokio::spawn(async move { maestro.interpret(score_item.0).await })
+                                .await;
+
+                        match interpretation_result {
+                            Ok(success) => info!("Score execution successful {success:?}"),
+                            Err(e) => {
+                                error!("Score execution failed {:#?}", e);
+                            }
+                        }
                     }
                 }
             }
             info!("STOPPING message channel receiver loop");
         });
+
         (handle, sender)
     }
 
@@ -105,6 +111,7 @@ impl HarmonyTUI {
 
         color_eyre::install()?;
         let mut terminal = ratatui::init();
+        log_panics::init();
         terminal.hide_cursor()?;
 
         // TODO improve performance here
@@ -130,17 +137,14 @@ impl HarmonyTUI {
         frame.set_cursor_position(Position::new(size.x / 2, size.y / 2));
 
         let [app_area, help_area] =
-            Layout::vertical([Constraint::Percentage(100), Constraint::Min(4)])
-                .areas(frame.area());
+            Layout::vertical([Constraint::Percentage(100), Constraint::Min(4)]).areas(frame.area());
 
         let help_block = Block::default().borders(Borders::TOP);
         frame.render_widget(&help_block, help_area);
         frame.render_widget(HelpWidget::new(), help_block.inner(help_area));
 
         let [list_area, output_area] =
-            Layout::horizontal([Constraint::Min(30), Constraint::Percentage(100)])
-                .areas(app_area);
-
+            Layout::horizontal([Constraint::Min(30), Constraint::Percentage(100)]).areas(app_area);
 
         let block = Block::default().borders(Borders::RIGHT);
         frame.render_widget(&block, list_area);
@@ -172,7 +176,9 @@ impl HarmonyTUI {
                     KeyCode::Char('q') | KeyCode::Esc => self.should_quit = true,
                     KeyCode::PageUp => self.tui_state.transition(TuiWidgetEvent::PrevPageKey),
                     KeyCode::PageDown => self.tui_state.transition(TuiWidgetEvent::NextPageKey),
-                    KeyCode::Char('G') | KeyCode::End => self.tui_state.transition(TuiWidgetEvent::EscapeKey),
+                    KeyCode::Char('G') | KeyCode::End => {
+                        self.tui_state.transition(TuiWidgetEvent::EscapeKey)
+                    }
                     _ => self.score.handle_event(event).await,
                 }
             }