AEP-8459: MemoryPerCPU Enforce #8459

Jrmy2402 · 2025-08-19T15:16:20Z

What type of PR is this?
/kind documentation
/kind feature
/kind api-change

What this PR does / why we need it:
AEP for #8420

k8s-ci-robot · 2025-08-19T15:16:23Z

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

linux-foundation-easycla · 2025-08-19T15:16:24Z

The committers listed above are authorized under a signed CLA.

✅ login: Jrmy2402 / name: Jérémy S (61e4f32, 35b9ab3, f4b20b6, 1766fc2, 5fed004, 5bf11a4)

k8s-ci-robot · 2025-08-19T15:16:30Z

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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

k8s-ci-robot · 2025-08-19T15:16:30Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Jrmy2402
Once this PR has been reviewed and has the lgtm label, please assign towca for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

vertical-pod-autoscaler/enhancements/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

adrianmoisey · 2025-08-19T15:18:48Z

/ok-to-test

k8s-triage-robot · 2025-08-19T15:50:53Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

omerap12

Thanks for this! some notes from me

omerap12 · 2025-08-20T19:55:12Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+        controlledResources: ["cpu", "memory"]
+        controlledValues: RequestsAndLimits
+        memoryPerCPU: "4Gi"


So in this case, VPA should skip the memory suggestion, right? We can also force this (for example, make sure memoryPerCPU and controlledResources.memory are mutually exclusive) . think just skipping is better. Just an idea.

What happens if the memory suggestion was greater than 4Gi per CPU? Surely it needs to be increased then?

I don't think so. You specify that for every cpu you will get 4Gi. the all idea is to have a "hard-coded" cpu-memory ratio. As I wrote in the comment below those use cases should be clear to the users.

The intent is that the ratio is strictly enforced in both directions.
Concretely: if the memory recommendation is higher than cpu * memoryPerCPU, then CPU will be increased accordingly. Likewise, if CPU is higher than memory / memoryPerCPU, then memory is increased.
I’ll update the AEP to make this explicit.

Thanks for the explanation, could you also update the use cases as Omer had suggested, since it's not clear to me why someone may want this feature.

Thanks for the feedback! I just pushed a commit updating the Motivation section to clarify the use cases, as suggested.

In our specific case, we want to use VPA to automatically scale our customers' services vertically, but since we charge them based on CPU with a guaranteed CPU-to-memory ratio, we need VPA to respect this fixed ratio in its recommendations.

Why do you want to express ratio as a resource.Quantity and not just a plain integer or float, if you're considering partials?

A different question, taking a step back, is there a possibility where users will be interested in providing a ration for different pair of resources? The answer will allow us to better match the name for this variable.

My initial reasoning was consistency with other VPA fields that already use resource.Quantity. But you’re right that semantically this field represents a ratio, not a resource amount. I’m fine switching to a float or plain integer if that’s the preferred approach.

I’m not 100% sure I understood the second question. What do you mean by "users will be interested in providing a ratio for different pair of resources"?

omerap12 · 2025-08-20T19:56:27Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+* If both CPU and memory are controlled, VPA enforces the ratio.  
+* Applies to Target, LowerBound, UpperBound, and UncappedTarget.  
+* If ratio cannot be applied (e.g., missing CPU), fallback to standard recommendations.  
+* With feature gate OFF: recommendations are unaffected.


Can we please improve this? it's very unclear to me.

Thanks for the feedback! I just pushed an update to clarify the wording and make it more explicit.

omerap12 · 2025-08-20T19:57:10Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+**When enabled**:  
+* VPA honors `memoryPerCPU` in recommendations.  


And ignore memory recommendation ...

When the memory suggestion is greater than 4Gi per CPU, the CPU will be increased to respect the memoryPerCPU ratio

omerap12 · 2025-08-20T19:58:23Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+
+### Test Plan
+
+* Unit tests ensuring ratio enforcement logic.  


We also need e2e tests. unit tests for AEP is not enough

I just pushed a commit updating the Test Plan section to explicitly mention adding e2e tests in addition to unit tests.
Thanks

I'd require tests ensuring that when the feature gate is on or off the values and validation is applied accordingly.

