-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Switch AdmissionReview from v1beta1 to v1 #4537
Switch AdmissionReview from v1beta1 to v1 #4537
Conversation
Welcome @adriananeci! |
/assign @krzysied |
8a4f038
to
1577ded
Compare
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.
I have some questions about migration:
- Can we drop dependency on
v1beta1
admission API? - If I have an existing cluster, with VPA webhook created using
v1beta1
admission API and switch to VPA usingv1
admission API what will happen? Will we overwrite the existing webhook? Will we have 2 webhooks? - What will happen if I need to rollback VPA to an older version? So I'll switch from VPA using
v1
admission API to older VPA usingv1beta1
admission API?
Yes, just removed
Tested it on a kind cluster and the old webhook will be replaced with the new one having
Everything should go smooth since the Validated it on a kind cluster:
|
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 for the answering quickly. This PR mostly looks good to me but I have a few small requests.
Please:
- fix commit message
AdmissionReviw
misses letter 'e' and - squash commits,
- sync (I think with Send UID too in AdmissionReview response #4538 merged I think there will be a small conflict to resolve).
e643ea1
to
33b0215
Compare
33b0215
to
dfce769
Compare
Rebased from master, squashed commits and fixed commit message |
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adriananeci, jbartosik 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 PR updates AdmissionReview from v1beta1 to v1. In V1, the UID field is mandatory and should be copied from the
request.uid
sent to the webhook based on https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#responseNo other notable changes between AdmissionReview
v1beta1
andv1
. AdmissionReviewv1
is available since k8s v1.16.