Handle exceptions and logging in the provision function

agent/testflinger_agent/agent.py

@@ -263,7 +263,7 @@ def process_jobs(self):
                     self.client.post_job_state(job.job_id, phase)
                     self.set_agent_state(phase)
                     provision_error_log = os.path.join(
-                        rundir, "provision_error.log"
+                        rundir, "provision-error.json"
                     )
                     if phase == "provision":
                         # Clear provision error log before starting
@@ -289,10 +289,7 @@ def process_jobs(self):
                             self.client.post_provision_log(
                                 job.job_id, exit_code, exit_event
                             )
-                            with open(
-                                provision_error_log, "a"
-                            ) as provision_error:
-                                detail = provision_error.read()
+                            detail = parse_provision_logs(provision_error_log)
                         event_emitter.emit_event(exit_event, detail)
                         if phase != "test":
                             logger.debug(
@@ -348,3 +345,25 @@ def retry_old_results(self):
             except TFServerError:
                 # Problems still, better luck next time?
                 pass
+
+    def parse_provision_logs(provision_error_log):
+        with open(provision_error_log, "a") as provision_error_file:
+            provision_file_contents = provision_error_file.read()
+            try:
+                exception_info = json.loads(provision_file_contents)[
+                    "exception_info"
+                ]
+                if exception_info["exception_cause"] == None:
+                    detail = "%s: %s" % (
+                        exception_info["exception_name"],
+                        exception_info["exception_message"],
+                    )
+                else:
+                    detail = "%s: %s caused by %s" % (
+                        exception_info["exception_name"],
+                        exception_info["exception_message"],
+                        exception_info["exception_cause"],
+                    )
+                return detail
+            except ValueError:
+                return ""

device-connectors/src/testflinger_device_connectors/devices/__init__.py

@@ -54,27 +54,25 @@
 
 
 class ProvisioningError(Exception):
-    def __init__(self, message: str = None):
-        if message is not None:
-            with open(
-                "provision_error.log", "w", encoding="utf-8"
-            ) as provision_error_file:
-                provision_error_file.write(message)
-                super().__init__(message)
-        else:
-            super().__init__()
+    pass
 
 
 class RecoveryError(Exception):
-    def __init__(self, message: str = None):
-        if message is not None:
-            with open(
-                "provision_error.log", "w", encoding="utf-8"
-            ) as provision_error_file:
-                provision_error_file.write(message)
-                super().__init__(message)
-        else:
-            super().__init__()
+    pass
+
+
+def log_provision_error(exception: Exception):
+    exception_info = {
+        "exception_info": {
+            "exception_name": type(exception).__name__,
+            "exception_message": str(exception),
+            "exception_cause": repr(exception.__cause__),
+        }
+    }
+    with open(
+        "provision-error.json", "w", encoding="utf-8"
+    ) as provision_error_file:
+        provision_error_file.write(json.dumps(exception_info))
 
 
 def SerialLogger(host=None, port=None, filename=None):

device-connectors/src/testflinger_device_connectors/devices/maas2/maas2.py

@@ -26,6 +26,7 @@
 from testflinger_device_connectors.devices import (
     ProvisioningError,
     RecoveryError,
+    log_provision_error,
 )
 from testflinger_device_connectors.devices.maas2.maas_storage import (
     MaasStorage,
@@ -70,19 +71,22 @@ def recover(self):
         self.node_release()
 
     def provision(self):
-        if self.config.get("reset_efi"):
-            self.reset_efi()
-        # Check if this is a device where we need to clear the tpm (dawson)
-        if self.config.get("clear_tpm"):
-            self.clear_tpm()
-        provision_data = self.job_data.get("provision_data")
-        # Default to a safe LTS if no distro is specified
-        distro = provision_data.get("distro", "xenial")
-        kernel = provision_data.get("kernel")
-        user_data = provision_data.get("user_data")
-        storage_data = provision_data.get("disks")
-
-        self.deploy_node(distro, kernel, user_data, storage_data)
+        try:
+            if self.config.get("reset_efi"):
+                self.reset_efi()
+            # Check if this is a device where we need to clear the tpm (dawson)
+            if self.config.get("clear_tpm"):
+                self.clear_tpm()
+            provision_data = self.job_data.get("provision_data")
+            # Default to a safe LTS if no distro is specified
+            distro = provision_data.get("distro", "xenial")
+            kernel = provision_data.get("kernel")
+            user_data = provision_data.get("user_data")
+            storage_data = provision_data.get("disks")
+            self.deploy_node(distro, kernel, user_data, storage_data)
+        except Exception as err:
+            log_provision_error(err)
+            raise
 
     def _install_efitools_snap(self):
         cmd = [

device-connectors/src/testflinger_device_connectors/devices/maas2/maas_storage.py

@@ -26,11 +26,7 @@
 
 
 class MaasStorageError(ProvisioningError):
-    def __init__(self, message: str = None):
-        if message is None:
-            super().__init__("MAAS Storage Error")
-        else:
-            super().__init__(f"MAAS Storage Error: {message}")
+    pass
 
 
 class MaasStorage:

device-connectors/src/testflinger_device_connectors/devices/maas2/tests/test_maas.py

@@ -18,13 +18,31 @@
 import json
 import pytest
 import yaml
+from unittest.mock import patch
 from testflinger_device_connectors.devices.maas2 import Maas2
 from testflinger_device_connectors.devices import ProvisioningError
 from testflinger_device_connectors.devices.maas2.maas_storage import (
     MaasStorageError,
 )
 
 
+@pytest.fixture
+def dummy_maas2(tmp_path):
+    config_yaml = tmp_path / "config.yaml"
+    config = {"maas_user": "user", "node_id": "abc", "agent_name": "agent001"}
+    config_yaml.write_text(yaml.safe_dump(config))
+
+    job_json = tmp_path / "job.json"
+    job = {"provision_data": {}}
+    job_json.write_text(json.dumps(job))
+
+    with patch(
+        "testflinger_device_connectors.devices.maas2.maas2.MaasStorage",
+        return_value=None,
+    ):
+        return Maas2(config=config_yaml, job_data=job_json)
+
+
 def test_maas2_agent_invalid_storage(tmp_path):
     """Test that the maas2 agent raises an exception when storage init fails"""
     config_yaml = tmp_path / "config.yaml"
@@ -40,3 +58,25 @@ def test_maas2_agent_invalid_storage(tmp_path):
 
     # MaasStorageError should also be a subclass of ProvisioningError
     assert isinstance(err.value, ProvisioningError)
+
+
+def test_maas2_error_file_logging(dummy_maas2):
+    open("provision-error.json", "w").close()
+    error_message = "my error message"
+    exception_info = {
+        "exception_name": "ProvisioningError",
+        "exception_message": error_message,
+        "exception_cause": "MaasStorageError()",
+    }
+    with patch.object(Maas2, "deploy_node") as mock_deploy_node:
+        provisioning_error = ProvisioningError(error_message)
+        provisioning_error.__cause__ = MaasStorageError()
+        mock_deploy_node.side_effect = provisioning_error
+        try:
+            dummy_maas2.provision()
+        except Exception:
+            with open("provision-error.json") as error_file:
+                assert (
+                    json.loads(error_file.read())["exception_info"]
+                    == exception_info
+                )

device-connectors/src/tests/test_devices.py

@@ -22,10 +22,6 @@
 from testflinger_device_connectors.devices import (
     DEVICE_CONNECTORS,
     get_device_stage_func,
-    ProvisioningError,
-)
-from testflinger_device_connectors.devices.maas2.maas_storage import (
-    MaasStorageError,
 )
 
 STAGES_CONNECTORS_PRODUCT = tuple(product(STAGES, DEVICE_CONNECTORS))
@@ -40,23 +36,3 @@ def test_get_device_stage_func(stage, device):
     orig_func = getattr(connector_instance, stage)
     func = get_device_stage_func(device, stage)
     assert func.__func__ is orig_func.__func__
-
-
-def test_provision_error_file_logging():
-    open("provision_error.log", "w").close()
-    error_message = "my error message"
-    try:
-        raise ProvisioningError(error_message)
-    except ProvisioningError:
-        with open("provision_error.log") as error_file:
-            assert error_file.read() == error_message
-
-
-def test_maas_storage_error_file_logging():
-    open("provision_error.log", "w").close()
-    error_message = "MAAS Storage Error"
-    try:
-        raise MaasStorageError()
-    except MaasStorageError:
-        with open("provision_error.log") as error_file:
-            assert error_file.read() == error_message