Skip to content

feat: deprecate UpdateMode Auto in VPA #8426

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 14, 2025
Merged

feat: deprecate UpdateMode Auto in VPA #8426

merged 3 commits into from
Aug 14, 2025
+132 −6

Conversation

rushmash91
Copy link
Member

@rushmash91 rushmash91 commented Aug 10, 2025 •
edited
Loading

What type of PR is this?

/kind deprecation

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #8424

Special notes for your reviewer:

This PR implements deprecation warnings for the VPA UpdateMode "Auto" value as part of the deprecation process. The Auto mode is now deprecated and users are encouraged to use explicit update modes like "Recreate", "Initial", or "InPlaceOrRecreate" instead.

Does this PR introduce a user-facing change?

VPA UpdateMode "Auto" is now deprecated. Users will see API warnings when creating VPAs with updateMode: "Auto" and should migrate to explicit modes like "Recreate", "Initial", or "InPlaceOrRecreate". The default updateMode for new VPAs without explicit configuration has changed from "Auto" to "Recreate".

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/documentation Categorizes issue or PR as related to documentation. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-area size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 10, 2025
@k8s-ci-robot k8s-ci-robot requested a review from jbartosik August 10, 2025 12:37
@rushmash91 rushmash91 force-pushed the deprecate-auto-updatemode branch from 1d726e6 to ac56983 Compare August 10, 2025 12:57
@rushmash91 rushmash91 changed the title feat: deprecate UpdateMode Auto in VPA feat: deprecate UpdateMode Auto in VPA Aug 10, 2025
@rushmash91 rushmash91 force-pushed the deprecate-auto-updatemode branch from ac56983 to 7ca3bb2 Compare August 10, 2025 13:13
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 10, 2025
omerap12
omerap12 reviewed Aug 10, 2025
View reviewed changes
vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go
Comment on lines +98 to +99
// Changed from UpdateModeAuto to UpdateModeRecreate as part of Auto mode deprecation
defaultUpdateMode := vpa_types.UpdateModeRecreate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect.
People who use the default setting would be suddenly and unexpectedly affected, because their VPA objects would now use Recreate by default.
Recreate is not really less stable than Auto here, but the rule shows we need a smooth change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought it might be ok since currently Auto = Recreate.
I still think we should revert this but I may be wrong here :)
@soltysh @adrianmoisey

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Let me know if its required or not 🙂

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this is only for newly created objects, in which case that's perfectly reasonable approach. Since you're starting the deprecation, you should not set the default to a deprecated value. As Omer mentioned, even though this might come as a surprise to some (Auto vs Recreate). But the outcome will be the same, given that currently Auto means Recreate.

@rushmash91 rushmash91 force-pushed the deprecate-auto-updatemode branch 2 times, most recently from 8c85301 to 3af6d54 Compare August 10, 2025 22:14
@rushmash91 rushmash91 force-pushed the deprecate-auto-updatemode branch from 3af6d54 to f99c5e1 Compare August 10, 2025 22:25
omerap12
omerap12 reviewed Aug 11, 2025
View reviewed changes
Copy link
Member

@omerap12 omerap12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!
Overall LGTM except this https://github.com/kubernetes/autoscaler/pull/8426/files#r2265295659.
/lgtm
/assign @adrianmoisey

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2025
soltysh
soltysh reviewed Aug 13, 2025
View reviewed changes
Copy link

@soltysh soltysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the warning you're adding there. Overall lgtm :)

vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go
Comment on lines +98 to +99
// Changed from UpdateModeAuto to UpdateModeRecreate as part of Auto mode deprecation
defaultUpdateMode := vpa_types.UpdateModeRecreate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this is only for newly created objects, in which case that's perfectly reasonable approach. Since you're starting the deprecation, you should not set the default to a deprecated value. As Omer mentioned, even though this might come as a surprise to some (Auto vs Recreate). But the outcome will be the same, given that currently Auto means Recreate.

@omerap12
Copy link
Member

since @soltysh LGTMed we can merge this
cc @adrianmoisey

@adrianmoisey
Copy link
Member

Oh, some of the documentation needs updating:

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 13, 2025
@rushmash91
Copy link
Member Author

@adrianmoisey Done!

omerap12
omerap12 reviewed Aug 13, 2025
View reviewed changes
Copy link
Member

@omerap12 omerap12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 13, 2025
@adrianmoisey
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianmoisey, rushmash91

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

The pull request process is described 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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 14, 2025
@k8s-ci-robot k8s-ci-robot merged commit a7df3cf into kubernetes:master Aug 14, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soltysh soltysh soltysh left review comments

@omerap12 omerap12 omerap12 left review comments

@adrianmoisey adrianmoisey Awaiting requested review from adrianmoisey

@jbartosik jbartosik Awaiting requested review from jbartosik

Assignees

@adrianmoisey adrianmoisey

@omerap12 omerap12

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/vertical-pod-autoscaler cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deprecate UpdateMode "Auto" in VPA
5 participants
@rushmash91 @omerap12 @adrianmoisey @k8s-ci-robot @soltysh