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

Patch strategy support #24

Closed
clux opened this issue May 26, 2019 · 3 comments
Closed

Patch strategy support #24

clux opened this issue May 26, 2019 · 3 comments
Labels
good first issue Good for newcomers help wanted Not immediately prioritised, please help!

Comments

@clux
Copy link
Member

clux commented May 26, 2019

Currently Api::patch and similar only support --type=merge (i.e. .header("Content-Type", "application/merge-patch+json")).

Would be good to support the 3 strategies.

Note that CRDs doesn't seem to support strategic merge atm, so need better tests. See kubernetes/kubernetes#52772.

@clux clux mentioned this issue May 27, 2019
@clux clux added enhancement good first issue Good for newcomers help wanted Not immediately prioritised, please help! labels May 31, 2019
@ragne
Copy link
Contributor

ragne commented Jul 2, 2019

Hi, I'd like to tackle on the issue. How resulting api should look like?

  • The simple idea is to add another parameter (enum) to Api::patch but it doesn't sound that great, though.
  • Second one is to add that parameter to PostParams (create another struct PatchParams?) and check in Api::patch like it was done for dry_run?

@clux
Copy link
Member Author

clux commented Jul 3, 2019

Hey, awesome! Please go right ahead.

I totally agree with the PatchParams, it's already close to what's in kubernetes under meta/v1#PatchOptions.

@clux
Copy link
Member Author

clux commented Jul 3, 2019

Funnily enough though, client-go still passes in an enum and a PatchOptions object 😕
https://github.com/kubernetes/client-go/blob/e6a502f77fded2b9d038629506553ae5bb6e0388/dynamic/client_test.go#L641

clux added a commit that referenced this issue Jul 10, 2019
#24 PatchParams and PatchStrategy implementation
@clux clux closed this as completed Jul 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Not immediately prioritised, please help!
Projects
None yet
Development

No branches or pull requests

2 participants