fix: idempotent helm installations for demos/stacks and ignoring of existing jobs (#386)

* helm idempotent installs

* ignore jobs on reapply, merge instead of apply for existing objects

* helm installs: uninstall and install chart instead of upgrade due to helmv3 force changes

* jobs: change failed job apply output

* remove playaround printing

* jobs: restructure code

* helm installs: add comment about procedure, cleanup

* jobs: add comment for immutability error handling

* add changelog entry

Author: xeniape

rust/stackable-cockpit/src/helm.rs

@@ -61,6 +61,9 @@ pub enum Error {
     #[snafu(display("failed to install Helm release"))]
     InstallRelease { source: InstallReleaseError },
 
+    #[snafu(display("failed to upgrade/install Helm release"))]
+    UpgradeRelease { source: InstallReleaseError },
+
     #[snafu(display("failed to uninstall Helm release ({error})"))]
     UninstallRelease { error: String },
 }
@@ -248,6 +251,78 @@ pub fn install_release_from_repo_or_registry(
     })
 }
 
+/// Upgrades a Helm release from a repo or registry.
+///
+/// This function expects the fully qualified Helm release name. In case of our
+/// operators this is: `<PRODUCT_NAME>-operator`.
+#[instrument(skip(values_yaml), fields(with_values = values_yaml.is_some(), indicatif.pb_show = true))]
+pub fn upgrade_or_install_release_from_repo_or_registry(
+    release_name: &str,
+    ChartVersion {
+        chart_source,
+        chart_name,
+        chart_version,
+    }: ChartVersion,
+    values_yaml: Option<&str>,
+    namespace: &str,
+    suppress_output: bool,
+) -> Result<InstallReleaseStatus, Error> {
+    // Ideally, each Helm invocation would spawn_blocking instead in/around helm_sys,
+    // but that requires a larger refactoring
+    block_in_place(|| {
+        debug!("Install/Upgrade Helm release from repo");
+        Span::current()
+            .pb_set_message(format!("Installing/Upgrading {chart_name} Helm chart").as_str());
+
+        if check_release_exists(release_name, namespace)? {
+            let release = get_release(release_name, namespace)?.ok_or(Error::InstallRelease {
+                source: InstallReleaseError::NoSuchRelease {
+                    name: release_name.to_owned(),
+                },
+            })?;
+
+            let current_version = release.version;
+
+            match chart_version {
+                Some(chart_version) => {
+                    if chart_version == current_version {
+                        return Ok(InstallReleaseStatus::ReleaseAlreadyInstalledWithVersion {
+                            requested_version: chart_version.to_string(),
+                            release_name: release_name.to_string(),
+                            current_version,
+                        });
+                    }
+                }
+                None => {
+                    return Ok(InstallReleaseStatus::ReleaseAlreadyInstalledUnspecified {
+                        release_name: release_name.to_string(),
+                        current_version,
+                    });
+                }
+            }
+        }
+
+        let full_chart_name = format!("{chart_source}/{chart_name}");
+        let chart_version = chart_version.unwrap_or(HELM_DEFAULT_CHART_VERSION);
+
+        debug!(
+            release_name,
+            chart_version, full_chart_name, "Installing Helm release"
+        );
+
+        upgrade_release(
+            release_name,
+            &full_chart_name,
+            chart_version,
+            values_yaml,
+            namespace,
+            suppress_output,
+        )?;
+
+        Ok(InstallReleaseStatus::Installed(release_name.to_string()))
+    })
+}
+
 /// Installs a Helm release.
 ///
 /// This function expects the fully qualified Helm release name. In case of our
@@ -281,6 +356,47 @@ fn install_release(
     Ok(())
 }
 
