Skip to content

VPA: Per-node recommendations (very early draft) #7978

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bboreham
Copy link
Contributor

What type of PR is this?

/kind feature

/kind api-change

What this PR does / why we need it:

This is a very early draft in pursuit of AEP-7942.

Adds an optional field scope on the VPA object, to let the VPA components generate and update more fine-grained recommendations.

Only one value of scope is implemented by this PR: node.
If this is set, record the node name as an extra label on each pod, then extract it when writing the recommendation.
Inside the admission-controller we recognize which node the pod will run on by unpacking nodeAffinity.

Special notes for your reviewer:

The change to version.go is intended as temporary while testing, not intended to be merged.

Does this PR introduce a user-facing change?

Adds an optional field `scope` on the VPA object, to let the VPA components generate and update more fine-grained recommendations. See AEP-7942.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

AEP-7942


Record the node name as an extra label on each pod, then extract it when
writing the recommendation.

Inside the admission-controller we recognize which node the pod will
run on by unpacking nodeAffinity.

Signed-off-by: Bryan Boreham <[email protected]>
@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 25, 2025
@k8s-ci-robot k8s-ci-robot requested a review from raywainman March 25, 2025 19:56
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 25, 2025
@voelzmo
Copy link
Contributor

voelzmo commented Mar 26, 2025

@bboreham Thanks for starting this! Can you give me a ping once this is in a state where you think it is mostly doing what it should do?

@voelzmo
Copy link
Contributor

voelzmo commented Mar 26, 2025

For future reference: with the current state, the VPA status gets a scope string for each containerRecommendation, resulting in a structure like this

conditions:
  - lastTransitionTime: "2025-03-26T12:35:08Z"
    status: "True"
    type: RecommendationProvided
recommendation:
  containerRecommendations:
    - containerName: pause
      lowerBound:
        cpu: 100m
        memory: 262144k
      scope: node=kind-control-plane
      target:
        cpu: 100m
        memory: 262144k
      uncappedTarget:
        cpu: 25m
        memory: 262144k
      upperBound:
        cpu: 185m
        memory: 262144k

So for clusters with a big number of nodes the status grows quite a lot.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bboreham
Once this PR has been reviewed and has the lgtm label, please assign voelzmo 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:

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

@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

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 k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vertical-pod-autoscaler cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants