Merge pull request redhat-performance#448 from grafuls/development

fix: better session handling

Author: sadsfae

.gitignore

@@ -113,3 +113,6 @@ venv.bak/
 # SCP Exports folder (Testing purpose)
 tests/exports/
 exports/
+
+# VSCode
+.vscode/

src/badfish/main.py

@@ -69,6 +69,16 @@ def __init__(self, _host, _username, _password, _logger, _retries, _loop=None):
         self.token = None
         self.vendor = None
 
+    async def __aenter__(self):
+        await self.init()
+        return self
+
+    async def __aexit__(self, exc_type, exc_val, exc_tb):
+        await self.delete_session()
+        if exc_type is not None:
+            self.logger.debug(f"Exiting context with exception: {exc_type.__name__}: {exc_val}")
+        return False
+
     async def init(self):
         self.session_uri = await self.find_session_uri()
         self.token = await self.validate_credentials()
@@ -2126,12 +2136,29 @@ async def take_screenshot(self):
         return True
 
     async def delete_session(self):
-        headers = {"content-type": "application/json"}
-        _uri = "%s%s" % (self.host_uri, self.session_id)
-        _response = await self.delete_request(_uri, headers=headers)
-        if _response.status not in [200, 201]:
-            raise BadfishException(f"Failed to delete X-Auth-Token for {self.host}")
-        return
+        try:
+            try:
+                if not self.session_id:
+                    self.logger.debug("No session ID found, skipping session deletion")
+                    return
+                headers = {"content-type": "application/json"}
+                _uri = "%s%s" % (self.host_uri, self.session_id)
+                try:
+                    _response = await self.delete_request(_uri, headers=headers)
+                    if _response.status in [200, 201]:
+                        self.logger.debug(f"Session successfully deleted for {self.host}")
+                    elif _response.status == 404:
+                        self.logger.debug(f"Session not found (404) for {self.host}, may have been already deleted")
+                    else:
+                        self.logger.warning(f"Unexpected status {_response.status} when deleting session for {self.host}.")
+                except Exception as ex:
+                    self.logger.warning(f"Failed to delete session for {self.host}: {ex}")
+            finally:
+                self.session_id = None
+                self.token = None
+        except Exception:
+            self.session_id = None
+            self.token = None
 
     async def get_scp_targets(self, op):
         uri = "%s%s" % (self.host_uri, self.manager_resource)
@@ -2564,6 +2591,7 @@ async def execute_badfish(_host, _args, logger, format_handler=None):
     get_nic_attribute = _args["get_nic_attribute"]
     set_nic_attribute = _args["set_nic_attribute"]
     result = True
+    badfish = None
 
     try:
         badfish = await badfish_factory(
@@ -2684,10 +2712,16 @@ async def execute_badfish(_host, _args, logger, format_handler=None):
         if pxe and not host_type:
             await badfish.set_next_boot_pxe()
 
-        await badfish.delete_session()
     except BadfishException as ex:
         logger.error(ex)
         result = False
+    finally:
+        if badfish and badfish.session_id:
+            try:
+                await badfish.delete_session()
+                logger.debug(f"Session closed for host: {_host}")
+            except BadfishException as ex:
+                logger.warning(f"Failed to close session for {_host}: {ex}")
 
     if _args["host_list"]:
         logger.info("*" * 48)

tests/config.py

@@ -372,12 +372,14 @@ def render_device_dict(index, device):
     "- INFO     - Attempting to clear job list instead.\n"
     "- WARNING  - Clearing job queue for job IDs: ['JID_498218641680'].\n"
     "- ERROR    - Job queue not cleared, there was something wrong with your request.\n"
+    "- WARNING  - Unexpected status 400 when deleting session for f01-h01-000-r630.host.io.\n"
 )
 RESPONSE_DELETE_JOBS_UNSUPPORTED_EXCEPTION = (
     "- WARNING  - iDRAC version installed does not support DellJobService\n"
     "- INFO     - Attempting to clear job list instead.\n"
     "- WARNING  - Clearing job queue for job IDs: ['JID_498218641680'].\n"
     "- ERROR    - Failed to communicate with server.\n"
+    "- WARNING  - Failed to delete session for f01-h01-000-r630.host.io: Failed to communicate with server.\n"
 )
 RESPONSE_DELETE_JOBS_SUPPORTED_EXCEPTION = "- ERROR    - Error reading response from host.\n"
 
@@ -616,7 +618,7 @@ def render_device_dict(index, device):
     "- INFO     -     ProcessorType: Accelerator\n"
 )
 
-RESPONSE_LS_GPU_SUMMARY_DATA_ERROR = "- ERROR    - GPU endpoint not available on host.\n"
+RESPONSE_LS_GPU_SUMMARY_DATA_ERROR = "- ERROR    - GPU endpoint not available on host.\n- WARNING  - Failed to delete session for f01-h01-000-r630.host.io: Failed to communicate with server.\n"
 RESPONSE_LS_GPU_SUMMARY_VALUE_ERROR = "- ERROR    - There was something wrong getting GPU summary values.\n"
 RESPONSE_LS_GPU_SUMMARY_BAD_JSON = "- ERROR    - There was something wrong getting GPU data\n"
 RESPONSE_LS_GPU_DETAILS_NOT_FOUND = "- ERROR    - There was something wrong getting host GPU details\n"

tests/test_base.py

@@ -19,16 +19,16 @@ async def get_application(self):
     @staticmethod
     def set_mock_response(mock, status, responses, post=False, headers=None):
         mock.return_value.__aenter__.return_value.name = responses
+        status_mock = MagicMock()
         if type(status) == list:
-            status_mock = MagicMock()
             if post:
                 dup_stats = [val for val in status for _ in range(2)]
                 type(status_mock).status = PropertyMock(side_effect=dup_stats)
             else:
                 type(status_mock).status = PropertyMock(side_effect=status)
-            mock.return_value.__aenter__.return_value = status_mock
         else:
-            mock.return_value.__aenter__.return_value.status = status
+            type(status_mock).status = PropertyMock(return_value=status)
+        mock.return_value.__aenter__.return_value = status_mock
         mock.return_value.__aenter__.return_value.read = AsyncMock()
         if type(responses) == list:
             mock.return_value.__aenter__.return_value.text = AsyncMock(

tests/test_bios_password.py

@@ -107,6 +107,7 @@ def test_set_bios_pass_cmd_failed(self, mock_get, mock_post, mock_delete):
         responses = INIT_RESP
         self.set_mock_response(mock_get, 200, responses)
         self.set_mock_response(mock_post, [200, 400], ["OK", "Bad Request"], True)
+        self.set_mock_response(mock_delete, 200, "OK")
         self.args = [self.option_arg, "--new-password", "new_pass"]
         _, err = self.badfish_call()
         assert err == BIOS_PASS_CHANGE_CMD_FAILED