feat(plugins): plugin controller deletion lifecycle

api/v1alpha1/plugin_types.go

@@ -158,6 +158,9 @@ const (
 	// HelmUninstallFailedReason is set when the Helm release could not be uninstalled.
 	HelmUninstallFailedReason greenhousemetav1alpha1.ConditionReason = "HelmUninstallFailed"
 
+	// HelmReleaseUninstallPendingReason is set while waiting for Flux to complete the Helm uninstall.
+	HelmReleaseUninstallPendingReason greenhousemetav1alpha1.ConditionReason = "HelmReleaseUninstallPending"
+
 	// OptionValueResolutionFailedReason is set when option values could not be resolved
 	OptionValueResolutionFailedReason greenhousemetav1alpha1.ConditionReason = "OptionValueResolutionFailed"
 

e2e/plugin/e2e_test.go

@@ -104,6 +104,10 @@ var _ = Describe("Plugin E2E", Ordered, func() {
 		scenarios.FluxControllerPluginDeletePolicyRetain(ctx, adminClient, env, remoteClusterName, team.Name)
 	})
 
+	It("should surface HelmRelease uninstall failure as a Plugin condition and retain the finalizer until resolved", func() {
+		scenarios.FluxControllerPluginDeletionLifecycle(ctx, adminClient, env, remoteClusterName, team.Name)
+	})
+
 	It("should resolve option values from direct plugin reference", func() {
 		By("setting up cluster role binding for OIDC on remote cluster")
 		expect.SetupOIDCClusterRoleBinding(ctx, remoteClient, remoteOIDCClusterRoleBindingName, remoteIntegrationCluster, env.TestNamespace)

e2e/plugin/scenarios/flux_controller.go

@@ -603,3 +603,146 @@ func FluxControllerPluginDeletePolicyRetain(ctx context.Context, adminClient cli
 		g.Expect(err).NotTo(HaveOccurred(), "should be able to uninstall the helm release for the plugin")
 	}).Should(Succeed(), "the retained Helm release should eventually be uninstalled from the remote cluster")
 }
+
+// FluxControllerPluginDeletionLifecycle verifies that the Plugin controller holds its finalizer
+// and surfaces the HelmRelease uninstall failure as a status condition when Flux reports an error.
+func FluxControllerPluginDeletionLifecycle(ctx context.Context, adminClient client.Client, env *shared.TestEnv, remoteClusterName, teamName string) {
+	const testFinalizer = "greenhouse.sap/e2e-test"
+
+	By("Creating plugin definition")
+	testPluginDefinition := fixtures.PreparePodInfoClusterPluginDefinition(env.TestNamespace, "6.9.0")
+	err := adminClient.Create(ctx, testPluginDefinition)
+	Expect(client.IgnoreAlreadyExists(err)).ToNot(HaveOccurred())
+
+	By("Checking the test plugin definition is ready")
+	Eventually(func(g Gomega) {
+		err = adminClient.Get(ctx, client.ObjectKeyFromObject(testPluginDefinition), testPluginDefinition)
+		g.Expect(err).NotTo(HaveOccurred())
+		g.Expect(testPluginDefinition.Status.IsReadyTrue()).To(BeTrue(), "the plugin definition should be ready")
+	}).Should(Succeed())
+
+	By("Adding labels to remote cluster")
+	remoteCluster := &greenhousev1alpha1.Cluster{}
+	err = adminClient.Get(ctx, client.ObjectKey{Name: remoteClusterName, Namespace: env.TestNamespace}, remoteCluster)
+	Expect(err).ToNot(HaveOccurred())
+	remoteCluster.Labels = map[string]string{"app": "test-deletion-lifecycle-cluster"}
+	err = adminClient.Update(ctx, remoteCluster)
+	Expect(err).ToNot(HaveOccurred())
+
+	By("Creating PluginPreset")
+	pluginPreset := test.NewPluginPreset("test-deletion-lifecycle-preset", env.TestNamespace,
+		test.WithPluginPresetLabel(greenhouseapis.LabelKeyOwnedBy, teamName),
+		test.WithPluginPresetPluginSpec(fixtures.PreparePlugin("test-deletion-lifecycle-preset", env.TestNamespace,
+			test.WithClusterPluginDefinition(testPluginDefinition.Name),
+			test.WithReleaseName("test-deletion-lifecycle"),
+			test.WithReleaseNamespace(env.TestNamespace),
+		).Spec),
+		test.WithPluginPresetClusterSelector(metav1.LabelSelector{MatchLabels: map[string]string{"app": "test-deletion-lifecycle-cluster"}}),
+	)
+	err = adminClient.Create(ctx, pluginPreset)
+	Expect(client.IgnoreAlreadyExists(err)).ToNot(HaveOccurred())
+
+	By("Waiting for the Plugin to be created and deployed")
+	plugin := &greenhousev1alpha1.Plugin{}
+	pluginKey := types.NamespacedName{Name: pluginPreset.Name + "-" + remoteClusterName, Namespace: env.TestNamespace}
+	Eventually(func(g Gomega) {
+		err = adminClient.Get(ctx, pluginKey, plugin)
+		g.Expect(err).ToNot(HaveOccurred())
+		g.Expect(plugin.Status.IsReadyTrue()).To(BeTrue(), "the plugin should be ready")
+	}).Should(Succeed(), "the plugin should eventually be created and ready")
+
+	helmRelease := &helmv2.HelmRelease{}
+	helmReleaseKey := types.NamespacedName{Name: pluginPreset.Name + "-" + remoteClusterName, Namespace: env.TestNamespace}
+
+	By("Adding a test finalizer to the HelmRelease to hold it during deletion")
+	Eventually(func(g Gomega) {
+		err = adminClient.Get(ctx, helmReleaseKey, helmRelease)
+		g.Expect(err).ToNot(HaveOccurred())
+		patch := helmRelease.DeepCopy()
+		patch.Finalizers = append(patch.Finalizers, testFinalizer)
+		g.Expect(adminClient.Patch(ctx, patch, client.MergeFrom(helmRelease))).To(Succeed())
+	}).Should(Succeed(), "should be able to add test finalizer to HelmRelease")
+
+	By("Deleting the PluginPreset to trigger Plugin deletion")
+	test.MustRemoveAnnotation(ctx, adminClient, pluginPreset, greenhousev1alpha1.PreventDeletionAnnotation)
+	Expect(adminClient.Delete(ctx, pluginPreset)).To(Succeed())
+
+	By("Waiting for the Plugin to enter deletion phase with uninstall-pending condition")
+	Eventually(func(g Gomega) {
+		err = adminClient.Get(ctx, pluginKey, plugin)
+		g.Expect(err).ToNot(HaveOccurred(), "Plugin must still exist while HelmRelease is held")
+		g.Expect(plugin.GetDeletionTimestamp()).ToNot(BeNil(), "Plugin must be marked for deletion")
+		cond := plugin.Status.GetConditionByType(greenhousev1alpha1.HelmReleaseDeployedCondition)
+		g.Expect(cond).ToNot(BeNil())
+		g.Expect(string(cond.Reason)).To(Equal(string(greenhousev1alpha1.HelmReleaseUninstallPendingReason)),
+			"condition should show uninstall is pending")
+	}).Should(Succeed(), "Plugin should enter deletion-pending state while HelmRelease is held")
+
+	By("Verifying the Plugin finalizer is retained while uninstall is pending")
+	err = adminClient.Get(ctx, pluginKey, plugin)
+	Expect(err).ToNot(HaveOccurred())
+	Expect(plugin.Finalizers).To(ContainElement("greenhouse.sap/cleanup"), "finalizer must be retained while HelmRelease exists")
+
+	By("Injecting an UninstallFailed condition on the HelmRelease to simulate a Flux failure")
+	Eventually(func(g Gomega) {
+		err = adminClient.Get(ctx, helmReleaseKey, helmRelease)
+		g.Expect(err).ToNot(HaveOccurred())
+		statusPatch := helmRelease.DeepCopy()
+		meta.SetStatusCondition(&statusPatch.Status.Conditions, metav1.Condition{
+			Type:               helmv2.ReleasedCondition,
+			Status:             metav1.ConditionFalse,
+			Reason:             helmv2.UninstallFailedReason,
+			Message:            "simulated uninstall failure for e2e test",
+			LastTransitionTime: metav1.Now(),
+		})
+		g.Expect(adminClient.Status().Patch(ctx, statusPatch, client.MergeFrom(helmRelease))).To(Succeed())
+	}).Should(Succeed(), "should be able to inject the uninstall failure condition")
+
+	By("Verifying the Plugin surfaces the uninstall failure and retains its finalizer")
+	Eventually(func(g Gomega) {
+		err = adminClient.Get(ctx, pluginKey, plugin)
+		g.Expect(err).ToNot(HaveOccurred(), "Plugin must still exist after uninstall failure")
+		g.Expect(plugin.GetDeletionTimestamp()).ToNot(BeNil(), "Plugin must remain in deletion phase")
+		g.Expect(plugin.Finalizers).To(ContainElement("greenhouse.sap/cleanup"), "finalizer must be retained after uninstall failure")
+		cond := plugin.Status.GetConditionByType(greenhousev1alpha1.HelmReleaseDeployedCondition)
+		g.Expect(cond).ToNot(BeNil())
+		g.Expect(string(cond.Reason)).To(Equal(string(greenhousev1alpha1.HelmUninstallFailedReason)),
+			"HelmReleaseDeployed condition should reflect the uninstall failure")
+	}).Should(Succeed(), "Plugin should surface the uninstall failure")
+
+	By("Removing the test finalizer to allow the HelmRelease and Plugin to be fully deleted")
+	Eventually(func(g Gomega) {
+		err = adminClient.Get(ctx, helmReleaseKey, helmRelease)
+		if apierrors.IsNotFound(err) {
+			return
+		}
+		g.Expect(err).ToNot(HaveOccurred())
+		patch := helmRelease.DeepCopy()
+		newFinalizers := make([]string, 0, len(patch.Finalizers))
+		for _, f := range patch.Finalizers {
+			if f != testFinalizer {
+				newFinalizers = append(newFinalizers, f)
+			}
+		}
+		patch.Finalizers = newFinalizers
+		g.Expect(adminClient.Patch(ctx, patch, client.MergeFrom(helmRelease))).To(Succeed())
+	}).Should(Succeed(), "should be able to remove the test finalizer from HelmRelease")
+
+	By("Verifying the Plugin is fully deleted once the HelmRelease is removed")
+	Eventually(func(g Gomega) {
+		err = adminClient.Get(ctx, pluginKey, plugin)
+		g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "Plugin should be fully deleted after HelmRelease is gone")
+	}).Should(Succeed(), "Plugin should eventually be fully garbage-collected")
+
+	By("Verifying the HelmRelease is fully deleted")
+	Eventually(func(g Gomega) {
+		err = adminClient.Get(ctx, helmReleaseKey, helmRelease)
+		g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "HelmRelease should be fully deleted")
+	}).Should(Succeed(), "HelmRelease should eventually be fully garbage-collected")
+
+	By("Verifying the PluginPreset is fully deleted once all Plugins are gone")
+	Eventually(func(g Gomega) {
+		err = adminClient.Get(ctx, client.ObjectKeyFromObject(pluginPreset), pluginPreset)
+		g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "PluginPreset should be fully deleted once all its Plugins are gone")
+	}).Should(Succeed(), "PluginPreset should eventually be fully garbage-collected")
+}

