VPA: [InPlaceOrRecreate] e2e tests #8072

maxcao13 · 2025-04-29T19:18:34Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds a lot of e2e tests to validate the InPlaceOrRecreate feature. I've tried to separate feature gated tests from regular tests.

I've added this check checkInPlaceOrRecreateTestsEnabled(f, true, true) to make sure that

InPlaceOrRecreate feature gate is enabled for the necessary components, otherwise skip
InPlacePodVerticalScaling feature gate is enabled on cluster
- This can be dropped after we do a release that bumps to k8s 1.33, i'm keeping it for now since we are still on 1.32

Still note quite sure how we want to handle VPA feature gated tests here. As of now, e2e tests will always run with InPlaceOrRecreate=true and all tests will run, but maybe we want to have a separate prow config which just exercises feature gated tests using special ginkgo test tags? We can ignore that for now in order to get this feature out the door, but maybe it's something to think about.

Which issue(s) this PR fixes:

Part of AEP-4016 [InPlaceOrRecreate]
This PR is part of the larger feature PR in #7673

Depends on: #7962
After it's merged I'll rebase.

Note that this PR is merging into the in-place-updates feature branch, which will be merged when this feature is all reviewed and ready.

Special notes for your reviewer:

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

[AEP-4016]: https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/enhancements/4016-in-place-updates-support

k8s-ci-robot · 2025-04-29T19:18:38Z

@maxcao13: The label(s) kind/test cannot be applied, because the repository doesn't have them.

In response to this:

What type of PR is this?

/kind test

What this PR does / why we need it:

Adds a lot of e2e tests to validate the InPlaceOrRecreate feature. I've tried to separate feature gated tests from regular tests.

I've added this check checkInPlaceOrRecreateTestsEnabled(f, true, true) to make sure that

InPlaceOrRecreate feature gate is enabled for the necessary components, otherwise skip

InPlacePodVerticalScaling feature gate is enabled on cluster

This can be dropped after we do a release that bumps to k8s 1.33, i'm keeping it for now since we are still on 1.32

Still note quite sure how we want to handle VPA feature gated tests here. As of now, e2e tests will always run with InPlaceOrRecreate=true and all tests will run, but maybe we want to have a separate prow config which just exercises feature gated tests using special ginkgo test tags? We can ignore that for now in order to get this feature out the door, but maybe it's something to think about.

Which issue(s) this PR fixes:

Part of AEP-4016 [InPlaceOrRecreate]
This PR is part of the larger feature PR in #7673

Depends on: #7962
After it's merged I'll rebase.

Note that this PR is merging into the in-place-updates feature branch, which will be merged when this feature is all reviewed and ready.

Special notes for your reviewer:

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
[AEP-4016]: https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/enhancements/4016-in-place-updates-support

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.

maxcao13 · 2025-04-29T19:20:17Z

/hold depends on #7962

omerap12 · 2025-04-30T06:55:10Z

/assign @raywainman for another pair of eyes

k8s-ci-robot · 2025-04-30T06:55:13Z

@omerap12: GitHub didn't allow me to assign the following users: another, pair, of, eyes, for.

Note that only kubernetes members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @raywainman for another pair of eyes

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.

raywainman · 2025-05-05T21:01:26Z

@maxcao13 do you mind rebasing once the other PR is merged?

Signed-off-by: Max Cao <[email protected]>

maxcao13 · 2025-05-05T21:04:53Z

/unhold
unholding since the updater PR merged and I've rebased.

vertical-pod-autoscaler/e2e/v1/actuation.go

omerap12 · 2025-05-06T10:29:20Z

vertical-pod-autoscaler/e2e/v1/common.go

