lock.ops.unlock_one_safe: Invert run-match logic

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>

diff --git a/teuthology/lock/ops.py b/teuthology/lock/ops.py
index 4fb6ba86a51aeb0189314e0ebbf2235f9145cd68..4c9cd4a6ab8befac69e979042eb5653040843dd2 100644 (file)
--- a/teuthology/lock/ops.py
+++ b/teuthology/lock/ops.py
@@ -186,16 +186,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):