-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Update Server-Side Apply KEP with upgrade strategy from client-side apply #1382
Update Server-Side Apply KEP with upgrade strategy from client-side apply #1382
Conversation
Welcome @julianvmodesto! |
Hi @julianvmodesto. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
8cce675
to
d52cdc6
Compare
cc @lavalamp |
keps/sig-api-machinery/0006-apply.md
Outdated
|
||
When upgrading the API server with server-side apply enabled, then existing objects | ||
with a `last-applied-configuration` annotation will be upgraded on the first | ||
use of server-side apply with `kubectl apply --server-side` by |
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.
this implies that when we turn SSA on for everything, we'll actually check for the presence of the annotation and not do the update logic if it is found (or at least not store the result)?
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.
Oh no, I'll fix this. We do want to proceed with and store the update logic after reading and converting the annotation.
I'll copy an example from Jenny's doc to better illustrate what will happen here. |
eb27f68
to
e90a8b2
Compare
a233224
to
aa78df0
Compare
I'll update this to match the current changes in kubernetes/kubernetes#84134 and kubernetes/kubernetes#90187 |
aa78df0
to
01531ae
Compare
Updated to match the changes in kubernetes/kubernetes#90187 |
/wg api-expression |
/lgtm |
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.
Just one minor comment.
keps/sig-api-machinery/0006-apply.md
Outdated
the user now wants to manage with server-side apply will be owned by the | ||
client-side apply field manager. | ||
|
||
In this case, users would need to force conflicts with `kubectl apply |
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.
It's not clear that this is a problem we seek to avoid, I'd reword to add something like "If we don't specifically handle this case, then".
01531ae
to
b15e2a2
Compare
/lgtm thanks! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: julianvmodesto, lavalamp 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 |
This change updates the server-side apply KEP with the strategy for upgrading client-side
kubectl apply
'slast-applied-configuration
annotation to server-side apply, as originally provided by @jennybuckley in Upgrade pathway: Client Side Apply to Server Side Apply.WIP here: kubernetes/kubernetes#84134
Umbrella: kubernetes/kubernetes#73723
/wg apply