Skip to content

Commit e6922db

Browse files
Kexin2000Kexin2000
committed
Add DependencyPolicyConflict event
Signed-off-by: Kexin2000 <3299133282@qq.com>
1 parent f3c4376 commit e6922db

File tree

4 files changed

+311
-11
lines changed

4 files changed

+311
-11
lines changed

‎pkg/dependenciesdistributor/dependencies_distributor.go‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ func (d *DependenciesDistributor) handleDependentResource(
362362
return err
363363
}
364364
attachedBinding := buildAttachedBinding(independentBinding, rawObject)
365-
return d.createOrUpdateAttachedBinding(attachedBinding)
365+
return d.createOrUpdateAttachedBinding(ctx, attachedBinding)
366366
case dependent.LabelSelector != nil:
367367
var selector labels.Selector
368368
var err error
@@ -375,7 +375,7 @@ func (d *DependenciesDistributor) handleDependentResource(
375375
}
376376
for _, rawObject := range rawObjects {
377377
attachedBinding := buildAttachedBinding(independentBinding, rawObject)
378-
if err := d.createOrUpdateAttachedBinding(attachedBinding); err != nil {
378+
if err := d.createOrUpdateAttachedBinding(ctx, attachedBinding); err != nil {
379379
return err
380380
}
381381
}
@@ -573,10 +573,10 @@ func (d *DependenciesDistributor) removeScheduleResultFromAttachedBindings(bindi
573573
return utilerrors.NewAggregate(errs)
574574
}
575575

576-
func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding *workv1alpha2.ResourceBinding) error {
576+
func (d *DependenciesDistributor) createOrUpdateAttachedBinding(ctx context.Context, attachedBinding *workv1alpha2.ResourceBinding) error {
577577
existBinding := &workv1alpha2.ResourceBinding{}
578578
bindingKey := client.ObjectKeyFromObject(attachedBinding)
579-
err := d.Client.Get(context.TODO(), bindingKey, existBinding)
579+
err := d.Client.Get(ctx, bindingKey, existBinding)
580580
if err == nil {
581581
// If this binding exists and its owner is not the input object, return error and let garbage collector
582582
// delete this binding and try again later. See https://github.com/karmada-io/karmada/issues/6034.
@@ -585,17 +585,32 @@ func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding
585585
"try again later after binding is garbage collected, see https://github.com/karmada-io/karmada/issues/6034", bindingKey)
586586
}
587587

588+
mergedRequiredBy := mergeBindingSnapshot(existBinding.Spec.RequiredBy, attachedBinding.Spec.RequiredBy)
589+
588590
// If the spec.Placement is nil, this means that existBinding is generated by the dependency mechanism.
589591
// If the spec.Placement is not nil, then it must be generated by PropagationPolicy.
590592
if existBinding.Spec.Placement == nil {
593+
// Pre-fetch referencing ResourceBindings once and reuse for conflict checks.
594+
rbs, err := d.resolveResourceBindingFromSnapshots(ctx, mergedRequiredBy)
595+
if err != nil {
596+
klog.Errorf("Failed to resolve referencing ResourceBindings for (%s): %v", bindingKey, err)
597+
return err
598+
}
599+
600+
crConflictDetected := d.conflictDetectedForConflictResolution(rbs)
601+
preserveConflictDetected := d.conflictDetectedForPreserveOnDeletion(rbs)
602+
603+
d.recordEventIfPolicyConflict(existBinding, crConflictDetected, preserveConflictDetected)
604+
591605
existBinding.Spec.ConflictResolution = attachedBinding.Spec.ConflictResolution
592606
}
593-
existBinding.Spec.RequiredBy = mergeBindingSnapshot(existBinding.Spec.RequiredBy, attachedBinding.Spec.RequiredBy)
607+
608+
existBinding.Spec.RequiredBy = mergedRequiredBy
594609
existBinding.Labels = util.DedupeAndMergeLabels(existBinding.Labels, attachedBinding.Labels)
595610
existBinding.Spec.Resource = attachedBinding.Spec.Resource
596611
existBinding.Spec.PreserveResourcesOnDeletion = attachedBinding.Spec.PreserveResourcesOnDeletion
597612

598-
if err := d.Client.Update(context.TODO(), existBinding); err != nil {
613+
if err := d.Client.Update(ctx, existBinding); err != nil {
599614
klog.Errorf("Failed to update resourceBinding(%s): %v", bindingKey, err)
600615
return err
601616
}
@@ -607,7 +622,7 @@ func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding
607622
return err
608623
}
609624

610-
return d.Client.Create(context.TODO(), attachedBinding)
625+
return d.Client.Create(ctx, attachedBinding)
611626
}
612627

