Add integration tests for recommender by adrianmoisey · Pull Request #9156 · kubernetes/autoscaler

adrianmoisey · 2026-02-01T13:25:35Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Currently the VPA doesn't have "integration" tests, which allows us to test various CLI flags to each component. The purpose of this change is to set the foundation to allow us to add integration tests.

The idea here is that we can write tests that execute faster without needing an entire Kubernetes cluster to validate these changes.

In theory this change also pushes us closer to the modern patterns that other controllers are using.

This PR only works on the updater, but if it makes sense to make similar changes to the admission-controller or updater, I'm happy to do so (I think the config change does make sense for all components).

This PR also highlighted that we use stop channels and contexts in some places where we could get away with only the context. That's something we can fix in future PRs.

Once this is accepted, I also need to figure out how to add this to CI.

Which issue(s) this PR fixes:

Relates to #8934

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

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

/cc @maxcao13 @omerap12

I don't have much envtest experience, so happy to make adjustments if there are better ways of handling any of this

Configure a struct that can be passed around, and populate that with CLI flags

if block ends with a return statement, so drop this else and outdent its block

Since it requires setup

k8s-ci-robot · 2026-02-01T13:25:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianmoisey

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~hack/OWNERS~~ [adrianmoisey]
~~vertical-pod-autoscaler/OWNERS~~ [adrianmoisey]

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

adrianmoisey · 2026-02-01T13:26:51Z

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

-// HamsterTargetRef is CrossVersionObjectReference of hamster app
-var HamsterTargetRef = &autoscaling.CrossVersionObjectReference{
-	APIVersion: "apps/v1",
-	Kind:       "Deployment",
-	Name:       "hamster-deployment",
-}


This is moved out of the e2e directory.
I didn't want to give the integration tests its own go.mod file, and I didn't want to make the VPA depend on things inside the e2e tests, so handle that, I moved this out of e2e so that it could be used in the integration tests.

adrianmoisey · 2026-02-01T13:28:29Z

/hold
(just to be sure it gets enough eyes before merging and doesn't merge accidentally)

maxcao13

I feel like we could move the flag refactoring to a separate PR to merge first, before we introduce this new test suite. It seems to me that we are doing both things in one PR, which is a little hard to review, but for the record I do like the changes :-)

maxcao13 · 2026-02-03T23:30:52Z

vertical-pod-autoscaler/common/flags.go

 }

+// ValidateCommonConfig performs validation of the common flags
+func ValidateCommonConfig(config *CommonFlags) {


This seems to be only used in the admission controller right now.

maxcao13 · 2026-02-04T00:54:12Z

vertical-pod-autoscaler/integration/recommender/recommender_test.go

+	}
+
+	// Configure the recommender to watch only the watched namespace
+	config := recommender_config.DefaultRecommenderConfig()


Some of the stuff in each test seems repeatable. Since we use ginkgo in the regular e2es, can we re-use it here and have some sort of beforeEach which will setup the configurations for the component under test?

maxcao13 · 2026-02-04T00:55:19Z

vertical-pod-autoscaler/pkg/admission-controller/config/config.go

+// ValidateAdmissionControllerConfig performs validation of the admission-controller flags
+func ValidateAdmissionControllerConfig(config *AdmissionControllerConfig) {
+	if len(config.CommonFlags.VpaObjectNamespace) > 0 && len(config.CommonFlags.IgnoredVpaObjectNamespaces) > 0 {
+		klog.ErrorS(nil, "--vpa-object-namespace and --ignored-vpa-object-namespaces are mutually exclusive and can't be set together.")
+		klog.FlushAndExit(klog.ExitFlushTimeout, 1)
+	}
+}


Do we still need this? I think we have that common controller config validation right?

maxcao13 · 2026-02-04T00:56:33Z

vertical-pod-autoscaler/pkg/utils/metrics/quality/quality.go

+	_ = prometheus.Register(usageRecommendationRelativeDiff)
+	_ = prometheus.Register(usageMissingRecommendationCounter)


Just curious why the changes here?

adrianmoisey · 2026-02-07T12:24:40Z

I feel like we could move the flag refactoring to a separate PR to merge first, before we introduce this new test suite. It seems to me that we are doing both things in one PR, which is a little hard to review, but for the record I do like the changes :-)

I had anticipated this and I've kept the changed in their own commits. I'll put up a PR for the flags

adrianmoisey · 2026-02-07T15:10:47Z

I feel like we could move the flag refactoring to a separate PR to merge first, before we introduce this new test suite. It seems to me that we are doing both things in one PR, which is a little hard to review, but for the record I do like the changes :-)

I had anticipated this and I've kept the changed in their own commits. I'll put up a PR for the flags

Here are the flags: #9195

I've addressed the comments you made on this PR and found a few small bugs.

k8s-ci-robot · 2026-02-12T07:09:38Z

PR needs rebase.

Details

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.

adrianmoisey added 8 commits February 1, 2026 11:38

Refactor updater's flags

624b164

Configure a struct that can be passed around, and populate that with CLI flags

Refactor other flags

e7b07d3

Move recommender start into it's own module

be47bf5

Wire up context, add stop channels, etc

179f2b4

Lint

31f2d61

if block ends with a return statement, so drop this else and outdent its block

Add some integration tests for recommender

019f3c2

Skip integration tests in regular testing

2a38777

Since it requires setup

Update flags

7197c34

k8s-ci-robot requested a review from maxcao13 February 1, 2026 13:25

k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Feb 1, 2026

k8s-ci-robot requested a review from omerap12 February 1, 2026 13:25

k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. do-not-merge/needs-area labels Feb 1, 2026

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. area/vertical-pod-autoscaler size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 1, 2026

adrianmoisey commented Feb 1, 2026

View reviewed changes

k8s-ci-robot removed the do-not-merge/needs-area label Feb 1, 2026

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

maxcao13 reviewed Feb 4, 2026

View reviewed changes

adrianmoisey mentioned this pull request Feb 7, 2026

VPA: Refactor how flags are set #9195

Open

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 12, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add integration tests for recommender#9156

Add integration tests for recommender#9156
adrianmoisey wants to merge 8 commits intokubernetes:masterfrom
adrianmoisey:adrian-envtest

adrianmoisey commented Feb 1, 2026 •

edited

Loading

Uh oh!

k8s-ci-robot commented Feb 1, 2026

Uh oh!

adrianmoisey Feb 1, 2026

Uh oh!

adrianmoisey commented Feb 1, 2026

Uh oh!

maxcao13 left a comment

Uh oh!

maxcao13 Feb 3, 2026

Uh oh!

maxcao13 Feb 4, 2026

Uh oh!

maxcao13 Feb 4, 2026

Uh oh!

maxcao13 Feb 4, 2026

Uh oh!

adrianmoisey commented Feb 7, 2026

Uh oh!

adrianmoisey commented Feb 7, 2026

Uh oh!

k8s-ci-robot commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		_ = prometheus.Register(usageRecommendationRelativeDiff)
		_ = prometheus.Register(usageMissingRecommendationCounter)

Conversation

adrianmoisey commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

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

Uh oh!

k8s-ci-robot commented Feb 1, 2026

Uh oh!

adrianmoisey Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

adrianmoisey commented Feb 1, 2026

Uh oh!

maxcao13 left a comment

Choose a reason for hiding this comment

Uh oh!

maxcao13 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

maxcao13 Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

maxcao13 Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

maxcao13 Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

adrianmoisey commented Feb 7, 2026

Uh oh!

adrianmoisey commented Feb 7, 2026

Uh oh!

k8s-ci-robot commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adrianmoisey commented Feb 1, 2026 •

edited

Loading