DM-55434: BPS Provisioner Service Job May End Before finalJob

doc/changes/DM-55434.bugfix.rst

@@ -0,0 +1 @@
+Add signal handling and resilience to default provisioningScript

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**

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}"

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"])

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")),

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

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)