diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 00bebdd2d56..32546118f41 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -330,7 +330,11 @@ func patchResource( return nil, err } - hasConflicts, err := mergepatch.HasConflicts(originalPatchMap, currentPatchMap) + patchMetaFromStruct, err := strategicpatch.NewPatchMetaFromStruct(versionedObj) + if err != nil { + return nil, err + } + hasConflicts, err := strategicpatch.MergingMapsHaveConflicts(originalPatchMap, currentPatchMap, patchMetaFromStruct) if err != nil { return nil, err } diff --git a/test/integration/apiserver/patch_test.go b/test/integration/apiserver/patch_test.go index 56bf90919c9..971d94c25d8 100644 --- a/test/integration/apiserver/patch_test.go +++ b/test/integration/apiserver/patch_test.go @@ -24,8 +24,6 @@ import ( "github.com/pborman/uuid" - "reflect" - "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -43,14 +41,30 @@ func TestPatchConflicts(t *testing.T) { ns := framework.CreateTestingNamespace("status-code", s, t) defer framework.DeleteTestingNamespace(ns, s, t) - // Create the object we're going to conflict on - clientSet.CoreV1().Secrets(ns.Name).Create(&v1.Secret{ + numOfConcurrentPatches := 2 * handlers.MaxRetryWhenPatchConflicts + + UIDs := make([]types.UID, numOfConcurrentPatches) + ownerRefs := []metav1.OwnerReference{} + for i := 0; i < numOfConcurrentPatches; i++ { + uid := types.UID(uuid.NewRandom().String()) + ownerName := fmt.Sprintf("owner-%d", i) + UIDs[i] = uid + ownerRefs = append(ownerRefs, metav1.OwnerReference{ + APIVersion: "example.com/v1", + Kind: "Foo", + Name: ownerName, + UID: uid, + }) + } + secret := &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "test", - // Populate annotations so the strategic patch descends, compares, and notices the $patch directive - Annotations: map[string]string{"initial": "value"}, + Name: "test", + OwnerReferences: ownerRefs, }, - }) + } + + // Create the object we're going to conflict on + clientSet.CoreV1().Secrets(ns.Name).Create(secret) client := clientSet.CoreV1().RESTClient() successes := int32(0) @@ -60,11 +74,10 @@ func TestPatchConflicts(t *testing.T) { // If the resource version of the object changed between attempts, that means another one of our patch requests succeeded. // That means if we run 2*MaxRetryWhenPatchConflicts patch attempts, we should see at least MaxRetryWhenPatchConflicts succeed. wg := sync.WaitGroup{} - for i := 0; i < (2 * handlers.MaxRetryWhenPatchConflicts); i++ { + for i := 0; i < numOfConcurrentPatches; i++ { wg.Add(1) go func(i int) { defer wg.Done() - annotationName := fmt.Sprintf("annotation-%d", i) labelName := fmt.Sprintf("label-%d", i) value := uuid.NewRandom().String() @@ -72,7 +85,7 @@ func TestPatchConflicts(t *testing.T) { Namespace(ns.Name). Resource("secrets"). Name("test"). - Body([]byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"%s"}, "annotations":{"$patch":"replace","%s":"%s"}}}`, labelName, value, annotationName, value))). + Body([]byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"%s"}, "ownerReferences":[{"$patch":"delete","uid":"%s"}]}}`, labelName, value, UIDs[i]))). Do(). Get() @@ -95,9 +108,14 @@ func TestPatchConflicts(t *testing.T) { t.Errorf("patch of %s was ineffective, expected %s=%s, got labels %#v", "secrets", labelName, value, accessor.GetLabels()) return } - // make sure the patch directive didn't get lost, and that the entire annotation map was replaced - if !reflect.DeepEqual(accessor.GetAnnotations(), map[string]string{annotationName: value}) { - t.Errorf("patch of %s with $patch directive was ineffective, didn't replace entire annotations map: %#v", "secrets", accessor.GetAnnotations()) + // make sure the patch directive didn't get lost, and that an entry in the ownerReference list was deleted. + found := findOwnerRefByUID(accessor.GetOwnerReferences(), UIDs[i]) + if err != nil { + t.Errorf("%v", err) + return + } + if found { + t.Errorf("patch of %s with $patch directive was ineffective, didn't delete the entry in the ownerReference slice: %#v", "secrets", UIDs[i]) } atomic.AddInt32(&successes, 1) @@ -112,3 +130,12 @@ func TestPatchConflicts(t *testing.T) { } } + +func findOwnerRefByUID(ownerRefs []metav1.OwnerReference, uid types.UID) bool { + for _, of := range ownerRefs { + if of.UID == uid { + return true + } + } + return false +}