VPA: Allow e2e tests to reference a custom namespace #8778

AryanBagade · 2025-11-09T20:05:26Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Enables VPA e2e tests to use a custom namespace via the VPA_NAMESPACE environment variable, while maintaining backward compatibility with the default kube-system namespace.
This makes the e2e tests consistent with the VPA components' ability to run in custom namespaces, as mentioned in issue #8752.

Which issue(s) this PR fixes:

Fixes #8752

Special notes for your reviewer:

This implementation follows the same pattern as PR #7654, using environment variables with sensible defaults to enable custom configurations without breaking existing tests.

The changes are minimal and only affect e2e test code:

Made VpaNamespace (e2e/v1/common.go) configurable via VPA_NAMESPACE env var
Made RecommenderNamespace (e2e/utils/common.go) configurable via VPA_NAMESPACE env var
Replaced hardcoded kube-system strings in deleteRecommender() function
Replaced hardcoded kube-system strings in webhook RoleBinding operations

All unit tests pass with race detection enabled.

Does this PR introduce a user-facing change?

NONE

linux-foundation-easycla · 2025-11-09T20:05:33Z

The committers listed above are authorized under a signed CLA.

✅ login: AryanBagade / name: Aryan Bagade (871b84b, 90f51c8)

k8s-ci-robot · 2025-11-09T20:05:35Z

Welcome @AryanBagade!

It looks like this is your first PR to kubernetes/autoscaler 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/autoscaler has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-11-09T20:05:36Z

Hi @AryanBagade. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

Enable VPA e2e tests to use a custom namespace via the VPA_NAMESPACE environment variable, while maintaining backward compatibility with the default kube-system namespace. This addresses issue kubernetes#8752 by making the e2e tests consistent with the VPA components' ability to run in custom namespaces. Changes: - Made VpaNamespace (e2e/v1/common.go) configurable via environment variable - Made RecommenderNamespace (e2e/utils/common.go) configurable via environment variable - Replaced hardcoded kube-system in deleteRecommender() function - Replaced hardcoded kube-system in webhook RoleBinding operations The implementation follows the same pattern as PR kubernetes#7654, using environment variables with sensible defaults to enable custom configurations without breaking existing tests.

AryanBagade · 2025-11-09T20:16:21Z

/release-note-none

adrianmoisey · 2025-11-10T13:21:25Z

Thanks for doing this!

/ok-to-test
/cc @maxcao13

omerap12

I thought the idea was to do something similar to this:
https://github.com/kubernetes/autoscaler/blob/master/vertical-pod-autoscaler/hack/dev-deploy-locally.sh#L60

(i.e. set the variable names during the binary compilation step)

omerap12 · 2025-11-10T13:39:18Z

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

