-
Notifications
You must be signed in to change notification settings - Fork 4.3k
VPA: Allow e2e tests to reference a custom namespace #8778
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ import ( | |
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "os" | ||
| "time" | ||
|
|
||
| ginkgo "github.com/onsi/ginkgo/v2" | ||
|
|
@@ -44,8 +45,6 @@ const ( | |
|
|
||
| // RecommenderDeploymentName is VPA recommender deployment name | ||
| RecommenderDeploymentName = "vpa-recommender" | ||
| // RecommenderNamespace is namespace to deploy VPA recommender | ||
| RecommenderNamespace = "kube-system" | ||
| // PollInterval is interval for polling | ||
| PollInterval = 10 * time.Second | ||
| // PollTimeout is timeout for polling | ||
|
|
@@ -57,6 +56,18 @@ const ( | |
| DefaultHamsterBackoffLimit = int32(10) | ||
| ) | ||
|
|
||
| var ( | ||
| // VpaNamespace is namespace to deploy VPA components. | ||
| // Can be overridden via VPA_NAMESPACE environment variable. | ||
| VpaNamespace = "kube-system" | ||
| ) | ||
|
|
||
| func init() { | ||
| if ns := os.Getenv("VPA_NAMESPACE"); ns != "" { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if there is no
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh I missed the |
||
| VpaNamespace = ns | ||
| } | ||
| } | ||
|
|
||
| // HamsterTargetRef is CrossVersionObjectReference of hamster app | ||
| var HamsterTargetRef = &autoscaling.CrossVersionObjectReference{ | ||
| APIVersion: "apps/v1", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should just remove this variable and use
VpaNamespacefor everything if that's possible?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, Sounds good!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm having two separate variables is redundant since they both serve the same purpose.
The change would be:
e2e/utils/common.goRecommenderNamespace→VpaNamespacee2e/v1/common.goutils.VpaNamespaceeverywhereDoes that sound good? Any other changes i need to make ????
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure that makes sense to me. Thanks for doing this :-)