-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(recommender): add enforce cpu memory ratio #8420
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
base: master
Are you sure you want to change the base?
Conversation
Welcome @Jrmy2402! |
Hi @Jrmy2402. 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-sigs/prow repository. |
d914a14
to
1986edf
Compare
5a27892
to
37f23aa
Compare
Thanks for this! |
/release-note feat(recommender): add enforce cpu memory ratio |
/ok-to-test |
I'm wondering if this change makes sense as a flag. It seems that moving these sorts of configurable option to the VPA API would seem to be more desirable, as a user may want to enable this on a subset of workloads controlled by the VPA |
Yeah totally agree here. |
Ok, thanks for the review. I'll rework the MR to handle this via the VPA API. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Jrmy2402 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 |
1db9ede
to
7786c6f
Compare
2f22344
to
a3ea8ba
Compare
a3ea8ba
to
0a85516
Compare
Update: switched from the global |
This should be behind an alpha feature flag with default off |
Does this type of change need an AEP? I assume so, since it's an API change. Sorry for the extra scope. |
Ok I will add an alpha feature flag with default off next week. |
We added a lot of AEPs lately, you can look at some of them to get the hang of it. It's pretty straightforward. |
It's similar to the KEP process for Kubernetes, but it's much more light weight. We are trying to ensure a high standard and also keep the standard similar to Kubernetes (ie: API review, feature gates, etc), but they aren't that hard to do, and as Omer suggested, take a look at the most recent ones. They live in https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/enhancements |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adds --enforce-cpu-memory-ratio flag to the VPA recommender, allowing users to enforce a fixed memory-per-CPU ratio (expressed as bytes per millicores) across all resource recommendations.
This ensures that CPU and memory recommendations remain balanced according to a user-defined ratio, automatically adjusting one resource when the other changes.
Special notes for your reviewer:
Does this PR introduce a user-facing change?
The ratio is applied to Target, LowerBound, UpperBound, and UncappedTarget recommendations.
The value of --enforce-cpu-memory-ratio should be specified in bytes per millicores (e.g., 4294967.296 for 1 CPU → 4 GiB).