Filter cluster events to trigger bundle deployment creation less often (#3796)

Author: manno

internal/cmd/controller/agentmanagement/controllers/cluster/import.go

@@ -204,6 +204,7 @@ func (i *importHandler) OnChange(key string, cluster *fleet.Cluster) (_ *fleet.C
 		return cluster, nil
 	}
 
+	// cluster.spec.KubeConfigSecret is empty when agent-initiated registration is used
 	if cluster.Spec.KubeConfigSecret == "" || agentDeployed(cluster) {
 		return cluster, nil
 	}

internal/cmd/controller/reconciler/bundle_controller.go

@@ -6,6 +6,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"maps"
+	"reflect"
 	"slices"
 	"time"
 
@@ -36,6 +38,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -79,7 +82,8 @@ func (r *BundleReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		For(&fleet.Bundle{}).
 		// Note: Maybe improve with WatchesMetadata, does it have access to labels?
 		Watches(
-			// Fan out from bundledeployment to bundle
+			// Fan out from bundledeployment to bundle, this is useful to update the
+			// bundle's status fields.
 			&fleet.BundleDeployment{},
 			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []ctrl.Request {
 				bd := a.(*fleet.BundleDeployment)
@@ -103,7 +107,7 @@ func (r *BundleReconciler) SetupWithManager(mgr ctrl.Manager) error {
 			builder.WithPredicates(bundleDeploymentStatusChangedPredicate()),
 		).
 		Watches(
-			// Fan out from cluster to bundle
+			// Fan out from cluster to bundle, this is useful for targeting and templating.
 			&fleet.Cluster{},
 			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []ctrl.Request {
 				cluster := a.(*fleet.Cluster)
@@ -123,13 +127,54 @@ func (r *BundleReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 				return requests
 			}),
-			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
+			builder.WithPredicates(clusterChangedPredicate()),
 		).
 		WithEventFilter(sharding.FilterByShardID(r.ShardID)).
 		WithOptions(controller.Options{MaxConcurrentReconciles: r.Workers}).
 		Complete(r)
 }
 
+// clusterChangedPredicate filters cluster events that relate to bundldeployment creation.
+func clusterChangedPredicate() predicate.Funcs {
+	return predicate.Funcs{
+		CreateFunc: func(e event.CreateEvent) bool {
+			return true
+		},
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			n := e.ObjectNew.(*fleet.Cluster)
+			o := e.ObjectOld.(*fleet.Cluster)
+			// cluster deletion will eventually trigger a delete event
+			if n == nil || !n.DeletionTimestamp.IsZero() {
+				return true
+			}
+			// labels and annotations are used for templating and targeting
+			if !maps.Equal(n.Labels, o.Labels) {
+				return true
+			}
+			if !maps.Equal(n.Annotations, o.Annotations) {
+				return true
+			}
+			// spec templateValues is used in templating
+			if !reflect.DeepEqual(n.Spec, o.Spec) {
+				return true
+			}
+			// this namespace contains the bundledeployments
+			if n.Status.Namespace != o.Status.Namespace {
+				return true
+			}
+			// this namespace indicates the agent is running
+			if n.Status.Agent.Namespace != o.Status.Agent.Namespace {
+				return true
+			}
+
+			return false
+		},
+		DeleteFunc: func(e event.DeleteEvent) bool {
+			return true
+		},
+	}
+}
+
 //+kubebuilder:rbac:groups=fleet.cattle.io,resources=bundles,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=fleet.cattle.io,resources=bundles/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=fleet.cattle.io,resources=bundles/finalizers,verbs=update
@@ -294,8 +339,9 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			continue
 		}
 
-		// NOTE we don't use the existing BundleDeployment, we discard annotations, status, etc
-		// copy labels from Bundle as they might have changed
+		// NOTE we don't re-use the existing BundleDeployment, we discard annotations, status, etc.
+		// and copy labels from Bundle as they might have changed.
+		// However, matchedTargets target.Deployment contains existing BundleDeployments.
 		bd := target.BundleDeployment()
 
 		// No need to check the deletion timestamp here before adding a finalizer, since the bundle has just