provision.fog: Mark nodes down in some cases

Specifically, if we time out waiting for the SSH port to open after a reimage,
we want to mark down immediately so that we can investigate.

Signed-off-by: Zack Cerza <zack@cerza.org>

Author: zmc

teuthology/dispatcher/supervisor.py

@@ -10,7 +10,7 @@
 
 from teuthology import exporter, dispatcher, kill, report, safepath
 from teuthology.config import config as teuth_config
-from teuthology.exceptions import SkipJob, MaxWhileTries
+from teuthology.exceptions import SkipJob, MaxWhileTries, ReimageFailureNeedsInvestigation
 from teuthology import setup_log_file, install_except_hook
 from teuthology.misc import get_user, archive_logs, compress_logs
 from teuthology.config import FakeNamespace
@@ -175,8 +175,14 @@ def run_job(job_config, teuth_bin_path, archive_dir, verbose):
         log.error('Child exited with code %d', p.returncode)
     else:
         log.info('Success!')
-    if 'targets' in job_config:
-        unlock_targets(job_config)
+    if 'targets' in job_config and job_config.get("unlock_on_failure", True):
+        unlock_targets(
+            job_config['targets'],
+            job_config['owner'],
+            job_config['name'],
+            job_config['job_id'],
+            job_config['archive_path'],
+        )
     return p.returncode
 
 def failure_is_reimage(failure_reason):
@@ -232,8 +238,28 @@ def reimage(job_config):
     try:
         reimaged = lock_ops.reimage_machines(ctx, targets, job_config['machine_type'])
     except Exception as e:
-        log.exception('Reimaging error. Unlocking machines...')
-        unlock_targets(job_config)
+        targets = job_config['targets'].copy()
+        log.exception('Reimaging error')
+        if isinstance(e, ReimageFailureNeedsInvestigation):
+            # This error requires further investigation. Mark the affected node
+            # down and leave it locked.
+            log.info(f"Marking {e.node_name} down for investigation")
+            lock_ops.update_lock(
+                e.node_name,
+                description=str(e.inner),
+                status='down',
+            )
+            targets = job_config['targets'].copy()
+            targets.pop(e.node_name)
+        if job_config.get("unlock_on_failure", True):
+            log.info('Unlocking machines...')
+            unlock_targets(
+                targets,
+                job_config['owner'],
+                job_config['name'],
+                job_config['job_id'],
+                job_config['archive_path'],
+            )
         # Reimage failures should map to the 'dead' status instead of 'fail'
         report.try_push_job_info(
             ctx.config,
@@ -252,20 +278,20 @@ def reimage(job_config):
     report.try_push_job_info(ctx.config, dict(status='running'))
 
 
-def unlock_targets(job_config):
+def unlock_targets(targets: dict, owner: str, run_name: str, job_id: str, archive_path: str):
     """
     Unlock machines only if locked and description matches.
 
     :param job_config:      dict, job config data
     """
-    machine_statuses = query.get_statuses(job_config['targets'].keys())
+    machine_statuses = query.get_statuses(targets.keys())
     locked = []
     for status in machine_statuses:
         name = shortname(status['name'])
         description = status['description']
         if not status['locked']:
             continue
-        if description != job_config['archive_path']:
+        if description != archive_path:
             log.warning(
                 "Was going to unlock %s but it was locked by another job: %s",
                 name, description
@@ -274,9 +300,8 @@ def unlock_targets(job_config):
         locked.append(name)
     if not locked:
         return
-    if job_config.get("unlock_on_failure", True):
-        log.info('Unlocking machines...')
-        lock_ops.unlock_safe(locked, job_config["owner"], job_config["name"], job_config["job_id"])
+    log.info('Unlocking machines...')
+    lock_ops.unlock_safe(locked, owner, run_name, job_id)
 
 
 def run_with_watchdog(process, job_config):

teuthology/exceptions.py

@@ -245,3 +245,15 @@ def __str__(self):
             prefix=prefix,
             message=self.message,
         )
+
+class ReimageFailure(Exception):
+    def __init__(self, node_name: str, message: str, inner: Optional[Exception] = None):
+        self.node_name: str = node_name
+        self.message: str = message
+        self.inner: Exception = inner
+
+    def __str__(self):
+        return f"Reimage of {self.node_name} failed with message: '{self.message}' (Inner exception: {self.inner})"
+
+class ReimageFailureNeedsInvestigation(ReimageFailure):
+    pass

teuthology/provision/fog.py

@@ -12,7 +12,7 @@
 
 from teuthology.config import config
 from teuthology.contextutil import safe_while
-from teuthology.exceptions import MaxWhileTries
+from teuthology.exceptions import MaxWhileTries, ReimageFailureNeedsInvestigation
 from teuthology.orchestra.opsys import OS
 from teuthology import misc
 
@@ -88,7 +88,19 @@ def create(self):
         except Exception:
             self.cancel_deploy_task(task_id)
             raise
-        self._wait_for_ready()
+        try:
+            self._wait_for_ready()
+        except MaxWhileTries as e:
+            # If the SSH port never opened, this requires investigation
+            if isinstance(e.last_exception, NoValidConnectionsError):
+                log.exception("Reimage failure")
+                raise ReimageFailureNeedsInvestigation(
+                    node_name=self.name,
+                    message=f"Reimage failed: {e.last_exception}",
+                    inner=e.last_exception,
+                )
+            else:
+                raise
         self._fix_hostname()
         self._verify_installed_os()
         self.log.info("Deploy complete!")
@@ -283,7 +295,8 @@ def cancel_deploy_task(self,  task_id):
     def _wait_for_ready(self):
         """ Attempt to connect to the machine via SSH """
         with safe_while(sleep=6, timeout=config.fog_wait_for_ssh_timeout) as proceed:
-            while proceed():
+            last_exception = None
+            while proceed(last_exception):
                 try:
                     self.remote.connect()
                     break
@@ -299,6 +312,7 @@ def _wait_for_ready(self):
                     # a mismatched host key in ~/.ssh/known_hosts, or
                     # something)
                     self.log.warning(e)
+                    last_exception = e
         sentinel_file = config.fog.get('sentinel_file', None)
         if sentinel_file:
             cmd = "while [ ! -e '%s' ]; do sleep 5; done" % sentinel_file