-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
AEP-4016: remove mention of 'partial updates' in testing plan #7896
base: master
Are you sure you want to change the base?
Conversation
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
/lgtm /assign @voelzmo |
/lgtm |
@@ -229,8 +229,6 @@ tested in the following scenarios: | |||
|
|||
* Admission controller applies recommendation to pod controlled by VPA. | |||
* In-place update applied to all containers of a pod. | |||
* Partial updates applied to some containers of a pod, some changes skipped (request in |
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.
Can we still mention this point somewhere in this AEP? Maybe in "Details still to consider" section? So this doesn't get lost. It'll be great to implement partial updates at some point :)
Signed-off-by: Max Cao <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: maxcao13, omerap12 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm Good call out on retaining the info, rather than deleting it |
/lgtm |
What type of PR is this?
/kind documentation
/kind cleanup
What this PR does / why we need it:
Removes mention of "partial updates" from the AEP-4016 testing plan.
This is because, VPA accepts whole pods for updates if at least 1 container is considered "out of recommendation bounds", it doesn't care about which one(s). If at least one container is out of bounds, the updater will try to in-place resize the entire pod and update all containers with the most current recs. We shouldn't need to introduce new logic to apply resizes only to the containers that actually have recommendations out of bounds.
Which issue(s) this PR fixes:
Fixes #
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: