Skip to content

patch_namespaced_deployment gets 500 as response when using json-patch #187

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

Closed
weltschraet opened this issue Apr 21, 2017 · 16 comments
Closed
Assignees

Comments

@weltschraet
Copy link

When using a RFC6902 formatted json-patch you get a 500 response.
The hack for the issue #93 introduced this bug.
Are there any plans to get rid of the hack?

To repoduce the problem the following can be done:

create the deployment with kubectl

kubectl run nginx --image=nginx:1.11.12

try to patch it

from kubernetes import client, config
config.load_kube_config()
client.configuration.debug = True
v1 = client.ExtensionsV1beta1Api()
patch = [{"op": "replace", "value": "nginx:1.11.13", "path": "/spec/template/spec/containers/0/image"}]
v1.patch_namespaced_deployment(name='nginx', namespace='default', body=patch)

log output

DEBUG Starting new HTTPS connection (1): xxx
send: b'PATCH /apis/extensions/v1beta1/namespaces/default/deployments/nginx HTTP/1.1\r\nHost: xxx\r\nAccept-Encoding: identity\r\nContent-Length: 95\r\nAccept: application/json\r\nContent-Type: application/strategic-merge-patch+json\r\nUser-Agent: Swagger-Codegen/1.0.0-alpha/python\r\nauthorization: Bearer xxx\r\n\r\n'
send: b'[{"op": "replace", "value": "nginx:1.11.13", "path": "/spec/template/spec/containers/0/image"}]'
reply: 'HTTP/1.1 500 Internal Server Error\r\n'
DEBUG https://xxx:443 "PATCH /apis/extensions/v1beta1/namespaces/default/deployments/nginx HTTP/1.1" 500 114
header: Content-Type header: Date header: Content-Length Traceback (most recent call last):
  File "api_test.py", line 25, in <module>
    main()
  File "api_test.py", line 21, in main
    print(v1.patch_namespaced_deployment(name='nginx', namespace='default', body=patch_test))
  File "/usr/local/lib/python3.6/site-packages/kubernetes/client/apis/extensions_v1beta1_api.py", line 5400, in patch_namespaced_deployment
    (data) = self.patch_namespaced_deployment_with_http_info(name, namespace, body, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/kubernetes/client/apis/extensions_v1beta1_api.py", line 5496, in patch_namespaced_deployment_with_http_info
    collection_formats=collection_formats)
  File "/usr/local/lib/python3.6/site-packages/kubernetes/client/api_client.py", line 329, in call_api
    _return_http_data_only, collection_formats, _preload_content, _request_timeout)
  File "/usr/local/lib/python3.6/site-packages/kubernetes/client/api_client.py", line 153, in __call_api
    _request_timeout=_request_timeout)
  File "/usr/local/lib/python3.6/site-packages/kubernetes/client/api_client.py", line 399, in request
    body=body)
  File "/usr/local/lib/python3.6/site-packages/kubernetes/client/rest.py", line 295, in PATCH
    body=body)
  File "/usr/local/lib/python3.6/site-packages/kubernetes/client/rest.py", line 231, in request
    raise ApiException(http_resp=r)
kubernetes.client.rest.ApiException: (500)
Reason: Internal Server Error
HTTP response headers: HTTPHeaderDict({'Content-Type': 'application/json', 'Date': 'Fri, 21 Apr 2017 12:05:35 GMT', 'Content-Length': '114'})
HTTP response body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Invalid JSON document","code":500}
@mbohlool
Copy link
Contributor

patch should be an object not an array of objects.

patch = {"op": "replace", "value": "nginx:1.11.13", "path": "/spec/template/spec/containers/0/image"}

@weltschraet
Copy link
Author

weltschraet commented Apr 21, 2017

as the RFC states it is a list of objects

  [
     { "op": "test", "path": "/a/b/c", "value": "foo" },
     { "op": "remove", "path": "/a/b/c" },
     { "op": "add", "path": "/a/b/c", "value": [ "foo", "bar" ] },
     { "op": "replace", "path": "/a/b/c", "value": 42 },
     { "op": "move", "from": "/a/b/c", "path": "/a/b/d" },
     { "op": "copy", "from": "/a/b/d", "path": "/a/b/e" }
   ]

if you mean that the api responses with a 200, when you patch just an object, yes you're right, but the patch is not applied.

if you comment out lines 157 and 158 (on master) in kubernetes/client/rest.py it works again. So the content-type in this case is wrong. the following lines breaks the patch in this case.

                    headers[
                        'Content-Type'] = 'application/strategic-merge-patch+json'

@mbohlool
Copy link
Contributor

@dims added those lines. @dims can the content type removed from header? or can we fix it to work with this case?

@dims
Copy link
Collaborator

dims commented Apr 22, 2017

@mbohlool : ack, i'll try to find a way to multiple ways to specify the patch request body
https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md#patch-operations

