Merge pull request #21174 from deads2k/make-patch-more-predictable

hack/test-cmd.sh

@@ -247,6 +247,7 @@ runTests() {
   secret_data=".data"
   secret_type=".type"
   deployment_image_field="(index .spec.template.spec.containers 0).image"
+  change_cause_annotation='.*kubernetes.io/change-cause.*'
 
   # Passing no arguments to create is an error
   ! kubectl create
@@ -458,9 +459,11 @@ runTests() {
 
   ## Patch pod can change image
   # Command
-  kubectl patch "${kube_flags[@]}" pod valid-pod -p='{"spec":{"containers":[{"name": "kubernetes-serve-hostname", "image": "nginx"}]}}'
+  kubectl patch "${kube_flags[@]}" pod valid-pod --record -p='{"spec":{"containers":[{"name": "kubernetes-serve-hostname", "image": "nginx"}]}}'
   # Post-condition: valid-pod POD has image nginx
   kube::test::get_object_assert pods "{{range.items}}{{$image_field}}:{{end}}" 'nginx:'
+  # Post-condition: valid-pod has the record annotation
+  kube::test::get_object_assert pods "{{range.items}}{{$annotations_field}}:{{end}}" "${change_cause_annotation}"
   # prove that patch can use different types 
   kubectl patch "${kube_flags[@]}" pod valid-pod --type="json" -p='[{"op": "replace", "path": "/spec/containers/0/image", "value":"nginx2"}]'
   # Post-condition: valid-pod POD has image nginx

pkg/kubectl/cmd/patch.go

@@ -139,18 +139,17 @@ func RunPatch(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []stri
 	}
 
 	helper := resource.NewHelper(client, mapping)
-	_, err = helper.Patch(namespace, name, patchType, patchBytes)
+	patchedObject, err := helper.Patch(namespace, name, patchType, patchBytes)
 	if err != nil {
 		return err
 	}
 	if cmdutil.ShouldRecord(cmd, info) {
-		patchBytes, err = cmdutil.ChangeResourcePatch(info, f.Command())
-		if err != nil {
-			return err
-		}
-		_, err = helper.Patch(namespace, name, api.StrategicMergePatchType, patchBytes)
-		if err != nil {
-			return err
+		if err := cmdutil.RecordChangeCause(patchedObject, f.Command()); err == nil {
+			// don't return an error on failure.  The patch itself succeeded, its only the hint for that change that failed
+			// don't bother checking for failures of this replace, because a failure to indicate the hint doesn't fail the command
+			// also, don't force the replacement.  If the replacement fails on a resourceVersion conflict, then it means this
+			// record hint is likely to be invalid anyway, so avoid the bad hint
+			resource.NewHelper(client, mapping).Replace(namespace, name, false, patchedObject)
 		}
 	}
 	cmdutil.PrintSuccess(mapper, shortOutput, out, "", name, "patched")