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 <letourneau.ian@gmail.com>
Reviewed-on: https://git.nationtech.io/NationTech/harmony/pulls/100

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<Outcome, InterpretError>,
     },
     TopologyStateChanged {

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"),
         }
     }
 }

harmony/src/domain/maestro/mod.rs

@@ -84,10 +84,8 @@ impl<T: Topology> Maestro<T> {
                 self.topology.name(),
             );
         }
-        debug!("Running score {score:?}");
+        debug!("Interpreting score {score:?}");
-        let interpret = score.create_interpret();
+        let result = score.interpret(&self.inventory, &self.topology).await;
-        debug!("Launching interpret {interpret:?}");
-        let result = interpret.execute(&self.inventory, &self.topology).await;
         debug!("Got result {result:?}");
         result
     }

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<T: Topology>:
     std::fmt::Debug + ScoreToString<T> + Send + Sync + CloneBoxScore<T> + SerializeScore<T>
 {
-    fn create_interpret(&self) -> Box<dyn Interpret<T>>;
+    async fn interpret(
+        &self,
+        inventory: &Inventory,
+        topology: &T,
+    ) -> Result<Outcome, InterpretError> {
+        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<dyn Interpret<T>>;
 }
 
 pub trait SerializeScore<T: Topology> {

harmony/src/domain/topology/k8s_anywhere.rs

@@ -86,8 +86,7 @@ impl PrometheusApplicationMonitoring<CRDPrometheus> for K8sAnywhereTopology {
         let result = self
             .get_k8s_prometheus_application_score(sender.clone(), receivers)
             .await
-            .create_interpret()
+            .interpret(inventory, self)
-            .execute(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()
+            .interpret(&Inventory::empty(), self)
-            .execute(&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
+                        let result = op_score.interpret(&Inventory::empty(), self).await;
-                            .create_interpret()
-                            .execute(&Inventory::empty(), self)
-                            .await;
 
                         return match result {
                             Ok(outcome) => match outcome.status {

harmony/src/domain/topology/oberservability/monitoring.rs

@@ -45,7 +45,7 @@ impl<S: AlertSender + Installable<T>, T: Topology> Interpret<T> for AlertingInte
     }
 
     fn get_name(&self) -> InterpretName {
-        todo!()
+        InterpretName::Alerting
     }
 
     fn get_version(&self) -> Version {

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",
                     }
                   }
                 ]

harmony/src/modules/application/features/continuous_delivery.rs

@@ -193,8 +193,7 @@ impl<
                     })],
                 };
                 score
-                    .create_interpret()
+                    .interpret(&Inventory::empty(), topology)
-                    .execute(&Inventory::empty(), topology)
                     .await
                     .unwrap();
             }

harmony/src/modules/application/features/helm_argocd_score.rs

@@ -51,10 +51,7 @@ impl<T: Topology + K8sclient + HelmCommand> Interpret<T> for ArgoInterpret {
         topology: &T,
     ) -> Result<Outcome, InterpretError> {
         error!("Uncomment below, only disabled for debugging");
-        self.score
+        self.score.interpret(inventory, topology).await?;
-            .create_interpret()
-            .execute(inventory, topology)
-            .await?;
 
         let k8s_client = topology.k8s_client().await?;
         k8s_client

harmony/src/modules/application/features/monitoring.rs

@@ -57,8 +57,7 @@ impl<
             namespace: namespace.clone(),
             host: "localhost".to_string(),
         };
-        ntfy.create_interpret()
+        ntfy.interpret(&Inventory::empty(), topology)
-            .execute(&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()
+            .interpret(&Inventory::empty(), topology)
-            .execute(&Inventory::empty(), topology)
             .await
             .unwrap();
         Ok(())

harmony/src/modules/helm/chart.rs

@@ -240,9 +240,11 @@ impl<T: Topology + HelmCommand> Interpret<T> for HelmChartInterpret {
             )),
         }
     }
+
     fn get_name(&self) -> InterpretName {
-        todo!()
+        InterpretName::HelmChart
     }
+
     fn get_version(&self) -> Version {
         todo!()
     }

harmony/src/modules/helm/command.rs

@@ -349,7 +349,7 @@ impl<T: Topology + K8sclient + HelmCommand> Interpret<T> for HelmChartInterpretV
     }
 
     fn get_name(&self) -> InterpretName {
-        todo!()
+        InterpretName::HelmCommand
     }
     fn get_version(&self) -> Version {
         todo!()

harmony/src/modules/k3d/install.rs

@@ -37,7 +37,7 @@ impl<T: Topology> Score<T> for K3DInstallationScore {
     }
 
     fn name(&self) -> String {
-        todo!()
+        "K3dInstallationScore".into()
     }
 }
 
@@ -51,20 +51,14 @@ impl<T: Topology> Interpret<T> for K3dInstallationInterpret {
     async fn execute(
         &self,
         _inventory: &Inventory,
-        topology: &T,
+        _topology: &T,
     ) -> Result<Outcome, InterpretError> {
-        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<T: Topology> Interpret<T> 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

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!()

harmony/src/modules/lamp.rs

@@ -128,10 +128,7 @@ impl<T: Topology + K8sclient + HelmCommand> Interpret<T> for LAMPInterpret {
 
         info!("Deploying score {deployment_score:#?}");
 
-        deployment_score
+        deployment_score.interpret(inventory, topology).await?;
-            .create_interpret()
-            .execute(inventory, topology)
-            .await?;
 
         info!("LAMP deployment_score {deployment_score:?}");
 
@@ -153,10 +150,7 @@ impl<T: Topology + K8sclient + HelmCommand> Interpret<T> for LAMPInterpret {
                 .map(|nbs| fqdn!(nbs.to_string().as_str())),
         };
 
-        lamp_ingress
+        lamp_ingress.interpret(inventory, topology).await?;
-            .create_interpret()
-            .execute(inventory, topology)
-            .await?;
 
         info!("LAMP lamp_ingress {lamp_ingress:?}");
 
@@ -166,7 +160,7 @@ impl<T: Topology + K8sclient + HelmCommand> Interpret<T> 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<PathBuf, Box<dyn std::error::Error>> {
         let mut dockerfile = Dockerfile::new();

harmony/src/modules/monitoring/application_monitoring/application_monitoring_score.rs

@@ -69,7 +69,7 @@ impl<T: Topology + PrometheusApplicationMonitoring<CRDPrometheus>> Interpret<T>
     }
 
     fn get_name(&self) -> InterpretName {
-        todo!()
+        InterpretName::ApplicationMonitoring
     }
 
     fn get_version(&self) -> Version {

harmony/src/modules/monitoring/kube_prometheus/prometheus.rs

@@ -119,8 +119,7 @@ impl KubePrometheus {
         topology: &T,
     ) -> Result<Outcome, InterpretError> {
         kube_prometheus_helm_chart_score(self.config.clone())
-            .create_interpret()
+            .interpret(inventory, topology)
-            .execute(inventory, topology)
             .await
     }
 }

harmony/src/modules/monitoring/ntfy/ntfy.rs

@@ -28,7 +28,7 @@ impl<T: Topology + HelmCommand + K8sclient> Score<T> for NtfyScore {
     }
 
     fn name(&self) -> String {
-        "Ntfy".to_string()
+        "NtfyScore".to_string()
     }
 }
 
@@ -96,8 +96,7 @@ impl<T: Topology + HelmCommand + K8sclient> Interpret<T> for NtfyInterpret {
         topology: &T,
     ) -> Result<Outcome, InterpretError> {
         ntfy_helm_chart_score(self.score.namespace.clone(), self.score.host.clone())
-            .create_interpret()
+            .interpret(inventory, topology)
-            .execute(inventory, topology)
             .await?;
 
         debug!("installed ntfy helm chart");
@@ -124,8 +123,9 @@ impl<T: Topology + HelmCommand + K8sclient> Interpret<T> for NtfyInterpret {
     }
 
     fn get_name(&self) -> InterpretName {
-        todo!()
+        InterpretName::Ntfy
     }
+
     fn get_version(&self) -> Version {
         todo!()
     }

harmony/src/modules/monitoring/prometheus/prometheus.rs

@@ -100,8 +100,7 @@ impl Prometheus {
         topology: &T,
     ) -> Result<Outcome, InterpretError> {
         prometheus_helm_chart_score(self.config.clone())
-            .create_interpret()
+            .interpret(inventory, topology)
-            .execute(inventory, topology)
             .await
     }
     pub async fn install_grafana<T: Topology + HelmCommand + Send + Sync>(
@@ -116,8 +115,7 @@ impl Prometheus {
 
         if let Some(ns) = namespace.as_deref() {
             grafana_helm_chart_score(ns)
-                .create_interpret()
+                .interpret(inventory, topology)
-                .execute(inventory, topology)
                 .await
         } else {
             Err(InterpretError::new(

harmony/src/modules/prometheus/k8s_prometheus_alerting_score.rs

@@ -99,7 +99,7 @@ impl<T: Topology + K8sclient + PrometheusApplicationMonitoring<CRDPrometheus>> I
     }
 
     fn get_name(&self) -> InterpretName {
-        todo!()
+        InterpretName::K8sPrometheusCrdAlerting
     }
 
     fn get_version(&self) -> Version {
@@ -118,7 +118,7 @@ impl<T: Topology + K8sclient + PrometheusApplicationMonitoring<CRDPrometheus>> 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();

harmony/src/modules/tenant/credentials.rs

@@ -17,7 +17,7 @@ impl<T: Topology + TenantCredentialManager> Score<T> for TenantCredentialScore {
     }
 
     fn name(&self) -> String {
-        todo!()
+        "TenantCredentialScore".into()
     }
 }
 

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(&section_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(&section_key).unwrap();
@@ -138,9 +148,15 @@ async fn handle_events() {
                         let _ = section.clear();
 
                         match outcome {
-                            Ok(outcome) => {
+                            Ok(outcome) => match outcome.status {
-                                progress::success(section, progress_bar, outcome.message);
+                                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}")
 }

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()