@TracyBin
Copy link

body = client.V1beta1Deployment(api_version='extensions/v1beta1', kind='Deployment',
                                    metadata=metadata, spec=spec, status=status)

   # call API
try:
        ext_v1_client.patch_namespaced_deployment(name=deployment_name, namespace=deployment_namespace, body=body, pretty='true')
except ApiException as e:
   

bsherman pushed a commit to bsherman/client-python that referenced this issue Apr 28, 2017
bsherman pushed a commit to bsherman/client-python that referenced this issue May 1, 2017
mbohlool added a commit that referenced this issue May 1, 2017
fix for #187 failure when using RFC 6902 json-patch operation
mbohlool pushed a commit to mbohlool/client-python that referenced this issue May 4, 2017
mbohlool pushed a commit to mbohlool/client-python that referenced this issue May 4, 2017
mbohlool pushed a commit to mbohlool/client-python that referenced this issue May 4, 2017
mbohlool pushed a commit to mbohlool/client-python that referenced this issue May 4, 2017
@mbohlool
Copy link
Contributor

I think this is fixed? please re-open if it is not fixed.

@chlung
Copy link

chlung commented Jul 12, 2017

@aosterkamp If I would like to update multiple values from your above example
[
{ "op": "replace", "path": "/a/b/c", "value": 42 },
{ "op": "replace", "path": "/a/b/d", "value": 43 }
]
Is it above correct? Can I do something like the following? Thanks
[
{ "op": "replace",
"something": [
{ "path": "/a/b/c", "value": 42},
{ "path": "/a/b/d", "value": 43}
]
}
]

@weltschraet
Copy link
Author

@chlung sorry for the late reply.

your first example is correct. The second one should not work as far as the RFC goes, but I haven't tested it.

@chlung
Copy link

chlung commented Jul 18, 2017

@aosterkamp thanks

@DjangoPeng
Copy link
Contributor

@mbohlool Does the issue fixed? I faced the same issue. Could you help have a look at my case?
I'd like to update replicas of deployment, which like the operation of kubectl scale --replicas deployment/xxx. So, I made a patch like this patch = {"spec": {"replicas": desired_replicas}}, and desired_replicas is a parameter which I would assign. I attached my code as below:

api_instance = client.ExtensionsV1beta1Api()
name = cccccx-11404185
namespace = "default"
patch = {"spec": {"replicas": 3}}
api_response = api_instance.patch_namespaced_deployment(name, namespace, patch)

The output as below:

kubernetes.client.rest.ApiException: (500)
Reason: Internal Server Error
HTTP response headers: HTTPHeaderDict({'Date': 'Thu, 17 Aug 2017 03:06:21 GMT', 'Content-Length': '117', 'Content-Type': 'application/json'})
HTTP response body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"unrecognized type: int32","code":500}

If I set the replicas as 0, it works. I don't know what's wrong with my patch.

@mbohlool
Copy link
Contributor

I would run kubectl -v9 with that command and look at json object it send to server and also enable debug here (to see the same thing) and compare them. There is a debug flag in configuration class but make sure you set it before creating the client.

@DjangoPeng
Copy link
Contributor

@mbohlool I made a plan b to scale deployment, which as below:

  1. Load the yaml file and get the body.
  2. Set body['spec']['replicas'] = desired_replicas .
  3. Call replace_namespaced_deployment(name, namespace, body) .

@mbohlool
Copy link
Contributor

mbohlool commented Aug 18, 2017

Be very careful about replace_namespaced_deployment specially when the client talks with clusters that has a version ahead of them. There would be dropped fields and unknown behaviours. We are making some designs to fix that, but till then, using replace is not recommended. I would spend a little more time figuring out the patch. I know people had problem with it if you search issues and the answer were always a slice formatting issue in the body of patch.

@DjangoPeng
Copy link
Contributor

@mbohlool Ok, look forward to your PR. Thanks!

@mbohlool
Copy link
Contributor

I don't think a PR here helps. What I am suggesting is to fix your problem, you can change the format of the patch to match what server expect. Other users of the client already figured that out and by searching into the issues, you can find your solution. Good luck and let me know if you need any help there.

@TracyBin
Copy link

TracyBin commented Aug 17, 2018

The patch object doesn't work at all, the following code is wrong

#patch = [{"op": "remove", "path": "/spec/template/spec/containers/0/volume_mounts"}]
patch = {"spec": {"/spec/template/spec/volumes/0/persistent_volume_claim": None}}
try:
    response = ext_v1_client.patch_namespaced_deployment(name=name, namespace=namespace, body=patch)
    print(response)
except ApiException as e:
    pass

@DjangoPeng Yours method is right

yliaog pushed a commit to yliaog/client-python that referenced this issue Jan 8, 2022
…-unavailable

Prevent 503s from killing the client during discovery
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants