feat: Add uninstall demo/stack feature by xeniape · Pull Request #429 · stackabletech/stackable-cockpit

xeniape · 2026-03-12T13:46:33Z

Description

Part of #187

This PR adds the uninstall subcommands to demo and stack commands. It implements the MVP features mentioned here #187 (comment)

The flag of skipping operators and CRD deletion was optional and I wasn't sure about the naming or if needs to be one command or two. Can also be just removed, added it because it was not much additional work.

Tested with all current demos/stacks. Some demos have edge cases in the deletion, will document that in the parent issue.

~~This PR also contains the DEMO/STACK parameter addition for stackabletech/demos#374 (comment)~~ stripped out into #432

Definition of Done Checklist

Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
Please make sure all these things are done and tick the boxes

Author

Changes are OpenShift compatible
CRD changes approved
Helm chart can be installed and deployed operator works
Integration tests passed (for non trivial changes)

Reviewer

Code contains useful comments
(Integration-)Test cases added
Documentation added or updated
Changelog updated
Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

Feature Tracker has been updated
Proper release label has been added

NickLarsenNZ · 2026-03-25T08:36:04Z

docs/modules/stackablectl/pages/commands/demo.adoc

+
+[NOTE]
+====
+The uninstall command deletes the namespace the demo was installed in. Therefore it is not possible to uninstall demos in the `default` namespace.


I feel like this should be opt-in behaviour. Or rather, if we created the namespace, it gets deleted.

There could be existing namespaces that shouldn't be deleted.

One such example is if we have the monitoring stack installed, and want to add and remove demos.

According to #187 (comment) "Update after a discussion on 2025-11-26:" it was decided the first version provides no option to be less destructive. Not sure which meeting that was or if it makes sense to change the requirements now.

There were also problems with deleting everything without deleting the namespace, see stackabletech/hdfs-operator#761 (comment). So maybe for the first version / MVP it is fine for now like this? But feel free to disagree

I left an idea below in how we could do it: #429 (comment)

But of course, if this implements what we agreed on, that's fine.
It does feel a little dangerous though.

Yes, I agree it feels dangerous, tried to avoid it at first but ran into the problems above, which unfortunately won't be solved by your idea I think. The manifests we would delete with that approach were also deleted with the help of the labels, so would be the same situation, unless I misunderstood something 😞

Another thing might be adding a confirmation dialogue like the one I added to the installation for the namespace. That it asks if the user really wants to uninstall since that would delete the namespace, and cancel the deletion otherwise.

NickLarsenNZ · 2026-03-25T08:37:55Z

docs/modules/stackablectl/pages/commands/stack.adoc

+
+[NOTE]
+====
+The uninstall command deletes the namespace the stack was installed in. Therefore it is not possible to uninstall stacks in the `default` namespace.


Same as my comment about demos.

NickLarsenNZ

Left a couple of comments about namespace deletion.

NickLarsenNZ · 2026-03-25T08:42:19Z

rust/stackable-cockpit/src/platform/demo/spec.rs

+        // Delete demo namespace
+        client
+            .delete_object(
+                &uninstall_parameters.demo_namespace,
+                &ApiResource::from_gvk(&GroupVersionKind {
+                    group: "".to_owned(),
+                    version: "v1".to_owned(),
+                    kind: "Namespace".to_owned(),
+                }),
+                None,
+            )
+            .await
+            .context(DeleteObjectSnafu)?;


Here's where I think we should have a check to see if the namespace was "owned" by the demo (via labels/annotations).

One thing we could consider is reading in the demo, get the GVKs and Helm charts, and uninstall in reverse order before uninstalling the namespace (if it was created by the demo).

Same would apply for stacks.

That could be a follow-up PR, but I think at least we should be careful with namespace deletion.

NickLarsenNZ · 2026-03-25T08:45:58Z

rust/stackable-cockpit/src/platform/stack/spec.rs

+        // Delete stack namespace
+        client
+            .delete_object(
+                &uninstall_parameters.stack_namespace,
+                &ApiResource::from_gvk(&GroupVersionKind {
+                    group: "".to_owned(),
+                    version: "v1".to_owned(),
+                    kind: "Namespace".to_owned(),
+                }),
+                None,
+            )
+            .await
+            .context(DeleteObjectSnafu)?;


Techassi

Left a few comments and questions.

Techassi · 2026-03-25T08:37:35Z

rust/stackable-cockpit/src/platform/demo/spec.rs

