From d248843b65479e0ce0624d5bf71f34ffc2320ee2 Mon Sep 17 00:00:00 2001 From: ymqytw Date: Tue, 22 Nov 2016 21:02:30 -0800 Subject: [PATCH] Revert "try old patch after new patch fails" This reverts commit f32696e7349c417526d9f243fd90960fc24b60ae. --- pkg/client/record/events_cache.go | 5 ++-- .../statusupdater/node_status_updater.go | 7 +++-- pkg/kubectl/cmd/annotate.go | 9 ++++-- pkg/kubectl/cmd/apply.go | 30 +++++++++---------- pkg/kubectl/cmd/edit.go | 22 ++++++-------- pkg/kubectl/cmd/label.go | 11 +++++-- pkg/kubectl/cmd/patch.go | 10 ++++++- pkg/kubectl/cmd/rollout/BUILD | 1 - pkg/kubectl/cmd/rollout/rollout_pause.go | 4 +-- pkg/kubectl/cmd/rollout/rollout_resume.go | 4 +-- pkg/kubectl/cmd/scale.go | 7 ++++- pkg/kubectl/cmd/set/helper.go | 11 ++++++- pkg/kubectl/cmd/set/set_image.go | 14 +++++++-- pkg/kubectl/cmd/set/set_resources.go | 4 +-- pkg/kubectl/cmd/taint.go | 8 +++-- pkg/kubectl/cmd/util/helpers.go | 15 ++++++++-- pkg/util/strategicpatch/BUILD | 1 + pkg/util/strategicpatch/patch.go | 18 +++++++++++ 18 files changed, 123 insertions(+), 58 deletions(-) diff --git a/pkg/client/record/events_cache.go b/pkg/client/record/events_cache.go index 5d988e237a2..6b2d6429e76 100644 --- a/pkg/client/record/events_cache.go +++ b/pkg/client/record/events_cache.go @@ -244,8 +244,9 @@ func (e *eventLogger) eventObserve(newEvent *api.Event) (*api.Event, []byte, err newData, _ := json.Marshal(event) oldData, _ := json.Marshal(eventCopy2) - // Defaulting to SMPatchVersion_1_5 is safe, since we only update Count and LastTimestamp, and none of them has list of primitives - patch, err = strategicpatch.CreateStrategicMergePatch(oldData, newData, event, strategicpatch.SMPatchVersion_1_5) + // TODO: need to figure out if we need to let eventObserve() use the new behavior of StrategicMergePatch. + // Currently default to old behavior now. Ref: issue #35936 + patch, err = strategicpatch.CreateStrategicMergePatch(oldData, newData, event, strategicpatch.SMPatchVersion_1_0) } // record our new observation diff --git a/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go b/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go index 32a9e1f46bd..ec58e8e5b10 100644 --- a/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go +++ b/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go @@ -59,6 +59,10 @@ type nodeStatusUpdater struct { } func (nsu *nodeStatusUpdater) UpdateNodeStatuses() error { + smPatchVersion, err := strategicpatch.GetServerSupportedSMPatchVersion(nsu.kubeClient.Discovery()) + if err != nil { + return err + } nodesToUpdate := nsu.actualStateOfWorld.GetVolumesToReportAttached() for nodeName, attachedVolumes := range nodesToUpdate { nodeObj, exists, err := nsu.nodeInformer.GetStore().GetByKey(string(nodeName)) @@ -107,9 +111,8 @@ func (nsu *nodeStatusUpdater) UpdateNodeStatuses() error { err) } - // Defaulting to SMPatchVersion_1_5 is safe, since updateNodeStatus doesn't update any lists of primitives patchBytes, err := - strategicpatch.CreateStrategicMergePatch(oldData, newData, node, strategicpatch.SMPatchVersion_1_5) + strategicpatch.CreateStrategicMergePatch(oldData, newData, node, smPatchVersion) if err != nil { return fmt.Errorf( "failed to CreateStrategicMergePatch for node %q. %v", diff --git a/pkg/kubectl/cmd/annotate.go b/pkg/kubectl/cmd/annotate.go index 552b13abed9..60ea3786a62 100644 --- a/pkg/kubectl/cmd/annotate.go +++ b/pkg/kubectl/cmd/annotate.go @@ -223,6 +223,12 @@ func (o AnnotateOptions) RunAnnotate(f cmdutil.Factory, cmd *cobra.Command) erro } outputObj = obj } else { + // retrieves server version to determine which SMPatchVersion to use. + smPatchVersion, err := cmdutil.GetServerSupportedSMPatchVersionFromFactory(f) + if err != nil { + return err + } + name, namespace := info.Name, info.Namespace oldData, err := json.Marshal(obj) if err != nil { @@ -239,8 +245,7 @@ func (o AnnotateOptions) RunAnnotate(f cmdutil.Factory, cmd *cobra.Command) erro if err != nil { return err } - // Defaulting to SMPatchVersion_1_5 is safe, since it just update the annotation which is a map[string]string - patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, obj, strategicpatch.SMPatchVersion_1_5) + patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, obj, smPatchVersion) createdPatch := err == nil if err != nil { glog.V(2).Infof("couldn't compute patch: %v", err) diff --git a/pkg/kubectl/cmd/apply.go b/pkg/kubectl/cmd/apply.go index 584ccaf9e09..4afdf55dcd1 100644 --- a/pkg/kubectl/cmd/apply.go +++ b/pkg/kubectl/cmd/apply.go @@ -195,6 +195,11 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *App visitedUids := sets.NewString() visitedNamespaces := sets.NewString() + smPatchVersion, err := cmdutil.GetServerSupportedSMPatchVersionFromFactory(f) + if err != nil { + return err + } + count := 0 err = r.Visit(func(info *resource.Info, err error) error { // In this method, info.Object contains the object retrieved from the server @@ -265,13 +270,13 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *App gracePeriod: options.GracePeriod, } - patchBytes, err := patcher.patch(info.Object, modified, info.Source, info.Namespace, info.Name) + patchBytes, err := patcher.patch(info.Object, modified, info.Source, info.Namespace, info.Name, smPatchVersion) if err != nil { return cmdutil.AddSourceToErr(fmt.Sprintf("applying patch:\n%s\nto:\n%v\nfor:", patchBytes, info), info.Source, err) } if cmdutil.ShouldRecord(cmd, info) { - patch, err := cmdutil.ChangeResourcePatch(info, f.Command()) + patch, err := cmdutil.ChangeResourcePatch(info, f.Command(), smPatchVersion) if err != nil { return err } @@ -507,7 +512,7 @@ type patcher struct { gracePeriod int } -func (p *patcher) patchSimple(obj runtime.Object, modified []byte, source, namespace, name string) ([]byte, error) { +func (p *patcher) patchSimple(obj runtime.Object, modified []byte, source, namespace, name string, smPatchVersion strategicpatch.StrategicMergePatchVersion) ([]byte, error) { // Serialize the current configuration of the object from the server. current, err := runtime.Encode(p.encoder, obj) if err != nil { @@ -531,27 +536,20 @@ func (p *patcher) patchSimple(obj runtime.Object, modified []byte, source, names } // Compute a three way strategic merge patch to send to server. - patch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, versionedObject, p.overwrite, strategicpatch.SMPatchVersion_1_5) + patch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, versionedObject, p.overwrite, smPatchVersion) + if err != nil { format := "creating patch with:\noriginal:\n%s\nmodified:\n%s\ncurrent:\n%s\nfor:" return nil, cmdutil.AddSourceToErr(fmt.Sprintf(format, original, modified, current), source, err) } + _, err = p.helper.Patch(namespace, name, api.StrategicMergePatchType, patch) - if err != nil { - // Retry SMPatchVersion_1_0 when applying the SMPatchVersion_1_5 patch - patch, err = strategicpatch.CreateThreeWayMergePatch(original, modified, current, versionedObject, p.overwrite, strategicpatch.SMPatchVersion_1_0) - if err != nil { - format := "creating patch with:\noriginal:\n%s\nmodified:\n%s\ncurrent:\n%s\nfor:" - return nil, cmdutil.AddSourceToErr(fmt.Sprintf(format, original, modified, current), source, err) - } - _, err = p.helper.Patch(namespace, name, api.StrategicMergePatchType, patch) - } return patch, err } -func (p *patcher) patch(current runtime.Object, modified []byte, source, namespace, name string) ([]byte, error) { +func (p *patcher) patch(current runtime.Object, modified []byte, source, namespace, name string, smPatchVersion strategicpatch.StrategicMergePatchVersion) ([]byte, error) { var getErr error - patchBytes, err := p.patchSimple(current, modified, source, namespace, name) + patchBytes, err := p.patchSimple(current, modified, source, namespace, name, smPatchVersion) for i := 1; i <= maxPatchRetry && errors.IsConflict(err); i++ { if i > triesBeforeBackOff { p.backOff.Sleep(backOffPeriod) @@ -560,7 +558,7 @@ func (p *patcher) patch(current runtime.Object, modified []byte, source, namespa if getErr != nil { return nil, getErr } - patchBytes, err = p.patchSimple(current, modified, source, namespace, name) + patchBytes, err = p.patchSimple(current, modified, source, namespace, name, smPatchVersion) } if err != nil && p.force { patchBytes, err = p.deleteAndCreate(modified, namespace, name) diff --git a/pkg/kubectl/cmd/edit.go b/pkg/kubectl/cmd/edit.go index 14814110018..8eaa67c3b22 100644 --- a/pkg/kubectl/cmd/edit.go +++ b/pkg/kubectl/cmd/edit.go @@ -424,8 +424,13 @@ func visitToPatch(originalObj runtime.Object, updates *resource.Info, results *editResults, file string) error { + smPatchVersion, err := cmdutil.GetServerSupportedSMPatchVersionFromFactory(f) + if err != nil { + return err + } + patchVisitor := resource.NewFlattenListVisitor(updates, resourceMapper) - err := patchVisitor.Visit(func(info *resource.Info, incomingErr error) error { + err = patchVisitor.Visit(func(info *resource.Info, incomingErr error) error { currOriginalObj := originalObj // if we're editing a list, then navigate the list to find the item that we're currently trying to edit @@ -486,7 +491,7 @@ func visitToPatch(originalObj runtime.Object, updates *resource.Info, preconditions := []strategicpatch.PreconditionFunc{strategicpatch.RequireKeyUnchanged("apiVersion"), strategicpatch.RequireKeyUnchanged("kind"), strategicpatch.RequireMetadataKeyUnchanged("name")} - patch, err := strategicpatch.CreateTwoWayMergePatch(originalJS, editedJS, currOriginalObj, strategicpatch.SMPatchVersion_1_5, preconditions...) + patch, err := strategicpatch.CreateTwoWayMergePatch(originalJS, editedJS, currOriginalObj, smPatchVersion, preconditions...) if err != nil { glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err) if strategicpatch.IsPreconditionFailed(err) { @@ -498,17 +503,8 @@ func visitToPatch(originalObj runtime.Object, updates *resource.Info, results.version = defaultVersion patched, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, api.StrategicMergePatchType, patch) if err != nil { - // Retry SMPatchVersion_1_0 when applying the SMPatchVersion_1_5 patch - patch, err = strategicpatch.CreateTwoWayMergePatch(originalJS, editedJS, currOriginalObj, strategicpatch.SMPatchVersion_1_0) - if err != nil { - glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err) - return err - } - patched, err = resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, api.StrategicMergePatchType, patch) - if err != nil { - fmt.Fprintln(out, results.addError(err, info)) - return nil - } + fmt.Fprintln(out, results.addError(err, info)) + return nil } info.Refresh(patched, true) cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, false, "edited") diff --git a/pkg/kubectl/cmd/label.go b/pkg/kubectl/cmd/label.go index d310be71f64..c5836b4d5ad 100644 --- a/pkg/kubectl/cmd/label.go +++ b/pkg/kubectl/cmd/label.go @@ -192,6 +192,14 @@ func (o *LabelOptions) RunLabel(f cmdutil.Factory, cmd *cobra.Command) error { return err } + smPatchVersion := strategicpatch.SMPatchVersionLatest + if !o.local { + smPatchVersion, err = cmdutil.GetServerSupportedSMPatchVersionFromFactory(f) + if err != nil { + return err + } + } + // only apply resource version locking on a single resource if !one && len(o.resourceVersion) > 0 { return fmt.Errorf("--resource-version may only be used with a single resource") @@ -246,8 +254,7 @@ func (o *LabelOptions) RunLabel(f cmdutil.Factory, cmd *cobra.Command) error { if !reflect.DeepEqual(oldData, newData) { dataChangeMsg = "labeled" } - // Defaulting to SMPatchVersion_1_5 is safe, since we only update labels and change cause, and none of them has list of primitives - patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, obj, strategicpatch.SMPatchVersion_1_5) + patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, obj, smPatchVersion) createdPatch := err == nil if err != nil { glog.V(2).Infof("couldn't compute patch: %v", err) diff --git a/pkg/kubectl/cmd/patch.go b/pkg/kubectl/cmd/patch.go index 6debf1f8624..7552bf68a4d 100644 --- a/pkg/kubectl/cmd/patch.go +++ b/pkg/kubectl/cmd/patch.go @@ -154,6 +154,14 @@ func RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin return err } + smPatchVersion := strategicpatch.SMPatchVersionLatest + if !options.Local { + smPatchVersion, err = cmdutil.GetServerSupportedSMPatchVersionFromFactory(f) + if err != nil { + return err + } + } + count := 0 err = r.Visit(func(info *resource.Info, err error) error { if err != nil { @@ -177,7 +185,7 @@ func RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin // 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()) + patch, err := cmdutil.ChangeResourcePatch(info, f.Command(), smPatchVersion) if err == nil { helper.Patch(info.Namespace, info.Name, api.StrategicMergePatchType, patch) } diff --git a/pkg/kubectl/cmd/rollout/BUILD b/pkg/kubectl/cmd/rollout/BUILD index 999e05e10b4..d90722d5113 100644 --- a/pkg/kubectl/cmd/rollout/BUILD +++ b/pkg/kubectl/cmd/rollout/BUILD @@ -32,7 +32,6 @@ go_library( "//pkg/runtime:go_default_library", "//pkg/util/errors:go_default_library", "//pkg/util/interrupt:go_default_library", - "//pkg/util/strategicpatch:go_default_library", "//pkg/watch:go_default_library", "//vendor:github.com/renstrom/dedent", "//vendor:github.com/spf13/cobra", diff --git a/pkg/kubectl/cmd/rollout/rollout_pause.go b/pkg/kubectl/cmd/rollout/rollout_pause.go index 427b8b7e642..3c4a32778c6 100644 --- a/pkg/kubectl/cmd/rollout/rollout_pause.go +++ b/pkg/kubectl/cmd/rollout/rollout_pause.go @@ -31,7 +31,6 @@ import ( "k8s.io/kubernetes/pkg/kubectl/resource" "k8s.io/kubernetes/pkg/runtime" utilerrors "k8s.io/kubernetes/pkg/util/errors" - "k8s.io/kubernetes/pkg/util/strategicpatch" ) // PauseConfig is the start of the data required to perform the operation. As new fields are added, add them here instead of @@ -135,8 +134,7 @@ func (o *PauseConfig) CompletePause(f cmdutil.Factory, cmd *cobra.Command, out i func (o PauseConfig) RunPause() error { allErrs := []error{} - // Defaulting to SMPatchVersion_1_5 is safe, since Pauser only update a boolean variable - for _, patch := range set.CalculatePatches(o.f, o.Infos, o.Encoder, strategicpatch.SMPatchVersion_1_5, o.Pauser) { + for _, patch := range set.CalculatePatches(o.f, o.Infos, o.Encoder, false, o.Pauser) { info := patch.Info if patch.Err != nil { allErrs = append(allErrs, fmt.Errorf("error: %s %q %v", info.Mapping.Resource, info.Name, patch.Err)) diff --git a/pkg/kubectl/cmd/rollout/rollout_resume.go b/pkg/kubectl/cmd/rollout/rollout_resume.go index af28cba335f..f247f19ff26 100644 --- a/pkg/kubectl/cmd/rollout/rollout_resume.go +++ b/pkg/kubectl/cmd/rollout/rollout_resume.go @@ -31,7 +31,6 @@ import ( "k8s.io/kubernetes/pkg/kubectl/resource" "k8s.io/kubernetes/pkg/runtime" utilerrors "k8s.io/kubernetes/pkg/util/errors" - "k8s.io/kubernetes/pkg/util/strategicpatch" ) // ResumeConfig is the start of the data required to perform the operation. As new fields are added, add them here instead of @@ -139,8 +138,7 @@ func (o *ResumeConfig) CompleteResume(f cmdutil.Factory, cmd *cobra.Command, out func (o ResumeConfig) RunResume() error { allErrs := []error{} - // Defaulting to SMPatchVersion_1_5 is safe, since Resumer only update a boolean variable - for _, patch := range set.CalculatePatches(o.f, o.Infos, o.Encoder, strategicpatch.SMPatchVersion_1_5, o.Resumer) { + for _, patch := range set.CalculatePatches(o.f, o.Infos, o.Encoder, false, o.Resumer) { info := patch.Info if patch.Err != nil { diff --git a/pkg/kubectl/cmd/scale.go b/pkg/kubectl/cmd/scale.go index c7f266b70d2..5b6a07ed017 100644 --- a/pkg/kubectl/cmd/scale.go +++ b/pkg/kubectl/cmd/scale.go @@ -139,6 +139,11 @@ func RunScale(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin return fmt.Errorf("cannot use --resource-version with multiple resources") } + smPatchVersion, err := cmdutil.GetServerSupportedSMPatchVersionFromFactory(f) + if err != nil { + return err + } + counter := 0 err = r.Visit(func(info *resource.Info, err error) error { if err != nil { @@ -164,7 +169,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, err := cmdutil.ChangeResourcePatch(info, f.Command(), smPatchVersion) if err != nil { return err } diff --git a/pkg/kubectl/cmd/set/helper.go b/pkg/kubectl/cmd/set/helper.go index b95abd17e87..97423908c35 100644 --- a/pkg/kubectl/cmd/set/helper.go +++ b/pkg/kubectl/cmd/set/helper.go @@ -123,8 +123,17 @@ type Patch struct { // If local is true, it will be default to use SMPatchVersionLatest to calculate a patch without contacting the server to // get the server supported SMPatchVersion. If you are using a patch's Patch field generated in local mode, be careful. // If local is false, it will talk to the server to check which StategicMergePatchVersion to use. -func CalculatePatches(f cmdutil.Factory, infos []*resource.Info, encoder runtime.Encoder, smPatchVersion strategicpatch.StrategicMergePatchVersion, mutateFn func(*resource.Info) (bool, error)) []*Patch { +func CalculatePatches(f cmdutil.Factory, infos []*resource.Info, encoder runtime.Encoder, local bool, mutateFn func(*resource.Info) (bool, error)) []*Patch { var patches []*Patch + smPatchVersion := strategicpatch.SMPatchVersionLatest + var err error + if !local { + smPatchVersion, err = cmdutil.GetServerSupportedSMPatchVersionFromFactory(f) + if err != nil { + return patches + } + } + for _, info := range infos { patch := &Patch{Info: info} patch.Before, patch.Err = runtime.Encode(encoder, info.Object) diff --git a/pkg/kubectl/cmd/set/set_image.go b/pkg/kubectl/cmd/set/set_image.go index a0e6486235a..f3ad8dbae19 100644 --- a/pkg/kubectl/cmd/set/set_image.go +++ b/pkg/kubectl/cmd/set/set_image.go @@ -164,8 +164,8 @@ func (o *ImageOptions) Validate() error { func (o *ImageOptions) Run() error { allErrs := []error{} - // Defauting to SMPatchVersion_1_5, since the func passed in doesn't update any lists of primitive - patches := CalculatePatches(o.f, o.Infos, o.Encoder, strategicpatch.SMPatchVersion_1_5, func(info *resource.Info) (bool, error) { + + patches := CalculatePatches(o.f, o.Infos, o.Encoder, o.Local, func(info *resource.Info) (bool, error) { transformed := false _, err := o.UpdatePodSpecForObject(info.Object, func(spec *api.PodSpec) error { for name, image := range o.ContainerImages { @@ -189,6 +189,14 @@ func (o *ImageOptions) Run() error { return transformed, err }) + smPatchVersion := strategicpatch.SMPatchVersionLatest + var err error + if !o.Local { + smPatchVersion, err = cmdutil.GetServerSupportedSMPatchVersionFromFactory(o.f) + if err != nil { + return err + } + } for _, patch := range patches { info := patch.Info if patch.Err != nil { @@ -215,7 +223,7 @@ 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 patch, err := cmdutil.ChangeResourcePatch(info, o.ChangeCause, smPatchVersion); err == nil { if obj, err = resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, api.StrategicMergePatchType, 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/set/set_resources.go b/pkg/kubectl/cmd/set/set_resources.go index 4601041b9b2..0aee531ac37 100644 --- a/pkg/kubectl/cmd/set/set_resources.go +++ b/pkg/kubectl/cmd/set/set_resources.go @@ -31,7 +31,6 @@ import ( "k8s.io/kubernetes/pkg/kubectl/resource" "k8s.io/kubernetes/pkg/runtime" utilerrors "k8s.io/kubernetes/pkg/util/errors" - "k8s.io/kubernetes/pkg/util/strategicpatch" ) var ( @@ -177,8 +176,7 @@ func (o *ResourcesOptions) Validate() error { func (o *ResourcesOptions) Run() error { allErrs := []error{} - // Defauting to SMPatchVersion_1_5, since the func passed in doesn't update any lists of primitive - patches := CalculatePatches(o.f, o.Infos, o.Encoder, strategicpatch.SMPatchVersion_1_5, func(info *resource.Info) (bool, error) { + patches := CalculatePatches(o.f, o.Infos, o.Encoder, cmdutil.GetDryRunFlag(o.Cmd), func(info *resource.Info) (bool, error) { transformed := false _, err := o.UpdatePodSpecForObject(info.Object, func(spec *api.PodSpec) error { containers, _ := selectContainers(spec.Containers, o.ContainerSelector) diff --git a/pkg/kubectl/cmd/taint.go b/pkg/kubectl/cmd/taint.go index c57ba901393..d68408e40c0 100644 --- a/pkg/kubectl/cmd/taint.go +++ b/pkg/kubectl/cmd/taint.go @@ -321,6 +321,11 @@ func (o TaintOptions) RunTaint() error { return err } + smPatchVersion, err := cmdutil.GetServerSupportedSMPatchVersionFromFactory(o.f) + if err != nil { + return err + } + return r.Visit(func(info *resource.Info, err error) error { if err != nil { return err @@ -343,8 +348,7 @@ func (o TaintOptions) RunTaint() error { if err != nil { return err } - // Defaulting to SMPatchVersion_1_5 is safe, since we don't update list of primitives. - patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, obj, strategicpatch.SMPatchVersion_1_5) + patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, obj, smPatchVersion) createdPatch := err == nil if err != nil { glog.V(2).Infof("couldn't compute patch: %v", err) diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go index 0e1af7815d8..54176b5d5d6 100644 --- a/pkg/kubectl/cmd/util/helpers.go +++ b/pkg/kubectl/cmd/util/helpers.go @@ -521,7 +521,7 @@ func RecordChangeCause(obj runtime.Object, changeCause string) error { // ChangeResourcePatch creates a strategic merge 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, smPatchVersion strategicpatch.StrategicMergePatchVersion) ([]byte, error) { oldData, err := json.Marshal(info.Object) if err != nil { return nil, err @@ -533,8 +533,7 @@ func ChangeResourcePatch(info *resource.Info, changeCause string) ([]byte, error if err != nil { return nil, err } - // Using SMPatchVersion_1_5, since RecordChangeCause() just update the annotation which is a map[string]string - return strategicpatch.CreateTwoWayMergePatch(oldData, newData, info.Object, strategicpatch.SMPatchVersion_1_5) + return strategicpatch.CreateTwoWayMergePatch(oldData, newData, info.Object, smPatchVersion) } // containsChangeCause checks if input resource info contains change-cause annotation. @@ -726,3 +725,13 @@ func RequireNoArguments(c *cobra.Command, args []string) { CheckErr(UsageError(c, fmt.Sprintf(`unknown command %q`, strings.Join(args, " ")))) } } + +// GetServerSupportedSMPatchVersionFromFactory is a wrapper of GetServerSupportedSMPatchVersion(), +// It takes a Factory, returns the max version the server supports. +func GetServerSupportedSMPatchVersionFromFactory(f Factory) (strategicpatch.StrategicMergePatchVersion, error) { + clientSet, err := f.ClientSet() + if err != nil { + return strategicpatch.Unknown, err + } + return strategicpatch.GetServerSupportedSMPatchVersion(clientSet.Discovery()) +} diff --git a/pkg/util/strategicpatch/BUILD b/pkg/util/strategicpatch/BUILD index 433b7a295d8..8340ba0c5ad 100644 --- a/pkg/util/strategicpatch/BUILD +++ b/pkg/util/strategicpatch/BUILD @@ -15,6 +15,7 @@ go_library( srcs = ["patch.go"], tags = ["automanaged"], deps = [ + "//pkg/client/typed/discovery:go_default_library", "//pkg/util/json:go_default_library", "//third_party/forked/golang/json:go_default_library", "//vendor:github.com/davecgh/go-spew/spew", diff --git a/pkg/util/strategicpatch/patch.go b/pkg/util/strategicpatch/patch.go index 442a176e943..f6c5e699eb8 100644 --- a/pkg/util/strategicpatch/patch.go +++ b/pkg/util/strategicpatch/patch.go @@ -21,6 +21,7 @@ import ( "reflect" "sort" + "k8s.io/kubernetes/pkg/client/typed/discovery" "k8s.io/kubernetes/pkg/util/json" forkedjson "k8s.io/kubernetes/third_party/forked/golang/json" @@ -1429,3 +1430,20 @@ func toYAML(v interface{}) (string, error) { return string(y), nil } + +// GetServerSupportedSMPatchVersion takes a discoveryClient, +// returns the max StrategicMergePatch version supported +func GetServerSupportedSMPatchVersion(discoveryClient discovery.DiscoveryInterface) (StrategicMergePatchVersion, error) { + serverVersion, err := discoveryClient.ServerVersion() + if err != nil { + return Unknown, err + } + serverGitVersion := serverVersion.GitVersion + if serverGitVersion >= string(SMPatchVersion_1_5) { + return SMPatchVersion_1_5, nil + } + if serverGitVersion >= string(SMPatchVersion_1_0) { + return SMPatchVersion_1_0, nil + } + return Unknown, fmt.Errorf("The version is too old: %v\n", serverVersion) +}