+
+// WaitForPodsUpdatedWithoutEviction waits for pods to be updated without any evictions taking place over the polling
+// interval.
+func WaitForPodsUpdatedWithoutEviction(f *framework.Framework, initialPods *apiv1.PodList) error {


The logic is kinda weird but I guess it's working... wondering if we can verify that a container has been updated in-place rather than just iterating on the names.

Not sure, I've thought about trying to track status.ContainerRestarts but doesn't hold up since we can't guarantee the container won't restart.

Maybe it's also possible to track the resize condition to go from nil -> PodResizeInProgress -> nil and not confirm it doesn't to Deferred or Infeasible or something, but that's probably not the best idea if the conditions change super fast.

Maybe someone else has better ideas, or maybe even someone from sig-node knows.

vertical-pod-autoscaler/e2e/v1/actuation.go

vertical-pod-autoscaler/e2e/v1/common.go

omerap12

Overall looks good. Great work, Max - really appreciated! I've added a few small comments.

omerap12 · 2025-05-06T10:58:37Z

I guess the e2e testing is taking longer now, right? I'm not sure how we could implement separate Prow job configurations specifically for feature-gated tests. We should definitely think about that, but it's not urgent right now.

maxcao13 · 2025-05-07T01:10:21Z

I guess the e2e testing is taking longer now, right? I'm not sure how we could implement separate Prow job configurations specifically for feature-gated tests. We should definitely think about that, but it's not urgent right now.

Yeah, it's taking a bit longer now, which probably isn't great since it'll slow all PRs down after this entire thing is merged to main.

I think it might be good to at least prepare for that here. I think we can use label filters to mark suites with Label("FG:InPlaceOrRecreate) using ginkgo.label-filter, so we can filter only feature-gated tests. My next commit will be this, I'd love to know what people think.

I've tried it locally and it works. You just have to specify --ginkgo.label-filter="FG:InPlaceOrRecreate" in your go test arguments.

EDIT: Although this doesn't account for the fact that after the feature gate is GA'd, we wouldn't be running the tests separately anymore. So either we are okay with the full_vpa tests being at most double in length, or we somehow find a way to make the tests more time efficient, or we have to start parallelizing some e2e tests somehow.

omerap12 · 2025-05-07T08:38:34Z

I guess the e2e testing is taking longer now, right? I'm not sure how we could implement separate Prow job configurations specifically for feature-gated tests. We should definitely think about that, but it's not urgent right now.

Yeah, it's taking a bit longer now, which probably isn't great since it'll slow all PRs down after this entire thing is merged to main.

I think it might be good to at least prepare for that here. I think we can use label filters to mark suites with Label("FG:InPlaceOrRecreate) using ginkgo.label-filter, so we can filter only feature-gated tests. My next commit will be this, I'd love to know what people think.

I've tried it locally and it works. You just have to specify --ginkgo.label-filter="FG:InPlaceOrRecreate" in your go test arguments.

I'm thinking about how we can integrate this into the CI process.
@adrianmoisey DevOps.

maxcao13 · 2025-05-08T18:10:02Z

Note to reviewers: The e2e tests I added aren't actually running right now. They require the FEATURE_GATES env var to be enabled in the tests. Not sure how we want to go about this.

vertical-pod-autoscaler/e2e/v1/actuation.go

vertical-pod-autoscaler/e2e/v1/common.go

Signed-off-by: Max Cao <[email protected]>

This commit refactors the VPA e2e test ginkgo wrappers so that they we can easily supply ginkgo decorators. This allows us to add ginkgo v2 labels to suites so that later we can run tests that only run FG tests. For now, this would only be useful for FG:InPlaceOrRecreate Signed-off-by: Max Cao <[email protected]>

raywainman · 2025-05-08T21:02:29Z

/lgtm

/approve

k8s-ci-robot · 2025-05-08T21:02:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maxcao13, 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:

~~vertical-pod-autoscaler/OWNERS~~ [raywainman]

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 added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 29, 2025

k8s-ci-robot requested a review from omerap12 April 29, 2025 19:18

k8s-ci-robot added the area/vertical-pod-autoscaler label Apr 29, 2025

k8s-ci-robot requested a review from voelzmo April 29, 2025 19:18

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 29, 2025

maxcao13 mentioned this pull request Apr 29, 2025

VPA: (InPlaceOrRecreate) Allow updater to actuate InPlaceOrRecreate updates #7962

Merged

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 29, 2025

maxcao13 mentioned this pull request Apr 29, 2025

VPA: Implement in-place updates support #7673

Closed

k8s-ci-robot assigned raywainman Apr 30, 2025

VPA: bump up overall e2e test timeout

80b0ffe

Signed-off-by: Max Cao <[email protected]>

maxcao13 force-pushed the in-place-updates-e2e branch from baa02c9 to 1680325 Compare May 5, 2025 21:04

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 5, 2025

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 5, 2025