Improve test robustness and simplify debugging ephemeral failures

tests/integration_tests/clouds.py

@@ -66,6 +66,7 @@ def __init__(
         self.cloud_instance = self._get_cloud_instance()
         self.initial_image_id = self._get_initial_image()
         self.snapshot_id: Optional[str] = None
+        self.test_failed = False
 
     @property
     def image_id(self):
@@ -178,6 +179,13 @@ def destroy(self):
                 "NOT cleaning cloud instance because KEEP_IMAGE or "
                 "KEEP_INSTANCE is True"
             )
+        elif (
+            integration_settings.KEEP_IMAGE == "ON_ERROR" and self.test_failed
+        ):
+            log.info(
+                'NOT cleaning cloud instance because KEEP_IMAGE="ON_ERROR" and'
+                "test failed"
+            )
         else:
             self.cloud_instance.clean()
 

tests/integration_tests/conftest.py

@@ -337,6 +337,8 @@ def _client(
         yield instance
         test_failed = request.session.testsfailed - previous_failures > 0
         _collect_artifacts(instance, request.node.nodeid, test_failed)
+        instance.test_failed = test_failed
+        session_cloud.test_failed = test_failed
     # conflicting requirements:
     # - pytest thinks that it can cleanup loggers after tests run
     # - pycloudlib thinks that at garbage collection is a good place to

tests/integration_tests/instances.py

@@ -2,6 +2,7 @@
 import logging
 import os
 import re
+import time
 import uuid
 from enum import Enum
 from pathlib import Path
@@ -67,21 +68,45 @@ def __init__(
         self.cloud = cloud
         self.instance = instance
         self.settings = settings
+        self.test_failed = False
         self._ip = ""
 
     def destroy(self):
+        wait = True
         if isinstance(self.instance, GceInstance):
-            self.instance.delete(wait=False)
+            wait = False
+        for i in range(1, 32):
+            try:
+                self.instance.delete(wait=wait)
+                break
+            except RuntimeError:
+                log.warning("Failed to clean instance, retrying.")
+                time.sleep(i)
         else:
-            self.instance.delete()
+            # This is just a cleanup mechanism. If we fail to clean the
+            # instance that doesn't mean that the test has actually failed.
+            log.error("Failed to clean instance")
 
     def restart(self):
         """Restart this instance (via cloud mechanism) and wait for boot.
 
         This wraps pycloudlib's `BaseInstance.restart`
         """
         log.info("Restarting instance and waiting for boot")
-        self.instance.restart()
+        last_exception = None
+        for i in range(1, 32):
+            try:
+                self.instance.restart()
+                break
+            except RuntimeError as e:
+                last_exception = e
+                log.warning("Failed to restart instance.")
+                time.sleep(i)
+        else:
+            # this is a requirement for tests, so we must fail to prevent
+            # running tests in an unexpected state
+            log.error("Failed to restart instance.")
+            raise last_exception
 
     def execute(self, command, *, use_sudo=True) -> Result:
         if self.instance.username == "root" and use_sudo is False:
@@ -329,7 +354,13 @@ def __enter__(self):
         return self
 
     def __exit__(self, exc_type, exc_val, exc_tb):
+
         if not self.settings.KEEP_INSTANCE:
             conftest.REAPER.reap(self)
+        elif (
+            integration_settings.KEEP_INSTANCE == "ON_ERROR"
+            and self.test_failed
+        ):
+            log.info("Keeping Instance (test failed) public ip: %s", self.ip())
         else:
-            log.info("Keeping Instance, public ip: %s", self.ip())
+            log.info("Keeping Instance public ip: %s", self.ip())

tests/integration_tests/integration_settings.py

@@ -9,6 +9,7 @@
 ##################################################################
 
 # Keep instance (mostly for debugging) when test is finished
+# set to "ON_ERROR" to only keep an instance when tests fail
 KEEP_INSTANCE = False
 # Keep snapshot image (mostly for debugging) when test is finished
 KEEP_IMAGE = False