-
Notifications
You must be signed in to change notification settings - Fork 103
End-to-end test case for cluster autoscaling #3302
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
|
Skipping CI for Draft Pull Request. |
8150495 to
e378c53
Compare
|
This PR is currently blocked by the update functionality introduced in #3541 |
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.
Thanks for the PR! Overall, things are looking pretty good.
- I think there's a few minor bugs to address
- More importantly, we need to double-check that all of these timeouts are correct. The current test execution time of just this suite is something like 3-5 hours if we're hitting those timeout conditions.
This is a pointer to an ClusterAutoscalingProfile struct, so it can be left unset to have the RP's default values take effect.
db7d7db to
9f806c2
Compare
|
/test e2e-parallel |
mbarnes
left a comment
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 can't approve this since I originally submitted the PR.
I think this is overall good for a first pass. Some parts of this just look like it's testing validation, which I don't think E2E is the right context for. We have unit and simulation tests for validation testing.
The last test ("it should respect cluster-wide node limits with nodepool autoscaling") is the most important since it tests the interaction between cluster-wide autoscaling parameters and node pools. We could use more test cases that demonstrate this interaction, but this one case is good enough to prove the values are getting down to the customer cluster.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: geoberle, mbarnes The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Requests appear to have been addressed. Dismissing to allow merging.
ARO-22328 - E2E Test Suite: Support global autoscaling configuration
What
Jira indicates @shitaljante may already be working on this. I'll sync with her to see where she's at with it. For now this is just a stash of what I had previously written.