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

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 (1766fc2, 35b9ab3, 5bf11a4, 5fed004, 61e4f32, 93d9437, f4b20b6)

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

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

omerap12

Overall LGTM with minor comments

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

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

/retitle AEP-8459: MemoryPerCPU Enforce

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

omerap12

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

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

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

/label api-review

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

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?

I think we're talking about two distinct things 😅 I was more asking about the validation case, where we ensure that the provided memory and cpu ensure we can reach the configured memoryPerCPU. Iow. if I specify cpu=1, memory=4Gi and use memoryPerCPU=5 then the math won't work and that should fail validation, but cpu=1, memory=4Gi and memoryPerCPU=3 will work, b/c that's achievable.

Whereas you're talking about the actual enforcement, which indeed will be "magical" 😉, and that's totally fine.

Does that make sense?

Absolutely, we were talking about two different layers.

I’ve pushed a commit to clarify the validation side: 93d9437

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

Jrmy2402 · 2025-09-30T07:44:23Z

Hi, @soltysh,

Did you check my answers to your comments?

Thanks in advance

Jrmy2402 · 2025-10-07T07:42:06Z

Hi,

@omerap12, @adrianmoisey do you think it could be merged as it is today, or do you have some comments?

Thanks in advance

omerap12 · 2025-10-07T09:37:46Z

Hi,

@omerap12, @adrianmoisey do you think it could be merged as it is today, or do you have some comments?

Thanks in advance

We can't merge this until a Kubernetes api reviewer lgtm it.
We will ping them :)

jackfrancis · 2025-10-07T15:46:21Z

/release-note-none

jackfrancis · 2025-10-07T15:47:05Z

/label tide/merge-method-squash

soltysh

From API perspective this seems good, my comments are non-blocking, but mostly clarifying.

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

soltysh · 2025-10-09T12:14:49Z

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 think we're talking about two distinct things 😅 I was more asking about the validation case, where we ensure that the provided memory and cpu ensure we can reach the configured memoryPerCPU. Iow. if I specify cpu=1, memory=4Gi and use memoryPerCPU=5 then the math won't work and that should fail validation, but cpu=1, memory=4Gi and memoryPerCPU=3 will work, b/c that's achievable.

Whereas you're talking about the actual enforcement, which indeed will be "magical" 😉, and that's totally fine.

Does that make sense?

Jrmy2402 · 2025-10-15T09:44:21Z

Thanks @soltysh, @omerap12, @adrianmoisey, @jackfrancis for the review.
What is the next step to merge the PR ?

omerap12 · 2025-10-15T14:11:18Z

Thanks! nothing blocking from my end.
/lgtm
/assign @adrianmoisey

adrianmoisey · 2025-10-15T19:01:55Z

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

+* Example 3: `memoryPerCPU = 4Gi`, with `--container-recommendation-max-allowed-memory=7Gi` or with `maxAllowed.memory=6Gi` set in VPA object
+  * Baseline recommendation: 2 CPUs, 4Gi memory
+  * UncappedTarget (ratio enforced): 2 CPUs, 8Gi
+  * Target (capped): 2 CPUs, 7Gi  ← memory capped by max-allowed-memory; ratio not fully satisfied


I'm unsure if this is a typo or if I'm not understanding something, but surely the Target on this one would be 2 CPUs, 6Gi memory, since maxAllowed.memory is 6Gi?

adrianmoisey · 2025-10-15T19:04:49Z

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

+* Admission ensures the provided `CPU` and `memory` bounds make the configured `memoryPerCPU` reachable—there must exist at least one (`cpu`, `memory`) such that `memory = cpu × memoryPerCPU`, otherwise the object is rejected. 
+  * Example: with cpu=1 and memory=4Gi, memoryPerCPU=5Gi is invalid (1×5Gi > 4Gi), while memoryPerCPU=3Gi is valid (1×3Gi ≤ 4Gi).


What is cpu and memory referring to here? Is this on Pod admission or is this the minAllowed/maxAllowed in the VPA spec?

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

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

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

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

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

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

vertical-pod-autoscaler/enhancements/8459-memory-per-cpu/README.md Outdated Show resolved Hide resolved

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

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed 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 Oct 7, 2025

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Oct 7, 2025

soltysh reviewed Oct 9, 2025

View reviewed changes

docs(aep-8459): clarify validation to ensure memoryPerCPU is reachable

93d9437

k8s-ci-robot assigned adrianmoisey and omerap12 Oct 15, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 15, 2025

adrianmoisey reviewed Oct 15, 2025

View reviewed changes


		### Behavior

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

		* Admission ensures the provided `CPU` and `memory` bounds make the configured `memoryPerCPU` reachable—there must exist at least one (`cpu`, `memory`) such that `memory = cpu × memoryPerCPU`, otherwise the object is rejected.
		* Example: with cpu=1 and memory=4Gi, memoryPerCPU=5Gi is invalid (1×5Gi > 4Gi), while memoryPerCPU=3Gi is valid (1×3Gi ≤ 4Gi).

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 • edited by jackfrancis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

omerap12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

omerap12 commented Aug 22, 2025

Uh oh!

Uh oh!

omerap12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

omerap12 commented Aug 31, 2025

Uh oh!

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!

Jrmy2402 Sep 15, 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!

Uh oh!

Uh oh!

Jrmy2402 commented Sep 30, 2025

Uh oh!

Jrmy2402 commented Oct 7, 2025

Uh oh!

omerap12 commented Oct 7, 2025

Uh oh!

jackfrancis commented Oct 7, 2025

Uh oh!

jackfrancis commented Oct 7, 2025

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jrmy2402 commented Oct 15, 2025

Uh oh!

omerap12 commented Oct 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jrmy2402 commented Aug 19, 2025 •

edited by jackfrancis

Loading

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

edited

Loading

Jrmy2402 Sep 15, 2025 •

edited

Loading