613628
// Start runs the distributor, never stop until context canceled.

‎pkg/dependenciesdistributor/dependencies_distributor_test.go‎

Lines changed: 169 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"reflect"
23+
"strings"
2324
"testing"
2425
"time"
2526

@@ -34,6 +35,7 @@ import (
3435
"k8s.io/client-go/dynamic"
3536
dynamicfake "k8s.io/client-go/dynamic/fake"
3637
"k8s.io/client-go/kubernetes/scheme"
38+
"k8s.io/client-go/tools/record"
3739
"k8s.io/utils/ptr"
3840
"sigs.k8s.io/controller-runtime/pkg/client"
3941
"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -42,6 +44,7 @@ import (
4244
configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
4345
policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
4446
workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
47+
"github.com/karmada-io/karmada/pkg/events"
4548
"github.com/karmada-io/karmada/pkg/util"
4649
"github.com/karmada-io/karmada/pkg/util/fedinformer/genericmanager"
4750
"github.com/karmada-io/karmada/pkg/util/fedinformer/keys"
@@ -2523,7 +2526,15 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
25232526
},
25242527
},
25252528
}
2526-
return fake.NewClientBuilder().WithScheme(Scheme).WithObjects(rb).Build()
2529+
2530+
// Add the referenced ResourceBindings that will be looked up during conflict checks.
2531+
ref1 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "default-binding-1", Namespace: "default-1"}}
2532+
ref2 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "default-binding-2", Namespace: "default-2"}}
2533+
ref3 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "default-binding-3", Namespace: "default-3"}}
2534+
ref4 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "test-binding-1", Namespace: "test-1"}}
2535+
ref5 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "test-binding-2", Namespace: "test-2"}}
2536+
2537+
return fake.NewClientBuilder().WithScheme(Scheme).WithObjects(rb, ref1, ref2, ref3, ref4, ref5).Build()
25272538
},
25282539
},
25292540
{
@@ -2826,7 +2837,15 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
28262837
},
28272838
},
28282839
}
2829-
return fake.NewClientBuilder().WithScheme(Scheme).WithObjects(rb).Build()
2840+
2841+
// Referenced ResourceBindings for conflict checks
2842+
ref1 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "default-binding-1", Namespace: "default-1"}}
2843+
ref2 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "default-binding-2", Namespace: "default-2"}}
2844+
ref3 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "default-binding-3", Namespace: "default-3"}}
2845+
ref4 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "test-binding-1", Namespace: "test-1"}}
2846+
ref5 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "test-binding-2", Namespace: "test-2"}}
2847+
2848+
return fake.NewClientBuilder().WithScheme(Scheme).WithObjects(rb, ref1, ref2, ref3, ref4, ref5).Build()
28302849
},
28312850
},
28322851
{
@@ -2933,7 +2952,12 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
29332952
},
29342953
},
29352954
}
2936-
return fake.NewClientBuilder().WithScheme(Scheme).WithObjects(rb).Build()
2955+
2956+
// Referenced ResourceBindings for conflict checks
2957+
ref1 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "default-binding-1", Namespace: "default-1"}}
2958+
ref2 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "test-binding-1", Namespace: "test-1"}}
2959+
2960+
return fake.NewClientBuilder().WithScheme(Scheme).WithObjects(rb, ref1, ref2).Build()
29372961
},
29382962
},
29392963
}
@@ -2942,7 +2966,7 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
29422966
d := &DependenciesDistributor{
29432967
Client: tt.setupClient(),
29442968
}
2945-
err := d.createOrUpdateAttachedBinding(tt.attachedBinding)
2969+
err := d.createOrUpdateAttachedBinding(context.TODO(), tt.attachedBinding)
29462970
if (err != nil) != tt.wantErr {
29472971
t.Errorf("createOrUpdateAttachedBinding() error = %v, wantErr %v", err, tt.wantErr)
29482972
}
@@ -2962,6 +2986,87 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
29622986
}
29632987
}
29642988

