DM-55434: BPS Provisioner Service Job May End Before finalJob#94
DM-55434: BPS Provisioner Service Job May End Before finalJob#94tcjennings wants to merge 1 commit into
Conversation
faed230 to
24d4b55
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #94 +/- ##
=======================================
Coverage 84.78% 84.78%
=======================================
Files 19 19
Lines 4260 4260
Branches 458 458
=======================================
Hits 3612 3612
Misses 570 570
Partials 78 78 ☔ View full report in Codecov by Harness. |
- trap SIGTERM in script and perform one final provisioner loop - keep provisionerJob alive until N consecutive allocateNodes failures
24d4b55 to
adabde5
Compare
mxk62
left a comment
There was a problem hiding this comment.
The changes looks fine. However, please do not forget to update package documentation! The default settings in lines 429 - 453 of doc/lsst.ctrl.bps.htcondor/userguide.rst must reflect any changes you'll make.
|
|
||
| allocating=1 | ||
| failures=0 | ||
| max_failures=10 |
There was a problem hiding this comment.
I'd think it would be better to let max_failure be a tunable parameter by introducing provisioningMaxFailures setting.
| wait $! || : | ||
| done | ||
| echo "$(date -u): Shutting down provisioningJob..." | ||
| sleep 20 |
There was a problem hiding this comment.
Again, it would be preferable to let the user adjust this value if needed. Can we use something like provisioningShutdownWaitTime to make it tunable?
| {provisioningExtraOptions} \ | ||
| '{provisioningPlatform}') | ||
|
|
||
| trap "allocating=0" SIGTERM |
There was a problem hiding this comment.
Don't we need to kill the background sleep when handling the signal too?
Updates default provisioningScript to handle SIGTERM correctly and complete a final provisioning attempt at shutdown.
Script tries to stay alive until 10 consecutive provisioning failures.
Checklist
doc/changes