From 408a978f26bc3996cadc4450810d7116aecb6266 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Sun, 22 Jan 2017 21:15:10 -0500 Subject: [PATCH] fix --record to work with unstructured objects --- pkg/kubectl/cmd/apply.go | 12 +++++------- pkg/kubectl/cmd/patch.go | 17 ++++++++++------- pkg/kubectl/cmd/scale.go | 5 ++--- pkg/kubectl/cmd/set/set_image.go | 4 ++-- pkg/kubectl/cmd/util/BUILD | 2 ++ pkg/kubectl/cmd/util/helpers.go | 25 +++++++++++++++++-------- 6 files changed, 38 insertions(+), 27 deletions(-) diff --git a/pkg/kubectl/cmd/apply.go b/pkg/kubectl/cmd/apply.go index 2c69cf1cbc5..07c2393023f 100644 --- a/pkg/kubectl/cmd/apply.go +++ b/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) + } } } diff --git a/pkg/kubectl/cmd/patch.go b/pkg/kubectl/cmd/patch.go index 19b2c0b4257..3e0f55342fa 100644 --- a/pkg/kubectl/cmd/patch.go +++ b/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++ diff --git a/pkg/kubectl/cmd/scale.go b/pkg/kubectl/cmd/scale.go index ff34ec50b8c..ef811f04fa6 100644 --- a/pkg/kubectl/cmd/scale.go +++ b/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 } diff --git a/pkg/kubectl/cmd/set/set_image.go b/pkg/kubectl/cmd/set/set_image.go index b079418edc1..8b43902353c 100644 --- a/pkg/kubectl/cmd/set/set_image.go +++ b/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) } } diff --git a/pkg/kubectl/cmd/util/BUILD b/pkg/kubectl/cmd/util/BUILD index 7f70692b427..7df57bacab3 100644 --- a/pkg/kubectl/cmd/util/BUILD +++ b/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", diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go index 5d4e0c83363..bc92403cc26 100644 --- a/pkg/kubectl/cmd/util/helpers.go +++ b/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.