+)
+
+func init() {
+	if ns := os.Getenv("VPA_NAMESPACE"); ns != "" {


What happens if there is no VPA_NAMESPACE env variable?

If the VPA_NAMESPACE environment variable is not set, os.Getenv("VPA_NAMESPACE") returns an empty string, so the condition ns != "" evaluates to false, and we don't override the default value. This means VpaNamespace and RecommenderNamespace remain as "kube-system", maintaining full backward compatibility.

oh I missed the VpaNamespace = "kube-system" above. thanks!

AryanBagade · 2025-11-10T20:01:50Z

I thought the idea was to do something similar to this:

I chose runtime environment variables

E2e tests benefit from runtime flexibility as you can run the same compiled test against different namespace configurations without recompiling
This matches the pattern used by other e2e test configuration (like KUBECONFIG, TEST_WITH_FEATURE_GATES_ENABLED, etc.)
It's simpler for developers like just set `VPA_NAMESPACE=custom-ns go test ./e2e/...

But if you prefer the build-time approach, I'm happy to adjust! What do you think?

maxcao13 · 2025-11-10T21:43:57Z

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

-	// RecommenderNamespace is namespace to deploy VPA recommender
-	RecommenderNamespace = "kube-system"


I'm wondering if we should just remove this variable and use VpaNamespace for everything if that's possible?

Sure, Sounds good!!

hmmm having two separate variables is redundant since they both serve the same purpose.
The change would be:

Keep the variable and init() function in e2e/utils/common.go

Rename RecommenderNamespace → VpaNamespace

Remove the duplicate from e2e/v1/common.go

Use utils.VpaNamespace everywhere

Does that sound good? Any other changes i need to make ????

Sure that makes sense to me. Thanks for doing this :-)

maxcao13 · 2025-11-10T21:47:57Z

I also think we need to configure these references too:

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

Lines 45 to 53 in 2eb783b

    
           ginkgo.By("Setting up the Admission Controller status") 
        
           stopCh := make(chan struct{}) 
        
           statusUpdater := status.NewUpdater( 
        
           	f.ClientSet, 
        
           	status.AdmissionControllerStatusName, 
        
           	status.AdmissionControllerStatusNamespace, 
        
           	statusUpdateInterval, 
        
           	"e2e test", 
        
           )

Right now they will always point to kube-system.

status.AdmissionControllerStatusNamespace

AryanBagade · 2025-11-10T21:52:13Z

right, I missed those references. Let me update those as well.

maxcao13 · 2025-11-10T21:54:35Z

I chose runtime environment variables

E2e tests benefit from runtime flexibility as you can run the same compiled test against different namespace configurations without recompiling
This matches the pattern used by other e2e test configuration (like KUBECONFIG, TEST_WITH_FEATURE_GATES_ENABLED, etc.)
It's simpler for developers like just set `VPA_NAMESPACE=custom-ns go test ./e2e/...

But if you prefer the build-time approach, I'm happy to adjust! What do you think?

+1 for me. it follows the other test configurations we've already been using.

Changes: - Consolidated to single VpaNamespace variable in e2e/utils/common.go - Added VPA_NAMESPACE environment variable support with init() function - Replaced all hardcoded namespace references in e2e tests: - e2e/v1/recommender.go deleteRecommender() function (1 instance) - e2e/v1/updater.go status namespace references (10 instances) - e2e/integration/recommender.go deployment operations (3 instances) - e2e/utils/webhook.go RoleBinding operations (2 instances) - Removed duplicate VpaNamespac from e2e/v1/common.go

maxcao13 · 2025-11-11T06:58:05Z

/lgtm

Thanks for the contribution!

omerap12 · 2025-11-11T08:09:12Z

I thought the idea was to do something similar to this:

I chose runtime environment variables

E2e tests benefit from runtime flexibility as you can run the same compiled test against different namespace configurations without recompiling

This matches the pattern used by other e2e test configuration (like KUBECONFIG, TEST_WITH_FEATURE_GATES_ENABLED, etc.)

It's simpler for developers like just set `VPA_NAMESPACE=custom-ns go test ./e2e/...

But if you prefer the build-time approach, I'm happy to adjust! What do you think?

Seems reasonable to me. I agree :)

omerap12

Thanks for this!
/label tide/merge-method-squash
/approve

k8s-ci-robot · 2025-11-11T08:11:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AryanBagade, omerap12

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~~ [omerap12]

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

