From b9a0b03d807de6204e2f01e3622993856097cde7 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 19 Mar 2025 12:35:11 -0600 Subject: [PATCH] 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 --- teuthology/lock/ops.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/teuthology/lock/ops.py b/teuthology/lock/ops.py index 4fb6ba86a5..4c9cd4a6ab 100644 --- 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): -- 2.39.5