Calculate Clusters resourceCount from BundleDeployments instead of Gi…

…tRepos (#3102)

e2e/single-cluster/status_test.go

@@ -75,6 +75,7 @@ var _ = Describe("Checks status updates happen for a simple deployment", Ordered
 				out, err := k.Get("cluster", "local", "-n", "fleet-local", "-o", "jsonpath='{.status.display.readyBundles}'")
 				g.Expect(err).ToNot(HaveOccurred(), out)
 
+				// Expected 2 bundles instead of just 1 because fleet-agent is also included here
 				g.Expect(out).Should(Equal("'2/2'"))
 			}).Should(Succeed())
 		})

internal/cmd/controller/gitops/reconciler/gitrepo_status.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/rancher/fleet/internal/cmd/controller/summary"
 	"github.com/rancher/fleet/internal/metrics"
+	"github.com/rancher/fleet/internal/resourcestatus"
 	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 	"github.com/rancher/wrangler/v3/pkg/condition"
 	"github.com/rancher/wrangler/v3/pkg/kstatus"
@@ -104,7 +105,7 @@ func SetStatusFromBundleDeployments(ctx context.Context, c client.Client, gitrep
 	gitrepo.Status.Display.Message = message
 	gitrepo.Status.Display.Error = len(message) > 0
 
-	setResources(list, gitrepo)
+	resourcestatus.SetGitRepoResources(list, gitrepo)
 
 	return nil
 }

internal/cmd/controller/reconciler/cluster_controller.go

@@ -6,14 +6,17 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"slices"
 	"sort"
 	"time"
 
 	"github.com/rancher/fleet/internal/cmd/controller/summary"
 	"github.com/rancher/fleet/internal/metrics"
+	"github.com/rancher/fleet/internal/resourcestatus"
 	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 	"github.com/rancher/fleet/pkg/durations"
 	"github.com/rancher/fleet/pkg/sharding"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	fleetutil "github.com/rancher/fleet/internal/cmd/controller/errorutil"
 	"github.com/rancher/wrangler/v3/pkg/condition"
@@ -43,11 +46,6 @@ var LongRetry = wait.Backoff{
 	Jitter:   0.1,
 }
 
-type repoKey struct {
-	repo string
-	ns   string
-}
-
 // ClusterReconciler reconciles a Cluster object
 type ClusterReconciler struct {
 	client.Client
@@ -105,6 +103,14 @@ func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
+func indexByNamespacedName[T metav1.Object](list []T) map[types.NamespacedName]T {
+	res := make(map[types.NamespacedName]T, len(list))
+	for _, obj := range list {
+		res[types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}] = obj
+	}
+	return res
+}
+
 //+kubebuilder:rbac:groups=fleet.cattle.io,resources=clusters,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=fleet.cattle.io,resources=clusters/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=fleet.cattle.io,resources=clusters/finalizers,verbs=update
@@ -144,20 +150,21 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	if err != nil {
 		return ctrl.Result{}, r.updateErrorStatus(ctx, req.NamespacedName, cluster.Status, err)
 	}
-
-	deleted := map[types.UID]bool{}
-	for _, bundle := range cleanup {
-		for _, bd := range bundleDeployments.Items {
-			if bd.Labels[fleet.BundleLabel] == bundle.Name && bd.Labels[fleet.BundleNamespaceLabel] == bundle.Namespace {
-				logger.V(1).Info("cleaning up bundleDeployment not matching the cluster", "bundledeployment", bd)
-				err := r.Delete(ctx, &bd)
-				if err != nil {
-					logger.V(1).Error(err, "deleting bundleDeployment returned an error")
-				}
-				deleted[bd.GetUID()] = true
+	toDeleteBundles := indexByNamespacedName(cleanup)
+
+	// Delete BundleDeployments for Bundles being removed while getting a filtered items list
+	bundleDeployments.Items = slices.DeleteFunc(bundleDeployments.Items, func(bd fleet.BundleDeployment) bool {
+		bundleNamespace := bd.Labels[fleet.BundleNamespaceLabel]
+		bundleName := bd.Labels[fleet.BundleLabel]
+		if _, ok := toDeleteBundles[types.NamespacedName{Namespace: bundleNamespace, Name: bundleName}]; ok {
+			logger.V(1).Info("cleaning up bundleDeployment not matching the cluster", "bundledeployment", bd)
+			if err := r.Delete(ctx, &bd); err != nil {
+				logger.V(1).Error(err, "deleting bundleDeployment returned an error")
 			}
+			return true
 		}
-	}
+		return false
+	})
 
 	// Count the number of gitrepo, bundledeployemt and deployed resources for this cluster
 	cluster.Status.DesiredReadyGitRepos = 0
@@ -169,33 +176,27 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return bundleDeployments.Items[i].Name < bundleDeployments.Items[j].Name
 	})
 
