Skip to content

Commit 127e00c

Browse files
rohan-019rohan-019
committed
optimizing-binding-controller
Signed-off-by: rohan-019 <[email protected]>
1 parent 70411ab commit 127e00c

File tree

4 files changed

+380
-2
lines changed

4 files changed

+380
-2
lines changed

‎pkg/controllers/binding/binding_controller.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"k8s.io/klog/v2"
3232
"k8s.io/utils/ptr"
3333
controllerruntime "sigs.k8s.io/controller-runtime"
34+
"sigs.k8s.io/controller-runtime/pkg/builder"
3435
"sigs.k8s.io/controller-runtime/pkg/client"
3536
"sigs.k8s.io/controller-runtime/pkg/controller"
3637
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -187,7 +188,7 @@ func (c *ResourceBindingController) checkDirectPurgeOrphanWorks(ctx context.Cont
187188
func (c *ResourceBindingController) SetupWithManager(mgr controllerruntime.Manager) error {
188189
return controllerruntime.NewControllerManagedBy(mgr).
189190
Named(ControllerName).
190-
For(&workv1alpha2.ResourceBinding{}).
191+
For(&workv1alpha2.ResourceBinding{}, builder.WithPredicates(newBindingPredicate())).
191192
WithEventFilter(predicate.GenerationChangedPredicate{}).
192193
Watches(&policyv1alpha1.OverridePolicy{}, handler.EnqueueRequestsFromMapFunc(c.newOverridePolicyFunc())).
193194
Watches(&policyv1alpha1.ClusterOverridePolicy{}, handler.EnqueueRequestsFromMapFunc(c.newOverridePolicyFunc())).

‎pkg/controllers/binding/binding_predicate.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
Copyright 2025 The Karmada Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package binding
18+
19+
import (
20+
"reflect"
21+
22+
"k8s.io/klog/v2"
23+
"sigs.k8s.io/controller-runtime/pkg/event"
24+
"sigs.k8s.io/controller-runtime/pkg/predicate"
25+
26+
workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
27+
)
28+
29+
// hasSignificantSpecFieldChanged checks if any significant spec fields have changed
30+
func hasSignificantSpecFieldChanged(oldObj, newObj interface{}) bool {
31+
var oldSpec, newSpec workv1alpha2.ResourceBindingSpec
32+
33+
switch oldBinding := oldObj.(type) {
34+
case *workv1alpha2.ResourceBinding:
35+
oldSpec = oldBinding.Spec
36+
case *workv1alpha2.ClusterResourceBinding:
37+
oldSpec = oldBinding.Spec
38+
default:
39+
klog.V(4).InfoS("Unknown object type in predicate", "type", reflect.TypeOf(oldObj))
40+
return false
41+
}
42+
43+
switch newBinding := newObj.(type) {
44+
case *workv1alpha2.ResourceBinding:
45+
newSpec = newBinding.Spec
46+
case *workv1alpha2.ClusterResourceBinding:
47+
newSpec = newBinding.Spec
48+
default:
49+
klog.V(4).InfoS("Unknown object type in predicate", "type", reflect.TypeOf(newObj))
50+
return false
51+
}
52+
53+
// Check critical fields that require reconciliation
54+
return !reflect.DeepEqual(oldSpec.RequiredBy, newSpec.RequiredBy) ||
55+
!reflect.DeepEqual(oldSpec.Clusters, newSpec.Clusters) ||
56+
!reflect.DeepEqual(oldSpec.GracefulEvictionTasks, newSpec.GracefulEvictionTasks) ||
57+
!reflect.DeepEqual(oldSpec.Resource, newSpec.Resource) ||
58+
!reflect.DeepEqual(oldSpec.Suspension, newSpec.Suspension) ||
59+
!reflect.DeepEqual(oldSpec.PreserveResourcesOnDeletion, newSpec.PreserveResourcesOnDeletion) ||
60+
oldSpec.ConflictResolution != newSpec.ConflictResolution
61+
}
62+
63+
// newBindingPredicate returns an inline predicate for ResourceBinding and ClusterResourceBinding
64+
// to optimize event filtering and reduce unnecessary reconciliations
65+
func newBindingPredicate() predicate.Predicate {
66+
return predicate.Funcs{
67+
CreateFunc: func(event.CreateEvent) bool {
68+
return true
69+
},
70+
UpdateFunc: func(e event.UpdateEvent) bool {
71+
// Trigger reconciliation when deletion starts to ensure timely finalizer/cleanup handling
72+
if e.ObjectOld.GetDeletionTimestamp() != e.ObjectNew.GetDeletionTimestamp() {
73+
return true
74+
}
75+
// Check if any critical spec fields have changed
76+
return hasSignificantSpecFieldChanged(e.ObjectOld, e.ObjectNew)
77+
},
78+
DeleteFunc: func(event.DeleteEvent) bool {
79+
// Ignore delete events; reconciliation is driven by generation bump and finalizers.
80+
// Cleanup is handled by controller logic (e.g., work deletion).
81+
return false
82+
},
83+
GenericFunc: func(event.GenericEvent) bool {
84+
// Process generic events (rarely used)
85+
return true
86+
},
87+
}
88+
}

‎pkg/controllers/binding/binding_predicate_test.go‎

Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
/*
2+
Copyright 2025 The Karmada Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package binding
18+
19+
import (
20+
"testing"
21+
"time"
22+
23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
"sigs.k8s.io/controller-runtime/pkg/event"
25+
26+
workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
27+
)
28+
29+
func TestResourceBindingPredicate_Create(t *testing.T) {
30+
predicate := newBindingPredicate()
31+
32+
// Test ResourceBinding create event
33+
rb := &workv1alpha2.ResourceBinding{
34+
ObjectMeta: metav1.ObjectMeta{
35+
Name: "test-rb",
36+
Namespace: "default",
37+
},
38+
}
39+
40+
createEvent := event.CreateEvent{Object: rb}
41+
if !predicate.Create(createEvent) {
42+
t.Error("Expected Create event to return true")
43+
}
44+
45+
// Test ClusterResourceBinding create event
46+
crb := &workv1alpha2.ClusterResourceBinding{
47+
ObjectMeta: metav1.ObjectMeta{
48+
Name: "test-crb",
49+
},
50+
}
51+
52+
createEventCRB := event.CreateEvent{Object: crb}
53+
if !predicate.Create(createEventCRB) {
54+
t.Error("Expected Create event for ClusterResourceBinding to return true")
55+
}
56+
}
57+
58+
func TestBindingPredicate_Delete(t *testing.T) {
59+
predicate := newBindingPredicate()
60+
61+
// Test ResourceBinding delete event
62+
rb := &workv1alpha2.ResourceBinding{
63+
ObjectMeta: metav1.ObjectMeta{
64+
Name: "test-rb",
65+
Namespace: "default",
66+
},
67+
}
68+
69+
deleteEvent := event.DeleteEvent{Object: rb}
70+
if predicate.Delete(deleteEvent) {
71+
t.Error("Expected Delete event to return false")
72+
}
73+
74+
// Test ClusterResourceBinding delete event
75+
crb := &workv1alpha2.ClusterResourceBinding{
76+
ObjectMeta: metav1.ObjectMeta{
77+
Name: "test-crb",
78+
},
79+
}
80+
81+
deleteEventCRB := event.DeleteEvent{Object: crb}
82+
if predicate.Delete(deleteEventCRB) {
83+
t.Error("Expected Delete event for ClusterResourceBinding to return false")
84+
}
85+
}
86+
87+
func TestBindingPredicate_Update(t *testing.T) {
88+
predicate := newBindingPredicate()
89+
90+
tests := []struct {
91+
name string
92+
oldObj *workv1alpha2.ResourceBinding
93+
newObj *workv1alpha2.ResourceBinding
94+
expected bool
95+
}{
96+
{
97+
name: "deletion timestamp set",
98+
oldObj: &workv1alpha2.ResourceBinding{
99+
ObjectMeta: metav1.ObjectMeta{
100+
Name: "test-rb",
101+
Namespace: "default",
102+
},
103+
},
104+
newObj: &workv1alpha2.ResourceBinding{
105+
ObjectMeta: metav1.ObjectMeta{
106+
Name: "test-rb",
107+
Namespace: "default",
108+
DeletionTimestamp: &metav1.Time{Time: time.Now()},
109+
},
110+
},
111+
expected: true,
112+
},
113+
{
114+
name: "clusters field change",
115+
oldObj: &workv1alpha2.ResourceBinding{
116+
ObjectMeta: metav1.ObjectMeta{
117+
Name: "test-rb",
118+
Namespace: "default",
119+
},
120+
Spec: workv1alpha2.ResourceBindingSpec{
121+
Clusters: []workv1alpha2.TargetCluster{
122+
{Name: "cluster1"},
123+
},
124+
},
125+
},
126+
newObj: &workv1alpha2.ResourceBinding{
127+
ObjectMeta: metav1.ObjectMeta{
128+
Name: "test-rb",
129+
Namespace: "default",
130+
},
131+
Spec: workv1alpha2.ResourceBindingSpec{
132+
Clusters: []workv1alpha2.TargetCluster{
133+
{Name: "cluster1"},
134+
{Name: "cluster2"},
135+
},
136+
},
137+
},
138+
expected: true,
139+
},
140+
{
141+
name: "resource field change",
142+
oldObj: &workv1alpha2.ResourceBinding{
143+
ObjectMeta: metav1.ObjectMeta{
144+
Name: "test-rb",
145+
Namespace: "default",
146+
},
147+
Spec: workv1alpha2.ResourceBindingSpec{
148+
Resource: workv1alpha2.ObjectReference{
149+
APIVersion: "apps/v1",
150+
Kind: "Deployment",
151+
Name: "test-deployment",
152+
Namespace: "default",
153+
},
154+
},
155+
},
156+
newObj: &workv1alpha2.ResourceBinding{
157+
ObjectMeta: metav1.ObjectMeta{
158+
Name: "test-rb",
159+
Namespace: "default",
160+
},
161+
Spec: workv1alpha2.ResourceBindingSpec{
162+
Resource: workv1alpha2.ObjectReference{
163+
APIVersion: "apps/v1",
164+
Kind: "Deployment",
165+
Name: "test-deployment-v2",
166+
Namespace: "default",
167+
},
168+
},
169+
},
170+
expected: true,
171+
},
172+
{
173+
name: "conflict resolution change",
174+
oldObj: &workv1alpha2.ResourceBinding{
175+
ObjectMeta: metav1.ObjectMeta{
176+
Name: "test-rb",
177+
Namespace: "default",
178+
},
179+
Spec: workv1alpha2.ResourceBindingSpec{
180+
ConflictResolution: "Abort",
181+
},
182+
},
183+
newObj: &workv1alpha2.ResourceBinding{
184+
ObjectMeta: metav1.ObjectMeta{
185+
Name: "test-rb",
186+
Namespace: "default",
187+
},
188+
Spec: workv1alpha2.ResourceBindingSpec{
189+
ConflictResolution: "Overwrite",
190+
},
191+
},
192+
expected: true,
193+
},
194+
{
195+
name: "no significant change",
196+
oldObj: &workv1alpha2.ResourceBinding{
197+
ObjectMeta: metav1.ObjectMeta{
198+
Name: "test-rb",
199+
Namespace: "default",
200+
},
201+
Spec: workv1alpha2.ResourceBindingSpec{
202+
Resource: workv1alpha2.ObjectReference{
203+
APIVersion: "apps/v1",
204+
Kind: "Deployment",
205+
Name: "test-deployment",
206+
Namespace: "default",
207+
},
208+
},
209+
},
210+
newObj: &workv1alpha2.ResourceBinding{
211+
ObjectMeta: metav1.ObjectMeta{
212+
Name: "test-rb",
213+
Namespace: "default",
214+
Labels: map[string]string{"test": "label"},
215+
},
216+
Spec: workv1alpha2.ResourceBindingSpec{
217+
Resource: workv1alpha2.ObjectReference{
218+
APIVersion: "apps/v1",
219+
Kind: "Deployment",
220+
Name: "test-deployment",
221+
Namespace: "default",
222+
},
223+
},
224+
},
225+
expected: false,
226+
},
227+
}
228+
229+
for _, tt := range tests {
230+
t.Run(tt.name, func(t *testing.T) {
231+
updateEvent := event.UpdateEvent{
232+
ObjectOld: tt.oldObj,
233+
ObjectNew: tt.newObj,
234+
}
235+
236+
result := predicate.Update(updateEvent)
237+
if result != tt.expected {
238+
t.Errorf("Expected %v, got %v", tt.expected, result)
239+
}
240+
})
241+
}
242+
}
243+
244+
func TestResourceBindingPredicate_Generic(t *testing.T) {
245+
predicate := newBindingPredicate()
246+
247+
genericEvent := event.GenericEvent{Object: &workv1alpha2.ResourceBinding{}}
248+
if !predicate.Generic(genericEvent) {
249+
t.Error("Expected Generic event to return true")
250+
}
251+
}
252+
253+
func TestResourceBindingPredicate_ClusterResourceBinding(t *testing.T) {
254+
predicate := newBindingPredicate()
255+
256+
// Test ClusterResourceBinding update with clusters change
257+
oldCRB := &workv1alpha2.ClusterResourceBinding{
258+
ObjectMeta: metav1.ObjectMeta{
259+
Name: "test-crb",
260+
},
261+
Spec: workv1alpha2.ResourceBindingSpec{
262+
Clusters: []workv1alpha2.TargetCluster{
263+
{Name: "cluster1"},
264+
},
265+
},
266+
}
267+
268+
newCRB := &workv1alpha2.ClusterResourceBinding{
269+
ObjectMeta: metav1.ObjectMeta{
270+
Name: "test-crb",
271+
},
272+
Spec: workv1alpha2.ResourceBindingSpec{
273+
Clusters: []workv1alpha2.TargetCluster{
274+
{Name: "cluster1"},
275+
{Name: "cluster2"},
276+
},
277+
},
278+
}
279+
280+
updateEvent := event.UpdateEvent{
281+
ObjectOld: oldCRB,
282+
ObjectNew: newCRB,
283+
}
284+
285+
if !predicate.Update(updateEvent) {
286+
t.Error("Expected ClusterResourceBinding update with clusters change to return true")
287+
}
288+
}

0 commit comments

Comments
 (0)