From 32bd3bac7840c7673d601457b14f581056ce6f47 Mon Sep 17 00:00:00 2001 From: Techassi Date: Wed, 3 Jan 2024 10:58:50 +0100 Subject: [PATCH 1/3] fix: Uninstall operators when removing release --- rust/stackable-cockpit/src/helm.rs | 15 +++++++++--- .../src/platform/operator/mod.rs | 1 - .../src/platform/release/spec.rs | 24 +++++++++++++++---- .../stackable-cockpit/src/platform/service.rs | 2 +- .../src/platform/stack/spec.rs | 1 - rust/stackablectl/CHANGELOG.md | 15 ++++++++---- rust/stackablectl/src/cmds/release.rs | 11 ++++----- 7 files changed, 47 insertions(+), 22 deletions(-) diff --git a/rust/stackable-cockpit/src/helm.rs b/rust/stackable-cockpit/src/helm.rs index 2333260a..11da6523 100644 --- a/rust/stackable-cockpit/src/helm.rs +++ b/rust/stackable-cockpit/src/helm.rs @@ -186,10 +186,12 @@ pub struct ChartVersion<'a> { pub chart_version: Option<&'a str>, } -/// Installs a Helm release +/// Installs a Helm release from a repo. +/// +/// This function expects the fully qualified Helm release name. In case of our +/// operators this is: `-operator`. #[instrument] pub fn install_release_from_repo( - operator_name: &str, release_name: &str, ChartVersion { repo_name, @@ -258,6 +260,10 @@ pub fn install_release_from_repo( Ok(InstallReleaseStatus::Installed(release_name.to_string())) } +/// Installs a Helm release. +/// +/// This function expects the fully qualified Helm release name. In case of our +/// operators this is: `-operator`. fn install_release( release_name: &str, chart_name: &str, @@ -289,7 +295,10 @@ fn install_release( Ok(()) } -/// Uninstall a Helm release +/// Uninstall a Helm release. +/// +/// This function expects the fully qualified Helm release name. In case of our +/// operators this is: `-operator`. #[instrument] pub fn uninstall_release( release_name: &str, diff --git a/rust/stackable-cockpit/src/platform/operator/mod.rs b/rust/stackable-cockpit/src/platform/operator/mod.rs index a9f82f47..f801e664 100644 --- a/rust/stackable-cockpit/src/platform/operator/mod.rs +++ b/rust/stackable-cockpit/src/platform/operator/mod.rs @@ -185,7 +185,6 @@ impl OperatorSpec { // Install using Helm helm::install_release_from_repo( - &self.name, &helm_name, helm::ChartVersion { chart_version: version.as_deref(), diff --git a/rust/stackable-cockpit/src/platform/release/spec.rs b/rust/stackable-cockpit/src/platform/release/spec.rs index 5b3343f6..4c6d6ae7 100644 --- a/rust/stackable-cockpit/src/platform/release/spec.rs +++ b/rust/stackable-cockpit/src/platform/release/spec.rs @@ -8,7 +8,10 @@ use utoipa::ToSchema; use crate::{ helm, - platform::{operator, product}, + platform::{ + operator::{self, OperatorSpec}, + product, + }, }; type Result = std::result::Result; @@ -52,10 +55,10 @@ impl ReleaseSpec { info!("Installing release"); for (product_name, product) in self.filter_products(include_products, exclude_products) { - info!("Installing product {}", product_name); + info!("Installing {}-operator", product_name); // Create operator spec - let operator = operator::OperatorSpec::new(product_name, Some(product.version.clone())) + let operator = OperatorSpec::new(product_name, Some(product.version.clone())) .context(OperatorSpecParseSnafu)?; // Install operator @@ -65,9 +68,20 @@ impl ReleaseSpec { Ok(()) } + #[instrument(skip_all)] pub fn uninstall(&self, namespace: &str) -> Result<()> { - for (product_name, _) in &self.products { - helm::uninstall_release(product_name, namespace, true).context(HelmUninstallSnafu)?; + info!("Uninstalling release"); + + for (product_name, product_spec) in &self.products { + info!("Uninstalling {}-operator", product_name); + + // Create operator spec + let operator = OperatorSpec::new(product_name, Some(product_spec.version.clone())) + .context(OperatorSpecParseSnafu)?; + + // Uninstall operator + helm::uninstall_release(&operator.helm_name(), namespace, true) + .context(HelmUninstallSnafu)?; } Ok(()) diff --git a/rust/stackable-cockpit/src/platform/service.rs b/rust/stackable-cockpit/src/platform/service.rs index cabd1055..cda0d481 100644 --- a/rust/stackable-cockpit/src/platform/service.rs +++ b/rust/stackable-cockpit/src/platform/service.rs @@ -192,7 +192,7 @@ pub async fn get_endpoint_urls_for_loadbalancer( .as_ref() .and_then(|s| s.load_balancer.as_ref()) .and_then(|l| l.ingress.as_ref()) - .and_then(|l| l.get(0)); + .and_then(|l| l.first()); if let Some(lb_host) = lb_host { let lb_host = lb_host.hostname.as_ref().or(lb_host.ip.as_ref()); diff --git a/rust/stackable-cockpit/src/platform/stack/spec.rs b/rust/stackable-cockpit/src/platform/stack/spec.rs index 483c1022..f670d408 100644 --- a/rust/stackable-cockpit/src/platform/stack/spec.rs +++ b/rust/stackable-cockpit/src/platform/stack/spec.rs @@ -286,7 +286,6 @@ impl StackSpec { // Install the Helm chart using the Helm wrapper helm::install_release_from_repo( - &helm_chart.release_name, &helm_chart.release_name, helm::ChartVersion { repo_name: &helm_chart.repo.name, diff --git a/rust/stackablectl/CHANGELOG.md b/rust/stackablectl/CHANGELOG.md index 63d824f0..351991f8 100644 --- a/rust/stackablectl/CHANGELOG.md +++ b/rust/stackablectl/CHANGELOG.md @@ -4,13 +4,20 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed + +- Fix `stackablectl release uninstall` command. It now deletes the operators included in the selected release correctly + ([#173]). + +[#173]: https://github.com/stackabletech/stackable-cockpit/pull/173 + ## [23.11.2] - 2024-01-02 ### Changed -- Bumped Rust version from `1.74.0` to `1.75.0` ([#172]). -- Bumped Rust and Go dependencies ([#135], [#162], [#167], [#168], [#170]). -- Renamed old output style `plain` to `table`. The new output option `plain` will output a reduced view (which removes +- Bump Rust version from `1.74.0` to `1.75.0` ([#172]). +- Bump Rust and Go dependencies ([#135], [#162], [#167], [#168], [#170]). +- Rename old output style `plain` to `table`. The new output option `plain` will output a reduced view (which removes borders from tables for example) ([#142], [#163]). [#135]: https://github.com/stackabletech/stackable-cockpit/pull/135 @@ -36,6 +43,6 @@ First official release of the `stackablectl` rewrite. ### Changed -- Bumped Rust version from `1.73.0` to `1.74.0` ([#151]). +- Bump Rust version from `1.73.0` to `1.74.0` ([#151]). [#151]: https://github.com/stackabletech/stackable-cockpit/pull/151 diff --git a/rust/stackablectl/src/cmds/release.rs b/rust/stackablectl/src/cmds/release.rs index 1285a934..44913cf0 100644 --- a/rust/stackablectl/src/cmds/release.rs +++ b/rust/stackablectl/src/cmds/release.rs @@ -145,7 +145,7 @@ impl ReleaseArgs { } } -#[instrument] +#[instrument(skip(cli, release_list))] async fn list_cmd( args: &ReleaseListArgs, cli: &Cli, @@ -199,7 +199,7 @@ async fn list_cmd( } } -#[instrument] +#[instrument(skip(cli, release_list))] async fn describe_cmd( args: &ReleaseDescribeArgs, cli: &Cli, @@ -260,14 +260,12 @@ async fn describe_cmd( } } -#[instrument] +#[instrument(skip(cli, release_list))] async fn install_cmd( args: &ReleaseInstallArgs, cli: &Cli, release_list: release::List, ) -> Result { - info!("Installing release"); - match release_list.get(&args.release) { Some(release) => { let mut output = cli.result(); @@ -306,13 +304,12 @@ async fn install_cmd( } } +#[instrument(skip(cli, release_list))] async fn uninstall_cmd( args: &ReleaseUninstallArgs, cli: &Cli, release_list: release::List, ) -> Result { - info!("Installing release"); - match release_list.get(&args.release) { Some(release) => { release From 6f914844bcc33d52d0a5feb44ae3d715debaacaa Mon Sep 17 00:00:00 2001 From: Techassi Date: Wed, 3 Jan 2024 11:01:37 +0100 Subject: [PATCH 2/3] Update linked PR in changelog --- rust/stackablectl/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/stackablectl/CHANGELOG.md b/rust/stackablectl/CHANGELOG.md index 351991f8..90d7702e 100644 --- a/rust/stackablectl/CHANGELOG.md +++ b/rust/stackablectl/CHANGELOG.md @@ -7,9 +7,9 @@ All notable changes to this project will be documented in this file. ### Fixed - Fix `stackablectl release uninstall` command. It now deletes the operators included in the selected release correctly - ([#173]). + ([#174]). -[#173]: https://github.com/stackabletech/stackable-cockpit/pull/173 +[#174]: https://github.com/stackabletech/stackable-cockpit/pull/174 ## [23.11.2] - 2024-01-02 From 1cdab46fa3e1742a1cc9af6a5e243ccd42cf205c Mon Sep 17 00:00:00 2001 From: Techassi Date: Wed, 3 Jan 2024 12:10:21 +0100 Subject: [PATCH 3/3] Add review suggestions --- rust/stackable-cockpit/src/platform/release/spec.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/stackable-cockpit/src/platform/release/spec.rs b/rust/stackable-cockpit/src/platform/release/spec.rs index 4c6d6ae7..96bc238a 100644 --- a/rust/stackable-cockpit/src/platform/release/spec.rs +++ b/rust/stackable-cockpit/src/platform/release/spec.rs @@ -55,7 +55,7 @@ impl ReleaseSpec { info!("Installing release"); for (product_name, product) in self.filter_products(include_products, exclude_products) { - info!("Installing {}-operator", product_name); + info!("Installing {product_name}-operator"); // Create operator spec let operator = OperatorSpec::new(product_name, Some(product.version.clone())) @@ -73,7 +73,7 @@ impl ReleaseSpec { info!("Uninstalling release"); for (product_name, product_spec) in &self.products { - info!("Uninstalling {}-operator", product_name); + info!("Uninstalling {product_name}-operator"); // Create operator spec let operator = OperatorSpec::new(product_name, Some(product_spec.version.clone()))