fix: delete plugin namespace after removing it by nmn3m · Pull Request #39 · mrgb7/playground

nmn3m · 2025-05-29T21:18:56Z

Fix

Added cleanup logic to ensure the namespace created by a plugin is properly removed upon its uninstallation.

Impact

- Prevents accumulation of unused namespaces.
- Keeps the playground environment clean and consistent.
- No changes to plugin behavior or isolation guarantees.

mrgb7 · 2025-05-30T19:14:34Z

 	},
 }

+func deleteNamespace(kubeConfigData, namespace string) error {


Include the namespace deletion as part of the plugin installation process. This helps maintain abstraction by keeping operational commands decoupled from the core logic, ensuring clearer separation of concerns.
e.g. in ArgoCD installer or helm installer you can set cleanup of the namespace.

Take a look into architecture docs here

I got it,
but do you mean plugin uninstallation process not installation process?

what i will do.

Move namespace cleanup logic from the command layer (remove.go) to the installer layer.

Create a shared cleanup Namespace utility function in the installer package to handle namespace deletion.

Add namespace cleanup as part of the UnInstall method in both Helm and ArgoCD installers.

Yep uninstall. When I said installation process I meant the layer sorry for confusion. Let me know if you need any help in architecture

@nmn3m First, investigate why the uninstall function in the Helm and ArgoCD installers does not clean up the namespace. Look into whether there are flags or options available in Helm and ArgoCD to ensure proper cleanup. Keep in mind that not all plugins are responsible for creating namespaces some are intended solely to update or override the configuration of other plugins.

Okay, Got it.

🧹 Uninstall Behavior & Namespace Ownership

Findings

Helm

helm uninstall only removes release-managed resources.

--namespace is used for lookup; does not trigger deletion.

No built-in support for namespace cleanup.

ArgoCD

Application deletion removes only managed resources.

Destination namespace is specified but not managed.

Issues

No tracking or handling of namespace ownership.

Plugin types vary:

Some create and own namespaces (e.g., ArgoCD, Cert-Manager).

Some reuse existing namespaces (e.g., Ingress controllers, TLS).

Some modify other namespaces.

Changes

Introduced a mechanism to identify if a plugin owns its namespace.

Namespace deletion is now conditional based on plugin type.

Clear separation between resource cleanup and namespace management logic.

🛠️ Code Improvements

Applied interface segregation to break import cycles instead of creating a new package.

🐛 Bug Fix: ArgoCD Uninstall Error

Fixed HTTP 415 - Invalid content type during ArgoCD app deletion.

Solution: Set proper header in argo.go:
req.Header.Set("Content-Type", "application/json")

mrgb7 · 2025-05-30T22:41:44Z

@@ -0,0 +1,57 @@
+package installer


The installer package includes the installers which implement installer interface, however here you added a utility. so it should be in different package

I got your point,
Do you have a problem if i add this utility inside k8s package as i see, i will fit there with client.go

Yes you can define a new function in the abstracted K8s client called DeleteNamespace

mrgb7 · 2025-05-30T22:44:09Z

+		return fmt.Errorf("failed to delete namespace %s: %w", namespace, err)
+	}
+
+	for {


There is no infinite stop here it will re-try for ever which is consuming the resources. also keep into account race condition between installer deleting the resource in background and deleting the namespace itself

mrgb7 · 2025-05-30T22:50:54Z

 }

+// Default implementation - plugins can override if needed
+func (b *BasePlugin) OwnsNamespace() bool {


You do not need this as the plugin has a string namespace field so you check if it is empty

Plugin Namespace Lifecycle Design

This design provides a clean and flexible way to handle plugin lifecycles:

Plugins that own their namespace (e.g., ArgoCD, Nginx, LoadBalancer)
→ Can safely delete the entire namespace during uninstallation.

Plugins that share a namespace (e.g., TLS sharing the cert-manager namespace)
→ Will leave the namespace intact during uninstallation to avoid disrupting other components.

Separation of concerns between:

GetNamespace() — Defines where the plugin runs.

OwnsNamespace() — Defines whether the plugin has exclusive control over that namespace.

This distinction is especially critical for shared environments, where deleting a shared namespace could unintentionally break other plugins or system components relying on it.

Namespaces are not shared by design each plugin either defines its own namespace or operates without one. keep it simple

mrgb7 · 2025-05-31T11:14:54Z

 		Version:         version,
 		KubeConfig:      kubeConfig,
 		RepoName:        plugin.GetRepoName(),
+		Plugin:          plugin,


I think this field never used ?

mrgb7 · 2025-05-31T11:18:16Z

@@ -1,5 +1,15 @@
 package installer

+type PluginInterface interface {


Could you explain more why we need this interface inside the installer interface under installer package

I forget to remove it, I was using it to solve Applied interface segregation to break import cycles instead of creating a new package

mrgb7

Thanks for your contribution 🚀

fix: delete plugin namespace after removing it

97c9e95

mrgb7 reviewed May 30, 2025

View reviewed changes

fix: delete plugin namespace after removing it

6b64f75

mrgb7 reviewed May 30, 2025

View reviewed changes

nmn3m added 3 commits May 31, 2025 13:14

fix: delete plugin namespace after removing it

951433f

fix: delete plugin namespace after removing it

f0f7dc5

fix: delete plugin namespace after removing it

88879f5

mrgb7 reviewed May 31, 2025

View reviewed changes

nmn3m added 3 commits May 31, 2025 14:44

fix: delete plugin namespace after removing it

6fc77be

fix: delete plugin namespace after removing it

1cf8bad

fix: delete plugin namespace after removing it

19c5466

mrgb7 approved these changes May 31, 2025

View reviewed changes

mrgb7 merged commit 6dbd278 into mrgb7:main May 31, 2025

		@@ -1,5 +1,15 @@
		package installer

		type PluginInterface interface {

Conversation

nmn3m commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix

Impact

Uh oh!

mrgb7 May 30, 2025 • 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!

nmn3m May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

what i will do.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrgb7 May 30, 2025 • 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

🧹 Uninstall Behavior & Namespace Ownership

Findings

Issues

Changes

🛠️ Code Improvements

🐛 Bug Fix: ArgoCD Uninstall Error

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

Plugin Namespace Lifecycle Design

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!

mrgb7 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nmn3m commented May 29, 2025 •

edited

Loading

mrgb7 May 30, 2025 •

edited

Loading

nmn3m May 30, 2025 •

edited

Loading

mrgb7 May 30, 2025 •

edited

Loading