2989+
func Test_createOrUpdateAttachedBinding_emitsConflictEvent(t *testing.T) {
2990+
Scheme := runtime.NewScheme()
2991+
utilruntime.Must(scheme.AddToScheme(Scheme))
2992+
utilruntime.Must(workv1alpha2.Install(Scheme))
2993+
2994+
// Existing attached binding to be updated (generated by dependency mechanism: Placement == nil)
2995+
exist := &workv1alpha2.ResourceBinding{
2996+
ObjectMeta: metav1.ObjectMeta{
2997+
Name: "test-binding",
2998+
Namespace: "test",
2999+
ResourceVersion: "1000",
3000+
OwnerReferences: []metav1.OwnerReference{{
3001+
UID: "uid-1",
3002+
Controller: ptr.To[bool](true),
3003+
}},
3004+
},
3005+
Spec: workv1alpha2.ResourceBindingSpec{},
3006+
}
3007+
3008+
// Referenced ResourceBindings with conflicting policies
3009+
rb1 := &workv1alpha2.ResourceBinding{
3010+
ObjectMeta: metav1.ObjectMeta{Name: "rb-1", Namespace: "ns-a"},
3011+
Spec: workv1alpha2.ResourceBindingSpec{
3012+
PreserveResourcesOnDeletion: ptr.To(true),
3013+
ConflictResolution: policyv1alpha1.ConflictOverwrite,
3014+
},
3015+
}
3016+
rb2 := &workv1alpha2.ResourceBinding{
3017+
ObjectMeta: metav1.ObjectMeta{Name: "rb-2", Namespace: "ns-b"},
3018+
Spec: workv1alpha2.ResourceBindingSpec{
3019+
PreserveResourcesOnDeletion: ptr.To(false),
3020+
},
3021+
}
3022+
3023+
attached := &workv1alpha2.ResourceBinding{
3024+
ObjectMeta: metav1.ObjectMeta{
3025+
Name: "test-binding",
3026+
Namespace: "test",
3027+
OwnerReferences: []metav1.OwnerReference{{
3028+
UID: "uid-1",
3029+
Controller: ptr.To[bool](true),
3030+
}},
3031+
},
3032+
Spec: workv1alpha2.ResourceBindingSpec{
3033+
Resource: workv1alpha2.ObjectReference{APIVersion: "apps/v1", Kind: "Deployment", Namespace: "fake-ns", Name: "demo-app", ResourceVersion: "1"},
3034+
RequiredBy: []workv1alpha2.BindingSnapshot{
3035+
{Namespace: "ns-a", Name: "rb-1"},
3036+
{Namespace: "ns-b", Name: "rb-2"},
3037+
},
3038+
},
3039+
}
3040+
3041+
fakeRec := record.NewFakeRecorder(10)
3042+
d := &DependenciesDistributor{
3043+
Client: fake.NewClientBuilder().WithScheme(Scheme).WithObjects(exist, rb1, rb2).Build(),
3044+
EventRecorder: fakeRec,
3045+
}
3046+
3047+
if err := d.createOrUpdateAttachedBinding(context.TODO(), attached); err != nil {
3048+
t.Fatalf("createOrUpdateAttachedBinding() error = %v", err)
3049+
}
3050+
3051+
select {
3052+
case e := <-fakeRec.Events:
3053+
if !strings.Contains(e, corev1.EventTypeWarning) {
3054+
t.Fatalf("expected Warning event, got %q", e)
3055+
}
3056+
if !strings.Contains(e, events.EventReasonDependencyPolicyConflict) {
3057+
t.Fatalf("expected reason %q in event, got %q", events.EventReasonDependencyPolicyConflict, e)
3058+
}
3059+
if !strings.Contains(e, "ConflictResolution conflicted (Overwrite vs Abort)") {
3060+
t.Fatalf("expected ConflictResolution conflicted hint in event, got %q", e)
3061+
}
3062+
if !strings.Contains(e, "PreserveResourcesOnDeletion conflicted (true vs false)") {
3063+
t.Fatalf("expected PreserveResourcesOnDeletion conflicted hint in event, got %q", e)
3064+
}
3065+
case <-time.After(1 * time.Second):
3066+
t.Fatalf("expected dependency policy conflict event, but none received")
3067+
}
3068+
}
3069+
29653070
func Test_buildAttachedBinding(t *testing.T) {
29663071
blockOwnerDeletion := true
29673072
isController := true
@@ -3432,3 +3537,63 @@ func Test_deleteBindingFromSnapshot(t *testing.T) {
34323537
})
34333538
}
34343539
}
3540+
3541+
func Test_resolveResourceBindingFromSnapshots_returnsBindings(t *testing.T) {
3542+
Scheme := runtime.NewScheme()
3543+
utilruntime.Must(scheme.AddToScheme(Scheme))
3544+
utilruntime.Must(workv1alpha2.Install(Scheme))
3545+
3546+
rb1 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "rb-1", Namespace: "ns-a"}}
3547+
rb2 := &workv1alpha2.ResourceBinding{ObjectMeta: metav1.ObjectMeta{Name: "rb-2", Namespace: "ns-b"}}
3548+
3549+
d := &DependenciesDistributor{
3550+
Client: fake.NewClientBuilder().WithScheme(Scheme).WithObjects(rb1, rb2).Build(),
3551+
}
3552+
3553+
snaps := []workv1alpha2.BindingSnapshot{
3554+
{Namespace: "ns-a", Name: "rb-1"},
3555+
{Namespace: "ns-b", Name: "rb-2"},
3556+
}
3557+
3558+
got, err := d.resolveResourceBindingFromSnapshots(context.TODO(), snaps)
3559+
if err != nil {
3560+
t.Fatalf("resolveResourceBindingFromSnapshots() error = %v", err)
3561+
}
3562+
if len(got) != 2 {
3563+
t.Fatalf("resolveResourceBindingFromSnapshots() len = %d, want 2", len(got))
3564+
}
3565+
if got[0].Name != "rb-1" || got[0].Namespace != "ns-a" {
3566+
t.Fatalf("resolveResourceBindingFromSnapshots()[0] = %s/%s, want ns-a/rb-1", got[0].Namespace, got[0].Name)
3567+
}
3568+
if got[1].Name != "rb-2" || got[1].Namespace != "ns-b" {
3569+
t.Fatalf("resolveResourceBindingFromSnapshots()[1] = %s/%s, want ns-b/rb-2", got[1].Namespace, got[1].Name)
3570+
}
3571+
}
3572+
3573+
func Test_conflictDetectedForConflictResolution_detectsConflict(t *testing.T) {
3574+
d := &DependenciesDistributor{}
3575+
3576+
rbs := []*workv1alpha2.ResourceBinding{
3577+
{Spec: workv1alpha2.ResourceBindingSpec{ConflictResolution: policyv1alpha1.ConflictOverwrite}},
3578+
{Spec: workv1alpha2.ResourceBindingSpec{}},
3579+
}
3580+
3581+
has := d.conflictDetectedForConflictResolution(rbs)
3582+
if !has {
3583+
t.Fatalf("conflictDetectedForConflictResolution() = false, want true")
3584+
}
3585+
}
3586+
3587+
func Test_conflictDetectedForPreserveOnDeletion_detectsConflict(t *testing.T) {
3588+
d := &DependenciesDistributor{}
3589+
3590+
rbs := []*workv1alpha2.ResourceBinding{
3591+
{Spec: workv1alpha2.ResourceBindingSpec{PreserveResourcesOnDeletion: ptr.To(true)}},
3592+
{Spec: workv1alpha2.ResourceBindingSpec{PreserveResourcesOnDeletion: ptr.To(false)}},
3593+
}
3594+
3595+
has := d.conflictDetectedForPreserveOnDeletion(rbs)
3596+
if !has {
3597+
t.Fatalf("conflictDetectedForPreserveOnDeletion() = false, want true")
3598+
}
3599+
}

0 commit comments

Comments
 (0)