]> 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)
committerDan Mick <dan.mick@redhat.com>
Fri, 12 Dec 2025 09:18:37 +0000 (01:18 -0800)
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 b9a0b03d807de6204e2f01e3622993856097cde7)

teuthology/lock/ops.py

index 5374249119f2b3dc7ffec2d7c206a7f4d3dd2b4b..e21551c4a677abd2567dc7a15e28a5ffc7750211 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 maybe_job.endswith(run_name):
+            log.info("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):