Merge pull request #473 from gianlucam76/unhealthy-cluster

If cluster is not ready, don't queue for reconciliation

Author: gianlucam76

controllers/clusterprofile_predicates.go

@@ -45,13 +45,6 @@ func ClusterPredicates(logger logr.Logger) predicate.Funcs {
 				return true
 			}
 
-			// return true if Cluster.Spec.Paused has changed from true to false
-			if oldCluster.Spec.Paused && !newCluster.Spec.Paused {
-				log.V(logs.LogVerbose).Info(
-					"Cluster was unpaused. Will attempt to reconcile associated ClusterProfiles.")
-				return true
-			}
-
 			// a label change migth change which clusters match which clusterprofile
 			if !reflect.DeepEqual(oldCluster.Labels, newCluster.Labels) {
 				log.V(logs.LogVerbose).Info(
@@ -68,6 +61,20 @@ func ClusterPredicates(logger logr.Logger) predicate.Funcs {
 				return true
 			}
 
+			// return true if Cluster.Spec.Paused has changed from true to false
+			if oldCluster.Spec.Paused && !newCluster.Spec.Paused {
+				log.V(logs.LogVerbose).Info(
+					"Cluster was unpaused. Will attempt to reconcile associated ClusterProfiles.")
+				return true
+			}
+
+			// return true if Cluster.Status.ControlPlaneReady has changed from false to true
+			if !oldCluster.Status.ControlPlaneReady && newCluster.Status.ControlPlaneReady {
+				log.V(logs.LogVerbose).Info(
+					"Cluster was not ready. Will attempt to reconcile associated ClusterProfiles.")
+				return true
+			}
+
 			// otherwise, return false
 			log.V(logs.LogVerbose).Info(
 				"Cluster did not match expected conditions.  Will not attempt to reconcile associated ClusterProfiles.")

controllers/clustersummary_controller.go

@@ -204,6 +204,17 @@ func (r *ClusterSummaryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		return reconcile.Result{Requeue: true, RequeueAfter: normalRequeueAfter}, nil
 	}
 
+	isReady, err := r.isReady(ctx, clusterSummary)
+	if err != nil {
+		return reconcile.Result{Requeue: true, RequeueAfter: normalRequeueAfter}, nil
+	}
+	if !isReady {
+		logger.V(logs.LogInfo).Info("cluster is not ready.")
+		// if cluster is not ready, do nothing and don't queue for reconciliation.
+		// When cluster becomes ready, all matching clusterSummaries will be requeued for reconciliation
+		return reconcile.Result{}, r.updateMaps(ctx, clusterSummaryScope, logger)
+	}
+
 	// Always close the scope when exiting this function so we can persist any ClusterSummary
 	// changes.
 	defer func() {
@@ -760,6 +771,13 @@ func (r *ClusterSummaryReconciler) shouldReconcile(clusterSummaryScope *scope.Cl
 		}
 	}
 
+	if len(clusterSummary.Spec.ClusterProfileSpec.KustomizationRefs) != 0 {
+		if !r.isFeatureDeployed(clusterSummaryScope.ClusterSummary, configv1alpha1.FeatureKustomize) {
+			logger.V(logs.LogDebug).Info("Mode set to one time. Kustomization resources not deployed yet. Reconciliation is needed.")
+			return true
+		}
+	}
+
 	return false
 }
 
@@ -812,6 +830,23 @@ func (r *ClusterSummaryReconciler) getReferenceMapForEntry(entry *corev1.ObjectR
 	return s
 }
 
+// isReady returns true if Sveltos/Cluster is ready
+func (r *ClusterSummaryReconciler) isReady(ctx context.Context,
+	clusterSummary *configv1alpha1.ClusterSummary) (bool, error) {
+
+	isClusterReady, err := clusterproxy.IsClusterReady(ctx, r.Client, clusterSummary.Spec.ClusterNamespace,
+		clusterSummary.Spec.ClusterName, clusterSummary.Spec.ClusterType)
+
+	if err != nil {
+		if apierrors.IsNotFound(err) {
+			return false, nil
+		}
+		return false, err
+	}
+
+	return isClusterReady, nil
+}
+
 // isPaused returns true if Sveltos/Cluster is paused or ClusterSummary has paused annotation.
 func (r *ClusterSummaryReconciler) isPaused(ctx context.Context,
 	clusterSummary *configv1alpha1.ClusterSummary) (bool, error) {

controllers/clustersummary_controller_test.go

@@ -65,6 +65,9 @@ var _ = Describe("ClustersummaryController", func() {
 					"dc": "eng",
 				},
 			},
+			Status: clusterv1.ClusterStatus{
+				ControlPlaneReady: true,
+			},
 		}
 
 		clusterProfile = &configv1alpha1.ClusterProfile{
@@ -92,11 +95,42 @@ var _ = Describe("ClustersummaryController", func() {
 
 		prepareForDeployment(clusterProfile, clusterSummary, cluster)
 
+		cluster.Status.ControlPlaneReady = true
+
 		// Get ClusterSummary so OwnerReference is set
 		Expect(testEnv.Get(context.TODO(),
 			types.NamespacedName{Namespace: clusterSummary.Namespace, Name: clusterSummary.Name}, clusterSummary)).To(Succeed())
 	})
 
+	It("isReady returns true if CAPI Cluster has Status.ControlPlaneReady set to true", func() {
+		cluster.Status.ControlPlaneReady = true
+
+		initObjects := []client.Object{
+			clusterProfile,
+			clusterSummary,
+			cluster,
+		}
+
+		c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(initObjects...).WithObjects(initObjects...).Build()
+
+		reconciler := &controllers.ClusterSummaryReconciler{
+			Client:            c,
+			Scheme:            scheme,
+			Deployer:          nil,
+			ClusterMap:        make(map[corev1.ObjectReference]*libsveltosset.Set),
+			ReferenceMap:      make(map[corev1.ObjectReference]*libsveltosset.Set),
+			ClusterSummaryMap: make(map[types.NamespacedName]*libsveltosset.Set),
+			PolicyMux:         sync.Mutex{},
+		}
+
+		Expect(controllers.IsReady(reconciler, context.TODO(), clusterSummary)).To(BeTrue())
+
+		cluster.Status.ControlPlaneReady = false
+		Expect(c.Status().Update(context.TODO(), cluster)).To(Succeed())
+
+		Expect(controllers.IsReady(reconciler, context.TODO(), clusterSummary)).To(BeFalse())
+	})
+
 	It("isPaused returns true if CAPI Cluster has Spec.Paused set", func() {
 		initObjects := []client.Object{
 			clusterProfile,
@@ -964,6 +998,13 @@ var _ = Describe("ClusterSummaryReconciler: requeue methods", func() {
 		Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed())
 
 		Expect(testEnv.Client.Create(context.TODO(), cluster)).To(Succeed())
+		Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed())
+		currentCluster := clusterv1.Cluster{}
+		Expect(testEnv.Client.Get(context.TODO(),
+			types.NamespacedName{Namespace: cluster.Namespace, Name: cluster.Name},
+			&currentCluster)).To(Succeed())
+		currentCluster.Status.ControlPlaneReady = true
+		Expect(testEnv.Client.Status().Update(ctx, &currentCluster)).To(Succeed())
 
 		configMap := createConfigMapWithPolicy(namespace, randomString(), fmt.Sprintf(editClusterRole, randomString()))
 		By(fmt.Sprintf("Creating %s %s/%s", configMap.Kind, configMap.Namespace, configMap.Name))
@@ -1033,6 +1074,16 @@ var _ = Describe("ClusterSummaryReconciler: requeue methods", func() {
 		Expect(testEnv.Client.Create(context.TODO(), ns)).To(Succeed())
 		Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed())
 
+		Expect(testEnv.Client.Create(context.TODO(), cluster)).To(Succeed())
+		Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed())
+		Expect(addTypeInformationToObject(scheme, cluster)).To(Succeed())
+		currentCluster := clusterv1.Cluster{}
+		Expect(testEnv.Client.Get(context.TODO(),
+			types.NamespacedName{Namespace: cluster.Namespace, Name: cluster.Name},
+			&currentCluster)).To(Succeed())
+		currentCluster.Status.ControlPlaneReady = true
+		Expect(testEnv.Client.Status().Update(ctx, &currentCluster)).To(Succeed())
+
 		Expect(testEnv.Client.Create(context.TODO(), referencingClusterSummary)).To(Succeed())
 		Expect(waitForObject(context.TODO(), testEnv.Client, referencingClusterSummary)).To(Succeed())
 		addOwnerReference(ctx, testEnv.Client, referencingClusterSummary, clusterProfile)
@@ -1041,10 +1092,6 @@ var _ = Describe("ClusterSummaryReconciler: requeue methods", func() {
 		Expect(waitForObject(context.TODO(), testEnv.Client, nonReferencingClusterSummary)).To(Succeed())
 		addOwnerReference(ctx, testEnv.Client, nonReferencingClusterSummary, clusterProfile)
 
-		Expect(testEnv.Client.Create(context.TODO(), cluster)).To(Succeed())
-		Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed())
-		Expect(addTypeInformationToObject(scheme, cluster)).To(Succeed())
-
 		clusterSummaryName := client.ObjectKey{
 			Name:      referencingClusterSummary.Name,
 			Namespace: referencingClusterSummary.Namespace,

controllers/clustersummary_transformations.go

@@ -176,7 +176,7 @@ func (r *ClusterSummaryReconciler) requeueClusterSummaryForCluster(
 		cluster.GetName(),
 	)
 
-	logger.V(logs.LogDebug).Info("reacting to CAPI Cluster change")
+	logger.V(logs.LogDebug).Info("reacting to Cluster change")
 
 	r.PolicyMux.Lock()
 	defer r.PolicyMux.Unlock()

controllers/export_test.go

@@ -52,6 +52,7 @@ var (
 	UndeployFeature                      = (*ClusterSummaryReconciler).undeployFeature
 	GetCurrentReferences                 = (*ClusterSummaryReconciler).getCurrentReferences
 	IsPaused                             = (*ClusterSummaryReconciler).isPaused
+	IsReady                              = (*ClusterSummaryReconciler).isReady
 	ShouldReconcile                      = (*ClusterSummaryReconciler).shouldReconcile
 	UpdateChartMap                       = (*ClusterSummaryReconciler).updateChartMap
 	ShouldRedeploy                       = (*ClusterSummaryReconciler).shouldRedeploy

controllers/suite_helpers_test.go

@@ -284,6 +284,12 @@ func prepareForDeployment(clusterProfile *configv1alpha1.ClusterProfile,
 
 	Expect(testEnv.Client.Create(context.TODO(), cluster)).To(Succeed())
 	Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed())
+	currentCluster := clusterv1.Cluster{}
+	Expect(testEnv.Client.Get(context.TODO(),
+		types.NamespacedName{Namespace: cluster.Namespace, Name: cluster.Name},
+		&currentCluster)).To(Succeed())
+	currentCluster.Status.ControlPlaneReady = true
+	Expect(testEnv.Client.Status().Update(ctx, &currentCluster)).To(Succeed())
 
 	// This method is invoked by different tests in parallel, all touching same clusterConfiguration.
 	// So add this logic in a Retry

go.mod

@@ -15,7 +15,7 @@ require (
 	github.com/onsi/ginkgo/v2 v2.15.0
 	github.com/onsi/gomega v1.31.1
 	github.com/pkg/errors v0.9.1
-	github.com/projectsveltos/libsveltos v0.24.0
+	github.com/projectsveltos/libsveltos v0.24.1-0.20240228174147-47c5e43b510a
 	github.com/prometheus/client_golang v1.18.0
 	github.com/spf13/pflag v1.0.5
 	github.com/yuin/gopher-lua v1.1.1

go.sum

@@ -335,8 +335,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/poy/onpar v1.1.2 h1:QaNrNiZx0+Nar5dLgTVp5mXkyoVFIbepjyEoGSnhbAY=
 github.com/poy/onpar v1.1.2/go.mod h1:6X8FLNoxyr9kkmnlqpK6LSoiOtrO6MICtWwEuWkLjzg=
-github.com/projectsveltos/libsveltos v0.24.0 h1:2wxUqLhIEJMRye9rHypc+uT0M++CunByuOA5Pu6TGH8=
-github.com/projectsveltos/libsveltos v0.24.0/go.mod h1:h2NUzyHkIc9u6T4NOQzpPnJmX8Rrumpmszo8wM8BUkA=
+github.com/projectsveltos/libsveltos v0.24.1-0.20240228174147-47c5e43b510a h1:6XlVKNkVn38RCz8kvLTejRg9VPEBnmSJ+cjcKA62o08=
+github.com/projectsveltos/libsveltos v0.24.1-0.20240228174147-47c5e43b510a/go.mod h1:h2NUzyHkIc9u6T4NOQzpPnJmX8Rrumpmszo8wM8BUkA=
 github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
 github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo=
 github.com/prometheus/client_golang v1.1.0/go.mod h1:I1FGZT9+L76gKKOs5djB6ezCbFQP1xR9D75/vuwEF3g=