reduce unnecessary watch events and bindingSpec change in the binding controller

Signed-off-by: changzhen <[email protected]>

Author: XiShanYongYe-Chang

pkg/controllers/binding/binding_controller.go

@@ -31,9 +31,11 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/utils/ptr"
 	controllerruntime "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"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/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -185,9 +187,27 @@ func (c *ResourceBindingController) checkDirectPurgeOrphanWorks(ctx context.Cont
 
 // SetupWithManager creates a controller and register to controller manager.
 func (c *ResourceBindingController) SetupWithManager(mgr controllerruntime.Manager) error {
+	bindingPredicateFn := predicate.Funcs{
+		CreateFunc: func(_ event.CreateEvent) bool {
+			return true
+		},
+		UpdateFunc: func(updateEvent event.UpdateEvent) bool {
+			oldBinding := updateEvent.ObjectOld.(*workv1alpha2.ResourceBinding)
+			newBinding := updateEvent.ObjectNew.(*workv1alpha2.ResourceBinding)
+			return careBindingSpecChanged(oldBinding.Spec, newBinding.Spec)
+		},
+		DeleteFunc: func(_ event.DeleteEvent) bool {
+			// The binding object has a finalizer. When the deletion timestamp is not nil,
+			// it means the object is being deleted and the controller will handle the
+			// deletion process, so the delete event can be ignored.
+			return false
+		},
+		GenericFunc: func(_ event.GenericEvent) bool { return false },
+	}
+
 	return controllerruntime.NewControllerManagedBy(mgr).
 		Named(ControllerName).
-		For(&workv1alpha2.ResourceBinding{}).
+		For(&workv1alpha2.ResourceBinding{}, builder.WithPredicates(bindingPredicateFn)).
 		WithEventFilter(predicate.GenerationChangedPredicate{}).
 		Watches(&policyv1alpha1.OverridePolicy{}, handler.EnqueueRequestsFromMapFunc(c.newOverridePolicyFunc())).
 		Watches(&policyv1alpha1.ClusterOverridePolicy{}, handler.EnqueueRequestsFromMapFunc(c.newOverridePolicyFunc())).

pkg/controllers/binding/cluster_resource_binding_controller.go

@@ -31,9 +31,11 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/utils/ptr"
 	controllerruntime "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"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/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -183,9 +185,26 @@ func (c *ClusterResourceBindingController) checkDirectPurgeOrphanWorks(ctx conte
 
 // SetupWithManager creates a controller and register to controller manager.
 func (c *ClusterResourceBindingController) SetupWithManager(mgr controllerruntime.Manager) error {
+	bindingPredicateFn := predicate.Funcs{
+		CreateFunc: func(_ event.CreateEvent) bool {
+			return true
+		},
+		UpdateFunc: func(updateEvent event.UpdateEvent) bool {
+			oldBinding := updateEvent.ObjectOld.(*workv1alpha2.ClusterResourceBinding)
+			newBinding := updateEvent.ObjectNew.(*workv1alpha2.ClusterResourceBinding)
+			return careBindingSpecChanged(oldBinding.Spec, newBinding.Spec)
+		},
+		DeleteFunc: func(_ event.DeleteEvent) bool {
+			// The binding object has a finalizer. When the deletion timestamp is not nil,
+			// it means the object is being deleted and the controller will handle the
+			// deletion process, so the delete event can be ignored.
+			return false
+		},
+		GenericFunc: func(_ event.GenericEvent) bool { return false },
+	}
 	return controllerruntime.NewControllerManagedBy(mgr).
 		Named(ClusterResourceBindingControllerName).
-		For(&workv1alpha2.ClusterResourceBinding{}).
+		For(&workv1alpha2.ClusterResourceBinding{}, builder.WithPredicates(bindingPredicateFn)).
 		Watches(&policyv1alpha1.ClusterOverridePolicy{}, handler.EnqueueRequestsFromMapFunc(c.newOverridePolicyFunc())).
 		WithEventFilter(predicate.GenerationChangedPredicate{}).
 		WithOptions(controller.Options{RateLimiter: ratelimiterflag.DefaultControllerRateLimiter[controllerruntime.Request](c.RateLimiterOptions)}).

pkg/controllers/binding/common.go

@@ -18,6 +18,7 @@ package binding
 
 import (
 	"context"
+	"reflect"
 	"strconv"
 	"time"
 
@@ -354,3 +355,38 @@ func shouldSuspendDispatching(suspension *workv1alpha2.Suspension, targetCluster
 	}
 	return suspendDispatching
 }
+
+type bindingSpecDigest struct {
+	Resource                    workv1alpha2.ObjectReference
+	Clusters                    []workv1alpha2.TargetCluster
+	GracefulEvictionTasks       []workv1alpha2.GracefulEvictionTask
+	RequiredBy                  []workv1alpha2.BindingSnapshot
+	ConflictResolution          policyv1alpha1.ConflictResolution
+	Suspension                  *workv1alpha2.Suspension
+	PreserveResourcesOnDeletion *bool
+}
+
+// careBindingSpecChanged checks whether the fields in bindingSpec that the binding controller cares about have changed.
+// This helps avoid triggering the binding controller to resync when irrelevant fields are modified.
+func careBindingSpecChanged(oldBindingSpec, newBindingSpec workv1alpha2.ResourceBindingSpec) bool {
+	oldDigest := bindingSpecDigest{
+		Resource:                    oldBindingSpec.Resource,
+		Clusters:                    oldBindingSpec.Clusters,
+		GracefulEvictionTasks:       oldBindingSpec.GracefulEvictionTasks,
+		RequiredBy:                  oldBindingSpec.RequiredBy,
+		ConflictResolution:          oldBindingSpec.ConflictResolution,
+		Suspension:                  oldBindingSpec.Suspension,
+		PreserveResourcesOnDeletion: oldBindingSpec.PreserveResourcesOnDeletion,
+	}
+	newDigest := bindingSpecDigest{
+		Resource:                    newBindingSpec.Resource,
+		Clusters:                    newBindingSpec.Clusters,
+		GracefulEvictionTasks:       newBindingSpec.GracefulEvictionTasks,
+		RequiredBy:                  newBindingSpec.RequiredBy,
+		ConflictResolution:          newBindingSpec.ConflictResolution,
+		Suspension:                  newBindingSpec.Suspension,
+		PreserveResourcesOnDeletion: newBindingSpec.PreserveResourcesOnDeletion,
+	}
+
+	return !reflect.DeepEqual(oldDigest, newDigest)
+}