Merge pull request #1207 from gianlucam76/resourcesummary-agentless

(bug) clean ResourceSummary instances when cluster is deleted

Author: gianlucam76

Makefile

@@ -73,7 +73,7 @@ KUBECTL := $(TOOLS_BIN_DIR)/kubectl
 
 GOVULNCHECK_VERSION := "v1.1.3"
 GOLANGCI_LINT_VERSION := "v1.64.7"
-CLUSTERCTL_VERSION := "v1.10.1"
+CLUSTERCTL_VERSION := "v1.10.2"
 
 KUSTOMIZE_VER := v5.3.0
 KUSTOMIZE_BIN := kustomize
@@ -194,7 +194,7 @@ endif
 # K8S_VERSION for the Kind cluster can be set as environment variable. If not defined,
 # this default value is used
 ifndef K8S_VERSION
-K8S_VERSION := v1.32.2
+K8S_VERSION := v1.33.0
 endif
 
 KIND_CONFIG ?= kind-cluster.yaml

controllers/clustersummary_controller.go

@@ -286,8 +286,17 @@ func (r *ClusterSummaryReconciler) reconcileDelete(
 
 		logger.V(logs.LogDebug).Info("remove drift-detection-manager resources from management cluster")
 		cs := clusterSummaryScope.ClusterSummary
-		if err := removeDriftDetectionManagerFromManagementCluster(ctx,
-			cs.Spec.ClusterNamespace, cs.Spec.ClusterName, cs.Spec.ClusterType, logger); err != nil {
+
+		err = removeStaleResourceSummary(ctx, cs.Spec.ClusterNamespace, cs.Spec.ClusterName, cs.Spec.ClusterType, logger)
+		if err != nil {
+			logger.V(logs.LogInfo).Info(
+				fmt.Sprintf("failed to remove resourceSummary instances from management cluster: %v", err))
+			return reconcile.Result{Requeue: true, RequeueAfter: deleteRequeueAfter}, nil
+		}
+
+		err := removeDriftDetectionManagerFromManagementCluster(ctx,
+			cs.Spec.ClusterNamespace, cs.Spec.ClusterName, cs.Spec.ClusterType, logger)
+		if err != nil {
 			logger.V(logs.LogInfo).Info(
 				fmt.Sprintf("failed to remove drift-detection-manager resources from management cluster: %v", err))
 			return reconcile.Result{Requeue: true, RequeueAfter: deleteRequeueAfter}, nil
@@ -439,7 +448,7 @@ func (r *ClusterSummaryReconciler) SetupWithManager(ctx context.Context, mgr ctr
 	}
 
 	if getAgentInMgmtCluster() {
-		go removeStaleDriftDetectionManager(ctx, r.Logger)
+		go removeStaleDriftDetectionResources(ctx, r.Logger)
 	}
 
 	initializeManager(ctrl.Log.WithName("watchers"), mgr.GetConfig(), mgr.GetClient())

controllers/export_test.go

@@ -207,6 +207,7 @@ var (
 )
 
 var (
-	AssociatedClusterExist = associatedClusterExist
-	RemoveDuplicates       = removeDuplicates
+	DeplAssociatedClusterExist = deplAssociatedClusterExist
+	RemoveStaleResourceSummary = removeStaleResourceSummary
+	RemoveDuplicates           = removeDuplicates
 )

controllers/handlers_helm.go

@@ -760,9 +760,7 @@ func handleInstall(ctx context.Context, clusterSummary *configv1beta1.ClusterSum
 	maxHistory := uint(getMaxHistoryValue(currentChart.Options))
 
 	if fs := getFeatureSummaryForFeatureID(clusterSummary, configv1beta1.FeatureHelm); fs != nil {
-		if fs.ConsecutiveFailures%maxHistory == 0 && fs.FailureMessage != nil &&
-			strings.Contains(*fs.FailureMessage, "context deadline exceeded") {
-
+		if fs.ConsecutiveFailures%maxHistory == 0 && fs.FailureMessage != nil {
 			err := doUninstallRelease(ctx, clusterSummary, currentChart, kubeconfig, registryOptions, logger)
 			if err != nil {
 				return nil, err

controllers/resourcesummary_collection.go

@@ -126,9 +126,19 @@ func collectResourceSummariesFromCluster(ctx context.Context, c client.Client, c
 	}
 
 	logger.V(logs.LogVerbose).Info("collecting ResourceSummaries from cluster")
+	listOptions := []client.ListOption{}
+	if getAgentInMgmtCluster() {
+		listOptions = []client.ListOption{
+			client.MatchingLabels{
+				sveltos_upgrade.ClusterNameLabel:               cluster.Name,
+				libsveltosv1beta1.ClusterSummaryNamespaceLabel: cluster.Namespace,
+			},
+		}
+	}
+
 	rsList := libsveltosv1beta1.ResourceSummaryList{}
 
-	err = clusterClient.List(ctx, &rsList)
+	err = clusterClient.List(ctx, &rsList, listOptions...)
 	if err != nil {
 		return err
 	}
@@ -138,7 +148,7 @@ func collectResourceSummariesFromCluster(ctx context.Context, c client.Client, c
 	for i := range rsList.Items {
 		rs := &rsList.Items[i]
 		if !rs.DeletionTimestamp.IsZero() {
-			// ignore deleted ClassifierReport
+			// ignore deleted resourceSummary
 			continue
 		}
 		if rs.Status.ResourcesChanged || rs.Status.HelmResourcesChanged || rs.Status.KustomizeResourcesChanged {

controllers/utils.go

@@ -50,6 +50,7 @@ import (
 	"github.com/projectsveltos/libsveltos/lib/clusterproxy"
 	"github.com/projectsveltos/libsveltos/lib/logsettings"
 	libsveltosset "github.com/projectsveltos/libsveltos/lib/set"
+	"github.com/projectsveltos/libsveltos/lib/sveltos_upgrade"
 )
 
 //+kubebuilder:rbac:groups=extension.projectsveltos.io,resources=yttsources,verbs=get;list;watch
@@ -498,7 +499,8 @@ func getSortedKustomizationRefs(clusterSummary *configv1beta1.ClusterSummary) []
 }
 
 // Identifies and removes drift detection deployments that are no longer associated with active clusters
-func removeStaleDriftDetectionManager(ctx context.Context, logger logr.Logger) {
+// Identifies and removes resourceSummary instances from clusters that are no longer existing
+func removeStaleDriftDetectionResources(ctx context.Context, logger logr.Logger) {
 	listOptions := []client.ListOption{
 		client.MatchingLabels{
 			driftDetectionFeatureLabelKey: driftDetectionFeatureLabelValue,
@@ -507,6 +509,7 @@ func removeStaleDriftDetectionManager(ctx context.Context, logger logr.Logger) {
 
 	for {
 		time.Sleep(time.Minute)
+
 		c := getManagementClusterClient()
 		driftDetectionDeployments := &appsv1.DeploymentList{}
 		err := c.List(ctx, driftDetectionDeployments, listOptions...)
@@ -518,7 +521,15 @@ func removeStaleDriftDetectionManager(ctx context.Context, logger logr.Logger) {
 		for i := range driftDetectionDeployments.Items {
 			depl := &driftDetectionDeployments.Items[i]
 
-			if !associatedClusterExist(ctx, c, depl, logger) {
+			exist, clusterNs, clusterName, clusterType := deplAssociatedClusterExist(ctx, c, depl, logger)
+			if !exist {
+				// find resourceSummaries from this cluster and remove those.
+				// Remove deployment only after this one succeed
+				err = removeStaleResourceSummary(ctx, clusterNs, clusterName, clusterType, logger)
+				if err != nil {
+					continue
+				}
+
 				logger.V(logsettings.LogInfo).Info(fmt.Sprintf("deleting driftDetection deployment %s/%s",
 					depl.Namespace, depl.Name))
 				_ = c.Delete(ctx, depl)
@@ -527,35 +538,72 @@ func removeStaleDriftDetectionManager(ctx context.Context, logger logr.Logger) {
 	}
 }
 
-func associatedClusterExist(ctx context.Context, c client.Client, depl *appsv1.Deployment, logger logr.Logger) bool {
+func removeStaleResourceSummary(ctx context.Context, clusterNamespace, clusterName string,
+	clusterType libsveltosv1beta1.ClusterType, logger logr.Logger) error {
+
+	c := getManagementClusterClient()
+
+	rsListOptions := []client.ListOption{
+		client.MatchingLabels{
+			sveltos_upgrade.ClusterNameLabel:               clusterName,
+			sveltos_upgrade.ClusterTypeLabel:               strings.ToLower(string(clusterType)),
+			libsveltosv1beta1.ClusterSummaryNamespaceLabel: clusterNamespace,
+		},
+	}
+
+	logger.V(logsettings.LogInfo).Info(fmt.Sprintf("MGIANLUC %v", rsListOptions))
+
+	resourceSummaries := &libsveltosv1beta1.ResourceSummaryList{}
+	err := c.List(ctx, resourceSummaries, rsListOptions...)
+	if err != nil {
+		logger.V(logsettings.LogInfo).Info(
+			fmt.Sprintf("failed to collect resourceSummary instances: %v", err))
+		return err
+	}
+
+	for i := range resourceSummaries.Items {
+		rs := &resourceSummaries.Items[i]
+		err = c.Delete(ctx, rs)
+		if err != nil {
+			logger.V(logsettings.LogInfo).Info(
+				fmt.Sprintf("failed to delete resourceSummary instance: %v", err))
+			return err
+		}
+	}
+
+	return nil
+}
+
+func deplAssociatedClusterExist(ctx context.Context, c client.Client, depl *appsv1.Deployment,
+	logger logr.Logger) (exist bool, clusterName, clusterNamespace string, clusterType libsveltosv1beta1.ClusterType) {
+
 	if depl.Labels == nil {
 		logger.V(logsettings.LogInfo).Info(fmt.Sprintf("driftDetection %s/%s has no label",
 			depl.Namespace, depl.Name))
-		return true
+		return true, "", "", ""
 	}
 
 	clusterNamespace, ok := depl.Labels[driftDetectionClusterNamespaceLabel]
 	if !ok {
 		logger.V(logsettings.LogInfo).Info(fmt.Sprintf("driftDetection %s/%s has no %s label",
 			depl.Namespace, depl.Name, driftDetectionClusterNamespaceLabel))
-		return true
+		return true, "", "", ""
 	}
 
-	clusterName, ok := depl.Labels[driftDetectionClusterNameLabel]
+	clusterName, ok = depl.Labels[driftDetectionClusterNameLabel]
 	if !ok {
 		logger.V(logsettings.LogInfo).Info(fmt.Sprintf("driftDetection %s/%s has no %s label",
 			depl.Namespace, depl.Name, driftDetectionClusterNameLabel))
-		return true
+		return true, "", "", ""
 	}
 
 	clusterTypeString, ok := depl.Labels[driftDetectionClusterTypeLabel]
 	if !ok {
 		logger.V(logsettings.LogInfo).Info(fmt.Sprintf("driftDetection %s/%s has no %s label",
 			depl.Namespace, depl.Name, driftDetectionClusterTypeLabel))
-		return true
+		return true, "", "", ""
 	}
 
-	var clusterType libsveltosv1beta1.ClusterType
 	if strings.EqualFold(clusterTypeString, string(libsveltosv1beta1.ClusterTypeSveltos)) {
 		clusterType = libsveltosv1beta1.ClusterTypeSveltos
 	} else if strings.EqualFold(clusterTypeString, string(libsveltosv1beta1.ClusterTypeCapi)) {
@@ -565,11 +613,11 @@ func associatedClusterExist(ctx context.Context, c client.Client, depl *appsv1.D
 	_, err := clusterproxy.GetCluster(ctx, c, clusterNamespace, clusterName, clusterType)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			return false
+			return false, clusterNamespace, clusterName, clusterType
 		}
 		logger.V(logsettings.LogInfo).Info(fmt.Sprintf("failed to get cluster %s:%s/%s: %v",
 			clusterNamespace, clusterName, clusterTypeString, err))
 	}
 
-	return true
+	return true, "", "", ""
 }

controllers/utils_test.go

@@ -19,6 +19,7 @@ package controllers_test
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -28,6 +29,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	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"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -428,7 +430,7 @@ var _ = Describe("getClusterProfileOwner ", func() {
 		}
 	})
 
-	It("associatedClusterExist returns false when associated cluster does not exist",
+	It("deplAssociatedClusterExist returns false when associated cluster does not exist",
 		func() {
 			clusterNamespace := randomString()
 			clusterName := randomString()
@@ -474,7 +476,8 @@ spec:
 			Expect(c.Get(context.TODO(),
 				types.NamespacedName{Namespace: u.GetNamespace(), Name: u.GetName()}, currentDepl)).To(Succeed())
 
-			Expect(controllers.AssociatedClusterExist(context.TODO(), c, currentDepl, logger)).To(BeFalse())
+			exist, _, _, _ := controllers.DeplAssociatedClusterExist(context.TODO(), c, currentDepl, logger)
+			Expect(exist).To(BeFalse())
 
 			sveltosCluster := libsveltosv1beta1.SveltosCluster{
 				ObjectMeta: metav1.ObjectMeta{
@@ -483,8 +486,57 @@ spec:
 				},
 			}
 			Expect(c.Create(context.TODO(), &sveltosCluster)).To(Succeed())
-			Expect(controllers.AssociatedClusterExist(context.TODO(), c, currentDepl, logger)).To(BeTrue())
+			exist, _, _, _ = controllers.DeplAssociatedClusterExist(context.TODO(), c, currentDepl, logger)
+			Expect(exist).To(BeTrue())
+		})
+
+	It("removeStaleResourceSummary removes stales resourceSummary instances",
+		func() {
+			clusterNamespace := randomString()
+			clusterName := randomString()
+			clusterType := libsveltosv1beta1.ClusterTypeSveltos
+
+			logger := textlogger.NewLogger(textlogger.NewConfig())
+
+			ns := &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: clusterNamespace,
+				},
+			}
+
+			Expect(testEnv.Create(context.TODO(), ns)).To(Succeed())
+			Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed())
+
+			rs := fmt.Sprintf(`apiVersion: lib.projectsveltos.io/v1beta1
+kind: ResourceSummary
+metadata:
+  labels:
+    projectsveltos.io/cluster-summary-namespace: %s
+    version.projectsveltos.io/clustername: %s
+    version.projectsveltos.io/clustertype: %s
+  name: deploy-cert-manager-sveltos-cluster1
+  namespace: %s
+`, clusterNamespace, clusterName, strings.ToLower(string(clusterType)), clusterNamespace)
+
+			u, err := k8s_utils.GetUnstructured([]byte(rs))
+			Expect(err).To(BeNil())
+
+			Expect(testEnv.Create(context.TODO(), u)).To(Succeed())
+			Expect(waitForObject(context.TODO(), testEnv.Client, u)).To(Succeed())
+
+			currentRS := &libsveltosv1beta1.ResourceSummary{}
+
+			Expect(controllers.RemoveStaleResourceSummary(context.TODO(), clusterNamespace, clusterName,
+				clusterType, logger)).To(Succeed())
 
+			Eventually(func() bool {
+				err = testEnv.Get(context.TODO(),
+					types.NamespacedName{Namespace: u.GetNamespace(), Name: u.GetName()}, currentRS)
+				if err == nil {
+					return false
+				}
+				return apierrors.IsNotFound(err)
+			}, timeout, pollingInterval).Should(BeTrue())
 		})
 })