-
Notifications
You must be signed in to change notification settings - Fork 71
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
test(e2e): add in chaos E2E test suite #61
Conversation
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.
/test
Pull Request Test Coverage Report for Build 8427608579Details
💛 - Coveralls |
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.
/test
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.
Can we do 5 runs showing its not flakey
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.
/test
My current benchmark is |
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.
Have a few questions to understand the goals of these tests
Look at the original pr for the chaos suite maybe? This might give us better insights as to the why behind these tests. @jonathan-innis any insights as I see you authored the original pr |
The extra information from the original PR:
makes sense why we'd get > 1 node for a 1 pod deployment. 35 still feels like a magic number to me though :) |
Also found this comment interesting for our consideration:
|
Yea, I'm not sure on the 35. Agree seems magic number-ish |
Checked with the AWS folks, and the < 35 nodes check was added due to testing as a typical upper limit with disruption. This is good enough for me right now, so I'll approve for now. Moreso for posterity's sake in case we want to adjust this limit in the future and how we might reason about doing so. |
Fixes #216
Description
How was this change tested?
9/10 successful runs required as the benchmark for adding a new test suite
Last 10 runs (10/10 successful):
Previous 10 runs (8/10 successful):
Older passing E2E test:
https://github.com/Azure/karpenter/actions/runs/7202151191
Does this change impact docs?
Release Note