From a56302017805d047e97394c3b4a62bbed5935504 Mon Sep 17 00:00:00 2001
From: Zack Cerza <zack@redhat.com>
Date: Wed, 8 Jun 2022 12:41:37 -0600
Subject: [PATCH] nuke.nuke: Rework lock-checking logic

Previously, we would call list_locks(), then iterate over the response,
each time iterating over the list of targets. If list_locks()
encountered an error and returned an empty response, we'd never actually
verify what we intended to. Instead, we should specifically query for
each target. This is far safer and faster.

Signed-off-by: Zack Cerza <zack@redhat.com>
---
 teuthology/nuke/__init__.py  | 31 +++++++++++++------------------
 teuthology/test/test_nuke.py | 13 ++++++++-----
 2 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/teuthology/nuke/__init__.py b/teuthology/nuke/__init__.py
index 09e16cbe0..3ec78bb82 100644
--- a/teuthology/nuke/__init__.py
+++ b/teuthology/nuke/__init__.py
@@ -239,26 +239,21 @@ def nuke(ctx, should_unlock, sync_clocks=True, noipmi=False, keep_logs=False, sh
     if 'targets' not in ctx.config:
         return
     total_unnuked = {}
-    targets = dict(ctx.config['targets'])
-    if ctx.name:
-        log.info('Checking targets against current locks')
-        locks = list_locks()
-        # Remove targets who's description doesn't match archive name.
-        for lock in locks:
-            for target in targets:
-                if target == lock['name']:
-                    if ctx.name not in lock['description']:
-                        del ctx.config['targets'][lock['name']]
-                        log.info(
-                            "Not nuking %s because description doesn't match",
-                            lock['name'])
-                    elif lock.get('up') is False:
-                        del ctx.config['targets'][lock['name']]
-                        log.info(
-                            "Not nuking %s because it is down",
-                            lock['name'])
+    log.info('Checking targets against current locks')
     with parallel() as p:
         for target, hostkey in ctx.config['targets'].items():
+            status = get_status(target)
+            if ctx.name and ctx.name not in status['description']:
+                total_unnuked[target] = hostkey
+                log.info(
+                    f"Not nuking {target} because description doesn't match: "
+                    f"{ctx.name} != {status['description']}"
+                )
+                continue
+            elif status.get('up') is False:
+                total_unnuked[target] = hostkey
+                log.info(f"Not nuking {target} because it is down")
+                continue
             p.spawn(
                 nuke_one,
                 ctx,
diff --git a/teuthology/test/test_nuke.py b/teuthology/test/test_nuke.py
index 726dccd41..6c02dee40 100644
--- a/teuthology/test/test_nuke.py
+++ b/teuthology/test/test_nuke.py
@@ -241,16 +241,18 @@ def test_nuke_internal():
         os_version='8.3',
         name='test_name',
     )
-    locks = [{'name': target, 'description': job_config['name']} for target
-             in job_config['targets'].keys()]
+    statuses = {
+        target: {'name': target, 'description': job_config['name']}
+        for target in job_config['targets'].keys()
+    }
     ctx = create_fake_context(job_config)
 
     # minimal call using defaults
     with patch.multiple(
             nuke,
             nuke_helper=DEFAULT,
-            list_locks=lambda: locks,
             unlock_one=DEFAULT,
+            get_status=lambda i: statuses[i],
             ) as m:
         nuke.nuke(ctx, True)
         m['nuke_helper'].assert_called_with(ANY, True, False, True)
@@ -260,8 +262,8 @@ def test_nuke_internal():
     with patch.multiple(
             nuke,
             nuke_helper=DEFAULT,
-            list_locks=lambda: locks,
             unlock_one=DEFAULT,
+            get_status=lambda i: statuses[i],
             ) as m:
         nuke.nuke(ctx, False)
         m['nuke_helper'].assert_called_with(ANY, False, False, True)
@@ -271,8 +273,8 @@ def test_nuke_internal():
     with patch.multiple(
             nuke,
             nuke_helper=DEFAULT,
-            list_locks=lambda: locks,
             unlock_one=DEFAULT,
+            get_status=lambda i: statuses[i],
             ) as m:
         nuke.nuke(ctx, False, True, False, True, False)
         m['nuke_helper'].assert_called_with(ANY, False, True, False)
@@ -284,6 +286,7 @@ def test_nuke_internal():
             nuke,
             nuke_helper=DEFAULT,
             unlock_one=DEFAULT,
+            get_status=lambda i: statuses[i],
             ) as m:
         nuke.nuke(ctx, True)
         m['nuke_helper'].assert_not_called()
-- 
2.47.3