Got it, thanks! I’ve updated the Test Plan so that unit tests explicitly cover both enforcement logic and the feature gate being on/off.

omerap12

Overall LGTM with minor comments

omerap12 · 2025-08-22T12:22:48Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+### Behavior
+
+* If both CPU and memory are controlled, VPA enforces the ratio.  
+* Applies to Target, LowerBound, UpperBound, and UncappedTarget.
+* Ratio enforcement is strict:
+  * If the memory recommendation would exceed `cpu * memoryPerCPU`, then **CPU is increased** to satisfy the ratio.
+  * If the CPU recommendation would exceed `memory / memoryPerCPU`, then **memory is increased** to satisfy the ratio.
+* If ratio cannot be applied (e.g., missing CPU), fallback to standard recommendations.  
+* With the `MemoryPerCPURatio` feature gate disabled, the `memoryPerCPU` field is ignored and recommendations fall back to standard VPA behavior.


Could you add some examples here to help users better understand how the algorithm behaves?

Done, I’ve added examples to clarify the behavior in commit 5fed004

omerap12 · 2025-08-22T12:24:20Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+* Components depending on the feature gate:
+  * recommender


@adrianmoisey , should we also add this to the admission controller? ( we talked about this stuff on Slack ).

To be consistent with the other feature gates, my vote is a yes

@Jrmy2402 , please update this too. you can see an example of it here: https://github.com/kubernetes/autoscaler/pull/8012/files#diff-ad66c76a76541b7991631925641b989fc402901440d8e0dcbc3591009eef52b9R226

I added the admission controller to the list, and I will update my MR #8420 this week after the AEP is merged

omerap12 · 2025-08-22T12:36:52Z

/retitle AEP-8459: MemoryPerCPU Enforce

