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>

diff --git a/teuthology/nuke/__init__.py b/teuthology/nuke/__init__.py
index 09e16cbe07a34a99e06e67be4e26cd95e107ed6e..3ec78bb8242743030f386ec95b9bad6b195dc427 100644 (file)
--- 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 726dccd41ce314b316f81e6d5c4ef5ef7ae0ef7d..6c02dee40a8b3754e544f09c59a9245b97bd1a56 100644 (file)
--- 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()