From 15512210e3e14b0c827d13334394d658dc7ddc88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Thu, 16 Jun 2022 11:12:17 +0300 Subject: [PATCH 1/6] (kubectl apply): Split patching types into functions and refactorings Patch type determination is done by checking existence of resource in schema and if it exists, it uses strategic merge patch. Otherwise, like for CRDs, it uses merge patch type. Currently, this code portion is not easily extensible and this PR splits required checks into their own function to increase extensibility. --- .../k8s.io/kubectl/pkg/cmd/apply/patcher.go | 124 ++++++++++++------ 1 file changed, 84 insertions(+), 40 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go index f14fdd06632..04001afce06 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go @@ -28,13 +28,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/jsonmergepatch" "k8s.io/apimachinery/pkg/util/mergepatch" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/cli-runtime/pkg/resource" - oapi "k8s.io/kube-openapi/pkg/util/proto" cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubectl/pkg/scheme" "k8s.io/kubectl/pkg/util" @@ -50,6 +50,8 @@ const ( triesBeforeBackOff = 1 ) +var createPatchErrFormat = "creating patch with:\noriginal:\n%s\nmodified:\n%s\ncurrent:\n%s\nfor:" + // Patcher defines options to patch OpenAPI objects. type Patcher struct { Mapping *meta.RESTMapping @@ -111,54 +113,30 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, source, names return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf("retrieving original configuration from:\n%v\nfor:", obj), source, err) } - var patchType types.PatchType var patch []byte - var lookupPatchMeta strategicpatch.LookupPatchMeta - var schema oapi.Schema - createPatchErrFormat := "creating patch with:\noriginal:\n%s\nmodified:\n%s\ncurrent:\n%s\nfor:" - // Create the versioned struct from the type defined in the restmapping - // (which is the API version we'll be submitting the patch to) - versionedObject, err := scheme.Scheme.New(p.Mapping.GroupVersionKind) - switch { - case runtime.IsNotRegisteredError(err): - // fall back to generic JSON merge patch - patchType = types.MergePatchType - preconditions := []mergepatch.PreconditionFunc{mergepatch.RequireKeyUnchanged("apiVersion"), - mergepatch.RequireKeyUnchanged("kind"), mergepatch.RequireMetadataKeyUnchanged("name")} - patch, err = jsonmergepatch.CreateThreeWayJSONMergePatch(original, modified, current, preconditions...) + patchType, err := p.getPatchType(p.Mapping.GroupVersionKind) + if err != nil { + return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf("getting patch type for %v:", p.Mapping.GroupVersionKind), source, err) + } + + switch patchType { + case types.MergePatchType: + patch, err = p.buildMergePatch(original, modified, current, source) if err != nil { - if mergepatch.IsPreconditionFailed(err) { - return nil, nil, fmt.Errorf("%s", "At least one of apiVersion, kind and name was changed") - } - return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf(createPatchErrFormat, original, modified, current), source, err) + return nil, nil, err } - case err != nil: - return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf("getting instance of versioned object for %v:", p.Mapping.GroupVersionKind), source, err) - case err == nil: - // Compute a three way strategic merge patch to send to server. - patchType = types.StrategicMergePatchType - + case types.StrategicMergePatchType: // Try to use openapi first if the openapi spec is available and can successfully calculate the patch. // Otherwise, fall back to baked-in types. - if p.OpenapiSchema != nil { - if schema = p.OpenapiSchema.LookupResource(p.Mapping.GroupVersionKind); schema != nil { - lookupPatchMeta = strategicpatch.PatchMetaFromOpenAPI{Schema: schema} - if openapiPatch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, lookupPatchMeta, p.Overwrite); err != nil { - fmt.Fprintf(errOut, "warning: error calculating patch from openapi spec: %v\n", err) - } else { - patchType = types.StrategicMergePatchType - patch = openapiPatch - } - } + patch, err = p.buildStrategicMergeFromOpenAPI(p.Mapping.GroupVersionKind, original, modified, current) + if err != nil { + // Warn user about problem and continue strategic merge patching using builtin types. + fmt.Fprintf(errOut, "warning: error calculating patch from openapi spec: %v\n", err) } if patch == nil { - lookupPatchMeta, err = strategicpatch.NewPatchMetaFromStruct(versionedObject) - if err != nil { - return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf(createPatchErrFormat, original, modified, current), source, err) - } - patch, err = strategicpatch.CreateThreeWayMergePatch(original, modified, current, lookupPatchMeta, p.Overwrite) + patch, err = p.buildStrategicMergeFromBuiltins(p.Mapping.GroupVersionKind, original, modified, current) if err != nil { return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf(createPatchErrFormat, original, modified, current), source, err) } @@ -180,6 +158,72 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, source, names return patch, patchedObj, err } +// getPatchType returns correct PatchType for the given GVK. +func (p *Patcher) getPatchType(gvk schema.GroupVersionKind) (types.PatchType, error) { + _, err := scheme.Scheme.New(gvk) + if err == nil { + return types.StrategicMergePatchType, nil + } + if runtime.IsNotRegisteredError(err) { + return types.MergePatchType, nil + } + + return types.MergePatchType, err +} + +// buildMergePatch builds patch according to the JSONMergePatch which is used for +// custom resource definitions. +func (p *Patcher) buildMergePatch(original, modified, current []byte, source string) ([]byte, error) { + preconditions := []mergepatch.PreconditionFunc{mergepatch.RequireKeyUnchanged("apiVersion"), + mergepatch.RequireKeyUnchanged("kind"), mergepatch.RequireMetadataKeyUnchanged("name")} + patch, err := jsonmergepatch.CreateThreeWayJSONMergePatch(original, modified, current, preconditions...) + if err != nil { + if mergepatch.IsPreconditionFailed(err) { + return nil, fmt.Errorf("%s", "At least one of apiVersion, kind and name was changed") + } + return nil, cmdutil.AddSourceToErr(fmt.Sprintf(createPatchErrFormat, original, modified, current), source, err) + } + + return patch, nil +} + +// buildStrategicMergeFromOpenAPI builds patch from OpenAPI if it is enabled. +// This is used for core types which is published in openapi. +func (p *Patcher) buildStrategicMergeFromOpenAPI(gvk schema.GroupVersionKind, original, modified, current []byte) ([]byte, error) { + if p.OpenapiSchema != nil { + if s := p.OpenapiSchema.LookupResource(gvk); s != nil { + lookupPatchMeta := strategicpatch.PatchMetaFromOpenAPI{Schema: s} + if openapiPatch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, lookupPatchMeta, p.Overwrite); err != nil { + return nil, err + } else { + return openapiPatch, nil + } + } + } + + return nil, nil +} + +// buildStrategicMergeFromStruct builds patch from struct. This is used when +// openapi endpoint is not working or user disables it by setting openapi-patch flag +// to false. +func (p *Patcher) buildStrategicMergeFromBuiltins(gvk schema.GroupVersionKind, original, modified, current []byte) ([]byte, error) { + versionedObject, err := scheme.Scheme.New(gvk) + if err != nil { + return nil, err + } + lookupPatchMeta, err := strategicpatch.NewPatchMetaFromStruct(versionedObject) + if err != nil { + return nil, err + } + patch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, lookupPatchMeta, p.Overwrite) + if err != nil { + return nil, err + } + + return patch, nil +} + // Patch tries to patch an OpenAPI resource. On success, returns the merge patch as well // the final patched object. On failure, returns an error. func (p *Patcher) Patch(current runtime.Object, modified []byte, source, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, error) { From d336f7df874dc978ebd97971e1fc93d891c21e7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 20 Jun 2022 10:04:18 +0300 Subject: [PATCH 2/6] Stop passing source as parameter This commit removes passing source field as parameter. Instead, this commit returns error verb and error functions to caller function. Caller function can add source field by generating correct message. --- .../k8s.io/kubectl/pkg/cmd/apply/patcher.go | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go index 04001afce06..05616ff81d4 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go @@ -100,62 +100,64 @@ func (p *Patcher) delete(namespace, name string) error { return err } -func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, source, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, error) { +func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, string, error) { // Serialize the current configuration of the object from the server. current, err := runtime.Encode(unstructured.UnstructuredJSONScheme, obj) if err != nil { - return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf("serializing current configuration from:\n%v\nfor:", obj), source, err) + return nil, nil, fmt.Sprintf("serializing current configuration from:\n%v\nfor:", obj), err } // Retrieve the original configuration of the object from the annotation. original, err := util.GetOriginalConfiguration(obj) if err != nil { - return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf("retrieving original configuration from:\n%v\nfor:", obj), source, err) + return nil, nil, fmt.Sprintf("retrieving original configuration from:\n%v\nfor:", obj), err } var patch []byte patchType, err := p.getPatchType(p.Mapping.GroupVersionKind) if err != nil { - return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf("getting patch type for %v:", p.Mapping.GroupVersionKind), source, err) + return nil, nil, fmt.Sprintf("getting patch type for %v:", p.Mapping.GroupVersionKind), err } switch patchType { case types.MergePatchType: - patch, err = p.buildMergePatch(original, modified, current, source) + patch, err = p.buildMergePatch(original, modified, current) if err != nil { - return nil, nil, err + return nil, nil, fmt.Sprintf(createPatchErrFormat, original, modified, current), err } case types.StrategicMergePatchType: // Try to use openapi first if the openapi spec is available and can successfully calculate the patch. // Otherwise, fall back to baked-in types. - patch, err = p.buildStrategicMergeFromOpenAPI(p.Mapping.GroupVersionKind, original, modified, current) - if err != nil { - // Warn user about problem and continue strategic merge patching using builtin types. - fmt.Fprintf(errOut, "warning: error calculating patch from openapi spec: %v\n", err) + if p.OpenapiSchema != nil { + patch, err = p.buildStrategicMergeFromOpenAPI(p.Mapping.GroupVersionKind, original, modified, current) + if err != nil { + // Warn user about problem and continue strategic merge patching using builtin types. + fmt.Fprintf(errOut, "warning: error calculating patch from openapi spec: %v\n", err) + } } if patch == nil { patch, err = p.buildStrategicMergeFromBuiltins(p.Mapping.GroupVersionKind, original, modified, current) if err != nil { - return nil, nil, cmdutil.AddSourceToErr(fmt.Sprintf(createPatchErrFormat, original, modified, current), source, err) + return nil, nil, fmt.Sprintf(createPatchErrFormat, original, modified, current), err } } } if string(patch) == "{}" { - return patch, obj, nil + return patch, obj, "", nil } if p.ResourceVersion != nil { patch, err = addResourceVersion(patch, *p.ResourceVersion) if err != nil { - return nil, nil, cmdutil.AddSourceToErr("Failed to insert resourceVersion in patch", source, err) + return nil, nil, "Failed to insert resourceVersion in patch", err } } patchedObj, err := p.Helper.Patch(namespace, name, patchType, patch, nil) - return patch, patchedObj, err + return patch, patchedObj, "", err } // getPatchType returns correct PatchType for the given GVK. @@ -173,7 +175,7 @@ func (p *Patcher) getPatchType(gvk schema.GroupVersionKind) (types.PatchType, er // buildMergePatch builds patch according to the JSONMergePatch which is used for // custom resource definitions. -func (p *Patcher) buildMergePatch(original, modified, current []byte, source string) ([]byte, error) { +func (p *Patcher) buildMergePatch(original, modified, current []byte) ([]byte, error) { preconditions := []mergepatch.PreconditionFunc{mergepatch.RequireKeyUnchanged("apiVersion"), mergepatch.RequireKeyUnchanged("kind"), mergepatch.RequireMetadataKeyUnchanged("name")} patch, err := jsonmergepatch.CreateThreeWayJSONMergePatch(original, modified, current, preconditions...) @@ -181,7 +183,7 @@ func (p *Patcher) buildMergePatch(original, modified, current []byte, source str if mergepatch.IsPreconditionFailed(err) { return nil, fmt.Errorf("%s", "At least one of apiVersion, kind and name was changed") } - return nil, cmdutil.AddSourceToErr(fmt.Sprintf(createPatchErrFormat, original, modified, current), source, err) + return nil, err } return patch, nil @@ -190,14 +192,12 @@ func (p *Patcher) buildMergePatch(original, modified, current []byte, source str // buildStrategicMergeFromOpenAPI builds patch from OpenAPI if it is enabled. // This is used for core types which is published in openapi. func (p *Patcher) buildStrategicMergeFromOpenAPI(gvk schema.GroupVersionKind, original, modified, current []byte) ([]byte, error) { - if p.OpenapiSchema != nil { - if s := p.OpenapiSchema.LookupResource(gvk); s != nil { - lookupPatchMeta := strategicpatch.PatchMetaFromOpenAPI{Schema: s} - if openapiPatch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, lookupPatchMeta, p.Overwrite); err != nil { - return nil, err - } else { - return openapiPatch, nil - } + if s := p.OpenapiSchema.LookupResource(gvk); s != nil { + lookupPatchMeta := strategicpatch.PatchMetaFromOpenAPI{Schema: s} + if openapiPatch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, lookupPatchMeta, p.Overwrite); err != nil { + return nil, err + } else { + return openapiPatch, nil } } @@ -228,7 +228,7 @@ func (p *Patcher) buildStrategicMergeFromBuiltins(gvk schema.GroupVersionKind, o // the final patched object. On failure, returns an error. func (p *Patcher) Patch(current runtime.Object, modified []byte, source, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, error) { var getErr error - patchBytes, patchObject, err := p.patchSimple(current, modified, source, namespace, name, errOut) + patchBytes, patchObject, errVerb, err := p.patchSimple(current, modified, namespace, name, errOut) if p.Retries == 0 { p.Retries = maxPatchRetry } @@ -240,10 +240,14 @@ func (p *Patcher) Patch(current runtime.Object, modified []byte, source, namespa if getErr != nil { return nil, nil, getErr } - patchBytes, patchObject, err = p.patchSimple(current, modified, source, namespace, name, errOut) + patchBytes, patchObject, errVerb, err = p.patchSimple(current, modified, namespace, name, errOut) } - if err != nil && (errors.IsConflict(err) || errors.IsInvalid(err)) && p.Force { - patchBytes, patchObject, err = p.deleteAndCreate(current, modified, namespace, name) + if err != nil { + if (errors.IsConflict(err) || errors.IsInvalid(err)) && p.Force { + patchBytes, patchObject, err = p.deleteAndCreate(current, modified, namespace, name) + } else if errVerb != "" { + err = cmdutil.AddSourceToErr(errVerb, source, err) + } } return patchBytes, patchObject, err } From 4ed5653a0bc8ca53ca6a917e634457bd7a2d8dae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 22 Jun 2022 09:35:49 +0300 Subject: [PATCH 3/6] Move getPatchType logic back to main function --- .../k8s.io/kubectl/pkg/cmd/apply/patcher.go | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go index 05616ff81d4..f3ba874b090 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go @@ -115,9 +115,13 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, namespace, na var patch []byte - patchType, err := p.getPatchType(p.Mapping.GroupVersionKind) + patchType := types.StrategicMergePatchType + _, err = scheme.Scheme.New(p.Mapping.GroupVersionKind) if err != nil { - return nil, nil, fmt.Sprintf("getting patch type for %v:", p.Mapping.GroupVersionKind), err + if !runtime.IsNotRegisteredError(err) { + return nil, nil, fmt.Sprintf("getting instance of versioned object for %v:", p.Mapping.GroupVersionKind), err + } + patchType = types.MergePatchType } switch patchType { @@ -160,19 +164,6 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, namespace, na return patch, patchedObj, "", err } -// getPatchType returns correct PatchType for the given GVK. -func (p *Patcher) getPatchType(gvk schema.GroupVersionKind) (types.PatchType, error) { - _, err := scheme.Scheme.New(gvk) - if err == nil { - return types.StrategicMergePatchType, nil - } - if runtime.IsNotRegisteredError(err) { - return types.MergePatchType, nil - } - - return types.MergePatchType, err -} - // buildMergePatch builds patch according to the JSONMergePatch which is used for // custom resource definitions. func (p *Patcher) buildMergePatch(original, modified, current []byte) ([]byte, error) { From 22b43d4edbee4f89984d1b905adb1f905f3ab90b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 22 Jun 2022 10:22:27 +0300 Subject: [PATCH 4/6] Use error wrapping in patchSimple --- .../k8s.io/kubectl/pkg/cmd/apply/patcher.go | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go index f3ba874b090..561dcce2074 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go @@ -22,8 +22,10 @@ import ( "io" "time" + "github.com/pkg/errors" + "github.com/jonboulle/clockwork" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -100,17 +102,17 @@ func (p *Patcher) delete(namespace, name string) error { return err } -func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, string, error) { +func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, error) { // Serialize the current configuration of the object from the server. current, err := runtime.Encode(unstructured.UnstructuredJSONScheme, obj) if err != nil { - return nil, nil, fmt.Sprintf("serializing current configuration from:\n%v\nfor:", obj), err + return nil, nil, errors.Wrapf(err, "serializing current configuration from:\n%v\nfor:", obj) } // Retrieve the original configuration of the object from the annotation. original, err := util.GetOriginalConfiguration(obj) if err != nil { - return nil, nil, fmt.Sprintf("retrieving original configuration from:\n%v\nfor:", obj), err + return nil, nil, errors.Wrapf(err, "retrieving original configuration from:\n%v\nfor:", obj) } var patch []byte @@ -119,7 +121,7 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, namespace, na _, err = scheme.Scheme.New(p.Mapping.GroupVersionKind) if err != nil { if !runtime.IsNotRegisteredError(err) { - return nil, nil, fmt.Sprintf("getting instance of versioned object for %v:", p.Mapping.GroupVersionKind), err + return nil, nil, errors.Wrapf(err, "getting instance of versioned object for %v:", p.Mapping.GroupVersionKind) } patchType = types.MergePatchType } @@ -128,7 +130,7 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, namespace, na case types.MergePatchType: patch, err = p.buildMergePatch(original, modified, current) if err != nil { - return nil, nil, fmt.Sprintf(createPatchErrFormat, original, modified, current), err + return nil, nil, errors.Wrapf(err, createPatchErrFormat, original, modified, current) } case types.StrategicMergePatchType: // Try to use openapi first if the openapi spec is available and can successfully calculate the patch. @@ -144,24 +146,24 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, namespace, na if patch == nil { patch, err = p.buildStrategicMergeFromBuiltins(p.Mapping.GroupVersionKind, original, modified, current) if err != nil { - return nil, nil, fmt.Sprintf(createPatchErrFormat, original, modified, current), err + return nil, nil, errors.Wrapf(err, createPatchErrFormat, original, modified, current) } } } if string(patch) == "{}" { - return patch, obj, "", nil + return patch, obj, nil } if p.ResourceVersion != nil { patch, err = addResourceVersion(patch, *p.ResourceVersion) if err != nil { - return nil, nil, "Failed to insert resourceVersion in patch", err + return nil, nil, errors.Wrap(err, "Failed to insert resourceVersion in patch") } } patchedObj, err := p.Helper.Patch(namespace, name, patchType, patch, nil) - return patch, patchedObj, "", err + return patch, patchedObj, err } // buildMergePatch builds patch according to the JSONMergePatch which is used for @@ -219,11 +221,11 @@ func (p *Patcher) buildStrategicMergeFromBuiltins(gvk schema.GroupVersionKind, o // the final patched object. On failure, returns an error. func (p *Patcher) Patch(current runtime.Object, modified []byte, source, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, error) { var getErr error - patchBytes, patchObject, errVerb, err := p.patchSimple(current, modified, namespace, name, errOut) + patchBytes, patchObject, err := p.patchSimple(current, modified, namespace, name, errOut) if p.Retries == 0 { p.Retries = maxPatchRetry } - for i := 1; i <= p.Retries && errors.IsConflict(err); i++ { + for i := 1; i <= p.Retries && apierrors.IsConflict(err); i++ { if i > triesBeforeBackOff { p.BackOff.Sleep(backOffPeriod) } @@ -231,13 +233,13 @@ func (p *Patcher) Patch(current runtime.Object, modified []byte, source, namespa if getErr != nil { return nil, nil, getErr } - patchBytes, patchObject, errVerb, err = p.patchSimple(current, modified, namespace, name, errOut) + patchBytes, patchObject, err = p.patchSimple(current, modified, namespace, name, errOut) } if err != nil { - if (errors.IsConflict(err) || errors.IsInvalid(err)) && p.Force { + if (apierrors.IsConflict(err) || apierrors.IsInvalid(err)) && p.Force { patchBytes, patchObject, err = p.deleteAndCreate(current, modified, namespace, name) - } else if errVerb != "" { - err = cmdutil.AddSourceToErr(errVerb, source, err) + } else { + err = cmdutil.AddSourceToErr("patching", source, err) } } return patchBytes, patchObject, err @@ -249,7 +251,7 @@ func (p *Patcher) deleteAndCreate(original runtime.Object, modified []byte, name } // TODO: use wait if err := wait.PollImmediate(1*time.Second, p.Timeout, func() (bool, error) { - if _, err := p.Helper.Get(namespace, name); !errors.IsNotFound(err) { + if _, err := p.Helper.Get(namespace, name); !apierrors.IsNotFound(err) { return false, err } return true, nil From be5f7c051b6105bc9a8d781edff86c264d446c91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 22 Jun 2022 10:35:36 +0300 Subject: [PATCH 5/6] Update dependencies for kubectl --- staging/src/k8s.io/kubectl/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staging/src/k8s.io/kubectl/go.mod b/staging/src/k8s.io/kubectl/go.mod index 4e0e8f2f31c..1a92cb1c1c7 100644 --- a/staging/src/k8s.io/kubectl/go.mod +++ b/staging/src/k8s.io/kubectl/go.mod @@ -23,6 +23,7 @@ require ( github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6 github.com/onsi/ginkgo v1.14.0 github.com/onsi/gomega v1.10.1 + github.com/pkg/errors v0.9.1 github.com/russross/blackfriday v1.5.2 github.com/spf13/cobra v1.4.0 github.com/spf13/pflag v1.0.5 @@ -77,7 +78,6 @@ require ( github.com/nxadm/tail v1.4.4 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect github.com/peterbourgon/diskv v2.0.1+incompatible // indirect - github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca // indirect go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect From 66c2f6069ebbfa41d0bef0f3959abda59b8483f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Tue, 28 Jun 2022 09:30:06 +0300 Subject: [PATCH 6/6] Move resource lookup into its own function --- .../k8s.io/kubectl/pkg/cmd/apply/patcher.go | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go index 561dcce2074..d3d157278e5 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go @@ -37,6 +37,7 @@ import ( "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/cli-runtime/pkg/resource" + "k8s.io/kube-openapi/pkg/util/proto" cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubectl/pkg/scheme" "k8s.io/kubectl/pkg/util" @@ -135,8 +136,8 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, namespace, na case types.StrategicMergePatchType: // Try to use openapi first if the openapi spec is available and can successfully calculate the patch. // Otherwise, fall back to baked-in types. - if p.OpenapiSchema != nil { - patch, err = p.buildStrategicMergeFromOpenAPI(p.Mapping.GroupVersionKind, original, modified, current) + if s := p.findOpenAPIResource(p.Mapping.GroupVersionKind); s != nil { + patch, err = p.buildStrategicMergeFromOpenAPI(s, original, modified, current) if err != nil { // Warn user about problem and continue strategic merge patching using builtin types. fmt.Fprintf(errOut, "warning: error calculating patch from openapi spec: %v\n", err) @@ -184,17 +185,21 @@ func (p *Patcher) buildMergePatch(original, modified, current []byte) ([]byte, e // buildStrategicMergeFromOpenAPI builds patch from OpenAPI if it is enabled. // This is used for core types which is published in openapi. -func (p *Patcher) buildStrategicMergeFromOpenAPI(gvk schema.GroupVersionKind, original, modified, current []byte) ([]byte, error) { - if s := p.OpenapiSchema.LookupResource(gvk); s != nil { - lookupPatchMeta := strategicpatch.PatchMetaFromOpenAPI{Schema: s} - if openapiPatch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, lookupPatchMeta, p.Overwrite); err != nil { - return nil, err - } else { - return openapiPatch, nil - } +func (p *Patcher) buildStrategicMergeFromOpenAPI(schema proto.Schema, original, modified, current []byte) ([]byte, error) { + lookupPatchMeta := strategicpatch.PatchMetaFromOpenAPI{Schema: schema} + if openapiPatch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, lookupPatchMeta, p.Overwrite); err != nil { + return nil, err + } else { + return openapiPatch, nil } +} - return nil, nil +// findOpenAPIResource finds schema of GVK in OpenAPI endpoint. +func (p *Patcher) findOpenAPIResource(gvk schema.GroupVersionKind) proto.Schema { + if p.OpenapiSchema == nil { + return nil + } + return p.OpenapiSchema.LookupResource(gvk) } // buildStrategicMergeFromStruct builds patch from struct. This is used when