Skip to content

VPA: Allow local dev and e2e scripts to run feature-gated features #7934

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

Conversation

maxcao13
Copy link
Member

@maxcao13 maxcao13 commented Mar 15, 2025

What type of PR is this?

/kind feature

What this PR does / why we need it:

  • Allows you to specify an env var 'FEATURE_GATES' which adds feature gates to all vpa components when running hack/vpa-up.sh and/or running local e2e tests. This or does NOT add the ability to run ci e2e tests with feature gates yet. This can be subject to change.
  • Also allows local e2e tests to run kind with a new kind-config file which enables KEP-1287 InPlacePodVerticalScaling feature gate.
  • Separates the admission-controller service into a separate deploy manifest in order for the feature-gate injection logic to work

Not sure if we want to include stuff in here to change the actual github ci workflow to include separate feature-gated tests (that only test InPlaceOrRecreate for example). We probably want to do that after the logic is actually merged into the feature branch, I'm thinking.

Which issue(s) this PR fixes:

This PR is part of the larger feature PR in #7673
Depends on https://github.com/kubernetes/autoscaler/pull/7932/files

Special notes for your reviewer:

This won't work until https://github.com/kubernetes/autoscaler/pull/7932/files is merged.

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 15, 2025
@k8s-ci-robot k8s-ci-robot requested a review from omerap12 March 15, 2025 02:13
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 15, 2025
@omerap12
Copy link
Member

Thanks for this! Let's wait for #7932.
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 15, 2025
@maxcao13 maxcao13 force-pushed the in-place-updates-scriptchanges branch from 0b6f2d6 to 1092727 Compare March 19, 2025 20:38
@adrianmoisey
Copy link
Member

Tests are broken due to #7946

@raywainman
Copy link
Contributor

/test all

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.

Tested and seems fine to me, thanks!
/unhold
/lgtm

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2025
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 23, 2025
Copy link
Contributor

@raywainman raywainman left a comment

Choose a reason for hiding this comment

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

/lgtm

@raywainman
Copy link
Contributor

Approving so you aren't stuck waiting for me. Feel free to remove hold when you've addressed my 1 nit comment :)

/approve

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maxcao13, omerap12, raywainman

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 Mar 23, 2025
Allows you to specify an env var FEATURE_GATES which adds feature gates to all vpa components during vpa-up and e2e tests.
Also allows local e2e tests to run kind with a new kind-config file which enables KEP-1287 InPlacePodVerticalScaling feature gate.
Separates the admission-controller service into a separate deploy manifest.

Signed-off-by: Max Cao <[email protected]>
@maxcao13 maxcao13 force-pushed the in-place-updates-scriptchanges branch from 1092727 to a99932a Compare March 24, 2025 14:51
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 24, 2025
@maxcao13
Copy link
Member Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 24, 2025
@adrianmoisey
Copy link
Member

Nit has been fixed:
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 24, 2025
@k8s-ci-robot k8s-ci-robot merged commit 4486391 into kubernetes:in-place-updates Mar 24, 2025
6 of 7 checks passed
@maxcao13 maxcao13 deleted the in-place-updates-scriptchanges branch March 24, 2025 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants