Optimize binding controllers with fine-grained event filtering to reduce redundant reconciliations

pkg/controllers/binding/binding_controller.go

@@ -31,6 +31,7 @@ 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"
@@ -187,7 +188,7 @@ func (c *ResourceBindingController) checkDirectPurgeOrphanWorks(ctx context.Cont
 func (c *ResourceBindingController) SetupWithManager(mgr controllerruntime.Manager) error {
 	return controllerruntime.NewControllerManagedBy(mgr).
 		Named(ControllerName).
-		For(&workv1alpha2.ResourceBinding{}).
+		For(&workv1alpha2.ResourceBinding{}, builder.WithPredicates(newBindingPredicate())).
 		WithEventFilter(predicate.GenerationChangedPredicate{}).
 		Watches(&policyv1alpha1.OverridePolicy{}, handler.EnqueueRequestsFromMapFunc(c.newOverridePolicyFunc())).
 		Watches(&policyv1alpha1.ClusterOverridePolicy{}, handler.EnqueueRequestsFromMapFunc(c.newOverridePolicyFunc())).

pkg/controllers/binding/binding_predicate.go

@@ -0,0 +1,88 @@
+/*
+Copyright 2025 The Karmada Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package binding
+
+import (
+	"reflect"
+
+	"k8s.io/klog/v2"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
+
+	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
+)
+
+// hasSignificantSpecFieldChanged checks if any significant spec fields have changed
+func hasSignificantSpecFieldChanged(oldObj, newObj interface{}) bool {
+	var oldSpec, newSpec workv1alpha2.ResourceBindingSpec
+
+	switch oldBinding := oldObj.(type) {
+	case *workv1alpha2.ResourceBinding:
+		oldSpec = oldBinding.Spec
+	case *workv1alpha2.ClusterResourceBinding:
+		oldSpec = oldBinding.Spec
+	default:
+		klog.V(4).InfoS("Unknown object type in predicate", "type", reflect.TypeOf(oldObj))
+		return false
+	}
+
+	switch newBinding := newObj.(type) {
+	case *workv1alpha2.ResourceBinding:
+		newSpec = newBinding.Spec
+	case *workv1alpha2.ClusterResourceBinding:
+		newSpec = newBinding.Spec
+	default:
+		klog.V(4).InfoS("Unknown object type in predicate", "type", reflect.TypeOf(newObj))
+		return false
+	}
+
+	// Check critical fields that require reconciliation
+	return !reflect.DeepEqual(oldSpec.RequiredBy, newSpec.RequiredBy) ||
+		!reflect.DeepEqual(oldSpec.Clusters, newSpec.Clusters) ||
+		!reflect.DeepEqual(oldSpec.GracefulEvictionTasks, newSpec.GracefulEvictionTasks) ||
+		!reflect.DeepEqual(oldSpec.Resource, newSpec.Resource) ||
+		!reflect.DeepEqual(oldSpec.Suspension, newSpec.Suspension) ||
+		!reflect.DeepEqual(oldSpec.PreserveResourcesOnDeletion, newSpec.PreserveResourcesOnDeletion) ||
+		oldSpec.ConflictResolution != newSpec.ConflictResolution
+}
+
+// newBindingPredicate returns an inline predicate for ResourceBinding and ClusterResourceBinding
+// to optimize event filtering and reduce unnecessary reconciliations
+func newBindingPredicate() predicate.Predicate {
+	return predicate.Funcs{
+		CreateFunc: func(event.CreateEvent) bool {
+			return true
+		},
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			// Trigger reconciliation when deletion starts to ensure timely finalizer/cleanup handling
+			if e.ObjectOld.GetDeletionTimestamp() != e.ObjectNew.GetDeletionTimestamp() {
+				return true
+			}
+			// Check if any critical spec fields have changed
+			return hasSignificantSpecFieldChanged(e.ObjectOld, e.ObjectNew)
+		},
+		DeleteFunc: func(event.DeleteEvent) bool {
+			// Ignore delete events; reconciliation is driven by generation bump and finalizers.
+			// Cleanup is handled by controller logic (e.g., work deletion).
+			return false
+		},
+		GenericFunc: func(event.GenericEvent) bool {
+			// Process generic events (rarely used)
+			return true
+		},
+	}
+}

pkg/controllers/binding/binding_predicate_test.go

@@ -0,0 +1,288 @@
+/*
+Copyright 2025 The Karmada Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package binding
+
+import (
+	"testing"
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+
+	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
+)
+
+func TestBindingPredicate_Create(t *testing.T) {
+	predicate := newBindingPredicate()
+
+	// Test ResourceBinding create event
+	rb := &workv1alpha2.ResourceBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-rb",
+			Namespace: "default",
+		},
+	}
+
+	createEvent := event.CreateEvent{Object: rb}
+	if !predicate.Create(createEvent) {
+		t.Error("Expected Create event to return true")
+	}
+
+	// Test ClusterResourceBinding create event
+	crb := &workv1alpha2.ClusterResourceBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-crb",
+		},
+	}
+
+	createEventCRB := event.CreateEvent{Object: crb}
+	if !predicate.Create(createEventCRB) {
+		t.Error("Expected Create event for ClusterResourceBinding to return true")
+	}
+}
+
+func TestBindingPredicate_Delete(t *testing.T) {
+	predicate := newBindingPredicate()
+
+	// Test ResourceBinding delete event
+	rb := &workv1alpha2.ResourceBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-rb",
+			Namespace: "default",
+		},
+	}
+
+	deleteEvent := event.DeleteEvent{Object: rb}
+	if predicate.Delete(deleteEvent) {
+		t.Error("Expected Delete event to return false")
+	}
+
+	// Test ClusterResourceBinding delete event
+	crb := &workv1alpha2.ClusterResourceBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-crb",
+		},
+	}
+
+	deleteEventCRB := event.DeleteEvent{Object: crb}
+	if predicate.Delete(deleteEventCRB) {
+		t.Error("Expected Delete event for ClusterResourceBinding to return false")
+	}
+}
+
+func TestBindingPredicate_Update(t *testing.T) {
+	predicate := newBindingPredicate()
+
+	tests := []struct {
+		name     string
+		oldObj   *workv1alpha2.ResourceBinding
+		newObj   *workv1alpha2.ResourceBinding
+		expected bool
+	}{
+		{
+			name: "deletion timestamp set",
+			oldObj: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-rb",
+					Namespace: "default",
+				},
+			},
+			newObj: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "test-rb",
+					Namespace:         "default",
+					DeletionTimestamp: &metav1.Time{Time: time.Now()},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "clusters field change",
+			oldObj: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-rb",
+					Namespace: "default",
+				},
+				Spec: workv1alpha2.ResourceBindingSpec{
+					Clusters: []workv1alpha2.TargetCluster{
+						{Name: "cluster1"},
+					},
+				},
+			},
+			newObj: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-rb",
+					Namespace: "default",
+				},
+				Spec: workv1alpha2.ResourceBindingSpec{
+					Clusters: []workv1alpha2.TargetCluster{
+						{Name: "cluster1"},
+						{Name: "cluster2"},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "resource field change",
+			oldObj: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-rb",
+					Namespace: "default",
+				},
+				Spec: workv1alpha2.ResourceBindingSpec{
+					Resource: workv1alpha2.ObjectReference{
+						APIVersion: "apps/v1",
+						Kind:       "Deployment",
+						Name:       "test-deployment",
+						Namespace:  "default",
+					},
+				},
+			},
+			newObj: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-rb",
+					Namespace: "default",
+				},
+				Spec: workv1alpha2.ResourceBindingSpec{
+					Resource: workv1alpha2.ObjectReference{
+						APIVersion: "apps/v1",
+						Kind:       "Deployment",
+						Name:       "test-deployment-v2",
+						Namespace:  "default",
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "conflict resolution change",
+			oldObj: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-rb",
+					Namespace: "default",
+				},
+				Spec: workv1alpha2.ResourceBindingSpec{
+					ConflictResolution: "Abort",
+				},
+			},
+			newObj: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-rb",
+					Namespace: "default",
+				},
+				Spec: workv1alpha2.ResourceBindingSpec{
+					ConflictResolution: "Overwrite",
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "no significant change",
+			oldObj: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-rb",
+					Namespace: "default",
+				},
+				Spec: workv1alpha2.ResourceBindingSpec{
+					Resource: workv1alpha2.ObjectReference{
+						APIVersion: "apps/v1",
+						Kind:       "Deployment",
+						Name:       "test-deployment",
+						Namespace:  "default",
+					},
+				},
+			},
+			newObj: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-rb",
+					Namespace: "default",
+					Labels:    map[string]string{"test": "label"},
+				},
+				Spec: workv1alpha2.ResourceBindingSpec{
+					Resource: workv1alpha2.ObjectReference{
+						APIVersion: "apps/v1",
+						Kind:       "Deployment",
+						Name:       "test-deployment",
+						Namespace:  "default",
+					},
+				},
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			updateEvent := event.UpdateEvent{
+				ObjectOld: tt.oldObj,
+				ObjectNew: tt.newObj,
+			}
+
+			result := predicate.Update(updateEvent)
+			if result != tt.expected {
+				t.Errorf("Expected %v, got %v", tt.expected, result)
+			}
+		})
+	}
+}
+
+func TestResourceBindingPredicate_Generic(t *testing.T) {
+	predicate := newBindingPredicate()
+
+	genericEvent := event.GenericEvent{Object: &workv1alpha2.ResourceBinding{}}
+	if !predicate.Generic(genericEvent) {
+		t.Error("Expected Generic event to return true")
+	}
+}
+
+func TestResourceBindingPredicate_ClusterResourceBinding(t *testing.T) {
+	predicate := newBindingPredicate()
+
+	// Test ClusterResourceBinding update with clusters change
+	oldCRB := &workv1alpha2.ClusterResourceBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-crb",
+		},
+		Spec: workv1alpha2.ResourceBindingSpec{
+			Clusters: []workv1alpha2.TargetCluster{
+				{Name: "cluster1"},
+			},
+		},
+	}
+
+	newCRB := &workv1alpha2.ClusterResourceBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-crb",
+		},
+		Spec: workv1alpha2.ResourceBindingSpec{
+			Clusters: []workv1alpha2.TargetCluster{
+				{Name: "cluster1"},
+				{Name: "cluster2"},
+			},
+		},
+	}
+
+	updateEvent := event.UpdateEvent{
+		ObjectOld: oldCRB,
+		ObjectNew: newCRB,
+	}
+
+	if !predicate.Update(updateEvent) {
+		t.Error("Expected ClusterResourceBinding update with clusters change to return true")
+	}
+}