]> git-server-git.apps.pok.os.sepia.ceph.com Git - teuthology.git/commitdiff
lock.ops.unlock_one_safe: Invert run-match logic
authorZack Cerza <zack@cerza.org>
Wed, 19 Mar 2025 18:35:11 +0000 (12:35 -0600)
committerZack Cerza <zack@cerza.org>
Fri, 9 Jan 2026 23:11:08 +0000 (16:11 -0700)
When unlock_one_safe is called with run_name, the caller means to express
"unlock this node if it belongs to this run".
When it is called with run_name and job_id, it means "unlock this node if it
belongs to this job in this run".
We had inverted the logic, causing leaks on reimage failures.

Signed-off-by: Zack Cerza <zack@cerza.org>
(cherry picked from commit 104ebb3e290c8a1de2d9b637ad327e8e640f54be)

teuthology/lock/ops.py

index 2e39fe744972c6b45a834005a2d6109c6560bc84..fb27313f3e0a4177073e78c0f4c7c4201466d550 100644 (file)
@@ -191,16 +191,23 @@ def unlock_safe(names: List[str], owner: str, run_name: str = "", job_id: str =
 def unlock_one_safe(name: str, owner: str, run_name: str = "", job_id: str = "") -> bool:
     node_status = query.get_status(name)
     if node_status.get("locked", False) is False:
-        log.warn(f"Refusing to unlock {name} since it is already unlocked")
+        log.info(f"Refusing to unlock {name} since it is already unlocked")
         return False
     maybe_job = query.node_active_job(name, node_status)
     if not maybe_job:
         return unlock_one(name, owner, node_status["description"], node_status)
-    if run_name and job_id and maybe_job.endswith(f"{run_name}/{job_id}"):
-            log.error(f"Refusing to unlock {name} since it has an active job: {run_name}/{job_id}")
+    if run_name:
+        if job_id and not maybe_job.endswith(f"{run_name}/{job_id}"):
+            log.info("Not unlocking {name} since it is running {maybe_job}, not {run_name}/{job_id}")
             return False
-    log.warning(f"Refusing to unlock {name} since it has an active job: {maybe_job}")
-    return False
+        elif not job_id and not maybe_job.endswith(run_name):
+            log.info(f"Not unlocking {name} since it is running {maybe_job}, not {run_name}")
+            return False
+        else:
+            return unlock_one(name, owner, node_status["description"], node_status)
+    else:
+        log.info(f"Refusing to unlock {name} since it has an active job: {maybe_job}")
+        return False
 
 
 def unlock_many(names, user):