Merge pull request #658 from gianlucam76/connection-status-and-sets

gianlucam76 · web-flow · commit 226a352c9ef1 · 2024-08-05T16:54:43.000+02:00
(feat) ClusterSet/Set: look at SveltosCluster ConnectionStatus
diff --git a/Makefile b/Makefile
@@ -25,7 +25,7 @@ ARCH ?= $(shell go env GOARCH)
 OS ?= $(shell uname -s | tr A-Z a-z)
 K8S_LATEST_VER ?= $(shell curl -s https://storage.googleapis.com/kubernetes-release/release/stable.txt)
 export CONTROLLER_IMG ?= $(REGISTRY)/$(IMAGE_NAME)
-TAG ?= main
+TAG ?= dev
 
 .PHONY: all
 all: build
diff --git a/config/crd/bases/config.projectsveltos.io_clusterprofiles.yaml b/config/crd/bases/config.projectsveltos.io_clusterprofiles.yaml
@@ -1623,6 +1623,8 @@ spec:
                             https://github.com/kubernetes/community/blob/master/contributors/design-proposals/api-machinery/api-group.md
                           type: string
                       type: object
+                  required:
+                  - patch
                   type: object
                 type: array
               policyRefs:
diff --git a/config/crd/bases/config.projectsveltos.io_clustersummaries.yaml b/config/crd/bases/config.projectsveltos.io_clustersummaries.yaml
@@ -1596,6 +1596,8 @@ spec:
                                 https://github.com/kubernetes/community/blob/master/contributors/design-proposals/api-machinery/api-group.md
                               type: string
                           type: object
+                      required:
+                      - patch
                       type: object
                     type: array
                   policyRefs:
diff --git a/config/crd/bases/config.projectsveltos.io_profiles.yaml b/config/crd/bases/config.projectsveltos.io_profiles.yaml
@@ -1623,6 +1623,8 @@ spec:
                             https://github.com/kubernetes/community/blob/master/contributors/design-proposals/api-machinery/api-group.md
                           type: string
                       type: object
+                  required:
+                  - patch
                   type: object
                 type: array
               policyRefs:
diff --git a/config/default/manager_auth_proxy_patch.yaml b/config/default/manager_auth_proxy_patch.yaml
@@ -15,4 +15,4 @@ spec:
         - "--report-mode=0"
         - --shard-key=
         - "--v=5"
-        - "--version=main"
+        - "--version=dev"
diff --git a/config/default/manager_image_patch.yaml b/config/default/manager_image_patch.yaml
@@ -8,5 +8,5 @@ spec:
     spec:
       containers:
       # Change the value of image field below to your controller image URL
-      - image: docker.io/projectsveltos/addon-controller:main
+      - image: docker.io/projectsveltos/addon-controller:dev
         name: controller
diff --git a/controllers/clusterset_controller.go b/controllers/clusterset_controller.go
@@ -143,7 +143,10 @@ func (r *ClusterSetReconciler) reconcileNormal(
 
 	setScope.SetMatchingClusterRefs(matchingCluster)
 
-	selectClusters(setScope)
+	err = selectClusters(ctx, r.Client, setScope, logger)
+	if err != nil {
+		return reconcile.Result{Requeue: true, RequeueAfter: normalRequeueAfter}
+	}
 
 	r.updateMaps(setScope)
 
diff --git a/controllers/export_test.go b/controllers/export_test.go
@@ -194,8 +194,9 @@ var (
 )
 
 var (
-	SelectClusters     = selectClusters
-	SelectMoreClusters = selectMoreClusters
+	SelectClusters              = selectClusters
+	SelectMoreClusters          = selectMoreClusters
+	PruneConnectionDownClusters = pruneConnectionDownClusters
 )
 
 var (
diff --git a/controllers/set_controller.go b/controllers/set_controller.go
@@ -152,7 +152,10 @@ func (r *SetReconciler) reconcileNormal(
 
 	setScope.SetMatchingClusterRefs(matchingCluster)
 
-	selectClusters(setScope)
+	err = selectClusters(ctx, r.Client, setScope, logger)
+	if err != nil {
+		return reconcile.Result{Requeue: true, RequeueAfter: normalRequeueAfter}
+	}
 
 	r.updateMaps(setScope)
 
diff --git a/controllers/set_utils.go b/controllers/set_utils.go
@@ -17,35 +17,46 @@ limitations under the License.
 package controllers
 
 import (
+	"context"
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/go-logr/logr"
 
 	"github.com/projectsveltos/addon-controller/pkg/scope"
+	libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1"
+	"github.com/projectsveltos/libsveltos/lib/clusterproxy"
 	logs "github.com/projectsveltos/libsveltos/lib/logsettings"
 	libsveltosset "github.com/projectsveltos/libsveltos/lib/set"
 )
 
-func selectClusters(setScope *scope.SetScope) {
+func selectClusters(ctx context.Context, c client.Client, setScope *scope.SetScope, logger logr.Logger) error {
 	status := setScope.GetStatus()
 	spec := setScope.GetSpec()
 
 	// Verify all currently selected are still ready
 	// Status.MatchingClusterRef only contains ready cluster
-	currentMatchingCluster := make(map[corev1.ObjectReference]bool)
-	for i := range status.MatchingClusterRefs {
-		currentMatchingCluster[status.MatchingClusterRefs[i]] = true
+	// Set must consider only cluster with connectionStatus set to Healthy. Prune matching cluster whose
+	// connection is down
+	healthyMatchingClusters, err := pruneConnectionDownClusters(ctx, c, status.MatchingClusterRefs, logger)
+	if err != nil {
+		return err
+	}
+
+	currentMatchingHealthyCluster := make(map[corev1.ObjectReference]bool)
+	for i := range healthyMatchingClusters {
+		currentMatchingHealthyCluster[healthyMatchingClusters[i]] = true
 	}
 
 	currentSelectedClusters := make([]corev1.ObjectReference, 0)
 	for i := range status.SelectedClusterRefs {
 		cluster := &status.SelectedClusterRefs[i]
-		if _, ok := currentMatchingCluster[*cluster]; ok {
+		if _, ok := currentMatchingHealthyCluster[*cluster]; ok {
 			currentSelectedClusters = append(currentSelectedClusters, *cluster)
 		}
 	}
@@ -57,7 +68,7 @@ func selectClusters(setScope *scope.SetScope) {
 	if len(currentSelectedClusters) == spec.MaxReplicas {
 		// Number of selected cluster matches the MaxReplicas, so there
 		// is nothing else to do
-		return
+		return nil
 	}
 
 	if spec.MaxReplicas == 0 {
@@ -67,11 +78,13 @@ func selectClusters(setScope *scope.SetScope) {
 		status.SelectedClusterRefs = currentSelectedClusters[:spec.MaxReplicas-1]
 	} else if len(currentSelectedClusters) < spec.MaxReplicas {
 		// select more clusters
-		selectMoreClusters(setScope)
+		selectMoreClusters(setScope, healthyMatchingClusters)
 	}
+
+	return nil
 }
 
-func selectMoreClusters(setScope *scope.SetScope) {
+func selectMoreClusters(setScope *scope.SetScope, healthyMatchingClusters []corev1.ObjectReference) {
 	status := setScope.GetStatus()
 	spec := setScope.GetSpec()
 
@@ -86,8 +99,8 @@ func selectMoreClusters(setScope *scope.SetScope) {
 		currentSelectedCluster[status.SelectedClusterRefs[i]] = true
 	}
 
-	for i := range status.MatchingClusterRefs {
-		cluster := &status.MatchingClusterRefs[i]
+	for i := range healthyMatchingClusters {
+		cluster := &healthyMatchingClusters[i]
 		if _, ok := currentSelectedCluster[*cluster]; !ok {
 			status.SelectedClusterRefs = append(status.SelectedClusterRefs, *cluster)
 			if len(status.SelectedClusterRefs) == spec.MaxReplicas {
@@ -128,3 +141,44 @@ func requeueForSet(set client.Object,
 
 	return requests
 }
+
+func pruneConnectionDownClusters(ctx context.Context, c client.Client, matchingClusters []corev1.ObjectReference,
+	logger logr.Logger) ([]corev1.ObjectReference, error) {
+
+	result := make([]corev1.ObjectReference, 0)
+	for i := range matchingClusters {
+		cluster := &matchingClusters[i]
+		clusterType := clusterproxy.GetClusterType(cluster)
+
+		if clusterType == libsveltosv1beta1.ClusterTypeSveltos {
+			sveltosCluster, err := getSveltosCluster(ctx, c, cluster.Namespace, cluster.Name)
+			if err != nil {
+				return nil, err
+			}
+			if sveltosCluster.Status.ConnectionStatus == libsveltosv1beta1.ConnectionDown {
+				logger.V(logs.LogDebug).Info(fmt.Sprintf("connection to sveltosCluster %s/%s is down. Ignore cluster",
+					cluster.Namespace, cluster.Name))
+				continue
+			}
+		}
+
+		result = append(result, *cluster)
+	}
+
+	return result, nil
+}
+
+func getSveltosCluster(ctx context.Context, c client.Client,
+	clusterNamespace, clusterName string) (*libsveltosv1beta1.SveltosCluster, error) {
+
+	clusterNamespacedName := types.NamespacedName{
+		Namespace: clusterNamespace,
+		Name:      clusterName,
+	}
+
+	cluster := &libsveltosv1beta1.SveltosCluster{}
+	if err := c.Get(ctx, clusterNamespacedName, cluster); err != nil {
+		return nil, err
+	}
+	return cluster, nil
+}
diff --git a/controllers/set_utils_test.go b/controllers/set_utils_test.go
@@ -17,19 +17,31 @@ limitations under the License.
 package controllers_test
 
 import (
+	"context"
+
+	"github.com/go-logr/logr"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/klog/v2/textlogger"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	"github.com/projectsveltos/addon-controller/controllers"
 	"github.com/projectsveltos/addon-controller/pkg/scope"
 	libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1"
 )
 
 var _ = Describe("Set utils", func() {
+	var logger logr.Logger
+
+	BeforeEach(func() {
+		logger = textlogger.NewLogger(textlogger.NewConfig())
+	})
+
 	It("selectMoreClusters does nothing when selected clusters matches MaxReplicas", func() {
 		selectedCluster := []corev1.ObjectReference{
 			{
@@ -79,7 +91,8 @@ var _ = Describe("Set utils", func() {
 			Set: &clusterSet,
 		}
 
-		controllers.SelectMoreClusters(setScope)
+		// Only ClusterAPI are used, so we can pass clusterSet.Status.MatchingClusterRefs
+		controllers.SelectMoreClusters(setScope, clusterSet.Status.MatchingClusterRefs)
 		Expect(len(clusterSet.Status.SelectedClusterRefs)).To(Equal(len(selectedCluster)))
 		for i := range selectedCluster {
 			Expect(clusterSet.Status.SelectedClusterRefs).To(ContainElement(selectedCluster[i]))
@@ -130,7 +143,8 @@ var _ = Describe("Set utils", func() {
 			Set: &clusterSet,
 		}
 
-		controllers.SelectMoreClusters(setScope)
+		// Only ClusterAPI are used, so we can pass clusterSet.Status.MatchingClusterRefs
+		controllers.SelectMoreClusters(setScope, clusterSet.Status.MatchingClusterRefs)
 		Expect(len(clusterSet.Status.SelectedClusterRefs)).To(Equal(clusterSet.Spec.MaxReplicas))
 	})
 
@@ -180,7 +194,8 @@ var _ = Describe("Set utils", func() {
 			Set: &clusterSet,
 		}
 
-		controllers.SelectMoreClusters(setScope)
+		// Only ClusterAPI are used, so we can pass clusterSet.Status.MatchingClusterRefs
+		controllers.SelectMoreClusters(setScope, setScope.GetStatus().MatchingClusterRefs)
 		Expect(len(clusterSet.Status.SelectedClusterRefs)).To(Equal(len(clusterSet.Status.MatchingClusterRefs)))
 	})
 
@@ -218,7 +233,107 @@ var _ = Describe("Set utils", func() {
 			Set: &clusterSet,
 		}
 
-		controllers.SelectClusters(setScope)
+		Expect(controllers.SelectClusters(context.TODO(), nil, setScope, logger)).To(Succeed())
 		Expect(len(clusterSet.Status.SelectedClusterRefs)).To(Equal(clusterSet.Spec.MaxReplicas))
 	})
+
+	It("pruneConnectionDownClusters remove SveltosCluster with connectionStatus set to Down", func() {
+		clusterSet := libsveltosv1beta1.ClusterSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: randomString(),
+			},
+			Spec: libsveltosv1beta1.Spec{
+				MaxReplicas: 2,
+				ClusterSelector: libsveltosv1beta1.Selector{
+					LabelSelector: metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							randomString(): randomString(),
+						},
+					},
+				},
+			},
+			Status: libsveltosv1beta1.Status{},
+		}
+
+		initObjects := []client.Object{}
+
+		clusterSet.Status.MatchingClusterRefs = make([]corev1.ObjectReference, 0)
+		// Add 3 SveltosCluster with Status.ConnectionStatus set to Down
+		for i := 0; i < 3; i++ {
+			sveltosCluster := libsveltosv1beta1.SveltosCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: randomString(),
+					Name:      randomString(),
+				},
+				Status: libsveltosv1beta1.SveltosClusterStatus{
+					ConnectionStatus: libsveltosv1beta1.ConnectionDown,
+					Ready:            true,
+				},
+			}
+
+			initObjects = append(initObjects, &sveltosCluster)
+
+			clusterSet.Status.MatchingClusterRefs = append(clusterSet.Status.MatchingClusterRefs,
+				corev1.ObjectReference{
+					Kind:       string(libsveltosv1beta1.ClusterTypeSveltos),
+					APIVersion: libsveltosv1beta1.GroupVersion.String(),
+					Namespace:  sveltosCluster.Namespace,
+					Name:       sveltosCluster.Name,
+				})
+		}
+
+		healthySveltosClusters := make(map[corev1.ObjectReference]bool)
+		// Add 3 SveltosCluster with Status.ConnectionStatus set to Healthy
+		for i := 0; i < 3; i++ {
+			sveltosCluster := libsveltosv1beta1.SveltosCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: randomString(),
+					Name:      randomString(),
+				},
+				Status: libsveltosv1beta1.SveltosClusterStatus{
+					ConnectionStatus: libsveltosv1beta1.ConnectionHealthy,
+					Ready:            true,
+				},
+			}
+
+			initObjects = append(initObjects, &sveltosCluster)
+
+			objectRef := &corev1.ObjectReference{
+				Kind:       string(libsveltosv1beta1.ClusterTypeSveltos),
+				APIVersion: libsveltosv1beta1.GroupVersion.String(),
+				Namespace:  sveltosCluster.Namespace,
+				Name:       sveltosCluster.Name,
+			}
+
+			clusterSet.Status.MatchingClusterRefs = append(clusterSet.Status.MatchingClusterRefs, *objectRef)
+
+			healthySveltosClusters[*objectRef] = true
+		}
+
+		c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(initObjects...).
+			WithObjects(initObjects...).Build()
+
+		Expect(addTypeInformationToObject(scheme, &clusterSet)).To(Succeed())
+
+		setScope := &scope.SetScope{
+			Set: &clusterSet,
+		}
+
+		result, err := controllers.PruneConnectionDownClusters(context.TODO(), c, clusterSet.Status.MatchingClusterRefs, logger)
+		Expect(err).To(BeNil())
+		Expect(len(result)).To(Equal(3))
+		for i := range result {
+			v := healthySveltosClusters[result[i]]
+			Expect(v).To(BeTrue())
+		}
+
+		Expect(controllers.SelectClusters(context.TODO(), c, setScope, logger)).To(Succeed())
+		Expect(len(clusterSet.Status.SelectedClusterRefs)).To(Equal(clusterSet.Spec.MaxReplicas))
+		// Verify only SveltosCluster with connectionStatus set to Healthy are picked
+		for i := range clusterSet.Status.SelectedClusterRefs {
+			clusterRef := clusterSet.Status.SelectedClusterRefs[i]
+			v := healthySveltosClusters[clusterRef]
+			Expect(v).To(BeTrue())
+		}
+	})
 })
diff --git a/go.mod b/go.mod
@@ -15,7 +15,7 @@ require (
 	github.com/onsi/ginkgo/v2 v2.19.0
 	github.com/onsi/gomega v1.33.1
 	github.com/pkg/errors v0.9.1
-	github.com/projectsveltos/libsveltos v0.35.1-0.20240726065655-ee3b7dc30da2
+	github.com/projectsveltos/libsveltos v0.35.1-0.20240805114830-bfe0e2b5e213
 	github.com/prometheus/client_golang v1.19.1
 	github.com/spf13/pflag v1.0.5
 	github.com/yuin/gopher-lua v1.1.1
diff --git a/go.sum b/go.sum
diff --git a/manifest/deployment-agentless.yaml b/manifest/deployment-agentless.yaml
diff --git a/manifest/deployment-shard.yaml b/manifest/deployment-shard.yaml
diff --git a/manifest/manifest.yaml b/manifest/manifest.yaml

Original file line number	Diff line number	Diff line change
`@@ -194,8 +194,9 @@ var (`
`194`	`194`	`)`
`195`	`195`
`196`	`196`	`var (`
`197`		`- SelectClusters = selectClusters`
`198`		`- SelectMoreClusters = selectMoreClusters`
	`197`	`+ SelectClusters = selectClusters`
	`198`	`+ SelectMoreClusters = selectMoreClusters`
	`199`	`+ PruneConnectionDownClusters = pruneConnectionDownClusters`
`199`	`200`	`)`
`200`	`201`
`201`	`202`	`var (`