Skip to content

Commit 12a7281

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

File tree

3 files changed

+202
-4
lines changed

3 files changed

+202
-4
lines changed

pkg/dependenciesdistributor/dependencies_distributor.go

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"encoding/json"
2222
"fmt"
23+
"strings"
2324

2425
corev1 "k8s.io/api/core/v1"
2526
"k8s.io/apimachinery/pkg/api/equality"
@@ -49,6 +50,7 @@ import (
4950
"sigs.k8s.io/controller-runtime/pkg/source"
5051

5152
configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
53+
policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
5254
workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
5355
"github.com/karmada-io/karmada/pkg/events"
5456
"github.com/karmada-io/karmada/pkg/resourceinterpreter"
@@ -585,16 +587,49 @@ func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding
585587
"try again later after binding is garbage collected, see https://github.com/karmada-io/karmada/issues/6034", bindingKey)
586588
}
587589

590+
mergedRequiredBy := mergeBindingSnapshot(existBinding.Spec.RequiredBy, attachedBinding.Spec.RequiredBy)
591+
592+
conflictDetected := false
593+
var conflictReasons []string
594+
588595
// If the spec.Placement is nil, this means that existBinding is generated by the dependency mechanism.
589596
// If the spec.Placement is not nil, then it must be generated by PropagationPolicy.
590597
if existBinding.Spec.Placement == nil {
598+
crConflictDetected, err := d.conflictDetectedForConflictResolution(mergedRequiredBy)
599+
if err != nil {
600+
klog.Errorf("Failed to check conflict for resourceBinding(%s): %v", bindingKey, err)
601+
return err
602+
}
603+
if crConflictDetected {
604+
conflictDetected = true
605+
conflictReasons = append(conflictReasons, "ConflictResolution mismatch (Overwrite vs Abort)")
606+
}
607+
608+
preserveConflictDetected, err := d.conflictDetectedForPreserveOnDeletion(mergedRequiredBy)
609+
if err != nil {
610+
klog.Errorf("Failed to check preserveOnDeletion conflict for resourceBinding(%s): %v", bindingKey, err)
611+
return err
612+
}
613+
if preserveConflictDetected {
614+
conflictDetected = true
615+
conflictReasons = append(conflictReasons, "PreserveResourcesOnDeletion mismatch (true vs false)")
616+
}
617+
591618
existBinding.Spec.ConflictResolution = attachedBinding.Spec.ConflictResolution
592619
}
593-
existBinding.Spec.RequiredBy = mergeBindingSnapshot(existBinding.Spec.RequiredBy, attachedBinding.Spec.RequiredBy)
620+
621+
existBinding.Spec.RequiredBy = mergedRequiredBy
594622
existBinding.Labels = util.DedupeAndMergeLabels(existBinding.Labels, attachedBinding.Labels)
595623
existBinding.Spec.Resource = attachedBinding.Spec.Resource
596624
existBinding.Spec.PreserveResourcesOnDeletion = attachedBinding.Spec.PreserveResourcesOnDeletion
597625

626+
if conflictDetected && d.EventRecorder != nil {
627+
// Emit a warning event indicating which policies conflicted.
628+
// Use placeholders to include the concrete conflicting dimensions in the message.
629+
message := "Dependency policy conflict detected: %s."
630+
d.EventRecorder.Eventf(existBinding, corev1.EventTypeWarning, events.EventReasonDependencyPolicyConflict, message, strings.Join(conflictReasons, "; "))
631+
}
632+
598633
if err := d.Client.Update(context.TODO(), existBinding); err != nil {
599634
klog.Errorf("Failed to update resourceBinding(%s): %v", bindingKey, err)
600635
return err
@@ -610,6 +645,54 @@ func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding
610645
return d.Client.Create(context.TODO(), attachedBinding)
611646
}
612647