internal/controller/plugin/plugin_controller_flux.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"time"
 
 	fluxstatus "github.com/fluxcd/cli-utils/pkg/kstatus/status"
 	helmv2 "github.com/fluxcd/helm-controller/api/v2"
@@ -55,8 +56,32 @@ func (r *PluginReconciler) EnsureFluxDeleted(ctx context.Context, plugin *greenh
 		return ctrl.Result{}, lifecycle.Failed, err
 	}
 
-	plugin.SetCondition(greenhousemetav1alpha1.FalseCondition(greenhousev1alpha1.HelmReleaseDeployedCondition, greenhousev1alpha1.HelmReleaseUninstalledReason, ""))
-	return ctrl.Result{}, lifecycle.Success, nil
+	// Observe the HelmRelease deletion: hold the finalizer until Flux completes the uninstall.
+	hr := &helmv2.HelmRelease{}
+	err := r.Get(ctx, types.NamespacedName{Name: plugin.Name, Namespace: plugin.Namespace}, hr)
+	if apierrors.IsNotFound(err) {
+		// HelmRelease is fully gone — uninstall complete.
+		plugin.SetCondition(greenhousemetav1alpha1.FalseCondition(greenhousev1alpha1.HelmReleaseDeployedCondition, greenhousev1alpha1.HelmReleaseUninstalledReason, ""))
+		return ctrl.Result{}, lifecycle.Success, nil
+	}
+	if err != nil {
+		plugin.SetCondition(greenhousemetav1alpha1.FalseCondition(greenhousev1alpha1.HelmReleaseDeployedCondition, greenhousev1alpha1.HelmUninstallFailedReason, err.Error()))
+		util.UpdatePluginReconcileTotalMetric(plugin, util.MetricResultError, util.MetricReasonClusterAccessFailed)
+		return ctrl.Result{}, lifecycle.Failed, err
+	}
+
+	// HelmRelease still exists; check whether Flux reported an explicit uninstall failure.
+	released := meta.FindStatusCondition(hr.Status.Conditions, helmv2.ReleasedCondition)
+	if released != nil && released.Reason == helmv2.UninstallFailedReason {
+		msg := released.Message
+		plugin.SetCondition(greenhousemetav1alpha1.FalseCondition(greenhousev1alpha1.HelmReleaseDeployedCondition, greenhousev1alpha1.HelmUninstallFailedReason, msg))
+		util.UpdatePluginReconcileTotalMetric(plugin, util.MetricResultError, util.MetricReasonUninstallHelmFailed)
+		return ctrl.Result{}, lifecycle.Failed, fmt.Errorf("helm uninstall failed: %s", msg)
+	}
+
+	// Flux is still running the uninstall; return Pending to keep the finalizer and requeue.
+	plugin.SetCondition(greenhousemetav1alpha1.FalseCondition(greenhousev1alpha1.HelmReleaseDeployedCondition, greenhousev1alpha1.HelmReleaseUninstallPendingReason, "waiting for HelmRelease to be removed"))
+	return ctrl.Result{RequeueAfter: 10 * time.Second}, lifecycle.Pending, nil
 }
 
 func (r *PluginReconciler) EnsureFluxCreated(ctx context.Context, plugin *greenhousev1alpha1.Plugin) (ctrl.Result, lifecycle.ReconcileResult, error) {

internal/controller/plugin/plugin_controller_flux_test.go

@@ -17,6 +17,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 
 	greenhouseapis "github.com/cloudoperators/greenhouse/api"
@@ -361,4 +362,153 @@ var _ = Describe("Flux Plugin Controller", Ordered, func() {
 				"Plugin without ExposedServices should not have plugin-exposed-services label")
 		}).Should(Succeed())
 	})
