Bugfix/connection fail reporting

nodescraper/base/oobanddataplugin.py

@@ -67,7 +67,7 @@ def run(  # type: ignore[override]
         system_interaction_level: Union[
             SystemInteractionLevel, str
         ] = SystemInteractionLevel.INTERACTIVE,
-        preserve_connection: bool = False,
+        preserve_connection: bool = True,
         data: Optional[Any] = None,
         collection_args: Optional[Any] = None,
         analysis_args: Optional[Any] = None,

nodescraper/connection/inband/inbandlocal.py

@@ -1,4 +1,4 @@
-###############################################################################
+##############################################################################
 #
 # MIT License
 #

nodescraper/connection/inband/inbandmanager.py

@@ -37,7 +37,6 @@
 from nodescraper.interfaces.connectionmanager import ConnectionManager
 from nodescraper.interfaces.taskresulthook import TaskResultHook
 from nodescraper.models import SystemInfo, TaskResult
-from nodescraper.utils import get_exception_traceback
 
 from .inband import InBandConnection
 from .inbandlocal import LocalShell
@@ -92,47 +91,46 @@ def connect(
             self.connection = LocalShell()
             return self.result
 
-        if not self.connection_args or not isinstance(self.connection_args, SSHConnectionParams):
-            if not self.connection_args:
-                message = "No SSH credentials provided"
-            else:
-                message = "Invalide SSH creddentials provided"
+        if not self.connection_args:
+            return self._fail_connect(
+                EventCategory.RUNTIME,
+                "Cannot connect to the system: no SSH credentials were provided.",
+                "No SSH connection arguments provided to InBandConnectionManager",
+                {"connection_args": None},
+            )
 
-            self._log_event(
-                category=EventCategory.RUNTIME,
-                description=message,
-                priority=EventPriority.CRITICAL,
-                console_log=True,
+        if not isinstance(self.connection_args, SSHConnectionParams):
+            return self._fail_connect(
+                EventCategory.RUNTIME,
+                "Cannot connect to the system: the SSH credentials are not valid.",
+                "SSH connection arguments are not SSHConnectionParams "
+                f"(got {type(self.connection_args).__name__})",
+                {"connection_args_type": type(self.connection_args).__name__},
             )
-            self.result.status = ExecutionStatus.EXECUTION_FAILURE
-            return self.result
 
+        hostname = self.connection_args.hostname
+        self.logger.info("Initializing SSH connection to system '%s'", hostname)
+        shell = RemoteShell(self.connection_args)
         try:
-            self.logger.info(
-                "Initializing SSH connection to system '%s'",
-                self.connection_args.hostname,
-            )
-            self.connection = RemoteShell(self.connection_args)
-            self.connection.connect_ssh()
+            shell.connect_ssh()
         except SSHConnectionError as exception:
-            self._log_event(
-                category=EventCategory.SSH,
-                description=f"{str(exception)}",
-                priority=EventPriority.CRITICAL,
-                console_log=True,
+            shell.client.close()
+            port = self.connection_args.port
+            return self._fail_connect(
+                EventCategory.SSH,
+                "Could not connect to the system over SSH.",
+                f"SSH connection to '{hostname}:{port}' failed: {exception}",
+                {
+                    "hostname": str(hostname),
+                    "port": port,
+                    "username": self.connection_args.username,
+                    "exception": str(exception),
+                },
             )
-            self.connection = None  # Nullify the connection since its not usable
-            self.result.status = ExecutionStatus.EXECUTION_FAILURE
-        except Exception as exception:
-            self._log_event(
-                category=EventCategory.SSH,
-                description=f"Exception during SSH: {str(exception)}",
-                data=get_exception_traceback(exception),
-                priority=EventPriority.CRITICAL,
-                console_log=True,
-            )
-            self.connection = None  # Nullify the connection since its not usable
-            self.result.status = ExecutionStatus.EXECUTION_FAILURE
+
+        self.connection = shell
+        self.result.message = f"SSH connection to '{hostname}' established successfully"
+        self.result.status = ExecutionStatus.OK
         return self.result
 
     def disconnect(self):

nodescraper/connection/inband/inbandremote.py

@@ -23,6 +23,7 @@
 # SOFTWARE.
 #
 ###############################################################################
+import logging
 import os
 import socket
 from typing import Type, Union
@@ -98,8 +99,10 @@ def connect_ssh(self):
             raise SSHConnectionError(f"Unable to establish SSH connection: {str(e)}") from e
         except EOFError as e:
             raise SSHConnectionError("EOFError during SSH connection") from e
+        except OSError as e:
+            raise SSHConnectionError(f"SSH network error: {e}") from e
         except Exception as e:
-            raise e
+            raise SSHConnectionError(f"Unexpected error during SSH connection: {str(e)}") from e
 
     def read_file(
         self,
@@ -176,6 +179,11 @@ def run_command(
             stderr_str = str(exc) or "SSH channel failed"
             stdout_str = ""
             exit_code = 124
+        except Exception as exc:
+            logging.getLogger(__name__).exception("Unexpected error running SSH command")
+            stderr_str = f"{type(exc).__name__}: {exc}"
+            stdout_str = ""
+            exit_code = 124
 
         return CommandArtifact(
             command=cmd_str,

nodescraper/connection/oob_ssh/oob_ssh_connection_manager.py

@@ -32,7 +32,6 @@
 from nodescraper.interfaces.connectionmanager import ConnectionManager
 from nodescraper.interfaces.taskresulthook import TaskResultHook
 from nodescraper.models import SystemInfo, TaskResult
-from nodescraper.utils import get_exception_traceback
 
 from ..inband.inband import InBandConnection
 from ..inband.inbandremote import RemoteShell, SSHConnectionError
@@ -66,62 +65,83 @@ def __init__(
 
     def connect(self) -> TaskResult:
         if not self.connection_args:
-            self._log_event(
-                category=EventCategory.RUNTIME,
-                description="No Redfish connection parameters provided for OOB SSH",
-                priority=EventPriority.CRITICAL,
-                console_log=True,
+            return self._fail_connect(
+                EventCategory.RUNTIME,
+                "Cannot connect to the BMC over SSH: no connection details were provided.",
+                "No Redfish connection parameters provided for OOB SSH",
+                {"connection_args": None},
             )
-            self.result.status = ExecutionStatus.EXECUTION_FAILURE
-            return self.result
 
         raw = self.connection_args
         if isinstance(raw, dict):
-            params = RedfishConnectionParams.model_validate(raw)
+            try:
+                params = RedfishConnectionParams.model_validate(raw)
+            except ValueError as exception:
+                return self._fail_connect(
+                    EventCategory.RUNTIME,
+                    "Cannot connect to the BMC over SSH: the connection details are not valid.",
+                    f"Invalid Redfish connection parameters for OOB SSH: {exception}",
+                    {"exception": str(exception)},
+                )
         elif isinstance(raw, RedfishConnectionParams):
             params = raw
         else:
-            self._log_event(
-                category=EventCategory.RUNTIME,
-                description="Redfish connection_args must be dict or RedfishConnectionParams",
-                priority=EventPriority.CRITICAL,
-                console_log=True,
+            return self._fail_connect(
+                EventCategory.RUNTIME,
+                "Cannot connect to the BMC over SSH: the connection details are not valid.",
+                "Redfish connection_args must be dict or RedfishConnectionParams "
+                f"(got {type(raw).__name__})",
+                {"connection_args_type": type(raw).__name__},
             )
-            self.result.status = ExecutionStatus.EXECUTION_FAILURE
-            return self.result
 
         if params.is_multi_target and params.host is None:
-            self.result.status = ExecutionStatus.NOT_RAN
-            self.result.message = (
+            message = (
                 "OOB SSH plugins use a single BMC host. "
                 "A targets list applies only to Redfish plugins."
             )
+            self._log_event(
+                category=EventCategory.RUNTIME,
+                description=message,
+                priority=EventPriority.WARNING,
+                console_log=True,
+            )
+            self.result.status = ExecutionStatus.NOT_RAN
+            self.result.message = message
             return self.result
 
         try:
             ssh_params = redfish_params_to_ssh(params)
-            self.logger.info("Initializing OOB SSH to BMC host %s", ssh_params.hostname)
-            self.connection = RemoteShell(ssh_params)
-            self.connection.connect_ssh()
-        except SSHConnectionError as exception:
-            self._log_event(
-                category=EventCategory.SSH,
-                description=str(exception),
-                priority=EventPriority.CRITICAL,
-                console_log=True,
+        except ValueError as exception:
+            return self._fail_connect(
+                EventCategory.RUNTIME,
+                "Cannot connect to the BMC over SSH: the connection details are not valid.",
+                f"Could not build OOB SSH parameters from the Redfish parameters: {exception}",
+                {
+                    "host": str(params.host),
+                    "exception": str(exception),
+                },
             )
-            self.result.status = ExecutionStatus.EXECUTION_FAILURE
-            self.connection = None
-        except Exception as exception:
-            self._log_event(
-                category=EventCategory.SSH,
-                description=f"Exception during OOB SSH: {exception!s}",
-                data=get_exception_traceback(exception),
-                priority=EventPriority.CRITICAL,
-                console_log=True,
+
+        hostname = ssh_params.hostname
+        self.logger.info("Initializing OOB SSH to BMC host %s", hostname)
+        shell = RemoteShell(ssh_params)
+        try:
+            shell.connect_ssh()
+        except SSHConnectionError as exception:
+            shell.client.close()
+            return self._fail_connect(
+                EventCategory.SSH,
+                "Could not connect to the BMC over SSH.",
+                f"OOB SSH connection to BMC '{hostname}:{ssh_params.port}' failed: {exception}",
+                {
+                    "hostname": str(hostname),
+                    "port": ssh_params.port,
+                    "username": ssh_params.username,
+                    "exception": str(exception),
+                },
             )
-            self.result.status = ExecutionStatus.EXECUTION_FAILURE
-            self.connection = None
+
+        self.connection = shell
         return self.result
 
     def disconnect(self) -> None:

nodescraper/connection/redfish/redfish_connection.py

@@ -188,10 +188,7 @@ def _transport_error_message(self, exc: Exception) -> str:
     def _execute_request(self, request_fn: Callable[[], _T]) -> _T:
         try:
             return request_fn()
-        except (
-            requests.exceptions.Timeout,
-            requests.exceptions.ConnectionError,
-        ) as exc:
+        except requests.exceptions.RequestException as exc:
             raise RedfishConnectionError(self._transport_error_message(exc)) from exc
         except socket.gaierror as exc:
             raise RedfishConnectionError(self._transport_error_message(exc)) from exc
@@ -251,7 +248,12 @@ def get(self, path: RedfishPath) -> dict[str, Any]:
                 f"GET {path_str} failed: {resp.status_code} {resp.reason}",
                 response=resp,
             )
-        return resp.json()
+        try:
+            return resp.json()
+        except ValueError as exc:
+            raise RedfishConnectionError(
+                f"GET {path_str} returned a body that is not valid JSON", response=resp
+            ) from exc
 
     def get_response(self, path: Union[str, "RedfishPath"]) -> RedfishHttpResponse:
         """GET a Redfish path and return the raw Response. path may be a string or RedfishPath."""