Repository: rancher/fleet
Pull request #3286 (2025-01-30 16:21:09)

Small refactor for setting resource statuses
<!-- Specify the issue ID that this pull request is solving -->
Refers to #3186

Also, changes `fromResources` to order BundleDeployments before processing instead of sorting the resulting resources, which should be more efficient while producing the same result.


Patch:
diff --git a/internal/cmd/controller/gitops/reconciler/status_controller.go b/internal/cmd/controller/gitops/reconciler/status_controller.go
index e99972a6..76e44f77 100644
--- a/internal/cmd/controller/gitops/reconciler/status_controller.go
+++ b/internal/cmd/controller/gitops/reconciler/status_controller.go
@@ -154,7 +154,7 @@ func setStatus(list *fleet.BundleDeploymentList, gitrepo *fleet.GitRepo) error {
 		return err
 	}
 
-	resourcestatus.SetResources(list, &gitrepo.Status.StatusBase)
+	resourcestatus.SetResources(list.Items, &gitrepo.Status.StatusBase)
 
 	summary.SetReadyConditions(&gitrepo.Status, "Bundle", gitrepo.Status.Summary)
 
diff --git a/internal/cmd/controller/helmops/reconciler/helmapp_status.go b/internal/cmd/controller/helmops/reconciler/helmapp_status.go
index 1b7f470b..38431175 100644
--- a/internal/cmd/controller/helmops/reconciler/helmapp_status.go
+++ b/internal/cmd/controller/helmops/reconciler/helmapp_status.go
@@ -121,7 +121,7 @@ func setStatusHelm(list *fleet.BundleDeploymentList, helmapp *fleet.HelmApp) err
 		return err
 	}
 
-	resourcestatus.SetResources(list, &helmapp.Status.StatusBase)
+	resourcestatus.SetResources(list.Items, &helmapp.Status.StatusBase)
 
 	summary.SetReadyConditions(&helmapp.Status, "Bundle", helmapp.Status.Summary)
 
diff --git a/internal/cmd/controller/reconciler/cluster_controller.go b/internal/cmd/controller/reconciler/cluster_controller.go
index b5e15161..01d89a66 100644
--- a/internal/cmd/controller/reconciler/cluster_controller.go
+++ b/internal/cmd/controller/reconciler/cluster_controller.go
@@ -141,9 +141,8 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	// increased log level, this triggers a lot
 	logger.V(4).Info("Reconciling cluster, cleaning old bundledeployments and updating status", "oldDisplay", cluster.Status.Display)
 