omerap12 · 2025-08-22T12:39:43Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+memory_bytes = cpu_cores * memoryPerCPU
+```
+
+## Design Details


Like https://github.com/kamarabbas99/autoscaler/blob/master/vertical-pod-autoscaler/enhancements/7862-cpu-startup-boost/README.md?plain=1#L126 this too should be capped.

Oh yes, thanks.
I’ve updated the AEP to mention that memoryPerCPU-based recommendations are also capped by --container-recommendation-max-allowed-cpu and --container-recommendation-max-allowed-memory, similar to the CPU Startup Boost case.

See commit: 5fed004

omerap12

Overall looks good we just need to address this https://github.com/kubernetes/autoscaler/pull/8459/files#r2295957092

omerap12 · 2025-08-23T15:38:55Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+* Example 4: Feature gate disabled
+  * Baseline recommendation: 1 CPU, 6Gi memory
+  * UncappedTarget: 1 CPU, 6Gi (ratio not applied)
+  * Target: 1 CPU, 6Gi


This is unneeded ( it's pretty clear that when the feature flag is off VPA works as usual ).

I removed this example, thanks

The example is still there, do you still need to push a change?

Oops, I forgot to push my last commit...I’ve pushed it now.

omerap12 · 2025-08-31T07:57:51Z

/label api-review

soltysh · 2025-09-03T14:05:44Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+        controlledResources: ["cpu", "memory"]
+        controlledValues: RequestsAndLimits
+        memoryPerCPU: "4Gi"


Why do you want to express ratio as a resource.Quantity and not just a plain integer or float, if you're considering partials?

soltysh · 2025-09-03T14:07:02Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+
+### Behavior
+
+* If both CPU and memory are controlled, VPA enforces the ratio.  


What if both (cpu and memory) are not specified? Should that be a validation error? It seems, like we should enforce that if you specify both you should get an error, this way we'll ensure that either you specify all the pieces of the puzzle, or none.

Initially, my thinking was to simply ignore memoryPerCPU if either CPU or memory was not specified in controlledResources.

But if the philosophy is rather to fail fast and return a validation error whenever memoryPerCPU is set without both CPU and memory being present, I’m fine with that approach too, I can update the AEP accordingly.

soltysh · 2025-09-03T14:08:11Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+* Applies to Target, LowerBound, UpperBound, and UncappedTarget.
+* Ratio enforcement is strict:
+  * If the memory recommendation would exceed `cpu * memoryPerCPU`, then **CPU is increased** to satisfy the ratio.
+  * If the CPU recommendation would exceed `memory / memoryPerCPU`, then **memory is increased** to satisfy the ratio.


I'm inclined to say we should error out if the math doesn't stand with the cpu and memory values, adjusting seems "magical", and I'd advice against it. Explicitness is always better.

I see your point, implicit adjustments can indeed feel “magical.”
In this case, though, the whole purpose of the feature is to enforce the ratio automatically: if CPU or memory drifts away from the configured ratio, VPA brings them back in line.

If we only validated and errored, users wouldn’t get the behavior they’re asking for (“always keep memory = cpu × memoryPerCPU”), they’d just see failures.
That would make the feature much less useful in practice.

Or maybe I didn’t fully understand your point?

soltysh · 2025-09-03T14:09:10Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+
+### Test Plan
+
+* Unit tests ensuring ratio enforcement logic.  


I'd require tests ensuring that when the feature gate is on or off the values and validation is applied accordingly.

soltysh · 2025-09-03T14:10:13Z

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md

+        controlledResources: ["cpu", "memory"]
+        controlledValues: RequestsAndLimits
+        memoryPerCPU: "4Gi"


A different question, taking a step back, is there a possibility where users will be interested in providing a ration for different pair of resources? The answer will allow us to better match the name for this variable.

k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Aug 19, 2025

k8s-ci-robot requested review from adrianmoisey and voelzmo August 19, 2025 15:16

k8s-ci-robot added area/vertical-pod-autoscaler size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed do-not-merge/needs-area labels Aug 19, 2025

Jrmy2402 force-pushed the AEP-cpu-memory-ratio branch from d7748f1 to 4ca0ab5 Compare August 19, 2025 15:16

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 19, 2025

docs: add AEP for MemoryPerCPU feature

61e4f32

Jrmy2402 force-pushed the AEP-cpu-memory-ratio branch from 4ca0ab5 to 61e4f32 Compare August 19, 2025 15:17

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 19, 2025

Jrmy2402 mentioned this pull request Aug 19, 2025

feat(recommender): add enforce cpu memory ratio #8420

Open

omerap12 reviewed Aug 20, 2025

View reviewed changes

update AEP

1766fc2

omerap12 reviewed Aug 22, 2025

View reviewed changes

k8s-ci-robot changed the title ~~docs: add AEP for MemoryPerCPU feature~~ AEP-8459: MemoryPerCPU Enforce Aug 22, 2025

omerap12 reviewed Aug 22, 2025

View reviewed changes

improve the Motivation

f4b20b6

Jrmy2402 force-pushed the AEP-cpu-memory-ratio branch from c5bbed0 to f4b20b6 Compare August 22, 2025 12:51

improve the Design Details

5fed004

omerap12 reviewed Aug 23, 2025

View reviewed changes

remove example 4

5bf11a4

k8s-ci-robot added the api-review Categorizes an issue or PR as actively needing an API review. label Aug 31, 2025

github-project-automation bot added this to API Reviews Aug 31, 2025

soltysh reviewed Sep 3, 2025

View reviewed changes

update test plan

35b9ab3

		When enabled:
		* VPA honors `memoryPerCPU` in recommendations.


		### Behavior

		* If both CPU and memory are controlled, VPA enforces the ratio.

AEP-8459: MemoryPerCPU Enforce #8459

Are you sure you want to change the base?

AEP-8459: MemoryPerCPU Enforce #8459

Conversation

Jrmy2402 commented Aug 19, 2025

Uh oh!

k8s-ci-robot commented Aug 19, 2025

Uh oh!

linux-foundation-easycla bot commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Aug 19, 2025

Uh oh!

k8s-ci-robot commented Aug 19, 2025

Uh oh!

adrianmoisey commented Aug 19, 2025

Uh oh!

k8s-triage-robot commented Aug 19, 2025

Uh oh!

omerap12 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

omerap12 Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

omerap12 Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

omerap12 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

omerap12 commented Aug 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Aug 19, 2025 •

edited

Loading

omerap12 Aug 21, 2025 •

edited

Loading

omerap12 Aug 20, 2025 •

edited

Loading

Jrmy2402 Sep 15, 2025 •

edited

Loading