648+
// conflictDetectedForConflictResolution checks if there is a conflict in ConflictResolution from referencing ResourceBindings.
649+
// Rules: any Overwrite -> Overwrite; else Abort. Returns whether a conflict was detected.
650+
func (d *DependenciesDistributor) conflictDetectedForConflictResolution(requiredBy []workv1alpha2.BindingSnapshot) (hasConflict bool, err error) {
651+
hasOverwrite := false
652+
hasAbort := false
653+
654+
for _, snap := range requiredBy {
655+
refRB := &workv1alpha2.ResourceBinding{}
656+
if err := d.Client.Get(context.TODO(), client.ObjectKey{Namespace: snap.Namespace, Name: snap.Name}, refRB); err != nil {
657+
return false, err
658+
}
659+
660+
cr := effectiveConflictResolution(refRB.Spec.ConflictResolution)
661+
if cr == policyv1alpha1.ConflictOverwrite {
662+
hasOverwrite = true
663+
} else {
664+
hasAbort = true
665+
}
666+
}
667+
668+
hasConflict = hasOverwrite && hasAbort
669+
return hasConflict, nil
670+
}
671+
672+
// conflictDetectedForPreserveOnDeletion checks if there is a conflict in PreserveResourcesOnDeletion from referencing ResourceBindings.
673+
// Rules: any true -> true; else false. Returns whether a conflict was detected.
674+
func (d *DependenciesDistributor) conflictDetectedForPreserveOnDeletion(requiredBy []workv1alpha2.BindingSnapshot) (hasConflict bool, err error) {
675+
seenTrue := false
676+
seenFalse := false
677+
678+
for _, snap := range requiredBy {
679+
refRB := &workv1alpha2.ResourceBinding{}
680+
if err := d.Client.Get(context.TODO(), client.ObjectKey{Namespace: snap.Namespace, Name: snap.Name}, refRB); err != nil {
681+
return false, err
682+
}
683+
684+
pres := ptr.Deref(refRB.Spec.PreserveResourcesOnDeletion, false)
685+
if pres {
686+
seenTrue = true
687+
} else {
688+
seenFalse = true
689+
}
690+
}
691+
692+
hasConflict = seenTrue && seenFalse
693+
return hasConflict, nil
694+
}
695+
613696
// Start runs the distributor, never stop until context canceled.
614697
func (d *DependenciesDistributor) Start(ctx context.Context) error {
615698
klog.Infof("Starting dependencies distributor.")
@@ -684,6 +767,14 @@ func (d *DependenciesDistributor) SetupWithManager(mgr controllerruntime.Manager
684767
})
685768
}
686769

770+
func effectiveConflictResolution(value policyv1alpha1.ConflictResolution) policyv1alpha1.ConflictResolution {
771+
if value == "" {
772+
return policyv1alpha1.ConflictAbort
773+
}
774+
775+
return value
776+
}
777+
687778
func generateBindingDependedLabels(bindingID, bindingNamespace, bindingName string) map[string]string {
688779
labelKey := generateBindingDependedLabelKey(bindingNamespace, bindingName)
689780
return map[string]string{labelKey: bindingID}

pkg/dependenciesdistributor/dependencies_distributor_test.go

Lines changed: 108 additions & 3 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
}
@@ -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(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 mismatch (Overwrite vs Abort)") {
3060+
t.Fatalf("expected ConflictResolution mismatch hint in event, got %q", e)
3061+
}
3062+
if !strings.Contains(e, "PreserveResourcesOnDeletion mismatch (true vs false)") {
3063+
t.Fatalf("expected PreserveResourcesOnDeletion mismatch 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

pkg/events/events.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ const (
6666
EventReasonSyncScheduleResultToDependenciesSucceed = "SyncScheduleResultToDependenciesSucceed"
6767
// EventReasonSyncScheduleResultToDependenciesFailed indicates sync schedule result to attached bindings failed.
6868
EventReasonSyncScheduleResultToDependenciesFailed = "SyncScheduleResultToDependenciesFailed"
69+
// EventReasonDependencyPolicyConflict indicates a dependency policy conflict was detected.
70+
EventReasonDependencyPolicyConflict = "DependencyPolicyConflict"
6971
)
7072

7173
// Define events for ResourceBinding, ClusterResourceBinding objects and their associated resources.

0 commit comments

Comments
 (0)