-
Notifications
You must be signed in to change notification settings - Fork 3.3k
add examples to demonstrate the usage of dynamic client #1448
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
add examples to demonstrate the usage of dynamic client #1448
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
1d75c7b
to
c936623
Compare
c936623
to
553f6ae
Compare
ingressroute_manifest_second["spec"]["entrypoints"] = ["web"] | ||
|
||
patch_ingressroute_one = ingressroute_api.patch( | ||
body=ingressroute_manifest_one, content_type="application/merge-patch+json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to use strategic-merge-patch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @yliaog, thanks for all your useful pointers.
In case of applying a patch on a custom_object with content_type="application/strategic-merge-patch"
as you suggested,
patch_ingressroute_second = ingressroute_api.patch(
body=ingressroute_manifest_second, content_type="application/strategic-merge-patch+json"
)
it throws the following error:
Reason: Unsupported Media Type
HTTP response headers: HTTPHeaderDict({'Audit-Id': 'a7b2fb82-0b68-40ee-8439-183cc9c1c0f0', 'Cache-Control': 'no-cache, private', 'Content-Type': 'application/json', 'X-Kubernetes-Pf-Flowschema-Uid': 'd4c6d349-a9e2-454c-b88f-5beb06b64c0b', 'X-Kubernetes-Pf-Prioritylevel-Uid': 'fa2d2d6c-312f-41bb-bc32-d00c53695884', 'Date': 'Mon, 03 May 2021 07:14:43 GMT', 'Content-Length': '293'})
HTTP response body: b'{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json, application/apply-patch+yaml","reason":"UnsupportedMediaType","code":415}\n'
I also see in the CustomObjectApi
not mentioning application/strategic-merge-patch
anywhere in the content-type
headers: https://github.com/kubernetes-client/python/blob/master/kubernetes/docs/CustomObjectsApi.md#http-request-headers-18
Could you please point me if content_type="application/strategic-merge-patch"
is actually not supported & needs to be implemented in the CustomObjectApi
or I'm doing something wrong here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
789e374#diff-5a785a7fc5b19b934975d4d49eb71d730de127362b35f89e368179b8355796c8 is the patch to support strategic merge patch.
@roycaihw do you know why is this error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, strategic merge patch is not supported currently for custom resources
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @yliaog, for confirming.
One question: as you pointed in the comments above, that there is already a patch for supporting strategic-merge-patch
. Is that something that would be available in the upcoming releases? Or that is not a patch for supporting strategic-merge-patch
using dynamic-client
?
Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
native k8s resources like "deployment" support strategic-merge-patch. CR does not currently have that support. The support needs to be on the k8s server side, not on the python client side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
understood. Thank you. :)
body=ingressroute_manifest_one, content_type="application/merge-patch+json" | ||
) | ||
patch_ingressroute_second = ingressroute_api.patch( | ||
body=ingressroute_manifest_second, content_type="application/merge-patch+json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
ingressroute_manifest_second["spec"]["entrypoints"] = ["web"] | ||
|
||
patch_ingressroute_one = ingressroute_api.patch( | ||
body=ingressroute_manifest_one, content_type="application/merge-patch+json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use strategic-merge-patch.
body=ingressroute_manifest_one, content_type="application/merge-patch+json" | ||
) | ||
patch_ingressroute_second = ingressroute_api.patch( | ||
body=ingressroute_manifest_second, content_type="application/merge-patch+json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
fa40565
to
62fd897
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Priyankasaggu11929, yliaog The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
62fd897
to
1d9ba9f
Compare
/lgtm |
1d9ba9f
to
22bb5b2
Compare
22bb5b2
to
1ff6ece
Compare
Hi @yliaog , could you please mark it Thank you! |
/lgtm |
What type of PR is this?
/kind documentation
What this PR does / why we need it:
This PR adds example python scripts to demonstrate the usage of dynamic client. Below are the files, I've added:
cluster_scoped_custom_resource.py

Output:
namespaced_custom_resource.py

Output:
configmap.py

Output:
node.py

Output:
replication_controller.py

Output:
service.py

Output:
Which issue(s) this PR fixes:
Fixes #1429
Does this PR introduce a user-facing change?