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

docs/user-guides/cluster/offboarding.md

@@ -37,6 +37,11 @@ Off-board a `Cluster` in Greenhouse is initiated by calling the command:
 kubectl --namespace=<greenhouse-organization-name> delete cluster <cluster-name>
 ```
 
+Another option to trigger `Cluster` off-boarding is manually deleting the cluster `Secret`, which can be done by calling the command:
+```shell
+kubectl --namespace=<greenhouse-organization-name> delete secret <cluster-name>
+```
+
 | :exclamation: Deleting the `Cluster` resource automatically uninstalls any deployed plugins as part of the cleanup process. |
 |-----------------------------------------------------------------------------------------------------------------------------|
 

e2e/cluster/e2e_test.go

@@ -135,6 +135,24 @@ var _ = Describe("Cluster E2E", Ordered, func() {
 			isOwner := shared.IsResourceOwnedByOwner(crb, sa)
 			Expect(isOwner).To(BeTrue(), "service account should have an owner reference")
 		})
+
+		It("should remove cluster on secret deletion", func() {
+			By("getting cluster secret")
+			secret := &corev1.Secret{}
+			err := adminClient.Get(ctx, client.ObjectKey{Name: remoteClusterHName, Namespace: env.TestNamespace}, secret)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("removing the secret resource")
+			err = adminClient.Delete(ctx, secret)
+			Expect(err).NotTo(HaveOccurred(), "cluster secret should exist")
+
+			By("checking the cluster resource is eventually deleted")
+			cluster := &greenhousev1alpha1.Cluster{}
+			Eventually(func(g Gomega) {
+				err := adminClient.Get(ctx, client.ObjectKey{Name: remoteClusterHName, Namespace: env.TestNamespace}, cluster)
+				g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "cluster resource should be deleted")
+			}).Should(Succeed(), "cluster resource should be deleted")
+		})
 	})
 
 	// the context executes the tests for Cluster where a secret of type kubeconfig is provided

internal/controller/cluster/bootstrap_controller.go

@@ -63,6 +63,10 @@ func (r *BootstrapReconciler) SetupWithManager(name string, mgr ctrl.Manager) er
 func (r *BootstrapReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	var kubeConfigSecret = new(corev1.Secret)
 	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)
+		}
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 	if kubeConfigSecret.Type == greenhouseapis.SecretTypeOIDCConfig {
@@ -217,6 +221,15 @@ func (r *BootstrapReconciler) getCluster(ctx context.Context, kubeConfigSecret *
 	return cluster, client.IgnoreNotFound(err)
 }
 
+func (r *BootstrapReconciler) requestClusterDeletion(ctx context.Context, namespacedName types.NamespacedName) error {
+	cluster := greenhousev1alpha1.Cluster{}
+	if err := r.Get(ctx, namespacedName, &cluster); err != nil {
+		return client.IgnoreNotFound(err)
+	}
+	log.FromContext(ctx).Info("Cluster secret not found, requesting Cluster deletion", "cluster", namespacedName.String())
+	return client.IgnoreNotFound(r.Delete(ctx, &cluster))
+}
+
 func enqueueSecretForCluster(_ context.Context, o client.Object) []ctrl.Request {
 	cluster, ok := o.(*greenhousev1alpha1.Cluster)
 	if !ok {

internal/controller/cluster/bootstrap_controller_test.go

@@ -8,6 +8,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
@@ -139,5 +140,26 @@ var _ = Describe("Bootstrap controller", Ordered, func() {
 			By("Deleting the kubeconfig secret and checking the cluster is deleted")
 			test.MustDeleteCluster(test.Ctx, test.K8sClient, cluster)
 		})
+		It("Should remove cluster when secret disappears", func() {
+			By("Creating a kubeconfig secret with label")
+			kubeConfigSecret := setup.CreateSecret(test.Ctx, bootstrapTestCase+"-secret-removal",
+				test.WithSecretType(greenhouseapis.SecretTypeKubeConfig),
+				test.WithSecretData(map[string][]byte{greenhouseapis.KubeConfigKey: remoteKubeConfig}),
+			)
+			By("Checking if the cluster is created")
+			cluster := &greenhousev1alpha1.Cluster{}
+			id := types.NamespacedName{Name: kubeConfigSecret.Name, Namespace: setup.Namespace()}
+			Eventually(func(g Gomega) bool {
+				g.Expect(test.K8sClient.Get(test.Ctx, id, cluster)).Should(Succeed(), "the cluster should have been created")
+				return true
+			}).Should(BeTrue(), "getting the cluster should succeed eventually")
+
+			test.MustDeleteSecret(test.Ctx, test.K8sClient, kubeConfigSecret)
+
+			Eventually(func(g Gomega) bool {
+				g.Expect(apierrors.IsNotFound(test.K8sClient.Get(test.Ctx, id, cluster))).Should(BeTrue(), "the cluster should be deleted")
+				return true
+			}).Should(BeTrue(), "the cluster should be deleted eventually")
+		})
 	})
 })

internal/controller/cluster/cluster_controller.go

@@ -294,8 +294,13 @@ func (r *RemoteClusterReconciler) EnsureDeleted(ctx context.Context, resource li
 
 	kubeConfigSecret := &corev1.Secret{}
 	if err := r.Get(ctx, types.NamespacedName{Namespace: cluster.GetNamespace(), Name: cluster.GetSecretName()}, kubeConfigSecret); err != nil {
-		return ctrl.Result{}, lifecycle.Failed, err
+		log.FromContext(ctx).Info("cannot fetch secret", "cluster", cluster.Name, "error", err.Error())
+		if client.IgnoreNotFound(err) != nil {
+			return ctrl.Result{}, lifecycle.Failed, err
+		}
+		return ctrl.Result{}, lifecycle.Success, nil
 	}
+
 	// early return if the cluster connectivity is via OIDC
 	if kubeConfigSecret.Type == greenhouseapis.SecretTypeOIDCConfig {
 		log.FromContext(ctx).Info("no resources to clean up", "secretType", kubeConfigSecret.Type, "cluster", cluster.Name)

internal/controller/cluster/status_test.go

@@ -235,31 +235,4 @@ var _ = Describe("Cluster status", Ordered, func() {
 			g.Expect(validCluster.Status.KubernetesVersion).ToNot(BeNil())
 		}).Should(Succeed())
 	})
-
-	It("should reconcile the status of a cluster without a secret", func() {
-		By("checking cluster conditions")
-		Eventually(func(g Gomega) {
-			g.Expect(test.K8sClient.Get(test.Ctx, types.NamespacedName{Name: invalidCluster.Name, Namespace: setup.Namespace()}, invalidCluster)).ShouldNot(HaveOccurred(), "There should be no error getting the cluster resource")
-			g.Expect(invalidCluster.Status.StatusConditions).ToNot(BeNil())
-			// Accessible condition validation.
-			accessibleCondition := invalidCluster.Status.GetConditionByType(greenhousev1alpha1.PermissionsVerified)
-			g.Expect(accessibleCondition).ToNot(BeNil(), "The Accessible condition should be present")
-			g.Expect(accessibleCondition.Status).To(Equal(metav1.ConditionUnknown), "The Accessible condition should be unknown")
-			g.Expect(accessibleCondition.Message).To(Equal("kubeconfig not valid - cannot validate cluster access"))
-			// ManagedResourcesDeployed condition validation.
-			managedResourcesDeployes := invalidCluster.Status.GetConditionByType(greenhousev1alpha1.ManagedResourcesDeployed)
-			g.Expect(managedResourcesDeployes).ToNot(BeNil(), "The ManagedResourcesDeployed condition should be present")
-			g.Expect(managedResourcesDeployes.Status).To(Equal(metav1.ConditionUnknown), "The ManagedResourcesDeployed condition should be unknown")
-			g.Expect(managedResourcesDeployes.Message).To(Equal("kubeconfig not valid - cannot validate managed resources"))
-			// KubeConfigValid condition validation.
-			kubeConfigValidCondition := invalidCluster.Status.GetConditionByType(greenhousev1alpha1.KubeConfigValid)
-			g.Expect(kubeConfigValidCondition).ToNot(BeNil(), "The KubeConfigValid condition should be present")
-			g.Expect(kubeConfigValidCondition.Status).To(Equal(metav1.ConditionFalse))
-			g.Expect(kubeConfigValidCondition.Message).To(ContainSubstring("Secret \"" + invalidCluster.Name + "\" not found"))
-			// Ready condition validation.
-			readyCondition := invalidCluster.Status.GetConditionByType(greenhousemetav1alpha1.ReadyCondition)
-			g.Expect(readyCondition).ToNot(BeNil(), "The ClusterReady condition should be present")
-			g.Expect(readyCondition.Status).To(Equal(metav1.ConditionFalse))
-		}).Should(Succeed())
-	})
 })

internal/test/util.go

@@ -14,6 +14,7 @@ import (
 	sourcev1 "github.com/fluxcd/source-controller/api/v1"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -88,6 +89,28 @@ func MustDeleteCluster(ctx context.Context, c client.Client, cluster *greenhouse
 	}).Should(BeTrue(), "the cluster should be deleted eventually", "key", client.ObjectKeyFromObject(cluster))
 }
 
+// MustDeleteSecret is used in the test context only and removes a secret by namespaced name.
+func MustDeleteSecret(ctx context.Context, c client.Client, secret *corev1.Secret) {
+	GinkgoHelper()
+
+	// Retry delete until the secret is gone - handles conflicts and waits for deletion to complete
+	Eventually(func() bool {
+		err := c.Get(ctx, client.ObjectKeyFromObject(secret), secret)
+		if err != nil {
+			return apierrors.IsNotFound(err)
+		}
+
+		err = c.Delete(ctx, secret)
+		if err != nil && !apierrors.IsNotFound(err) {
+			return false // Delete failed, will retry
+		}
+
+		// Make sure that the secret is gone
+		err = c.Get(ctx, client.ObjectKeyFromObject(secret), secret)
+		return apierrors.IsNotFound(err)
+	}).Should(BeTrue(), "the secret should be deleted eventually", "key", client.ObjectKeyFromObject(secret))
+}
+
 // SetClusterReadyCondition sets the ready condition of the cluster resource.
 func SetClusterReadyCondition(ctx context.Context, c client.Client, cluster *greenhousev1alpha1.Cluster, readyStatus metav1.ConditionStatus) error {
 	_, err := clientutil.PatchStatus(ctx, c, cluster, func() error {