feat(clusters): delete cluster if secret is deleted (#2060) by jellonek · Pull Request #2142 · cloudoperators/greenhouse

jellonek · 2026-07-21T14:42:44Z

Description

This PR adds the feature requested in #2060 - cluster resource deletion on secret deletion.

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

Related Tickets & Documents

Closes #2060

Added tests?

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

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

Implements automatic Cluster cleanup when the backing Cluster Secret is deleted by extending the cluster bootstrap controller’s Secret deletion handling, and adds test/docs coverage for the offboarding path.

Changes:

Trigger Cluster deletion when the controller receives a reconcile request for a missing kubeconfig/OIDC Secret.
Add integration/e2e test scenarios covering “secret deleted ⇒ cluster deleted”.
Document Secret deletion as an offboarding option.

Reviewed changes

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

Show a summary per file

File	Description
internal/controller/cluster/bootstrap_controller.go	Deletes the Cluster when its source Secret is missing (e.g., after Secret deletion).
internal/controller/cluster/bootstrap_controller_test.go	Adds an envtest case verifying Cluster deletion after Secret deletion.
internal/test/util.go	Adds a test helper to reliably delete Secrets in tests.
e2e/cluster/e2e_test.go	Adds an e2e scenario validating Cluster deletion after Secret deletion.
docs/user-guides/cluster/offboarding.md	Documents Secret deletion as an alternative offboarding trigger.

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

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

docs/user-guides/cluster/offboarding.md:40

Minor grammar issue: “..., what can be done ...” should be “..., which can be done ...” (or rephrase) to keep the sentence correct and clear.

Another option to trigger `Cluster` off-boarding is manually deleting the cluster `Secret`, what can be done by calling the command:

 	if err := r.Get(ctx, req.NamespacedName, kubeConfigSecret); err != nil {
+		if client.IgnoreNotFound(err) == nil {
+			// missing secret - initiate cluster deletion if it already exists
+			return ctrl.Result{}, r.requestClusterDeletion(ctx, req.NamespacedName)
+		}


jellonek · 2026-07-22T11:58:06Z

On hold, as removal of secret is breaking the logic in RemoteClusterReconciler.EnsureDeleted.

Closes #2060 Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

jellonek · 2026-07-24T08:35:20Z

Ready for review.

abhijith-darshan · 2026-07-24T11:43:01Z

 	if err := r.Get(ctx, req.NamespacedName, kubeConfigSecret); err != nil {
+		if client.IgnoreNotFound(err) == nil {
+			// missing secret - initiate cluster deletion if it already exists
+			return ctrl.Result{}, r.requestClusterDeletion(ctx, req.NamespacedName)


This won’t work. If secret is missing we cannot clean up helm releases. During bootstrap process please add a finalizer to secret so you can initiate cluster deletion, which in turn will initiate plugins deletion.

mikolajkucinski · 2026-07-24T11:59:07Z

+		if client.IgnoreNotFound(err) != nil {
+			return ctrl.Result{}, lifecycle.Failed, err
+		}
+		return ctrl.Result{}, lifecycle.Success, nil


Hmm doesn't lifecycle.Success early-return skips deleteClusterMetrics(cluster) which every other Success path in EnsureDeleted calls?

Yes, it's skipping it, as there is no secret with configuration data needed to make the connection, which is needed to perform that cleanup.
I have added this early return as a best effort to unblock the deletion which without that would be hang in "terminating" state.

Anyway - instead of trying to unblock deletion there - i'll be going into "finalizer on a secret" path suggested by Abhi.

mikolajkucinski · 2026-07-24T11:59:45Z

 		}).Should(Succeed())
 	})
-
-	It("should reconcile the status of a cluster without a secret", func() {


Aren't we loosing coverage here?

We are loosing the coverage for the thing, which is removed by this PR. This whole task is to remove cluster which does not have already "access secret" - without which cluster can not be accessed, so if the cluster has to be removed, there is no way to reconcile its status.

Copilot AI review requested due to automatic review settings July 21, 2026 14:42

jellonek requested review from a team as code owners July 21, 2026 14:42

github-actions Bot added size/M documentation Improvements or additions to documentation labels Jul 21, 2026

Copilot started reviewing on behalf of jellonek July 21, 2026 14:43 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread internal/controller/cluster/bootstrap_controller.go

Comment thread internal/controller/cluster/bootstrap_controller_test.go Outdated

Comment thread e2e/cluster/e2e_test.go

Comment thread e2e/cluster/e2e_test.go

Comment thread docs/user-guides/cluster/offboarding.md Outdated

jellonek requested a review from Copilot July 21, 2026 15:08

Copilot started reviewing on behalf of jellonek July 21, 2026 15:09 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

jellonek force-pushed the delete-cluster-on-gone-secret branch 2 times, most recently from 95a3d96 to d8fd2d5 Compare July 21, 2026 15:28

github-actions Bot added size/L and removed size/M labels Jul 22, 2026

jellonek force-pushed the delete-cluster-on-gone-secret branch from cddfab9 to ad3ce10 Compare July 23, 2026 07:42

jellonek added 5 commits July 24, 2026 10:34

feat(clusters): delete cluster if secret is deleted (#2060)

36235fa

Closes #2060 Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

docs(clusters): update info about secret removal

e250673

Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

fix(chore): address comments from review

1087cde

Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

test(clusters): remove obsolete test

9613514

Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

fix(clusters): best effort when secret is missing

426ed5d

Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

jellonek force-pushed the delete-cluster-on-gone-secret branch from 2516b2a to 426ed5d Compare July 24, 2026 08:34

abhijith-darshan reviewed Jul 24, 2026

View reviewed changes

mikolajkucinski reviewed Jul 24, 2026

View reviewed changes

Uh oh!

Conversation

jellonek commented Jul 21, 2026

Description

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

Related Tickets & Documents

Added tests?

Added to documentation?

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

jellonek commented Jul 22, 2026

Uh oh!

jellonek commented Jul 24, 2026

Uh oh!

abhijith-darshan Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

jellonek Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mikolajkucinski Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

jellonek Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

jellonek Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mikolajkucinski Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

jellonek Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants