Skip to content

Commit

Permalink
fix the attached binding deletion problem
Browse files Browse the repository at this point in the history
Signed-off-by: whitewindmills <[email protected]>
  • Loading branch information
whitewindmills committed Jan 13, 2025
1 parent be674c7 commit 8699d21
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 0 deletions.
7 changes: 7 additions & 0 deletions pkg/dependenciesdistributor/dependencies_distributor.go
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,13 @@ func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding
bindingKey := client.ObjectKeyFromObject(attachedBinding)
err := d.Client.Get(context.TODO(), bindingKey, existBinding)
if err == nil {
// If this binding exists and its owner is not the input object, return error and let garbage collector
// delete this binding and try again later. See https://github.com/karmada-io/karmada/issues/2090.
if ownerRef := metav1.GetControllerOfNoCopy(existBinding); ownerRef != nil && ownerRef.UID != attachedBinding.OwnerReferences[0].UID {
return fmt.Errorf("failed to update resourceBinding(%s) due to different owner reference UID, will "+
"try again later after binding is garbage collected, see https://github.com/karmada-io/karmada/issues/2090", bindingKey)
}

// If the spec.Placement is nil, this means that existBinding is generated by the dependency mechanism.
// If the spec.Placement is not nil, then it must be generated by PropagationPolicy.
if existBinding.Spec.Placement == nil {
Expand Down
49 changes: 49 additions & 0 deletions pkg/dependenciesdistributor/dependencies_distributor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"k8s.io/client-go/dynamic"
dynamicfake "k8s.io/client-go/dynamic/fake"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/utils/pointer"

Check failure on line 37 in pkg/dependenciesdistributor/dependencies_distributor_test.go

View workflow job for this annotation

GitHub Actions / lint

SA1019: "k8s.io/utils/pointer" is deprecated: Use functions in k8s.io/utils/ptr instead: ptr.To to obtain a pointer, ptr.Deref to dereference a pointer, ptr.Equal to compare dereferenced pointers. (staticcheck)
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/event"
Expand Down Expand Up @@ -2224,6 +2225,12 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
Namespace: "test",
ResourceVersion: "1000",
Labels: map[string]string{"app": "nginx"},
OwnerReferences: []metav1.OwnerReference{
{
UID: "foo-bar",
Controller: pointer.Bool(true),
},
},
},
Spec: workv1alpha2.ResourceBindingSpec{
Resource: workv1alpha2.ObjectReference{
Expand Down Expand Up @@ -2274,6 +2281,12 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
Namespace: "test",
ResourceVersion: "1001",
Labels: map[string]string{"app": "nginx", "foo": "bar"},
OwnerReferences: []metav1.OwnerReference{
{
UID: "foo-bar",
Controller: pointer.Bool(true),
},
},
},
Spec: workv1alpha2.ResourceBindingSpec{
Resource: workv1alpha2.ObjectReference{
Expand Down Expand Up @@ -2344,6 +2357,12 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
Namespace: "test",
ResourceVersion: "1000",
Labels: map[string]string{"foo": "bar"},
OwnerReferences: []metav1.OwnerReference{
{
UID: "foo-bar",
Controller: pointer.Bool(true),
},
},
},
Spec: workv1alpha2.ResourceBindingSpec{
RequiredBy: []workv1alpha2.BindingSnapshot{
Expand Down Expand Up @@ -2390,6 +2409,12 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
Name: "test-binding",
Namespace: "test",
Labels: map[string]string{"app": "nginx"},
OwnerReferences: []metav1.OwnerReference{
{
UID: "foo-bar",
Controller: pointer.Bool(true),
},
},
},
Spec: workv1alpha2.ResourceBindingSpec{
Resource: workv1alpha2.ObjectReference{
Expand Down Expand Up @@ -2440,6 +2465,12 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
Namespace: "test",
ResourceVersion: "1",
Labels: map[string]string{"app": "nginx"},
OwnerReferences: []metav1.OwnerReference{
{
UID: "foo-bar",
Controller: pointer.Bool(true),
},
},
},
Spec: workv1alpha2.ResourceBindingSpec{
Resource: workv1alpha2.ObjectReference{
Expand Down Expand Up @@ -2495,6 +2526,12 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
Namespace: "test",
ResourceVersion: "1000",
Labels: map[string]string{"app": "nginx"},
OwnerReferences: []metav1.OwnerReference{
{
UID: "foo-bar",
Controller: pointer.Bool(true),
},
},
},
Spec: workv1alpha2.ResourceBindingSpec{
Resource: workv1alpha2.ObjectReference{
Expand Down Expand Up @@ -2546,6 +2583,12 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
Namespace: "test",
ResourceVersion: "1001",
Labels: map[string]string{"app": "nginx", "foo": "bar"},
OwnerReferences: []metav1.OwnerReference{
{
UID: "foo-bar",
Controller: pointer.Bool(true),
},
},
},
Spec: workv1alpha2.ResourceBindingSpec{
Resource: workv1alpha2.ObjectReference{
Expand Down Expand Up @@ -2617,6 +2660,12 @@ func Test_createOrUpdateAttachedBinding(t *testing.T) {
Namespace: "test",
ResourceVersion: "1000",
Labels: map[string]string{"foo": "bar"},
OwnerReferences: []metav1.OwnerReference{
{
UID: "foo-bar",
Controller: pointer.Bool(true),
},
},
},
Spec: workv1alpha2.ResourceBindingSpec{
RequiredBy: []workv1alpha2.BindingSnapshot{
Expand Down

0 comments on commit 8699d21

Please sign in to comment.