+        transfer_client: &xfer::Client,
+        stack: StackSpec,
+    ) -> Result<(), Error> {
+        // Uninstall Helm Charts


note: I think this comments seems misplaced and needs to move a few lines lower.

Techassi · 2026-03-25T08:39:56Z

rust/stackable-cockpit/src/platform/demo/spec.rs

+        // Delete demo namespace
+        client
+            .delete_object(
+                &uninstall_parameters.demo_namespace,
+                &ApiResource::from_gvk(&GroupVersionKind {
+                    group: "".to_owned(),
+                    version: "v1".to_owned(),
+                    kind: "Namespace".to_owned(),
+                }),
+                None,
+            )
+            .await
+            .context(DeleteObjectSnafu)?;


note: I think we should abstract that away in an associated function called delete_namespace similar to what we have to create a namespace. See:

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

Lines 443 to 460 in 32cf847

pub async fn create_namespace(&self, name: String) -> Result<()> {

let namespace_api: Api<Namespace> = Api::all(self.client.clone());

namespace_api

.create(

&PostParams::default(),

&Namespace {

metadata: ObjectMeta {

name: Some(name),

..Default::default()

},

..Default::default()

},

)

.await

.context(KubeClientPatchSnafu)?;

Ok(())

}

Techassi · 2026-03-25T08:42:43Z

rust/stackable-cockpit/src/platform/demo/spec.rs

+                "stackable.tech/demo",
+                &uninstall_parameters.demo_name,


question: Do we want to use a validated Label instead of a free-form string here?

Techassi · 2026-03-25T08:43:54Z

rust/stackable-cockpit/src/platform/stack/spec.rs

 #[cfg_attr(feature = "openapi", derive(ToSchema))]
 pub struct StackSpec {
-    /// A short description of the demo
+    /// A short description of the stack


Hehe, nice catch!

Techassi · 2026-03-25T08:50:15Z

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

-        parameters.insert("STACK".to_owned(), stack_name.into());
-        if let Some(demo_name) = demo_name {
-            parameters.insert("DEMO".to_owned(), demo_name.into());
-        }


question: Just that we are on the same track: These are now added earlier and are part of the passed parameters, right?

question: Out of curiosity, what was the reason you did that? I assume the demo and stack name are needed earlier in the call chain now?

Techassi · 2026-03-25T09:00:09Z

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

+            if let Some(objectlist) = objects {
+                for object in objectlist {
+                    if let Some(value) = object.labels().get(label_key) {
+                        if value.eq(label_value) {
+                            self.delete_object(
+                                &object.metadata.name.unwrap(),
+                                &api_resource,
+                                object.metadata.namespace.as_deref(),
+                            )
+                            .await?;
+                        }
+                    }
+                }
+            }
+        }


note: I think we should try reducing the amount of right-shift here. We might be able to use something like:

let Some(object_list) = objects else { continue; }

Techassi · 2026-03-25T09:03:24Z

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

+        while let Ok(delete_status) = delete_request {
+            match delete_status {
+                // Left side of Either, when deletion is in progress
+                either::Either::Left(_) => {
+                    Span::current()
+                        .pb_set_message(&format!("Deleting {} {}", api_resource.kind, object_name));
+                    // Short sleep to reduce number of requests
+                    sleep(time::Duration::from_millis(600)).await;
+                    // Resend request to get deletion status
+                    delete_request = object_api
+                        .delete(object_name, &DeleteParams::foreground())
+                        .await;
+                }
+                // Right side of Either, when deletion is done
+                either::Either::Right(_) => {
+                    return Ok(());
+                }
+            }
+        }


note: We might want to use kube's is_deleted for this instead: https://docs.rs/kube/latest/kube/runtime/wait/conditions/fn.is_deleted.html

Techassi · 2026-03-25T09:08:28Z

rust/stackablectl/src/cmds/demo.rs

+    let demo_namespace = tracing_indicatif::suspend_tracing_indicatif(
+        || -> Result<String, CmdError> {
+            if args.namespaces.namespace == DEFAULT_NAMESPACE {
+                if Confirm::new()
+                .with_prompt(
+                    format!(
+                        "Demos installed in the '{DEFAULT_NAMESPACE}' namespace cannot be uninstalled with stackablectl. Install the demo in the '{demo_namespace}' namespace instead?",
+                    demo_namespace = args.demo_name.clone())
+                )
+                .default(true)
+                .interact()
+                .context(ConfirmDialogSnafu)? {
+                    // User selected to install in suggested namespace
+                    Ok(args.demo_name.clone())
+                } else {
+                    // User selected to install in default namespace
+                    Ok(args.namespaces.namespace.clone())
+                }
+            } else {
+                Ok(args.namespaces.namespace.clone())
+            }
+        },
+    )?;


note: Well that's a mouthful to read. Can we somehow make this easier to grasp?

Techassi · 2026-03-25T09:11:01Z

rust/stackablectl/src/cmds/demo.rs

+                if Confirm::new()
+                .with_prompt(
+                    format!(
+                        "Demos installed in the '{DEFAULT_NAMESPACE}' namespace cannot be uninstalled with stackablectl. Install the demo in the '{demo_namespace}' namespace instead?",


note: Use the debug impl for String instead, which already includes quotes:

Suggested change

"Demos installed in the '{DEFAULT_NAMESPACE}' namespace cannot be uninstalled with stackablectl. Install the demo in the '{demo_namespace}' namespace instead?",

"Demos installed in the {DEFAULT_NAMESPACE:?} namespace cannot be uninstalled with stackablectl. Install the demo in the {demo_namespace:?} namespace instead?",

Techassi · 2026-03-25T09:36:30Z

rust/stackablectl/src/cmds/demo.rs

+        .context(BuildListSnafu)?;
+
+    // Get the stack spec based on the name defined in the demo spec
+    let stack = stack_list.get(&demo.stack).context(NoSuchStackSnafu {


note: already extracting the stack here (and then having to clone it via to_owned below) is a completely different approach compared to installing a demo. Why don't we use the same approach here as well?

feat: Add uninstall demo/stack feature

c9aec77

xeniape self-assigned this Mar 12, 2026

xeniape added 2 commits March 12, 2026 14:55

Merge branch 'main' into feat/add-demo-stack-deletion

d8476f3

changelog, some fixes after merge, cargo clippy

5b366b4

This was referenced Mar 12, 2026

fix: Enable Demo/Stack installations in non-default namespaces stackabletech/demos#374

Open

Implement option to uninstall Stacks and Demos #187

Open

xeniape added 3 commits March 12, 2026 15:33

pre-commit fixes

d6dc693

pre-commit fix

f80624a

regenerate-nix

1066138

xeniape added this to Stackable Engineering Mar 12, 2026

xeniape moved this to Development: Waiting for Review in Stackable Engineering Mar 12, 2026

sbernauer mentioned this pull request Mar 17, 2026

feat: Add STACK and DEMO templating parameters #432

Merged

11 tasks

xeniape added 3 commits March 23, 2026 15:31

Merge branch 'main' into feat/add-demo-stack-deletion

73515e2

adjustments after main merge

0e0658f

further uninstall adjustments

9f9b923

Techassi self-requested a review March 23, 2026 15:32

xeniape added 3 commits March 23, 2026 19:30

small word fix

7da2228

cargo update

70fd483

add documentation

c187cff

NickLarsenNZ moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Mar 25, 2026

NickLarsenNZ reviewed Mar 25, 2026

View reviewed changes

Techassi reviewed Mar 25, 2026

View reviewed changes

	pub async fn create_namespace(&self, name: String) -> Result<()> {
	let namespace_api: Api<Namespace> = Api::all(self.client.clone());
	namespace_api
	.create(
	&PostParams::default(),
	&Namespace {
	metadata: ObjectMeta {
	name: Some(name),
	..Default::default()
	},
	..Default::default()
	},
	)
	.await
	.context(KubeClientPatchSnafu)?;

	Ok(())
	}

	"Demos installed in the '{DEFAULT_NAMESPACE}' namespace cannot be uninstalled with stackablectl. Install the demo in the '{demo_namespace}' namespace instead?",
	"Demos installed in the {DEFAULT_NAMESPACE:?} namespace cannot be uninstalled with stackablectl. Install the demo in the {demo_namespace:?} namespace instead?",

Uh oh!

Conversation

xeniape commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Definition of Done Checklist

Author

Reviewer

Acceptance

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xeniape Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NickLarsenNZ left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NickLarsenNZ Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Techassi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xeniape commented Mar 12, 2026 •

edited

Loading

xeniape Mar 25, 2026 •

edited

Loading

NickLarsenNZ Mar 25, 2026 •

edited

Loading