-	repos := map[repoKey]bool{}
-	for _, bd := range bundleDeployments.Items {
-		// do not count bundledeployments that were just deleted
-		if deleted[bd.GetUID()] {
-			continue
-		}
+	resourcestatus.SetClusterResources(bundleDeployments, cluster)
 
-		bd := bd
+	repos := map[types.NamespacedName]bool{}
+	for _, bd := range bundleDeployments.Items {
 		state := summary.GetDeploymentState(&bd)
 		summary.IncrementState(&cluster.Status.Summary, bd.Name, state, summary.MessageFromDeployment(&bd), bd.Status.ModifiedStatus, bd.Status.NonReadyStatus)
 		cluster.Status.Summary.DesiredReady++
 
-		repo := bd.Labels[fleet.RepoLabel]
-		ns := bd.Labels[fleet.BundleNamespaceLabel]
-		if repo != "" && ns != "" {
+		repoNamespace, repoName := bd.Labels[fleet.BundleNamespaceLabel], bd.Labels[fleet.RepoLabel]
+		if repoNamespace != "" && repoName != "" {
 			// a gitrepo is ready if its bundledeployments are ready, take previous state into account
-			repos[repoKey{repo: repo, ns: ns}] = (state == fleet.Ready) || repos[repoKey{repo: repo, ns: ns}]
+			repoKey := types.NamespacedName{Namespace: repoNamespace, Name: repoName}
+			repos[repoKey] = (state == fleet.Ready) || repos[repoKey]
 		}
 	}
 
 	// a cluster is ready if all its gitrepos are ready and the resources are ready too
 	allReady := true
 	for repo, ready := range repos {
 		gitrepo := &fleet.GitRepo{}
-		err := r.Get(ctx, types.NamespacedName{Namespace: repo.ns, Name: repo.repo}, gitrepo)
-		if err == nil {
-			summary.IncrementResourceCounts(&cluster.Status.ResourceCounts, gitrepo.Status.ResourceCounts)
+		if err := r.Get(ctx, repo, gitrepo); err == nil {
 			cluster.Status.DesiredReadyGitRepos++
 			if ready {
 				cluster.Status.ReadyGitRepos++

internal/cmd/controller/gitops/reconciler/resourcekey.go → internal/resourcestatus/resourcekey.go

@@ -1,4 +1,4 @@
-package reconciler
+package resourcestatus
 
 import (
 	"encoding/json"
@@ -8,14 +8,20 @@ import (
 	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 )
 
-func setResources(list *fleet.BundleDeploymentList, gitrepo *fleet.GitRepo) {
+func SetGitRepoResources(list *fleet.BundleDeploymentList, gitrepo *fleet.GitRepo) {
 	s := summaryState(gitrepo.Status.Summary)
 	r, errors := fromResources(list, s)
 	gitrepo.Status.ResourceErrors = errors
 	gitrepo.Status.ResourceCounts = countResources(r)
 	gitrepo.Status.Resources = merge(r)
 }
 
+func SetClusterResources(list *fleet.BundleDeploymentList, cluster *fleet.Cluster) {
+	s := summaryState(cluster.Status.Summary)
+	r, _ := fromResources(list, s)
+	cluster.Status.ResourceCounts = countResources(r)
+}
+
 // merge takes a list of GitRepo resources and deduplicates resources deployed to multiple clusters,
 // ensuring that for such resources, the output contains a single resource entry with a field summarizing
 // its status on each cluster.

internal/cmd/controller/gitops/reconciler/resourcekey_test.go → internal/resourcestatus/resourcekey_test.go

@@ -1,4 +1,4 @@
-package reconciler
+package resourcestatus
 
 import (
 	. "github.com/onsi/ginkgo/v2"
@@ -114,7 +114,7 @@ var _ = Describe("Resourcekey", func() {
 	})
 
 	It("returns a list", func() {
-		setResources(list, gitrepo)
+		SetGitRepoResources(list, gitrepo)
 
 		Expect(gitrepo.Status.Resources).To(HaveLen(2))
 		Expect(gitrepo.Status.Resources).To(ContainElement(fleet.GitRepoResource{