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

Can't replicate kubectl apply -f functionality with Python client #1028

Closed
Datamance opened this issue Dec 5, 2019 · 14 comments
Closed

Can't replicate kubectl apply -f functionality with Python client #1028

Datamance opened this issue Dec 5, 2019 · 14 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@Datamance
Copy link

Datamance commented Dec 5, 2019

What happened (please include outputs or screenshots):
The kubectl CLI uses a StrategicMergePatch type in the underlying API request for apply.

The Python client should be able to do the same by letting us pass a Content-Type of 'application/strategic-merge-patch+json' into the headers dict, but this odd little piece of generated code precludes us from doing so

header_params['Content-Type'] = self.api_client.select_header_content_type( # noqa: E501

I would like to leverage the Python client as much as possible without resorting to kubectl. Is there a way to get around this for the time being?

Thanks in advance!

Environment:

  • Kubernetes version (kubectl version):

  • OS (e.g., MacOS 10.13.6):
    10.15.1

  • Python version (python --version)
    3.8.0

  • Python client version (pip list | grep kubernetes)
    10.0.1

@Datamance Datamance added the kind/bug Categorizes issue or PR as related to a bug. label Dec 5, 2019
@Datamance
Copy link
Author

OK, I see there appears to be some activity on this precise issue now. The Java client seemed to be having the same kind of issue in 2017, to boot. Codegen seems to be a bit unwieldy.

Still interested in any workarounds that stay in Python-land!

@roycaihw
Copy link
Member

We patched the generated rest client to allow using strategic merge patch:

if headers['Content-Type'] == 'application/json-patch+json':
if not isinstance(body, list):
headers['Content-Type'] = \
'application/strategic-merge-patch+json'

That being said, kubectl apply is more than sending a strategic merge patch-- it has logic to do a three-way diff to compose the patch. A better way to do that is to use the new server-side apply feature kubernetes/enhancements#555. We should support sending patch with application/apply-patch content type

@Datamance
Copy link
Author

@roycaihw it looks like this feature is not generally available as of 1.17?

@roycaihw
Copy link
Member

No it is not. From what I heard it was supported for a sub-portion of Kubernetes APIs in 1.15.

@Datamance
Copy link
Author

@roycaihw given that this client doesn't implement the three-way diff logic found in the kubectl tool, and you don't yet support hooking into the server-side apply feature, if we want to emulate this behavior right now - do we have to shell out to kubectl? If so, when would you expect the server-side apply feature to be generally available, and how long after that would you be adding support in the Python client?

Related - is this the logic that you're referring to? If so, I might try my hand at implementing it myself - especially if the only two requirements are 1) getting the patch body correct and 2) having the correct content-type.

@Datamance
Copy link
Author

also @roycaihw doesn’t that rest client patch preclude me from doing anything other than a strategic merge patch? I’m on a mobile device so it’s a little hard to look at code right now but I’m unclear as to when body is going to be a list - perhaps this is only then for when I’m patching multiple resources? But then, what if I want to do a strategic merge patch to multiple resources? Or do another type of patch on a single resource?

@couaky
Copy link

couaky commented Dec 20, 2019

Hi !

As I understand with the current patched generator, all PATCH requests will use application/strategic-merge-patch+json or application/json-patch+json if body is a list and there is no workaround to use the other type application/merge-patch+json until the mentioned merge request will be done ?

Thanks.

@Datamance
Copy link
Author

@roycaihw when approximately will this client be supporting the application/apply-patch content type? Also I'm guessing that they'll support a +json prefix as well but just wanted to make 100% sure given that the documentation only mentions a application/apply-patch+yaml

@Datamance
Copy link
Author

@roycaihw and @micw523 - it looks like the two of you are the major contributors to this repository. Are you able to answer to some of the questions above, or at least provide some sort of status update?

@micw523
Copy link
Contributor

micw523 commented Jan 29, 2020

I'd be watching the status of PR #959. There are propositions of how to deal with it, but it might require upstream (generator) changes.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 30, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 30, 2020
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

6 participants