From 29a261575bfecf99e10241b98cfc1e5b60249906 Mon Sep 17 00:00:00 2001 From: Ian Letourneau Date: Sat, 9 Aug 2025 22:56:23 +0000 Subject: [PATCH] refactor: Interpret score with a provided method on Score (#100) First step in a direction to better orchestrate the core flow, even though it feels weird to move this logic into the `Score`. We'll refactor this as soon as we have a better solution. Co-authored-by: Ian Letourneau Reviewed-on: https://git.nationtech.io/NationTech/harmony/pulls/100 --- harmony/src/domain/instrumentation.rs | 2 + harmony/src/domain/interpret/mod.rs | 16 ++++++++ harmony/src/domain/maestro/mod.rs | 6 +-- harmony/src/domain/score.rs | 40 ++++++++++++++++++- harmony/src/domain/topology/k8s_anywhere.rs | 11 ++--- .../topology/oberservability/monitoring.rs | 2 +- harmony/src/domain/topology/tenant/k8s.rs | 2 +- .../features/continuous_delivery.rs | 3 +- .../application/features/helm_argocd_score.rs | 5 +-- .../application/features/monitoring.rs | 6 +-- harmony/src/modules/helm/chart.rs | 4 +- harmony/src/modules/helm/command.rs | 2 +- harmony/src/modules/k3d/install.rs | 25 +++--------- harmony/src/modules/k8s/resource.rs | 2 +- harmony/src/modules/lamp.rs | 14 ++----- .../application_monitoring_score.rs | 2 +- .../monitoring/kube_prometheus/prometheus.rs | 3 +- harmony/src/modules/monitoring/ntfy/ntfy.rs | 8 ++-- .../monitoring/prometheus/prometheus.rs | 6 +-- .../k8s_prometheus_alerting_score.rs | 4 +- harmony/src/modules/tenant/credentials.rs | 2 +- harmony_cli/src/cli_logger.rs | 30 +++++++++++--- harmony_cli/src/theme.rs | 1 + 23 files changed, 118 insertions(+), 78 deletions(-) diff --git a/harmony/src/domain/instrumentation.rs b/harmony/src/domain/instrumentation.rs index eaea6f7..79787ec 100644 --- a/harmony/src/domain/instrumentation.rs +++ b/harmony/src/domain/instrumentation.rs @@ -13,11 +13,13 @@ pub enum HarmonyEvent { InterpretExecutionStarted { topology: String, interpret: String, + score: String, message: String, }, InterpretExecutionFinished { topology: String, interpret: String, + score: String, outcome: Result, }, TopologyStateChanged { diff --git a/harmony/src/domain/interpret/mod.rs b/harmony/src/domain/interpret/mod.rs index fca1817..cfbf2b5 100644 --- a/harmony/src/domain/interpret/mod.rs +++ b/harmony/src/domain/interpret/mod.rs @@ -24,6 +24,14 @@ pub enum InterpretName { TenantInterpret, Application, ArgoCD, + Alerting, + Ntfy, + HelmChart, + HelmCommand, + K8sResource, + Lamp, + ApplicationMonitoring, + K8sPrometheusCrdAlerting, } impl std::fmt::Display for InterpretName { @@ -42,6 +50,14 @@ impl std::fmt::Display for InterpretName { InterpretName::TenantInterpret => f.write_str("Tenant"), InterpretName::Application => f.write_str("Application"), InterpretName::ArgoCD => f.write_str("ArgoCD"), + InterpretName::Alerting => f.write_str("Alerting"), + InterpretName::Ntfy => f.write_str("Ntfy"), + InterpretName::HelmChart => f.write_str("HelmChart"), + InterpretName::HelmCommand => f.write_str("HelmCommand"), + InterpretName::K8sResource => f.write_str("K8sResource"), + InterpretName::Lamp => f.write_str("LAMP"), + InterpretName::ApplicationMonitoring => f.write_str("ApplicationMonitoring"), + InterpretName::K8sPrometheusCrdAlerting => f.write_str("K8sPrometheusCrdAlerting"), } } } diff --git a/harmony/src/domain/maestro/mod.rs b/harmony/src/domain/maestro/mod.rs index 721fb40..d9587c8 100644 --- a/harmony/src/domain/maestro/mod.rs +++ b/harmony/src/domain/maestro/mod.rs @@ -84,10 +84,8 @@ impl Maestro { self.topology.name(), ); } - debug!("Running score {score:?}"); - let interpret = score.create_interpret(); - debug!("Launching interpret {interpret:?}"); - let result = interpret.execute(&self.inventory, &self.topology).await; + debug!("Interpreting score {score:?}"); + let result = score.interpret(&self.inventory, &self.topology).await; debug!("Got result {result:?}"); result } diff --git a/harmony/src/domain/score.rs b/harmony/src/domain/score.rs index a7eb9c7..140d7f1 100644 --- a/harmony/src/domain/score.rs +++ b/harmony/src/domain/score.rs @@ -1,15 +1,51 @@ use std::collections::BTreeMap; +use async_trait::async_trait; use serde::Serialize; use serde_value::Value; -use super::{interpret::Interpret, topology::Topology}; +use super::{ + instrumentation::{self, HarmonyEvent}, + interpret::{Interpret, InterpretError, Outcome}, + inventory::Inventory, + topology::Topology, +}; +#[async_trait] pub trait Score: std::fmt::Debug + ScoreToString + Send + Sync + CloneBoxScore + SerializeScore { - fn create_interpret(&self) -> Box>; + async fn interpret( + &self, + inventory: &Inventory, + topology: &T, + ) -> Result { + let interpret = self.create_interpret(); + + instrumentation::instrument(HarmonyEvent::InterpretExecutionStarted { + topology: topology.name().into(), + interpret: interpret.get_name().to_string(), + score: self.name(), + message: format!("{} running...", interpret.get_name()), + }) + .unwrap(); + let result = interpret.execute(inventory, topology).await; + + instrumentation::instrument(HarmonyEvent::InterpretExecutionFinished { + topology: topology.name().into(), + interpret: interpret.get_name().to_string(), + score: self.name(), + outcome: result.clone(), + }) + .unwrap(); + + result + } + fn name(&self) -> String; + + #[doc(hidden)] + fn create_interpret(&self) -> Box>; } pub trait SerializeScore { diff --git a/harmony/src/domain/topology/k8s_anywhere.rs b/harmony/src/domain/topology/k8s_anywhere.rs index 4df1ef2..b21f385 100644 --- a/harmony/src/domain/topology/k8s_anywhere.rs +++ b/harmony/src/domain/topology/k8s_anywhere.rs @@ -86,8 +86,7 @@ impl PrometheusApplicationMonitoring for K8sAnywhereTopology { let result = self .get_k8s_prometheus_application_score(sender.clone(), receivers) .await - .create_interpret() - .execute(inventory, self) + .interpret(inventory, self) .await; match result { @@ -173,8 +172,7 @@ impl K8sAnywhereTopology { async fn try_install_k3d(&self) -> Result<(), PreparationError> { let result = self .get_k3d_installation_score() - .create_interpret() - .execute(&Inventory::empty(), self) + .interpret(&Inventory::empty(), self) .await; match result { @@ -293,10 +291,7 @@ impl K8sAnywhereTopology { debug!("installing prometheus operator"); let op_score = prometheus_operator_helm_chart_score(sender.namespace.clone()); - let result = op_score - .create_interpret() - .execute(&Inventory::empty(), self) - .await; + let result = op_score.interpret(&Inventory::empty(), self).await; return match result { Ok(outcome) => match outcome.status { diff --git a/harmony/src/domain/topology/oberservability/monitoring.rs b/harmony/src/domain/topology/oberservability/monitoring.rs index 7fa6eb4..c2e93d6 100644 --- a/harmony/src/domain/topology/oberservability/monitoring.rs +++ b/harmony/src/domain/topology/oberservability/monitoring.rs @@ -45,7 +45,7 @@ impl, T: Topology> Interpret for AlertingInte } fn get_name(&self) -> InterpretName { - todo!() + InterpretName::Alerting } fn get_version(&self) -> Version { diff --git a/harmony/src/domain/topology/tenant/k8s.rs b/harmony/src/domain/topology/tenant/k8s.rs index 9b08b66..f2edb05 100644 --- a/harmony/src/domain/topology/tenant/k8s.rs +++ b/harmony/src/domain/topology/tenant/k8s.rs @@ -236,7 +236,7 @@ impl K8sTenantManager { //need to find a way to automatically detect the ip address from the docker //network "ipBlock": { - "cidr": "172.24.0.0/16", + "cidr": "172.18.0.0/16", } } ] diff --git a/harmony/src/modules/application/features/continuous_delivery.rs b/harmony/src/modules/application/features/continuous_delivery.rs index de778fb..e53bd36 100644 --- a/harmony/src/modules/application/features/continuous_delivery.rs +++ b/harmony/src/modules/application/features/continuous_delivery.rs @@ -193,8 +193,7 @@ impl< })], }; score - .create_interpret() - .execute(&Inventory::empty(), topology) + .interpret(&Inventory::empty(), topology) .await .unwrap(); } diff --git a/harmony/src/modules/application/features/helm_argocd_score.rs b/harmony/src/modules/application/features/helm_argocd_score.rs index ff79740..66b23f0 100644 --- a/harmony/src/modules/application/features/helm_argocd_score.rs +++ b/harmony/src/modules/application/features/helm_argocd_score.rs @@ -51,10 +51,7 @@ impl Interpret for ArgoInterpret { topology: &T, ) -> Result { error!("Uncomment below, only disabled for debugging"); - self.score - .create_interpret() - .execute(inventory, topology) - .await?; + self.score.interpret(inventory, topology).await?; let k8s_client = topology.k8s_client().await?; k8s_client diff --git a/harmony/src/modules/application/features/monitoring.rs b/harmony/src/modules/application/features/monitoring.rs index 4c7632c..1ffdace 100644 --- a/harmony/src/modules/application/features/monitoring.rs +++ b/harmony/src/modules/application/features/monitoring.rs @@ -57,8 +57,7 @@ impl< namespace: namespace.clone(), host: "localhost".to_string(), }; - ntfy.create_interpret() - .execute(&Inventory::empty(), topology) + ntfy.interpret(&Inventory::empty(), topology) .await .expect("couldn't create interpret for ntfy"); @@ -95,8 +94,7 @@ impl< alerting_score.receivers.push(Box::new(ntfy_receiver)); alerting_score - .create_interpret() - .execute(&Inventory::empty(), topology) + .interpret(&Inventory::empty(), topology) .await .unwrap(); Ok(()) diff --git a/harmony/src/modules/helm/chart.rs b/harmony/src/modules/helm/chart.rs index c4321d4..b21aa50 100644 --- a/harmony/src/modules/helm/chart.rs +++ b/harmony/src/modules/helm/chart.rs @@ -240,9 +240,11 @@ impl Interpret for HelmChartInterpret { )), } } + fn get_name(&self) -> InterpretName { - todo!() + InterpretName::HelmChart } + fn get_version(&self) -> Version { todo!() } diff --git a/harmony/src/modules/helm/command.rs b/harmony/src/modules/helm/command.rs index 60c6fb6..149d6c6 100644 --- a/harmony/src/modules/helm/command.rs +++ b/harmony/src/modules/helm/command.rs @@ -349,7 +349,7 @@ impl Interpret for HelmChartInterpretV } fn get_name(&self) -> InterpretName { - todo!() + InterpretName::HelmCommand } fn get_version(&self) -> Version { todo!() diff --git a/harmony/src/modules/k3d/install.rs b/harmony/src/modules/k3d/install.rs index 9490dee..d51e005 100644 --- a/harmony/src/modules/k3d/install.rs +++ b/harmony/src/modules/k3d/install.rs @@ -37,7 +37,7 @@ impl Score for K3DInstallationScore { } fn name(&self) -> String { - todo!() + "K3dInstallationScore".into() } } @@ -51,20 +51,14 @@ impl Interpret for K3dInstallationInterpret { async fn execute( &self, _inventory: &Inventory, - topology: &T, + _topology: &T, ) -> Result { - instrumentation::instrument(HarmonyEvent::InterpretExecutionStarted { - topology: topology.name().into(), - interpret: "k3d-installation".into(), - message: "installing k3d...".into(), - }) - .unwrap(); - let k3d = k3d_rs::K3d::new( self.score.installation_path.clone(), Some(self.score.cluster_name.clone()), ); - let outcome = match k3d.ensure_installed().await { + + match k3d.ensure_installed().await { Ok(_client) => { let msg = format!("k3d cluster '{}' installed ", self.score.cluster_name); debug!("{msg}"); @@ -73,16 +67,7 @@ impl Interpret for K3dInstallationInterpret { Err(msg) => Err(InterpretError::new(format!( "failed to ensure k3d is installed : {msg}" ))), - }; - - instrumentation::instrument(HarmonyEvent::InterpretExecutionFinished { - topology: topology.name().into(), - interpret: "k3d-installation".into(), - outcome: outcome.clone(), - }) - .unwrap(); - - outcome + } } fn get_name(&self) -> InterpretName { InterpretName::K3dInstallation diff --git a/harmony/src/modules/k8s/resource.rs b/harmony/src/modules/k8s/resource.rs index 3c0b2bf..b6709ea 100644 --- a/harmony/src/modules/k8s/resource.rs +++ b/harmony/src/modules/k8s/resource.rs @@ -89,7 +89,7 @@ where )) } fn get_name(&self) -> InterpretName { - todo!() + InterpretName::K8sResource } fn get_version(&self) -> Version { todo!() diff --git a/harmony/src/modules/lamp.rs b/harmony/src/modules/lamp.rs index 29228d8..1a853ea 100644 --- a/harmony/src/modules/lamp.rs +++ b/harmony/src/modules/lamp.rs @@ -128,10 +128,7 @@ impl Interpret for LAMPInterpret { info!("Deploying score {deployment_score:#?}"); - deployment_score - .create_interpret() - .execute(inventory, topology) - .await?; + deployment_score.interpret(inventory, topology).await?; info!("LAMP deployment_score {deployment_score:?}"); @@ -153,10 +150,7 @@ impl Interpret for LAMPInterpret { .map(|nbs| fqdn!(nbs.to_string().as_str())), }; - lamp_ingress - .create_interpret() - .execute(inventory, topology) - .await?; + lamp_ingress.interpret(inventory, topology).await?; info!("LAMP lamp_ingress {lamp_ingress:?}"); @@ -166,7 +160,7 @@ impl Interpret for LAMPInterpret { } fn get_name(&self) -> InterpretName { - todo!() + InterpretName::Lamp } fn get_version(&self) -> Version { @@ -215,7 +209,7 @@ impl LAMPInterpret { repository: None, }; - score.create_interpret().execute(inventory, topology).await + score.interpret(inventory, topology).await } fn build_dockerfile(&self, score: &LAMPScore) -> Result> { let mut dockerfile = Dockerfile::new(); diff --git a/harmony/src/modules/monitoring/application_monitoring/application_monitoring_score.rs b/harmony/src/modules/monitoring/application_monitoring/application_monitoring_score.rs index b016db5..1bcaacc 100644 --- a/harmony/src/modules/monitoring/application_monitoring/application_monitoring_score.rs +++ b/harmony/src/modules/monitoring/application_monitoring/application_monitoring_score.rs @@ -69,7 +69,7 @@ impl> Interpret } fn get_name(&self) -> InterpretName { - todo!() + InterpretName::ApplicationMonitoring } fn get_version(&self) -> Version { diff --git a/harmony/src/modules/monitoring/kube_prometheus/prometheus.rs b/harmony/src/modules/monitoring/kube_prometheus/prometheus.rs index 4cf2b47..98970e6 100644 --- a/harmony/src/modules/monitoring/kube_prometheus/prometheus.rs +++ b/harmony/src/modules/monitoring/kube_prometheus/prometheus.rs @@ -119,8 +119,7 @@ impl KubePrometheus { topology: &T, ) -> Result { kube_prometheus_helm_chart_score(self.config.clone()) - .create_interpret() - .execute(inventory, topology) + .interpret(inventory, topology) .await } } diff --git a/harmony/src/modules/monitoring/ntfy/ntfy.rs b/harmony/src/modules/monitoring/ntfy/ntfy.rs index a640bf4..2622c28 100644 --- a/harmony/src/modules/monitoring/ntfy/ntfy.rs +++ b/harmony/src/modules/monitoring/ntfy/ntfy.rs @@ -28,7 +28,7 @@ impl Score for NtfyScore { } fn name(&self) -> String { - "Ntfy".to_string() + "NtfyScore".to_string() } } @@ -96,8 +96,7 @@ impl Interpret for NtfyInterpret { topology: &T, ) -> Result { ntfy_helm_chart_score(self.score.namespace.clone(), self.score.host.clone()) - .create_interpret() - .execute(inventory, topology) + .interpret(inventory, topology) .await?; debug!("installed ntfy helm chart"); @@ -124,8 +123,9 @@ impl Interpret for NtfyInterpret { } fn get_name(&self) -> InterpretName { - todo!() + InterpretName::Ntfy } + fn get_version(&self) -> Version { todo!() } diff --git a/harmony/src/modules/monitoring/prometheus/prometheus.rs b/harmony/src/modules/monitoring/prometheus/prometheus.rs index 934f1ae..a207d5a 100644 --- a/harmony/src/modules/monitoring/prometheus/prometheus.rs +++ b/harmony/src/modules/monitoring/prometheus/prometheus.rs @@ -100,8 +100,7 @@ impl Prometheus { topology: &T, ) -> Result { prometheus_helm_chart_score(self.config.clone()) - .create_interpret() - .execute(inventory, topology) + .interpret(inventory, topology) .await } pub async fn install_grafana( @@ -116,8 +115,7 @@ impl Prometheus { if let Some(ns) = namespace.as_deref() { grafana_helm_chart_score(ns) - .create_interpret() - .execute(inventory, topology) + .interpret(inventory, topology) .await } else { Err(InterpretError::new( diff --git a/harmony/src/modules/prometheus/k8s_prometheus_alerting_score.rs b/harmony/src/modules/prometheus/k8s_prometheus_alerting_score.rs index e806c63..df8fc49 100644 --- a/harmony/src/modules/prometheus/k8s_prometheus_alerting_score.rs +++ b/harmony/src/modules/prometheus/k8s_prometheus_alerting_score.rs @@ -99,7 +99,7 @@ impl> I } fn get_name(&self) -> InterpretName { - todo!() + InterpretName::K8sPrometheusCrdAlerting } fn get_version(&self) -> Version { @@ -118,7 +118,7 @@ impl> I impl K8sPrometheusCRDAlertingInterpret { async fn crd_exists(&self, crd: &str) -> bool { let status = Command::new("sh") - .args(["-c", "kubectl get crd -A | grep -i", crd]) + .args(["-c", &format!("kubectl get crd -A | grep -i {crd}")]) .status() .map_err(|e| InterpretError::new(format!("could not connect to cluster: {}", e))) .unwrap(); diff --git a/harmony/src/modules/tenant/credentials.rs b/harmony/src/modules/tenant/credentials.rs index 0e5917a..2ee24ec 100644 --- a/harmony/src/modules/tenant/credentials.rs +++ b/harmony/src/modules/tenant/credentials.rs @@ -17,7 +17,7 @@ impl Score for TenantCredentialScore { } fn name(&self) -> String { - todo!() + "TenantCredentialScore".into() } } diff --git a/harmony_cli/src/cli_logger.rs b/harmony_cli/src/cli_logger.rs index 12665a6..455a4eb 100644 --- a/harmony_cli/src/cli_logger.rs +++ b/harmony_cli/src/cli_logger.rs @@ -107,12 +107,21 @@ async fn handle_events() { HarmonyEvent::InterpretExecutionStarted { topology, interpret, + score, message, } => { let section_key = if (*sections).contains_key(&topology_key(&topology)) { topology_key(&topology) + } else if (*sections).contains_key(&score_key(&score)) { + score_key(&interpret) } else { - interpret_key(&interpret) + let key = score_key(&score); + let section = progress::new_section(format!( + "\n{} Interpreting score: {score}...", + crate::theme::EMOJI_SCORE, + )); + (*sections).insert(key.clone(), section); + key }; let section = (*sections).get(§ion_key).unwrap(); let progress_bar = progress::add_spinner(section, message); @@ -122,13 +131,14 @@ async fn handle_events() { HarmonyEvent::InterpretExecutionFinished { topology, interpret, + score, outcome, } => { let has_topology = (*sections).contains_key(&topology_key(&topology)); let section_key = if has_topology { topology_key(&topology) } else { - interpret_key(&interpret) + score_key(&score) }; let section = (*sections).get(§ion_key).unwrap(); @@ -138,9 +148,15 @@ async fn handle_events() { let _ = section.clear(); match outcome { - Ok(outcome) => { - progress::success(section, progress_bar, outcome.message); - } + Ok(outcome) => match outcome.status { + harmony::interpret::InterpretStatus::SUCCESS => { + progress::success(section, progress_bar, outcome.message) + } + harmony::interpret::InterpretStatus::NOOP => { + progress::skip(section, progress_bar, outcome.message) + } + _ => progress::error(section, progress_bar, outcome.message), + }, Err(err) => { progress::error(section, progress_bar, err.to_string()); } @@ -162,6 +178,10 @@ fn topology_key(topology: &str) -> String { format!("topology-{topology}") } +fn score_key(score: &str) -> String { + format!("score-{score}") +} + fn interpret_key(interpret: &str) -> String { format!("interpret-{interpret}") } diff --git a/harmony_cli/src/theme.rs b/harmony_cli/src/theme.rs index ee25077..6a059c5 100644 --- a/harmony_cli/src/theme.rs +++ b/harmony_cli/src/theme.rs @@ -8,6 +8,7 @@ pub static EMOJI_SKIP: Emoji<'_, '_> = Emoji("⏭️", ""); pub static EMOJI_ERROR: Emoji<'_, '_> = Emoji("⚠️", ""); pub static EMOJI_DEPLOY: Emoji<'_, '_> = Emoji("🚀", ""); pub static EMOJI_TOPOLOGY: Emoji<'_, '_> = Emoji("📦", ""); +pub static EMOJI_SCORE: Emoji<'_, '_> = Emoji("🎶", ""); lazy_static! { pub static ref SPINNER_STYLE: ProgressStyle = ProgressStyle::default_spinner()