Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support json patch & json merge patch in addition to strategic patch #357

Closed
Stono opened this issue Oct 18, 2018 · 4 comments
Closed

Support json patch & json merge patch in addition to strategic patch #357

Stono opened this issue Oct 18, 2018 · 4 comments

Comments

@Stono
Copy link

Stono commented Oct 18, 2018

Error message: the body of the request was in an unknown format - accepted media types include: application/json-patch+json,application/merge-patch+json

Using

@atcloud.patch_service_check(check.metadata.name, {metadata: {annotations: {key: 'value'}}}, check.metadata.namespace)

Any ideas?

@Stono
Copy link
Author

Stono commented Oct 18, 2018

This monkeypatch resolved it:

module Kubeclient
  module ClientMixin
    def patch_entity(resource_name, name, patch, namespace = nil)
      ns_prefix = build_namespace_prefix(namespace)
      response = handle_exception do
        rest_client[ns_prefix + resource_name + "/#{name}"]
          .patch(
            patch.to_json,
            { 'Content-Type' => 'application/merge-patch+json' }.merge(@headers)
          )
      end
      format_response(@as, response.body)
    end
  end 
end 

K8 1.10.7

@cben
Copy link
Collaborator

cben commented Oct 23, 2018

Is that a custom resource (CRD)?
per kubernetes/kubernetes#58414, strategic merge patch (which is what kubeclient currently sends) for CRDs is unsupported and not planned.

Your change above redefines patch_* methods to take a different format, this isn't backward compatible, I don't think we can do that in kubeclient.

But we clearly need to add a way to choose patch format.
An additional param? Would have to be optional, default to strategic.
Separate methods for each format? Say add json_patch_* and merge_patch_* in addition to patch_* (and document patch_* doesn't always work).
I prefer separate methods, as the signature is simpler — each method takes a fixed "type" rather than overloaded based on another arg. Plus less mental pressure to use the suboptimal "default".

What do you think?

PS. doc on the formats: https://kubernetes.io/docs/tasks/run-application/update-api-object-kubectl-patch/

@Stono
Copy link
Author

Stono commented Oct 25, 2018

Hey @cben - yes this is a patch against a CRD so that makes sense indeed and I agree with you, well named functions for the purpose over an expanding list of arguments.

@cben cben added help wanted and removed bug labels Oct 30, 2018
@cben cben changed the title Unable to patch Support json patch & json merge patch in addition to strategic patch Dec 27, 2018
@cben
Copy link
Collaborator

cben commented Dec 27, 2018

I'll be happy to take a PR. Otherwise, I'm unlikely to get to this any time soon, I need to focus on bugs first (especially #318).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants