Merge pull request #42327 from deads2k/api-06-debug

Automatic merge from submit-queue

show patch error as string not byte slice

Debugging for https://github.com/kubernetes/kubernetes/issues/39471 shows

```
client_test.go:309: Failed updating patchpod with patch type application/json-patch+json: Operation cannot be fulfilled on pods "patchpod": there is a meaningful conflict:
	 diff1=[123 34 109 101 116 97 100 97 116 97 34 58 123 34 108 97 98 101 108 115 34 58 123 34 102 111 111 34 58 110 117 108 108 125 44 34 114 101 115 111 117 114 99 101 86 101 114 115 105 111 110 34 58 34 49 52 50 50 34 125 125]
	, diff2=[123 34 109 101 116 97 100 97 116 97 34 58 123 34 108 97 98 101 108 115 34 58 110 117 108 108 125 125]
```

I don't speak byte slice, so this makes it a string for next time.

@sttts

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go

@@ -570,6 +570,7 @@ func patchResource(
 		originalObjMap          map[string]interface{}
 		originalPatchMap        map[string]interface{}
 		lastConflictErr         error
+		originalResourceVersion string
 	)
 
 	// applyPatch is called every time GuaranteedUpdate asks for the updated object,
@@ -582,12 +583,18 @@ func patchResource(
 			return nil, errors.NewNotFound(resource.GroupResource(), name)
 		}
 
+		currentResourceVersion := ""
+		if currentMetadata, err := meta.Accessor(currentObject); err == nil {
+			currentResourceVersion = currentMetadata.GetResourceVersion()
+		}
+
 		switch {
 		case originalObjJS == nil && originalObjMap == nil:
 			// first time through,
 			// 1. apply the patch
 			// 2. save the original and patched to detect whether there were conflicting changes on retries
 
+			originalResourceVersion = currentResourceVersion
 			objToUpdate := patcher.New()
 
 			// For performance reasons, in case of strategicpatch, we avoid json
@@ -690,11 +697,12 @@ func patchResource(
 			if err != nil {
 				return nil, err
 			}
+
 			if hasConflicts {
 				diff1, _ := json.Marshal(currentPatchMap)
 				diff2, _ := json.Marshal(originalPatchMap)
-				patchDiffErr := fmt.Errorf("there is a meaningful conflict:\n diff1=%v\n, diff2=%v\n", diff1, diff2)
+				patchDiffErr := fmt.Errorf("there is a meaningful conflict (firstResourceVersion: %q, currentResourceVersion: %q):\n diff1=%v\n, diff2=%v\n", originalResourceVersion, currentResourceVersion, string(diff1), string(diff2))
-				glog.V(4).Infof("patchResource failed for resource %s, because there is a meaningful conflict.\n diff1=%v\n, diff2=%v\n", name, diff1, diff2)
+				glog.V(4).Infof("patchResource failed for resource %s, because there is a meaningful conflict(firstResourceVersion: %q, currentResourceVersion: %q):\n diff1=%v\n, diff2=%v\n", name, originalResourceVersion, currentResourceVersion, string(diff1), string(diff2))
 
 				// Return the last conflict error we got if we have one
 				if lastConflictErr != nil {

test/integration/client/client_test.go

@@ -268,14 +268,26 @@ func TestPatch(t *testing.T) {
 	pb := patchBodies[c.Core().RESTClient().APIVersion()]
 
 	execPatch := func(pt types.PatchType, body []byte) error {
-		return c.Core().RESTClient().Patch(pt).
+		result := c.Core().RESTClient().Patch(pt).
 			Resource(resource).
 			Namespace(ns.Name).
 			Name(name).
 			Body(body).
-			Do().
+			Do()
-			Error()
+		if result.Error() != nil {
+			return result.Error()
 		}
+
+		// trying to chase flakes, this should give us resource versions of objects as we step through
+		jsonObj, err := result.Raw()
+		if err != nil {
+			t.Log(err)
+		} else {
+			t.Logf("%v", string(jsonObj))
+		}
+		return nil
+	}
+
 	for k, v := range pb {
 		// add label
 		err := execPatch(k, v.AddLabelBody)