feat(helm): refactor helm execution to use topology-specific commands
Some checks failed
Run Check Script / check (pull_request) Failing after 8s

Refactors the `HelmChartInterpret` to move away from the `helm-wrapper-rs` crate in favor of a custom command builder pattern. This allows the `HelmCommand` trait to provide topology-specific configurations, such as `kubeconfig` and `kube-context`, directly to the `helm` CLI.

- Implements `get_helm_command` for `K8sAnywhereTopology` to inject configuration flags.
- Replaces `DefaultHelmExecutor` with a manual `Command` construction in `run_helm_command`.
- Updates `HelmChartInterpret` to pass the topology through to repository and installation logic.
- Cleans up unused imports and removes the temporary `HelmCommand` implementation for `LocalhostTopology`.
This commit is contained in:
2026-01-08 23:42:54 -05:00
parent 77583a1ad1
commit 6933280575
3 changed files with 80 additions and 113 deletions

View File

@@ -1112,7 +1112,21 @@ impl MultiTargetTopology for K8sAnywhereTopology {
} }
} }
impl HelmCommand for K8sAnywhereTopology {} impl HelmCommand for K8sAnywhereTopology {
fn get_helm_command(&self) -> Command {
let mut cmd = Command::new("helm");
if let Some(k) = &self.config.kubeconfig {
cmd.args(["--kubeconfig", k]);
}
if let Some(c) = &self.config.k8s_context {
cmd.args(["--kube-context", c]);
}
info!("Using helm command {cmd:?}");
cmd
}
}
#[async_trait] #[async_trait]
impl TenantManager for K8sAnywhereTopology { impl TenantManager for K8sAnywhereTopology {
@@ -1133,7 +1147,7 @@ impl TenantManager for K8sAnywhereTopology {
#[async_trait] #[async_trait]
impl Ingress for K8sAnywhereTopology { impl Ingress for K8sAnywhereTopology {
async fn get_domain(&self, service: &str) -> Result<String, PreparationError> { async fn get_domain(&self, service: &str) -> Result<String, PreparationError> {
use log::{debug, trace, warn}; use log::{trace, warn};
let client = self.k8s_client().await?; let client = self.k8s_client().await?;

View File

@@ -2,7 +2,7 @@ use async_trait::async_trait;
use derive_new::new; use derive_new::new;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use super::{HelmCommand, PreparationError, PreparationOutcome, Topology}; use super::{PreparationError, PreparationOutcome, Topology};
#[derive(new, Clone, Debug, Serialize, Deserialize)] #[derive(new, Clone, Debug, Serialize, Deserialize)]
pub struct LocalhostTopology; pub struct LocalhostTopology;
@@ -19,6 +19,3 @@ impl Topology for LocalhostTopology {
}) })
} }
} }
// TODO: Delete this, temp for test
impl HelmCommand for LocalhostTopology {}

View File

