From 22752960f9d2dc7c1816c46a9558f30ce2f2bed6 Mon Sep 17 00:00:00 2001 From: Jean-Gabriel Gill-Couture Date: Fri, 25 Apr 2025 11:32:02 -0400 Subject: [PATCH] fix(k8s_anywhere): Ensure k3d cluster is started before use - Refactor k3d cluster management to explicitly start the cluster. - Introduce `start_cluster` function to ensure cluster is running before operations. - Improve error handling and logging during cluster startup. - Update `create_cluster` and other related functions to utilize the new startup mechanism. - Enhance reliability and prevent potential issues caused by an uninitialized cluster. - Add `run_k3d_command` to handle k3d commands with logging and error handling. --- examples/lamp/src/main.rs | 1 + harmony/src/domain/topology/k8s_anywhere.rs | 21 +++-- k3d/src/lib.rs | 85 ++++++++++++++++----- opnsense-config/src/config/config.rs | 2 +- opnsense-config/src/lib.rs | 2 +- 5 files changed, 81 insertions(+), 30 deletions(-) diff --git a/examples/lamp/src/main.rs b/examples/lamp/src/main.rs index d070871..27e5df6 100644 --- a/examples/lamp/src/main.rs +++ b/examples/lamp/src/main.rs @@ -8,6 +8,7 @@ use harmony::{ #[tokio::main] async fn main() { + // let _ = env_logger::Builder::from_default_env().filter_level(log::LevelFilter::Info).try_init(); let lamp_stack = LAMPScore { name: "harmony-lamp-demo".to_string(), domain: Url::Url(url::Url::parse("https://lampdemo.harmony.nationtech.io").unwrap()), diff --git a/harmony/src/domain/topology/k8s_anywhere.rs b/harmony/src/domain/topology/k8s_anywhere.rs index 780bc92..5325915 100644 --- a/harmony/src/domain/topology/k8s_anywhere.rs +++ b/harmony/src/domain/topology/k8s_anywhere.rs @@ -116,17 +116,22 @@ impl K8sAnywhereTopology { info!("Starting K8sAnywhere installation"); self.try_install_k3d().await?; let k3d_score = self.get_k3d_installation_score(); - match k3d_rs::K3d::new(k3d_score.installation_path, Some(k3d_score.cluster_name)) - .get_client() - .await - { - Ok(client) => Ok(Some(K8sState { + // I feel like having to rely on the k3d_rs crate here is a smell + // I think we should have a way to interact more deeply with scores/interpret. Maybe the + // K3DInstallationScore should expose a method to get_client ? Not too sure what would be a + // good implementation due to the stateful nature of the k3d thing. Which is why I went + // with this solution for now + let k3d = k3d_rs::K3d::new(k3d_score.installation_path, Some(k3d_score.cluster_name)); + let state = match k3d.get_client().await { + Ok(client) => K8sState { _client: K8sClient::new(client), _source: K8sSource::LocalK3d, message: "Successfully installed K3D cluster and acquired client".to_string(), - })), + }, Err(_) => todo!(), - } + }; + + Ok(Some(state)) } } @@ -154,7 +159,7 @@ struct K8sAnywhereConfig { #[async_trait] impl Topology for K8sAnywhereTopology { fn name(&self) -> &str { - todo!() + "K8sAnywhereTopology" } async fn ensure_ready(&self) -> Result { diff --git a/k3d/src/lib.rs b/k3d/src/lib.rs index 88d5963..ff63704 100644 --- a/k3d/src/lib.rs +++ b/k3d/src/lib.rs @@ -117,7 +117,7 @@ impl K3d { /// 2. It has proper executable permissions (on Unix systems) /// 3. It responds correctly to a simple command (`k3d --version`) pub fn is_installed(&self) -> bool { - let binary_path = self.base_dir.join(K3D_BIN_FILE_NAME); + let binary_path = self.get_k3d_binary_path(); if !binary_path.exists() { debug!("K3d binary not found at {:?}", binary_path); @@ -131,15 +131,15 @@ impl K3d { self.can_execute_binary_check(&binary_path) } - /// Verifies if the specified cluster is already running + /// Verifies if the specified cluster is already created /// /// Executes `k3d cluster list ` and checks for a successful response, /// indicating that the cluster exists and is registered with k3d. pub fn is_cluster_initialized(&self) -> bool { - let cluster_name = match &self.cluster_name { - Some(name) => name, - None => { - debug!("No cluster name specified, can't verify if cluster is initialized"); + let cluster_name = match self.get_cluster_name() { + Ok(name) => name, + Err(_) => { + debug!("Could not get cluster name, can't verify if cluster is initialized"); return false; } }; @@ -152,6 +152,13 @@ impl K3d { self.verify_cluster_exists(&binary_path, cluster_name) } + fn get_cluster_name(&self) -> Result<&String, String> { + match &self.cluster_name { + Some(name) => Ok(name), + None => Err("No cluster name available".to_string()), + } + } + /// Creates a new k3d cluster with the specified name /// /// This method: @@ -163,22 +170,29 @@ impl K3d { /// - `Ok(Client)` - Successfully created cluster and connected client /// - `Err(String)` - Error message detailing what went wrong pub async fn initialize_cluster(&self) -> Result { - let cluster_name = match &self.cluster_name { - Some(name) => name, - None => return Err("No cluster name specified for initialization".to_string()), + let cluster_name = match self.get_cluster_name() { + Ok(name) => name, + Err(_) => return Err("Could not get cluster_name, cannot initialize".to_string()), }; - let binary_path = self.base_dir.join(K3D_BIN_FILE_NAME); - if !binary_path.exists() { - return Err(format!("K3d binary not found at {:?}", binary_path)); - } - info!("Initializing k3d cluster '{}'", cluster_name); - self.create_cluster(&binary_path, cluster_name)?; + self.create_cluster(cluster_name)?; self.create_kubernetes_client().await } + fn get_k3d_binary_path(&self) -> PathBuf { + self.base_dir.join(K3D_BIN_FILE_NAME) + } + + fn get_k3d_binary(&self) -> Result { + let path = self.get_k3d_binary_path(); + if !path.exists() { + return Err(format!("K3d binary not found at {:?}", path)); + } + Ok(path) + } + /// Ensures k3d is installed and the cluster is initialized /// /// This method provides a complete setup flow: @@ -206,6 +220,8 @@ impl K3d { return self.initialize_cluster().await; } + self.start_cluster().await?; + info!("K3d and cluster are already properly set up"); self.create_kubernetes_client().await } @@ -282,11 +298,27 @@ impl K3d { } } - fn create_cluster(&self, binary_path: &PathBuf, cluster_name: &str) -> Result<(), String> { - let output = std::process::Command::new(binary_path) - .args(["cluster", "create", cluster_name]) - .output() - .map_err(|e| format!("Failed to execute k3d command: {}", e))?; + pub fn run_k3d_command(&self, args: I) -> Result + where + I: IntoIterator, + S: AsRef, + { + let binary_path = self.get_k3d_binary()?; + let output = std::process::Command::new(binary_path).args(args).output(); + match output { + Ok(output) => { + let stderr = String::from_utf8_lossy(&output.stderr); + debug!("stderr : {}", stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + debug!("stdout : {}", stdout); + Ok(output) + } + Err(e) => Err(format!("Failed to execute k3d command: {}", e)), + } + } + + fn create_cluster(&self, cluster_name: &str) -> Result<(), String> { + let output = self.run_k3d_command(["cluster", "create", cluster_name])?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); @@ -310,6 +342,19 @@ impl K3d { false => Err("Cannot get client! Cluster not initialized yet".to_string()), } } + + async fn start_cluster(&self) -> Result<(), String> { + let cluster_name = self.get_cluster_name()?; + let output = self.run_k3d_command(["cluster", "start", cluster_name])?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(format!("Failed to start cluster: {}", stderr)); + } + + info!("Successfully started k3d cluster '{}'", cluster_name); + Ok(()) + } } #[cfg(test)] diff --git a/opnsense-config/src/config/config.rs b/opnsense-config/src/config/config.rs index 10dab61..3f00c44 100644 --- a/opnsense-config/src/config/config.rs +++ b/opnsense-config/src/config/config.rs @@ -23,7 +23,7 @@ pub struct Config { } impl Serialize for Config { - fn serialize(&self, serializer: S) -> Result + fn serialize(&self, _serializer: S) -> Result where S: serde::Serializer, { diff --git a/opnsense-config/src/lib.rs b/opnsense-config/src/lib.rs index f83b3e0..a497133 100644 --- a/opnsense-config/src/lib.rs +++ b/opnsense-config/src/lib.rs @@ -10,11 +10,11 @@ mod test { use std::net::Ipv4Addr; use crate::Config; - use pretty_assertions::assert_eq; #[cfg(opnsenseendtoend)] #[tokio::test] async fn test_public_sdk() { + use pretty_assertions::assert_eq; let mac = "11:22:33:44:55:66"; let ip = Ipv4Addr::new(10, 100, 8, 200); let hostname = "test_hostname";