diff --git a/doc/changes/DM-55434.bugfix.rst b/doc/changes/DM-55434.bugfix.rst new file mode 100644 index 0000000..8b3e626 --- /dev/null +++ b/doc/changes/DM-55434.bugfix.rst @@ -0,0 +1 @@ +Add signal handling and resilience to default provisioningScript diff --git a/doc/lsst.ctrl.bps.htcondor/userguide.rst b/doc/lsst.ctrl.bps.htcondor/userguide.rst index c8d8570..ed03fd2 100644 --- a/doc/lsst.ctrl.bps.htcondor/userguide.rst +++ b/doc/lsst.ctrl.bps.htcondor/userguide.rst @@ -426,31 +426,58 @@ settings: .. code-block:: yaml - provisioning: - provisioningNodeCount: 10 - provisioningMaxIdleTime: 900 - provisioningCheckInterval: 600 - provisioningQueue: "milano" - provisioningAccountingUser: "rubin:developers" - provisioningExtraOptions: "" - provisioningPlatform: "s3df" - provisioningScript: | - #!/bin/bash - set -e - set -x - while true; do - ${CTRL_EXECUTE_DIR}/bin/allocateNodes.py \ - --account {provisioningAccountingUser} \ - --auto \ - --node-count {provisioningNodeCount} \ - --maximum-wall-clock {provisioningMaxWallTime} \ - --glidein-shutdown {provisioningMaxIdleTime} \ - --queue {provisioningQueue} \ - {provisioningExtraOptions} \ - {provisioningPlatform} - sleep {provisioningCheckInterval} - done - exit 0 + provisioning: + provisioningNodeCount: 10 + provisioningMaxIdleTime: 900 + provisioningCheckInterval: 600 + provisioningMaxConsecutiveFailures: 10 + provisioningShutdownAllowanceSeconds: 20 + provisioningQueue: "milano" + provisioningAccountingUser: "rubin:developers" + provisioningExtraOptions: "" + provisioningPlatform: "s3df" + provisioningScript: | + #!/bin/bash + set -x + + allocating=1 + failures=0 + max_failures='{provisioningMaxConsecutiveFailures}' + + allocate=("${CTRL_EXECUTE_DIR}/bin/allocateNodes.py" \ + --account '{provisioningAccountingUser}' \ + --auto \ + --node-count '{provisioningNodeCount}' \ + --maximum-wall-clock '{provisioningMaxWallTime}' \ + --glidein-shutdown '{provisioningMaxIdleTime}' \ + --queue '{provisioningQueue}' \ + --nodeset '{nodeset}' \ + {provisioningExtraOptions} \ + '{provisioningPlatform}') + + trap 'allocating=0; test -n "$bgpid" && kill -SIGTERM "$bgpid";' SIGTERM + + echo "$(date -u): starting provisioningJob" + while (( allocating )); do + if "${allocate[@]}"; then + failures=0 + else + echo "$(date -u): allocatedNodes failed" >&2 + failures=$((failures + 1)) + fi + if [[ $failures -ge $max_failures ]]; then + echo "$(date -u): Too many consecutive allocateNodes failures, exiting" >&2 + exit 1 + fi + sleep {provisioningCheckInterval} & + bgpid=$! + wait $bgpid || : + done + echo "$(date -u): Shutting down provisioningJob..." + sleep '{provisioningShutdownAllowanceSeconds}' + echo "$(date -u): Running FINAL provisioner" + "${allocate[@]}" + exit 0 ``allocateNodes.py`` requires a small configuration file located in the user's directory to work. With automatic provisioning enabled **ctrl_bps_htcondor** diff --git a/python/lsst/ctrl/bps/htcondor/etc/htcondor_defaults.yaml b/python/lsst/ctrl/bps/htcondor/etc/htcondor_defaults.yaml index 702135b..afe9794 100644 --- a/python/lsst/ctrl/bps/htcondor/etc/htcondor_defaults.yaml +++ b/python/lsst/ctrl/bps/htcondor/etc/htcondor_defaults.yaml @@ -10,27 +10,54 @@ provisioning: provisioningNodeCount: 10 provisioningMaxIdleTime: 900 provisioningCheckInterval: 600 + provisioningMaxConsecutiveFailures: 10 + provisioningShutdownAllowanceSeconds: 20 provisioningQueue: "milano" provisioningAccountingUser: "rubin:developers" provisioningExtraOptions: "" provisioningPlatform: "s3df" provisioningScript: | #!/bin/bash - set -e set -x - while true; do - ${CTRL_EXECUTE_DIR}/bin/allocateNodes.py \ - --account {provisioningAccountingUser} \ - --auto \ - --node-count {provisioningNodeCount} \ - --maximum-wall-clock {provisioningMaxWallTime} \ - --glidein-shutdown {provisioningMaxIdleTime} \ - --queue {provisioningQueue} \ - --nodeset {nodeset} \ - {provisioningExtraOptions} \ - {provisioningPlatform} - sleep {provisioningCheckInterval} + + allocating=1 + failures=0 + max_failures='{provisioningMaxConsecutiveFailures}' + + allocate=("${CTRL_EXECUTE_DIR}/bin/allocateNodes.py" \ + --account '{provisioningAccountingUser}' \ + --auto \ + --node-count '{provisioningNodeCount}' \ + --maximum-wall-clock '{provisioningMaxWallTime}' \ + --glidein-shutdown '{provisioningMaxIdleTime}' \ + --queue '{provisioningQueue}' \ + --nodeset '{nodeset}' \ + {provisioningExtraOptions} \ + '{provisioningPlatform}') + + trap 'allocating=0; test -n "$bgpid" && kill -SIGTERM "$bgpid";' SIGTERM + + echo "$(date -u): starting provisioningJob" + while (( allocating )); do + if "${allocate[@]}"; then + failures=0 + else + echo "$(date -u): allocatedNodes failed" >&2 + failures=$((failures + 1)) + fi + if [[ $failures -ge $max_failures ]]; then + echo "$(date -u): Too many consecutive allocateNodes failures, exiting" >&2 + exit 1 + fi + sleep {provisioningCheckInterval} & + bgpid=$! + wait $bgpid || : done + + echo "$(date -u): Shutting down provisioningJob..." + sleep '{provisioningShutdownAllowanceSeconds}' + echo "$(date -u): Running FINAL provisioner" + "${allocate[@]}" exit 0 provisioningScriptConfig: | config.platform["{provisioningPlatform}"].user.name="${USER}" diff --git a/python/lsst/ctrl/bps/htcondor/lssthtc.py b/python/lsst/ctrl/bps/htcondor/lssthtc.py index 1461214..a753409 100644 --- a/python/lsst/ctrl/bps/htcondor/lssthtc.py +++ b/python/lsst/ctrl/bps/htcondor/lssthtc.py @@ -222,6 +222,9 @@ class WmsNodeType(IntEnum): "periodic_remove", "accounting_group", "accounting_group_user", + "kill_sig", + "want_graceful_removal", + "job_max_vacate_time", } HTC_VALID_JOB_DAG_KEYS = { "dir", @@ -1144,7 +1147,7 @@ def add_service_job(self, job): Parameters ---------- job : `HTCJob` - HTCJob to add to the HTCDag as a FINAL job. + HTCJob to add to the HTCDag as a SERVICE job. """ # Add dag level attributes to each job job.add_job_attrs(self.graph["attr"]) diff --git a/python/lsst/ctrl/bps/htcondor/provisioner.py b/python/lsst/ctrl/bps/htcondor/provisioner.py index 634ba70..b853625 100644 --- a/python/lsst/ctrl/bps/htcondor/provisioner.py +++ b/python/lsst/ctrl/bps/htcondor/provisioner.py @@ -192,6 +192,9 @@ def provision(self, dag: HTCDag) -> None: "executable": f"{self.script_name}", "should_transfer_files": "NO", "getenv": "True", + "kill_sig": "SIGTERM", + "want_graceful_removal": "True", + "job_max_vacate_time": "60", } cmds |= { "output": str(job.subfile.with_suffix(".$(Cluster).out")), diff --git a/tests/test_htcondor_service.py b/tests/test_htcondor_service.py index 31df2de..5022ac2 100644 --- a/tests/test_htcondor_service.py +++ b/tests/test_htcondor_service.py @@ -227,7 +227,7 @@ def testPrepareProvision(self, mock_write): prov_script = Path(tmpdir) / "provisioningJob.bash" self.assertTrue(prov_script.is_file()) script_contents = prov_script.read_text() - self.assertIn(f"--nodeset {timestamp}", script_contents) + self.assertIn(f"--nodeset '{timestamp}'", script_contents) def testSubmitWithConfigPath(self): """Only testing value for wms_config_path being passed diff --git a/tests/test_provisioner.py b/tests/test_provisioner.py index 37820f9..de262cc 100644 --- a/tests/test_provisioner.py +++ b/tests/test_provisioner.py @@ -149,11 +149,15 @@ def testProvision(self): "output": f"jobs/{script.stem}/{script.stem}.$(Cluster).out", "error": f"jobs/{script.stem}/{script.stem}.$(Cluster).out", "log": f"jobs/{script.stem}/{script.stem}.$(Cluster).log", + "kill_sig": "SIGTERM", + "want_graceful_removal": "True", + "job_max_vacate_time": "60", } dag = HTCDag(name="default") with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as tmpdir: self.config[".provisioning.provisioningMaxWallTime"] = 3600 + self.config[".provisioning.provisioningMaxConsecutiveFailures"] = 5 self.config[".provisioning.provisioningScriptConfigPath"] = f"{tmpdir}/condor-info.py" self.config[".bps_defined.nodeset"] = "20260129T163505Z" @@ -162,6 +166,9 @@ def testProvision(self): provisioner.prepare(script.name, prefix=tmpdir) provisioner.provision(dag) + script_payload = (Path(tmpdir) / script.name).read_text() + self.assertIn("max_failures='5'", script_payload) + self.assertIsNotNone(dag.graph["service_job"]) self.assertEqual(dag.graph["service_job"].name, script.stem) self.assertEqual(dag.graph["service_job"].label, script.stem)