+/// Upgrades a Helm release.
+/// If a release with the specified `chart_name` does not already exist,
+/// this function installs it instead.
+///
+/// This function expects the fully qualified Helm release name. In case of our
+/// operators this is: `<PRODUCT_NAME>-operator`.
+#[instrument(fields(with_values = values_yaml.is_some()))]
+fn upgrade_release(
+    release_name: &str,
+    chart_name: &str,
+    chart_version: &str,
+    values_yaml: Option<&str>,
+    namespace: &str,
+    suppress_output: bool,
+) -> Result<(), Error> {
+    // In Helm 3 the behavior of the `--force` option has changed
+    // It no longer deletes and re-installs a resource https://github.com/helm/helm/issues/7082#issuecomment-559558318
+    // Because of that, conflict errors might appear, which fail the upgrade, even if `helm upgrade --force` is used
+    // Therefore we uninstall the previous release (if present) and install the new one
+    uninstall_release(release_name, namespace, suppress_output)?;
+
+    let result = helm_sys::install_helm_release(
+        release_name,
+        chart_name,
+        chart_version,
+        values_yaml.unwrap_or(""),
+        namespace,
+        suppress_output,
+    );
+
+    if let Some(error) = helm_sys::to_helm_error(&result) {
+        error!("Go wrapper function go_install_helm_release encountered an error: {error}");
+
+        return Err(Error::UpgradeRelease {
+            source: InstallReleaseError::HelmWrapper { error },
+        });
+    }
+
+    Ok(())
+}
+
 /// Uninstall a Helm release.
 ///
 /// This function expects the fully qualified Helm release name. In case of our

rust/stackable-cockpit/src/platform/manifests.rs

@@ -116,7 +116,7 @@ pub trait InstallManifestsExt {
                         .context(SerializeOptionsSnafu)?;
 
                     // Install the Helm chart using the Helm wrapper
-                    helm::install_release_from_repo_or_registry(
+                    helm::upgrade_or_install_release_from_repo_or_registry(
                         &helm_chart.release_name,
                         helm::ChartVersion {
                             chart_source: &helm_chart.repo.name,

rust/stackable-cockpit/src/utils/k8s/client.rs

@@ -15,7 +15,7 @@ use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_operator::{commons::listener::Listener, kvp::Labels};
 use tokio::sync::RwLock;
 use tracing::{Span, info, instrument};
-use tracing_indicatif::span_ext::IndicatifSpanExt as _;
+use tracing_indicatif::{indicatif_eprintln, span_ext::IndicatifSpanExt as _};
 
 #[cfg(doc)]
 use crate::utils::k8s::ListParamsExt;
@@ -144,13 +144,48 @@ impl Client {
                 }
             };
 
-            api.patch(
-                &object.name_any(),
-                &PatchParams::apply("stackablectl"),
-                &Patch::Apply(object),
-            )
-            .await
-            .context(KubeClientPatchSnafu)?;
+            if let Some(existing_object) = api
+                .get_opt(&object.name_any())
+                .await
+                .context(KubeClientFetchSnafu)?
+            {
+                object.metadata.resource_version = existing_object.resource_version();
+
+                api
+                    .patch(
+                        &object.name_any(),
+                        &PatchParams::apply("stackablectl"),
+                        &Patch::Merge(object.clone()),
+                    )
+                    .await
+                    .or_else(|e| {
+                        // If re-applying a Job fails due to immutability, print out the failed manifests instead of erroring out,
+                        // so the user can decide if the existing Job needs a deletion and recreation
+                        match (resource.kind.as_ref(), e) {
+                            // Errors for immutability in Kubernetes do not return meaningful `code`, `status`, or `reason` to filter on
+                            // Currently we have to check the `message` for the actual error we are looking for
+                            ("Job", kube::Error::Api(e)) if e.message.contains("field is immutable") => {
+                                indicatif_eprintln!(
+                                    "Deploying {kind}/{object_name} manifest failed due to immutability",
+                                    kind = resource.kind,
+                                    object_name = object.name_any().clone()
+                                );
+                                Ok(object)
+                            }
+                            (_, e) => {
+                                Err(e).context(KubeClientPatchSnafu)
+                            }
+                        }
+                    })?;
+            } else {
+                api.patch(
+                    &object.name_any(),
+                    &PatchParams::apply("stackablectl"),
+                    &Patch::Apply(object.clone()),
+                )
+                .await
+                .context(KubeClientPatchSnafu)?;
+            }
         }
 
         Ok(())

rust/stackablectl/CHANGELOG.md

@@ -7,8 +7,12 @@ All notable changes to this project will be documented in this file.
 ### Fixed
 
 - nix: Update nixpkgs and upgrade nodejs-18 to nodejs_20 ([#384]).
+- Switch to idempotent Helm installations for demos and stacks ([#386]).
+- Ignore failed re-application of Jobs due to immutability in demo and stack installations.
+  Display those manifests to the user, so they can decide if they need to delete and recreate it ([#386]).
 
 [#384]: https://github.com/stackabletech/stackable-cockpit/pull/384
+[#386]: https://github.com/stackabletech/stackable-cockpit/pull/386
 
 ## [1.0.0] - 2025-06-02