VPA: Allow e2e tests to reference a custom namespace (kubernetes#8778) * VPA: Allow e2e tests to reference a custom namespace Enable VPA e2e tests to use a custom namespace via the VPA_NAMESPACE environment variable, while maintaining backward compatibility with the default kube-system namespace. This addresses issue kubernetes#8752 by making the e2e tests consistent with the VPA components' ability to run in custom namespaces. Changes: - Made VpaNamespace (e2e/v1/common.go) configurable via environment variable - Made RecommenderNamespace (e2e/utils/common.go) configurable via environment variable - Replaced hardcoded kube-system in deleteRecommender() function - Replaced hardcoded kube-system in webhook RoleBinding operations The implementation follows the same pattern as PR kubernetes#7654, using environment variables with sensible defaults to enable custom configurations without breaking existing tests. * VPA: Allow e2e tests to reference a custom namespace Changes: - Consolidated to single VpaNamespace variable in e2e/utils/common.go - Added VPA_NAMESPACE environment variable support with init() function - Replaced all hardcoded namespace references in e2e tests: - e2e/v1/recommender.go deleteRecommender() function (1 instance) - e2e/v1/updater.go status namespace references (10 instances) - e2e/integration/recommender.go deployment operations (3 instances) - e2e/utils/webhook.go RoleBinding operations (2 instances) - Removed duplicate VpaNamespac from e2e/v1/common.go Signed-off-by: Max Cao <[email protected]>

* VPA: Allow e2e tests to reference a custom namespace Enable VPA e2e tests to use a custom namespace via the VPA_NAMESPACE environment variable, while maintaining backward compatibility with the default kube-system namespace. This addresses issue kubernetes#8752 by making the e2e tests consistent with the VPA components' ability to run in custom namespaces. Changes: - Made VpaNamespace (e2e/v1/common.go) configurable via environment variable - Made RecommenderNamespace (e2e/utils/common.go) configurable via environment variable - Replaced hardcoded kube-system in deleteRecommender() function - Replaced hardcoded kube-system in webhook RoleBinding operations The implementation follows the same pattern as PR kubernetes#7654, using environment variables with sensible defaults to enable custom configurations without breaking existing tests. * VPA: Allow e2e tests to reference a custom namespace Changes: - Consolidated to single VpaNamespace variable in e2e/utils/common.go - Added VPA_NAMESPACE environment variable support with init() function - Replaced all hardcoded namespace references in e2e tests: - e2e/v1/recommender.go deleteRecommender() function (1 instance) - e2e/v1/updater.go status namespace references (10 instances) - e2e/integration/recommender.go deployment operations (3 instances) - e2e/utils/webhook.go RoleBinding operations (2 instances) - Removed duplicate VpaNamespac from e2e/v1/common.go

k8s-ci-robot added the do-not-merge/needs-area label Nov 9, 2025

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 9, 2025

k8s-ci-robot requested review from adrianmoisey and raywainman November 9, 2025 20:05

AryanBagade force-pushed the feature/8752-vpa-e2e-custom-namespace branch from 6bf0cee to 90f51c8 Compare November 9, 2025 20:15

k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Nov 9, 2025

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 9, 2025

k8s-ci-robot requested a review from maxcao13 November 10, 2025 13:21

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 10, 2025

omerap12 reviewed Nov 10, 2025

View reviewed changes

maxcao13 reviewed Nov 10, 2025

View reviewed changes

AryanBagade requested review from maxcao13 and omerap12 November 10, 2025 22:31

k8s-ci-robot assigned maxcao13 Nov 11, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 11, 2025

omerap12 approved these changes Nov 11, 2025

View reviewed changes

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Nov 11, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 11, 2025

k8s-ci-robot merged commit 7fe519c into kubernetes:master Nov 11, 2025
13 checks passed

		// RecommenderNamespace is namespace to deploy VPA recommender
		RecommenderNamespace = "kube-system"

VPA: Allow e2e tests to reference a custom namespace #8778

VPA: Allow e2e tests to reference a custom namespace #8778

Uh oh!

Conversation

AryanBagade commented Nov 9, 2025

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?

Uh oh!

linux-foundation-easycla bot commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Nov 9, 2025

Uh oh!

k8s-ci-robot commented Nov 9, 2025

Uh oh!

AryanBagade commented Nov 9, 2025

Uh oh!

adrianmoisey commented Nov 10, 2025

Uh oh!

omerap12 left a comment

Choose a reason for hiding this comment

Uh oh!

omerap12 Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

AryanBagade Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

omerap12 Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

AryanBagade commented Nov 10, 2025

Uh oh!

maxcao13 Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

AryanBagade Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

AryanBagade Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

maxcao13 Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

maxcao13 commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AryanBagade commented Nov 10, 2025

Uh oh!

maxcao13 commented Nov 10, 2025

Uh oh!

maxcao13 commented Nov 11, 2025

Uh oh!

omerap12 commented Nov 11, 2025

Uh oh!

omerap12 left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Nov 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

linux-foundation-easycla bot commented Nov 9, 2025 •

edited

Loading

maxcao13 commented Nov 10, 2025 •

edited

Loading