From 901e436c22e3d17f88e58a810cf9beb898bdc317 Mon Sep 17 00:00:00 2001
From: Milos Tomic <59831542+tmilos77@users.noreply.github.com>
Date: Wed, 25 Sep 2024 11:07:13 +0200
Subject: [PATCH] fix: prevent reinstallation of CRDs when module disabled
 (#659)

---
 pkg/skr/runtime/looper/checker.go | 35 +++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/pkg/skr/runtime/looper/checker.go b/pkg/skr/runtime/looper/checker.go
index 883ad7c60..dcc514a9e 100644
--- a/pkg/skr/runtime/looper/checker.go
+++ b/pkg/skr/runtime/looper/checker.go
@@ -19,7 +19,7 @@ type checker struct {
 }
 
 func (c *checker) IsReady(ctx context.Context, skrCluster cluster.Cluster) bool {
-	timeout := time.Now().Add(time.Second * 3)
+	timeout := time.Now().Add(time.Millisecond * 3500)
 	interval := time.Second
 	for {
 		select {
@@ -40,11 +40,42 @@ func (c *checker) IsReady(ctx context.Context, skrCluster cluster.Cluster) bool
 	}
 }
 
+// singleCheck returns true if it is ok to proceed with SKR connection that implies installation of CRDs and
+// running controllers. It is important to consider both provisioning and deprovisioning phase:
+// * Provisioning
+//   - CloudResources CRD is not yet installed by KLM
+//   - CloudResources default instance is not yet created by KLM
+//
+// * Deprovisioning
+//   - There are no CloudResources instances since they all have been deleted - KLM marked for deletion,
+//     CloudManager connected to SKR and uninstalled CRDs and removed finalized, K8S deleted CloudResources.
+//     If CloudManager connects again before the KCP Scope is reconciled and this SKR removed from active
+//     we must ensure connection does not proceed and installs CRDs again.
+//   - There is CloudResources instance, but it's marked for deletion and has no finalizer since CloudManager
+//     already have connected, deleted CRDs and removed finalizer, and now K8S API should delete this resource.
+//     Not sure if practically possible, but implementing it just in case.
 func (c *checker) singleCheck(ctx context.Context, skrCluster cluster.Cluster) bool {
 	list := &cloudresourcesv1beta1.CloudResourcesList{}
 	err := skrCluster.GetAPIReader().List(ctx, list)
 	if err != nil {
-		c.logger.Error(err, "SKR readiness failed")
+		c.logger.Error(err, "SKR readiness failed - CloudResources CRD not installed")
+		return false
+	}
+	if len(list.Items) == 0 {
+		c.logger.Error(err, "SKR readiness failed - no CloudResources created")
+		return false
+	}
+	allDeletedAndNoFinalizer := true
+	for _, item := range list.Items {
+		// has deletion timestamp and has no finalizers
+		if !item.DeletionTimestamp.IsZero() && len(item.Finalizers) == 0 {
+			continue
+		}
+		allDeletedAndNoFinalizer = false
+		break
+	}
+	if allDeletedAndNoFinalizer {
+		c.logger.Error(err, "SKR readiness failed - all CloudResources instances are being deleted and have no finalizer")
 		return false
 	}
 	return true