-	bundleDeployments := &fleet.BundleDeploymentList{}
-	err = r.List(ctx, bundleDeployments, client.InNamespace(cluster.Status.Namespace))
-	if err != nil {
+	bundleDeploymentList := &fleet.BundleDeploymentList{}
+	if err = r.List(ctx, bundleDeploymentList, client.InNamespace(cluster.Status.Namespace)); err != nil {
 		return ctrl.Result{}, r.updateErrorStatus(ctx, req.NamespacedName, cluster.Status, err)
 	}
 
@@ -155,7 +154,7 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	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 {
+	bundleDeployments := slices.DeleteFunc(bundleDeploymentList.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 {
@@ -174,14 +173,14 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	cluster.Status.ResourceCounts = fleet.ResourceCounts{}
 	cluster.Status.Summary = fleet.BundleSummary{}
 
-	sort.Slice(bundleDeployments.Items, func(i, j int) bool {
-		return bundleDeployments.Items[i].Name < bundleDeployments.Items[j].Name
+	sort.Slice(bundleDeployments, func(i, j int) bool {
+		return bundleDeployments[i].Name < bundleDeployments[j].Name
 	})
 
 	resourcestatus.SetClusterResources(bundleDeployments, cluster)
 
 	repos := map[types.NamespacedName]bool{}
-	for _, bd := range bundleDeployments.Items {
+	for _, bd := range bundleDeployments {
 		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++
diff --git a/internal/resourcestatus/resourcekey.go b/internal/resourcestatus/resourcekey.go
index bb5bf1d1..f7c1aa72 100644
--- a/internal/resourcestatus/resourcekey.go
+++ b/internal/resourcestatus/resourcekey.go
@@ -9,7 +9,7 @@ import (
 	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 )
 
-func SetResources(list *fleet.BundleDeploymentList, status *fleet.StatusBase) {
+func SetResources(list []fleet.BundleDeployment, status *fleet.StatusBase) {
 	byCluster, errors := fromResources(list)
 	status.ResourceErrors = errors
 	status.Resources = aggregateResourceStatesClustersMap(byCluster)
@@ -17,7 +17,7 @@ func SetResources(list *fleet.BundleDeploymentList, status *fleet.StatusBase) {
 	status.PerClusterResourceCounts = resourceCountsPerCluster(list)
 }
 
-func SetClusterResources(list *fleet.BundleDeploymentList, cluster *fleet.Cluster) {
+func SetClusterResources(list []fleet.BundleDeployment, cluster *fleet.Cluster) {
 	cluster.Status.ResourceCounts = sumResourceCounts(list)
 }
 
@@ -25,9 +25,9 @@ func key(resource fleet.Resource) string {
 	return resource.Type + "/" + resource.ID
 }
 
-func resourceCountsPerCluster(list *fleet.BundleDeploymentList) map[string]*fleet.ResourceCounts {
+func resourceCountsPerCluster(items []fleet.BundleDeployment) map[string]*fleet.ResourceCounts {
 	res := make(map[string]*fleet.ResourceCounts)
-	for _, bd := range list.Items {
+	for _, bd := range items {
 		clusterID := bd.Labels[fleet.ClusterNamespaceLabel] + "/" + bd.Labels[fleet.ClusterLabel]
 		if _, ok := res[clusterID]; !ok {
 			res[clusterID] = &fleet.ResourceCounts{}
@@ -44,14 +44,21 @@ type resourceStateEntry struct {
 
 type resourceStatesByResourceKey map[fleet.ResourceKey][]resourceStateEntry
 
+func clusterID(bd fleet.BundleDeployment) string {
+	return bd.Labels[fleet.ClusterNamespaceLabel] + "/" + bd.Labels[fleet.ClusterLabel]
+}
+
 // fromResources inspects a list of BundleDeployments and returns a list of per-cluster states per resource keys.
 // It also returns a list of errors messages produced that may have occurred during processing
-func fromResources(list *fleet.BundleDeploymentList) (resourceStatesByResourceKey, []string) {
+func fromResources(items []fleet.BundleDeployment) (resourceStatesByResourceKey, []string) {
+	sort.Slice(items, func(i, j int) bool {
+		return clusterID(items[i]) < clusterID(items[j])
+	})
 	var (
 		errors    []string
 		resources = make(resourceStatesByResourceKey)
 	)
-	for _, bd := range list.Items {
+	for _, bd := range items {
 		clusterID := bd.Labels[fleet.ClusterNamespaceLabel] + "/" + bd.Labels[fleet.ClusterLabel]
 
 		bdResources, errs := bundleDeploymentResources(bd)
@@ -65,11 +72,6 @@ func fromResources(list *fleet.BundleDeploymentList) (resourceStatesByResourceKe
 			resources[key] = append(resources[key], resourceStateEntry{state, bd.Status.IncompleteState})
 		}
 	}
-	for _, entries := range resources {
-		sort.Slice(entries, func(i, j int) bool {
-			return entries[i].ClusterID < entries[j].ClusterID
-		})
-	}
 
 	sort.Strings(errors)
 
@@ -197,9 +199,9 @@ func aggregateResourceStatesClustersMap(resourceKeyStates resourceStatesByResour
 	return result
 }
 
-func sumResourceCounts(list *fleet.BundleDeploymentList) fleet.ResourceCounts {
+func sumResourceCounts(items []fleet.BundleDeployment) fleet.ResourceCounts {
 	var res fleet.ResourceCounts
-	for _, bd := range list.Items {
+	for _, bd := range items {
 		summary.IncrementResourceCounts(&res, bd.Status.ResourceCounts)
 	}
 	return res


Test patch:
diff --git a/internal/resourcestatus/resourcekey_test.go b/internal/resourcestatus/resourcekey_test.go
index 6c106471..7d8dc965 100644
--- a/internal/resourcestatus/resourcekey_test.go
+++ b/internal/resourcestatus/resourcekey_test.go
@@ -12,140 +12,138 @@ import (
 )
 
 func TestSetResources(t *testing.T) {
-	list := &fleet.BundleDeploymentList{
-		Items: []fleet.BundleDeployment{
-			{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "bd1",
-					Namespace: "ns1-cluster1-ns",
-					Labels: map[string]string{
-						fleet.ClusterLabel:          "cluster1",
-						fleet.ClusterNamespaceLabel: "c-ns1",
-					},
+	list := []fleet.BundleDeployment{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "bd1",
+				Namespace: "ns1-cluster1-ns",
+				Labels: map[string]string{
+					fleet.ClusterLabel:          "cluster1",
+					fleet.ClusterNamespaceLabel: "c-ns1",
 				},
-				Spec: fleet.BundleDeploymentSpec{
-					DeploymentID: "id2",
-				},
-				Status: fleet.BundleDeploymentStatus{
-					Ready:               false,
-					NonModified:         true,
-					AppliedDeploymentID: "id1",
-					Resources: []fleet.BundleDeploymentResource{
-						{
-							Kind:       "Deployment",
-							APIVersion: "v1",
-							Name:       "web",
-							Namespace:  "default",
-						},
-						{
-							// extra service for one cluster
-							Kind:       "Service",
-							APIVersion: "v1",
-							Name:       "web-svc",
-							Namespace:  "default",
-						},
+			},
+			Spec: fleet.BundleDeploymentSpec{
+				DeploymentID: "id2",
+			},
+			Status: fleet.BundleDeploymentStatus{
+				Ready:               false,
+				NonModified:         true,
+				AppliedDeploymentID: "id1",
+				Resources: []fleet.BundleDeploymentResource{
+					{
+						Kind:       "Deployment",
+						APIVersion: "v1",
+						Name:       "web",
+						Namespace:  "default",
 					},
-					ResourceCounts: fleet.ResourceCounts{
-						DesiredReady: 2,
-						WaitApplied:  2,
+					{
+						// extra service for one cluster
+						Kind:       "Service",
+						APIVersion: "v1",
+						Name:       "web-svc",
+						Namespace:  "default",
 					},
 				},
-			},
-			{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "bd1",
-					Namespace: "ns1-cluster2-ns",
-					Labels: map[string]string{
-						fleet.ClusterLabel:          "cluster2",
-						fleet.ClusterNamespaceLabel: "c-ns1",
-					},
+				ResourceCounts: fleet.ResourceCounts{
+					DesiredReady: 2,
+					WaitApplied:  2,
 				},
-				Status: fleet.BundleDeploymentStatus{
-					Ready:       true,
-					NonModified: true,
-					Resources: []fleet.BundleDeploymentResource{
-						{
-							Kind:       "Deployment",
-							APIVersion: "v1",
-							Name:       "web",
-							Namespace:  "default",
-						},
-					},
-					ResourceCounts: fleet.ResourceCounts{
-						DesiredReady: 1,
-						Ready:        1,
-					},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "bd1",
+				Namespace: "ns1-cluster2-ns",
+				Labels: map[string]string{
+					fleet.ClusterLabel:          "cluster2",
+					fleet.ClusterNamespaceLabel: "c-ns1",
 				},
 			},
-			{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "bd2",
-					Namespace: "ns1-cluster2-ns",
-					Labels: map[string]string{
-						fleet.ClusterLabel:          "cluster2",
-						fleet.ClusterNamespaceLabel: "c-ns1",
+			Status: fleet.BundleDeploymentStatus{
+				Ready:       true,
+				NonModified: true,
+				Resources: []fleet.BundleDeploymentResource{
+					{
+						Kind:       "Deployment",
+						APIVersion: "v1",
+						Name:       "web",
+						Namespace:  "default",
 					},
 				},
-				Status: fleet.BundleDeploymentStatus{
-					Ready:       true,
-					NonModified: true,
-					Resources: []fleet.BundleDeploymentResource{
-						{
-							Kind:       "ConfigMap",
-							APIVersion: "v1",
-							Name:       "cm-web",
-							Namespace:  "default",
-						},
-					},
-					ResourceCounts: fleet.ResourceCounts{
-						DesiredReady: 1,
-						Ready:        1,
-					},
+				ResourceCounts: fleet.ResourceCounts{
+					DesiredReady: 1,
+					Ready:        1,
 				},
 			},
-			{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "bd1",
-					Namespace: "ns2-cluster1",
-					Labels: map[string]string{
-						fleet.ClusterLabel:          "cluster1",
-						fleet.ClusterNamespaceLabel: "c-ns2",
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "bd2",
+				Namespace: "ns1-cluster2-ns",
+				Labels: map[string]string{
+					fleet.ClusterLabel:          "cluster2",
+					fleet.ClusterNamespaceLabel: "c-ns1",
+				},
+			},
+			Status: fleet.BundleDeploymentStatus{
+				Ready:       true,
+				NonModified: true,
+				Resources: []fleet.BundleDeploymentResource{
+					{
+						Kind:       "ConfigMap",
+						APIVersion: "v1",
+						Name:       "cm-web",
+						Namespace:  "default",
 					},
 				},
-				Spec: fleet.BundleDeploymentSpec{
-					DeploymentID: "id2",
+				ResourceCounts: fleet.ResourceCounts{
+					DesiredReady: 1,
+					Ready:        1,
 				},
-				Status: fleet.BundleDeploymentStatus{
-					Ready:               false,
-					NonModified:         true,
-					AppliedDeploymentID: "id1",
-					NonReadyStatus: []fleet.NonReadyStatus{
-						{
-							Kind:       "Deployment",
-							APIVersion: "v1",
-							Name:       "web",
-							Namespace:  "default",
-							Summary: summary.Summary{
-								State:         "Pending",
-								Error:         true,
-								Transitioning: true,
-								Message:       []string{"message1", "message2"},
-							},
-						},
-					},
-					Resources: []fleet.BundleDeploymentResource{
-						{
-							Kind:       "Deployment",
-							APIVersion: "v1",
-							Name:       "web",
-							Namespace:  "default",
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "bd1",
+				Namespace: "ns2-cluster1",
+				Labels: map[string]string{
+					fleet.ClusterLabel:          "cluster1",
+					fleet.ClusterNamespaceLabel: "c-ns2",
+				},
+			},
+			Spec: fleet.BundleDeploymentSpec{
+				DeploymentID: "id2",
+			},
+			Status: fleet.BundleDeploymentStatus{
+				Ready:               false,
+				NonModified:         true,
+				AppliedDeploymentID: "id1",
+				NonReadyStatus: []fleet.NonReadyStatus{
+					{
+						Kind:       "Deployment",
+						APIVersion: "v1",
+						Name:       "web",
+						Namespace:  "default",
+						Summary: summary.Summary{
+							State:         "Pending",
+							Error:         true,
+							Transitioning: true,
+							Message:       []string{"message1", "message2"},
 						},
 					},
-					ResourceCounts: fleet.ResourceCounts{
-						DesiredReady: 1,
-						NotReady:     1,
+				},
+				Resources: []fleet.BundleDeploymentResource{
+					{
+						Kind:       "Deployment",
+						APIVersion: "v1",
+						Name:       "web",
+						Namespace:  "default",
 					},
 				},
+				ResourceCounts: fleet.ResourceCounts{
+					DesiredReady: 1,
+					NotReady:     1,
+				},
 			},
 		},
 	}