+
+	It("should hold the finalizer while Flux is uninstalling the HelmRelease", func() {
+		deletionPlugin := test.NewPlugin(test.Ctx, "test-deletion-lifecycle", test.TestNamespace,
+			test.WithCluster("test-flux-cluster"),
+			test.WithClusterPluginDefinition("test-flux-plugindefinition"),
+			test.WithReleaseName("release-deletion-lifecycle"),
+			test.WithReleaseNamespace(test.TestNamespace),
+			test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testPluginTeam.Name),
+		)
+
+		By("creating the Plugin")
+		Expect(test.K8sClient.Create(test.Ctx, deletionPlugin)).To(Succeed())
+
+		By("waiting for the HelmRelease to be created")
+		helmRelease := &helmv2.HelmRelease{}
+		releaseKey := types.NamespacedName{Name: deletionPlugin.Name, Namespace: deletionPlugin.Namespace}
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, releaseKey, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred(), "HelmRelease should be created")
+		}).Should(Succeed())
+
+		By("adding the Flux finalizer to simulate a real Flux-managed HelmRelease")
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, releaseKey, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred())
+			controllerutil.AddFinalizer(helmRelease, helmv2.HelmReleaseFinalizer)
+			err = test.K8sClient.Update(test.Ctx, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred())
+		}).Should(Succeed())
+
+		By("deleting the Plugin")
+		Expect(test.K8sClient.Delete(test.Ctx, deletionPlugin)).To(Succeed())
+
+		By("verifying the Plugin has the deletion-pending condition while HelmRelease exists")
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, client.ObjectKeyFromObject(deletionPlugin), deletionPlugin)
+			g.Expect(err).ToNot(HaveOccurred(), "Plugin should still exist with finalizer")
+			cond := deletionPlugin.Status.GetConditionByType(greenhousev1alpha1.HelmReleaseDeployedCondition)
+			g.Expect(cond).ToNot(BeNil())
+			g.Expect(cond.Reason).To(Equal(greenhousev1alpha1.HelmReleaseUninstallPendingReason),
+				"HelmReleaseDeployed condition should show uninstall is pending")
+		}).Should(Succeed())
+
+		By("verifying the Plugin finalizer is still present")
+		err := test.K8sClient.Get(test.Ctx, client.ObjectKeyFromObject(deletionPlugin), deletionPlugin)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(deletionPlugin.Finalizers).To(ContainElement("greenhouse.sap/cleanup"), "finalizer must be retained while HelmRelease exists")
+
+		By("simulating Flux completing the uninstall by removing the HelmRelease finalizer and deleting it")
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, releaseKey, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred())
+			helmRelease.Finalizers = nil
+			err = test.K8sClient.Update(test.Ctx, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred())
+		}).Should(Succeed())
+		Expect(test.K8sClient.Delete(test.Ctx, helmRelease)).To(Or(Succeed(), MatchError(ContainSubstring("not found"))))
+
+		By("verifying the Plugin is eventually garbage-collected after the HelmRelease is gone")
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, client.ObjectKeyFromObject(deletionPlugin), deletionPlugin)
+			g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "Plugin should be deleted once HelmRelease is gone")
+		}).Should(Succeed())
+	})
+
+	It("should surface HelmRelease uninstall failure as a status condition", func() {
+		failPlugin := test.NewPlugin(test.Ctx, "test-uninstall-failure", test.TestNamespace,
+			test.WithCluster("test-flux-cluster"),
+			test.WithClusterPluginDefinition("test-flux-plugindefinition"),
+			test.WithReleaseName("release-uninstall-failure"),
+			test.WithReleaseNamespace(test.TestNamespace),
+			test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testPluginTeam.Name),
+		)
+
+		By("creating the Plugin")
+		Expect(test.K8sClient.Create(test.Ctx, failPlugin)).To(Succeed())
+
+		By("waiting for the HelmRelease to be created")
+		helmRelease := &helmv2.HelmRelease{}
+		releaseKey := types.NamespacedName{Name: failPlugin.Name, Namespace: failPlugin.Namespace}
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, releaseKey, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred(), "HelmRelease should be created")
+		}).Should(Succeed())
+
+		By("adding the Flux finalizer to simulate a real Flux-managed HelmRelease")
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, releaseKey, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred())
+			controllerutil.AddFinalizer(helmRelease, helmv2.HelmReleaseFinalizer)
+			err = test.K8sClient.Update(test.Ctx, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred())
+		}).Should(Succeed())
+
+		By("deleting the Plugin")
+		Expect(test.K8sClient.Delete(test.Ctx, failPlugin)).To(Succeed())
+
+		By("waiting for the Plugin to enter deletion-pending state")
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, client.ObjectKeyFromObject(failPlugin), failPlugin)
+			g.Expect(err).ToNot(HaveOccurred())
+			cond := failPlugin.Status.GetConditionByType(greenhousev1alpha1.HelmReleaseDeployedCondition)
+			g.Expect(cond).ToNot(BeNil())
+			g.Expect(cond.Reason).To(Equal(greenhousev1alpha1.HelmReleaseUninstallPendingReason))
+		}).Should(Succeed())
+
+		By("simulating Flux reporting an uninstall failure on the HelmRelease")
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, releaseKey, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred())
+			helmRelease.Status.Conditions = []metav1.Condition{
+				{
+					Type:               helmv2.ReleasedCondition,
+					Status:             metav1.ConditionFalse,
+					Reason:             helmv2.UninstallFailedReason,
+					Message:            "helm uninstall: resource deletion timeout",
+					LastTransitionTime: metav1.Now(),
+				},
+			}
+			err = test.K8sClient.Status().Update(test.Ctx, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred())
+		}).Should(Succeed())
+
+		By("verifying the Plugin surfaces the uninstall failure as a condition")
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, client.ObjectKeyFromObject(failPlugin), failPlugin)
+			g.Expect(err).ToNot(HaveOccurred(), "Plugin should still exist since uninstall failed")
+			cond := failPlugin.Status.GetConditionByType(greenhousev1alpha1.HelmReleaseDeployedCondition)
+			g.Expect(cond).ToNot(BeNil())
+			g.Expect(cond.IsFalse()).To(BeTrue())
+			g.Expect(cond.Reason).To(Equal(greenhousev1alpha1.HelmUninstallFailedReason),
+				"condition reason should reflect uninstall failure")
+		}).Should(Succeed())
+
+		By("verifying the Plugin finalizer is retained after uninstall failure")
+		err := test.K8sClient.Get(test.Ctx, client.ObjectKeyFromObject(failPlugin), failPlugin)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(failPlugin.Finalizers).To(ContainElement("greenhouse.sap/cleanup"), "finalizer must be retained after uninstall failure")
+
+		By("cleaning up: force-remove the HelmRelease so the Plugin can be collected")
+		Eventually(func(g Gomega) {
+			err := test.K8sClient.Get(test.Ctx, releaseKey, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred())
+			helmRelease.Finalizers = nil
+			err = test.K8sClient.Update(test.Ctx, helmRelease)
+			g.Expect(err).ToNot(HaveOccurred())
+		}).Should(Succeed())
+		Expect(test.K8sClient.Delete(test.Ctx, helmRelease)).To(Or(Succeed(), MatchError(ContainSubstring("not found"))))
+	})
 })