-
Notifications
You must be signed in to change notification settings - Fork 356
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: ensure make unslurmcluster always runs in CI #9420
Conversation
✅ Deploy Preview for determined-ui canceled.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9420 +/- ##
==========================================
- Coverage 51.24% 45.81% -5.43%
==========================================
Files 746 718 -28
Lines 110663 108876 -1787
Branches 2778 2778
==========================================
- Hits 56713 49886 -6827
- Misses 53776 58816 +5040
Partials 174 174
Flags with carried forward coverage won't be shown. Click here to find out more. |
3756b32
to
907a7e7
Compare
Added you as a reviewer @dannysauer since I told you I'd do this on a thread yesterday. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9420 +/- ##
==========================================
- Coverage 51.24% 48.61% -2.64%
==========================================
Files 746 1233 +487
Lines 110663 158972 +48309
Branches 2778 2778
==========================================
+ Hits 56713 77283 +20570
- Misses 53776 81515 +27739
Partials 174 174
Flags with carried forward coverage won't be shown. Click here to find out more. |
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.
Still gon' leave resources around if the job is interrupted by a second PR or otherwise canceled (because CircleCI has no finalizer), but this is an improvement either way. :)
Yeah, I agree, this probably needs to get added to the terminator script at some point. But this at least fixes the specific issue that caused us to have an explosion of vpcs. |
merging over some unrelated failures (known flakes on main) |
When cluster creation fails halfway, pkill determined-mast will exit with 1. CircleCI runs these inline script with set -e, so that failure causes make unslurmcluster to no run, leaving stray resources behind.
Ticket
Description
When cluster creation fails halfway,
pkill determined-mast
will exit with 1. CircleCI runs these inline script withset -e
, so that failure causesmake unslurmcluster
to not run and leave stray TF resources behind.Test Plan
Is a test.
Checklist
docs/release-notes/
.See Release Note for details.