@@ -6,15 +6,11 @@ use crate::topology::{HelmCommand, Topology};
use async_trait::async_trait; use async_trait::async_trait;
use harmony_types::id::Id; use harmony_types::id::Id;
use harmony_types::net::Url; use harmony_types::net::Url;
use helm_wrapper_rs;
use helm_wrapper_rs::blocking::{DefaultHelmExecutor, HelmExecutor};
use log::{debug, info, warn}; use log::{debug, info, warn};
pub use non_blank_string_rs::NonBlankString; pub use non_blank_string_rs::NonBlankString;
use serde::Serialize; use serde::Serialize;
use std::collections::HashMap; use std::collections::HashMap;
use std::path::Path; use std::process::{Output, Stdio};
use std::process::{Command, Output, Stdio};
use std::str::FromStr;
use temp_file::TempFile; use temp_file::TempFile;
#[derive(Debug, Clone, Serialize)] #[derive(Debug, Clone, Serialize)]
@@ -65,7 +61,7 @@ pub struct HelmChartInterpret {
pub score: HelmChartScore, pub score: HelmChartScore,
} }
impl HelmChartInterpret { impl HelmChartInterpret {
fn add_repo(&self) -> Result<(), InterpretError> { fn add_repo<T: HelmCommand>(&self, topology: &T) -> Result<(), InterpretError> {
let repo = match &self.score.repository { let repo = match &self.score.repository {
Some(repo) => repo, Some(repo) => repo,
None => { None => {
@@ -84,7 +80,7 @@ impl HelmChartInterpret {
add_args.push("--force-update"); add_args.push("--force-update");
} }
let add_output = run_helm_command(&add_args)?; let add_output = run_helm_command(topology, &add_args)?;
let full_output = format!( let full_output = format!(
"{}\n{}", "{}\n{}",
String::from_utf8_lossy(&add_output.stdout), String::from_utf8_lossy(&add_output.stdout),
@@ -100,23 +96,19 @@ impl HelmChartInterpret {
} }
} }
fn run_helm_command(args: &[&str]) -> Result<Output, InterpretError> { fn run_helm_command<T: HelmCommand>(topology: &T, args: &[&str]) -> Result<Output, InterpretError> {
let command_str = format!("helm {}", args.join(" ")); let mut helm_cmd = topology.get_helm_command();
debug!( helm_cmd.args(args);
"Got KUBECONFIG: `{}`",
std::env::var("KUBECONFIG").unwrap_or("".to_string())
);
debug!("Running Helm command: `{}`", command_str);
let output = Command::new("helm") debug!("Running Helm command: `{:?}`", helm_cmd);
.args(args)
let output = helm_cmd
.stdout(Stdio::piped()) .stdout(Stdio::piped())
.stderr(Stdio::piped()) .stderr(Stdio::piped())
.output() .output()
.map_err(|e| { .map_err(|e| {
InterpretError::new(format!( InterpretError::new(format!(
"Failed to execute helm command '{}': {}. Is helm installed and in PATH?", "Failed to execute helm command '{helm_cmd:?}': {e}. Is helm installed and in PATH?",
command_str, e
)) ))
})?; })?;
@@ -124,13 +116,13 @@ fn run_helm_command(args: &[&str]) -> Result<Output, InterpretError> {
let stdout = String::from_utf8_lossy(&output.stdout); let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr); let stderr = String::from_utf8_lossy(&output.stderr);
warn!( warn!(
"Helm command `{}` failed with status: {}\nStdout:\n{}\nStderr:\n{}", "Helm command `{helm_cmd:?}` failed with status: {}\nStdout:\n{stdout}\nStderr:\n{stderr}",
command_str, output.status, stdout, stderr output.status
); );
} else { } else {
debug!( debug!(
"Helm command `{}` finished successfully. Status: {}", "Helm command `{helm_cmd:?}` finished successfully. Status: {}",
command_str, output.status output.status
); );
} }
@@ -142,7 +134,7 @@ impl<T: Topology + HelmCommand> Interpret<T> for HelmChartInterpret {
async fn execute( async fn execute(
&self, &self,
_inventory: &Inventory, _inventory: &Inventory,
_topology: &T, topology: &T,
) -> Result<Outcome, InterpretError> { ) -> Result<Outcome, InterpretError> {
let ns = self let ns = self
.score .score
@@ -150,98 +142,62 @@ impl<T: Topology + HelmCommand> Interpret<T> for HelmChartInterpret {
.as_ref() .as_ref()
.unwrap_or_else(|| todo!("Get namespace from active kubernetes cluster")); .unwrap_or_else(|| todo!("Get namespace from active kubernetes cluster"));
let tf: TempFile; self.add_repo(topology)?;
let yaml_path: Option<&Path> = match self.score.values_yaml.as_ref() {
Some(yaml_str) => { let mut args = if self.score.install_only {
tf = temp_file::with_contents(yaml_str.as_bytes()); vec!["install"]
debug!( } else {
"values yaml string for chart {} :\n {yaml_str}", vec!["upgrade", "--install"]
self.score.chart_name
);
Some(tf.path())
}
None => None,
}; };
self.add_repo()?; args.extend(vec![
let helm_executor = DefaultHelmExecutor::new_with_opts(
&NonBlankString::from_str("helm").unwrap(),
None,
900,
false,
false,
);
let mut helm_options = Vec::new();
if self.score.create_namespace {
helm_options.push(NonBlankString::from_str("--create-namespace").unwrap());
}
if self.score.install_only {
let chart_list = match helm_executor.list(Some(ns)) {
Ok(charts) => charts,
Err(e) => {
return Err(InterpretError::new(format!(
"Failed to list scores in namespace {:?} because of error : {}",
self.score.namespace, e
)));
}
};
if chart_list
.iter()
.any(|item| item.name == self.score.release_name.to_string())
{
info!(
"Release '{}' already exists in namespace '{}'. Skipping installation as install_only is true.",
self.score.release_name, ns
);
return Ok(Outcome::success(format!(
"Helm Chart '{}' already installed to namespace {ns} and install_only=true",
self.score.release_name
)));
} else {
info!(
"Release '{}' not found in namespace '{}'. Proceeding with installation.",
self.score.release_name, ns
);
}
}
let res = helm_executor.install_or_upgrade(
ns,
&self.score.release_name, &self.score.release_name,
&self.score.chart_name, &self.score.chart_name,
self.score.chart_version.as_ref(), "--namespace",
self.score.values_overrides.as_ref(), &ns,
yaml_path, ]);
Some(&helm_options),
);
let status = match res { if self.score.create_namespace {
Ok(status) => status, args.push("--create-namespace");
Err(err) => return Err(InterpretError::new(err.to_string())), }
};
match status { if let Some(version) = &self.score.chart_version {
helm_wrapper_rs::HelmDeployStatus::Deployed => Ok(Outcome::success(format!( args.push("--version");
args.push(&version);
}
let tf: TempFile;
if let Some(yaml_str) = &self.score.values_yaml {
tf = temp_file::with_contents(yaml_str.as_bytes());
args.push("--values");
args.push(tf.path().to_str().unwrap());
}
let overrides_strings: Vec<String>;
if let Some(overrides) = &self.score.values_overrides {
overrides_strings = overrides
.iter()
.map(|(key, value)| format!("{key}={value}"))
.collect();
for o in overrides_strings.iter() {
args.push("--set");
args.push(&o);
}
}
let output = run_helm_command(topology, &args)?;
if output.status.success() {
Ok(Outcome::success(format!(
"Helm Chart {} deployed", "Helm Chart {} deployed",
self.score.release_name self.score.release_name
))), )))
helm_wrapper_rs::HelmDeployStatus::PendingInstall => Ok(Outcome::running(format!( } else {
"Helm Chart {} pending install...", Err(InterpretError::new(format!(
self.score.release_name "Helm Chart {} installation failed: {}",
))), self.score.release_name,
helm_wrapper_rs::HelmDeployStatus::PendingUpgrade => Ok(Outcome::running(format!( String::from_utf8_lossy(&output.stderr)
"Helm Chart {} pending upgrade...", )))
self.score.release_name
))),
helm_wrapper_rs::HelmDeployStatus::Failed => Err(InterpretError::new(format!(
"Helm Chart {} installation failed",
self.score.release_name
))),
} }
} }