-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
chore: Optimize e2e tests by improving EnsureCleanState #20942
base: master
Are you sure you want to change the base?
chore: Optimize e2e tests by improving EnsureCleanState #20942
Conversation
✅ Preview Environment deployed on Bunnyshell
See: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
6bc0692
to
c67bcae
Compare
Hm, namespace deletion is slow, so waiting for it is too bad. |
6523fea
to
ba114a0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #20942 +/- ##
=========================================
Coverage ? 55.03%
=========================================
Files ? 324
Lines ? 55465
Branches ? 0
=========================================
Hits ? 30523
Misses ? 22331
Partials ? 2611 ☔ View full report in Codecov by Sentry. |
Seems to save just about 1 minute, but I'll take it. |
@andrii-korotkov-verkada it's not clear to me why this change would be faster than the current implementation. If it saves 1m it seems more like background noise than anything else... |
We don't run extra kubectl commands if there's nothing to cleanup, saving ~50ms per call. Also, I think directly using APIs to create gpg is a bit faster than running cli. I've expected more savings, but I'll take even 1 minute. |
There's been log messages for kubectl with label selector saying there's nothing to remove, so I'm getting rid of those. Also, I batch multiple resources of the same type to be deleted together, via one command. Not sure how much this saves, but seems good. |
If savings are <5min, I think simpler code is worth the time. Cleaning up logs is an independently good thing, but also probably not worth more difficult-to-read code. |
New code reads fine to me except the |
I'll get rid of len(args) > and replace with something more readable. |
ba114a0
to
a656f9b
Compare
Closes argoproj#20941 Batch some kubectl operations, create gpg using APIs instead of CLI. Signed-off-by: Andrii Korotkov <[email protected]>
a656f9b
to
2cc4c36
Compare
Closes #20941
Batch some kubectl operations, create gpg using APIs instead of CLI.
Checklist: