fix --record to work with unstructured objects

pkg/kubectl/cmd/apply.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/golang/glog"
 	"github.com/jonboulle/clockwork"
 	"github.com/spf13/cobra"
 
@@ -295,13 +296,10 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti
 			}
 
 			if cmdutil.ShouldRecord(cmd, info) {
-				patch, err := cmdutil.ChangeResourcePatch(info, f.Command())
-				if err != nil {
-					return err
-				}
-				_, err = helper.Patch(info.Namespace, info.Name, types.StrategicMergePatchType, patch)
-				if err != nil {
-					return cmdutil.AddSourceToErr(fmt.Sprintf("applying patch:\n%s\nto:\n%v\nfor:", patch, info), info.Source, err)
+				if patch, patchType, err := cmdutil.ChangeResourcePatch(info, f.Command()); err == nil {
+					if _, err = helper.Patch(info.Namespace, info.Name, patchType, patch); err != nil {
+						glog.V(4).Infof("error recording reason: %v", err)
+					}
 				}
 			}
 

pkg/kubectl/cmd/patch.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 
 	jsonpatch "github.com/evanphx/json-patch"
+	"github.com/golang/glog"
 	"github.com/spf13/cobra"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -181,14 +182,16 @@ func RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin
 			if err != nil {
 				return err
 			}
+			// Record the change as a second patch to avoid trying to merge with a user's patch data
 			if cmdutil.ShouldRecord(cmd, info) {
-				// 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
-				patch, err := cmdutil.ChangeResourcePatch(info, f.Command())
-				if err == nil {
-					helper.Patch(info.Namespace, info.Name, types.StrategicMergePatchType, patch)
+				// Copy the resource info and update with the result of applying the user's patch
+				infoCopy := *info
+				infoCopy.Object = patchedObj
+				infoCopy.VersionedObject = patchedObj
+				if patch, patchType, err := cmdutil.ChangeResourcePatch(&infoCopy, f.Command()); err == nil {
+					if _, err = helper.Patch(info.Namespace, info.Name, patchType, patch); err != nil {
+						glog.V(4).Infof("error recording reason: %v", err)
+					}
 				}
 			}
 			count++

pkg/kubectl/cmd/scale.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/spf13/cobra"
 
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
@@ -164,7 +163,7 @@ func RunScale(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin
 			return err
 		}
 		if cmdutil.ShouldRecord(cmd, info) {
-			patchBytes, err := cmdutil.ChangeResourcePatch(info, f.Command())
+			patchBytes, patchType, err := cmdutil.ChangeResourcePatch(info, f.Command())
 			if err != nil {
 				return err
 			}
@@ -174,7 +173,7 @@ func RunScale(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin
 				return err
 			}
 			helper := resource.NewHelper(client, mapping)
-			_, err = helper.Patch(info.Namespace, info.Name, types.StrategicMergePatchType, patchBytes)
+			_, err = helper.Patch(info.Namespace, info.Name, patchType, patchBytes)
 			if err != nil {
 				return err
 			}

pkg/kubectl/cmd/set/set_image.go

@@ -238,8 +238,8 @@ func (o *ImageOptions) Run() error {
 
 		// record this change (for rollout history)
 		if o.Record || cmdutil.ContainsChangeCause(info) {
-			if patch, err := cmdutil.ChangeResourcePatch(info, o.ChangeCause); err == nil {
-				if obj, err = resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, types.StrategicMergePatchType, patch); err != nil {
+			if patch, patchType, err := cmdutil.ChangeResourcePatch(info, o.ChangeCause); err == nil {
+				if obj, err = resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, patchType, patch); err != nil {
 					fmt.Fprintf(o.Err, "WARNING: changes to %s/%s can't be recorded: %v\n", info.Mapping.Resource, info.Name, err)
 				}
 			}

pkg/kubectl/cmd/util/BUILD

@@ -53,7 +53,9 @@ go_library(
         "//vendor:k8s.io/apimachinery/pkg/runtime",
         "//vendor:k8s.io/apimachinery/pkg/runtime/schema",
         "//vendor:k8s.io/apimachinery/pkg/runtime/serializer/json",
+        "//vendor:k8s.io/apimachinery/pkg/types",
         "//vendor:k8s.io/apimachinery/pkg/util/errors",
+        "//vendor:k8s.io/apimachinery/pkg/util/json",
         "//vendor:k8s.io/apimachinery/pkg/util/sets",
         "//vendor:k8s.io/apimachinery/pkg/util/strategicpatch",
         "//vendor:k8s.io/apimachinery/pkg/version",

pkg/kubectl/cmd/util/helpers.go

@@ -18,7 +18,6 @@ package util
 
 import (
 	"bytes"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -38,7 +37,9 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apimachinery/pkg/util/json"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	"k8s.io/client-go/tools/clientcmd"
@@ -520,27 +521,35 @@ func RecordChangeCause(obj runtime.Object, changeCause string) error {
 	return nil
 }
 
-// ChangeResourcePatch creates a strategic merge patch between the origin input resource info
+// ChangeResourcePatch creates a patch between the origin input resource info
 // and the annotated with change-cause input resource info.
-func ChangeResourcePatch(info *resource.Info, changeCause string) ([]byte, error) {
+func ChangeResourcePatch(info *resource.Info, changeCause string) ([]byte, types.PatchType, error) {
 	// Get a versioned object
 	obj, err := info.Mapping.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion())
 	if err != nil {
-		return nil, err
+		return nil, types.StrategicMergePatchType, err
 	}
 
 	oldData, err := json.Marshal(obj)
 	if err != nil {
-		return nil, err
+		return nil, types.StrategicMergePatchType, err
 	}
 	if err := RecordChangeCause(obj, changeCause); err != nil {
-		return nil, err
+		return nil, types.StrategicMergePatchType, err
 	}
 	newData, err := json.Marshal(obj)
 	if err != nil {
-		return nil, err
+		return nil, types.StrategicMergePatchType, err
+	}
+
+	switch obj := obj.(type) {
+	case *unstructured.Unstructured:
+		patch, err := jsonpatch.CreateMergePatch(oldData, newData)
+		return patch, types.MergePatchType, err
+	default:
+		patch, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, obj)
+		return patch, types.StrategicMergePatchType, err
 	}
-	return strategicpatch.CreateTwoWayMergePatch(oldData, newData, obj)
 }
 
 // containsChangeCause checks if input resource info contains change-cause annotation.