feat(plugins): plugin controller deletion lifecycle

[Zaggy21]

Description

Fixes the plugin deletion lifecycle where the greenhouse.sap/cleanup finalizer was removed before Flux had finished running helm uninstall, potentially leaving orphaned Helm resources on the target cluster.

EnsureFluxDeleted now:

• Issues the delete on the HelmRelease and requeues,
• If the HelmRelease is still present, checks Flux's ReleasedCondition: an UninstallFailed reason surfaces as HelmUninstallFailedReason on the Plugin; otherwise sets HelmReleaseUninstallPendingReason and relies on the HelmRelease watch to re-trigger reconciliation,
• Only returns lifecycle.Success (removing the finalizer) once the HelmRelease is gone.

What type of PR is this? (check all applicable)

• 🍕 Feature
• 🐛 Bug Fix
• 📝 Documentation Update
• 🎨 Style
• 🧑‍💻 Code Refactor
• 🔥 Performance Improvements
• ✅ Test
• 🤖 Build
• 🔁 CI
• 📦 Chore (Release)
• ⏩ Revert

Related Tickets & Documents

• Closes [FEAT] - Plugin controller deletion lifecycle #1885

Added tests?

• 👍 yes
• 🙅 no, because they aren't needed
• 🙋 no, because I need help
• Separate ticket for tests # (issue/pr)

Adds unit tests covering the pending and failure paths, and an e2e scenario that injects an uninstall failure via a test finalizer on the HelmRelease and verifies the Plugin surfaces the condition and retains its finalizer until resolved.

Added to documentation?

• 📜 README.md
• 🤝 Documentation pages updated
• 🙅 no documentation needed
• (if applicable) generated OpenAPI docs for CRD changes

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• New and existing unit tests pass locally with my changes

[Copilot]

Pull request overview

This PR improves the Plugin controller’s deletion lifecycle to ensure the greenhouse.sap/cleanup finalizer is not removed until Flux has fully completed the HelmRelease uninstall (and to surface Flux uninstall failures on the Plugin status), preventing orphaned Helm resources.

Changes:

• Update EnsureFluxDeleted to wait for the Flux HelmRelease to be fully removed, requeue while uninstall is pending, and surface UninstallFailed via Plugin conditions.
• Skip Plugin reconciliation when the target Cluster is already being deleted (via DeletionTimestamp) by requeueing and setting a deletion-related condition.
• Add unit + e2e coverage for pending uninstall and uninstall-failure paths during Plugin deletion.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/controller/plugin/util.go	Adds handling to defer reconciliation when the referenced Cluster is already being deleted.
internal/controller/plugin/plugin_controller_flux.go	Holds Plugin finalizer until HelmRelease deletion completes; detects Flux uninstall failure and reports it on Plugin conditions.
internal/controller/plugin/plugin_controller_flux_test.go	Adds unit tests for pending uninstall and uninstall failure condition surfacing.
e2e/plugin/scenarios/flux_controller.go	Adds an e2e scenario that injects HelmRelease uninstall failure and verifies Plugin behavior/finalizer retention.
e2e/plugin/e2e_test.go	Registers the new deletion lifecycle e2e scenario.
api/v1alpha1/plugin_types.go	Introduces HelmReleaseUninstallPendingReason condition reason constant.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

internal/controller/plugin/plugin_controller_flux_test.go:456

• This Eventually block appends the Flux finalizer on every retry without checking if it’s already present. If the closure retries after a successful update, the finalizer can be duplicated. Consider guarding the append with a contains-check (or use an idempotent patch) to keep the test stable.

		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())
			helmRelease.Finalizers = append(helmRelease.Finalizers, helmv2.HelmReleaseFinalizer)
			err = test.K8sClient.Update(test.Ctx, helmRelease)
			g.Expect(err).ToNot(HaveOccurred())
		}).Should(Succeed())

[abhijith-darshan]

Why do we need a dedicated scenario? 😄

Every scenario should satisfy this

[Zaggy21]

It's to cover the HelmRelease uninstall failure scenario as per the acceptance criterion. Can't really add it seamlessly to other scenarios. The basic check for "Plugin stays pending until HelmRelease is deleted" could be added to other scenarios, but it doesn't add much value. It's also covered in unit tests. I'd leave it as it is.

[abhijith-darshan]

We cannot rely on unit tests. There is no work load in there to guarantee clean ups.

There is already failing plugin e2e scenarios... in one of those we can expect HelmRelease to remain which should already be the case.

A dedicated scenario just for this is not needed.

[abhijith-darshan]

So to simplify -

During the clean up of every scenario -

• Expect that the HelmRelease is also gone
• in Existing failing scenarios - Expect both Plugin and HelmRelease remain.

[Zaggy21]

Done. Removed the dedicated scenario and added assertions to FluxControllerPluginDependencies's teardown: after deleting the presets, it verifies both Plugins are marked for deletion before the global plugin is removed and Flux can complete the uninstall.

[abhijith-darshan]

minor improvements