From 7e4ffe7c80c36fb9de797c70982c928bb7c31b76 Mon Sep 17 00:00:00 2001 From: cid Date: Tue, 25 Feb 2025 01:30:31 +0100 Subject: [PATCH] Include all relevant error messages in exception Retry failures report only the last error which could be misleading, so include all relevant errors in the final exception. Closes-Bug: #2098977 Change-Id: I8c0fb0328a6b3ee084813961d9a959af996a6dcb Signed-off-by: Afonne-CID --- ironic/drivers/modules/redfish/boot.py | 17 ++++- .../unit/drivers/modules/redfish/test_boot.py | 74 ++++++++++++++++--- 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index de1142b4c7..7c6035c562 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -274,8 +274,15 @@ def _insert_vmedia(task, managers, boot_url, boot_device): return if err_msgs: - exc_msg = ("All virtual media mount attempts failed. " - "Most recent error: " + err_msgs[-1]) + if len(err_msgs) == 1: + exc_msg = _("All virtual media mount attempts failed. " + "Error encountered: %s") % err_msgs[0] + else: + exc_msg = _("All virtual media mount attempts failed. " + "First error: %(first)s. Final error: %(final)s") % { + 'first': err_msgs[0], + 'final': err_msgs[-1] + } else: exc_msg = 'No suitable virtual media device found' raise exception.InvalidParameterValue(exc_msg) @@ -347,8 +354,12 @@ def _insert_vmedia_in_resource(task, resource, boot_url, boot_device, LOG.warning(err_msg) continue except sushy.exceptions.ServerSideError as e: + err_msg = str(e) + if err_msg not in err_msgs: + err_msgs.append(err_msg) + e.node_uuid = task.node.uuid - raise + return False LOG.info("Inserted boot media %(boot_url)s into " "%(boot_device)s for node " diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index 825d879835..c941574ae0 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -1420,6 +1420,54 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): self.assertEqual(mock_vmedia_dvd_2.insert_media.call_count, 1) + @mock.patch('time.sleep', lambda *args, **kwargs: None) + @mock.patch.object(redfish_boot, '_has_vmedia_via_systems', autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test__insert_vmedia_multiple_errors(self, mock_sys, mock_vmd_sys): + mock_vmd_sys.return_value = False + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + err_msgs = [] + first_error = "Previous error" + err_msgs.append(first_error) + + mock_vmedia_cd = mock.MagicMock( + inserted=False, + media_types=[sushy.VIRTUAL_MEDIA_CD]) + + error = "Unable to locate the ISO or IMG image file" + mock_response = mock.MagicMock() + mock_response.json.return_value = { + "error": {"message": error} + } + + server_error = sushy.exceptions.ServerSideError( + "POST", 'img-url', mock_response) + server_error.message = error + + mock_vmedia_cd.insert_media.side_effect = server_error + mock_manager = mock.MagicMock() + mock_manager.virtual_media.get_members.return_value = [ + mock_vmedia_cd] + + result = redfish_boot._insert_vmedia_in_resource( + task, mock_manager, 'img-url', + sushy.VIRTUAL_MEDIA_CD, err_msgs) + + self.assertFalse(result) + self.assertEqual(2, len(err_msgs)) + self.assertIn(first_error, err_msgs[0]) + self.assertIn(error, err_msgs[1]) + + joined_errors = "; ".join(err_msgs) + self.assertIn(first_error, joined_errors) + self.assertIn(error, joined_errors) + + self.assertRaises( + exception.InvalidParameterValue, + redfish_boot._insert_vmedia, + task, [mock_manager], 'img-url', sushy.VIRTUAL_MEDIA_CD) + @mock.patch.object(redfish_boot, '_has_vmedia_via_systems', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test__insert_vmedia_already_inserted(self, mock_sys, mock_vmd_sys): @@ -1454,18 +1502,19 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): ) mock_manager = mock.MagicMock() - def clear_and_raise(*args, **kwargs): - mock_vmedia_cd.insert_media.side_effect = None - raise sushy.exceptions.ServerSideError( - "POST", 'img-url', mock.MagicMock()) - mock_vmedia_cd.insert_media.side_effect = clear_and_raise + mock_response = mock.MagicMock() + mock_response.json.return_value = { + "error": {"message": "Device is ejecting"} + } + + mock_vmedia_cd.insert_media.side_effect = mock_response mock_manager.virtual_media.get_members.return_value = [ mock_vmedia_cd] redfish_boot._insert_vmedia( task, [mock_manager], 'img-url', sushy.VIRTUAL_MEDIA_CD) - self.assertEqual(mock_vmedia_cd.insert_media.call_count, 2) + self.assertEqual(mock_vmedia_cd.insert_media.call_count, 1) @mock.patch('time.sleep', lambda *args, **kwargs: None) @mock.patch.object(redfish_boot, '_has_vmedia_via_systems', autospec=True) @@ -2691,18 +2740,19 @@ class RedfishVirtualMediaBootViaSystemTestCase(db_base.DbTestCase): ) mock_manager = mock.MagicMock() - def clear_and_raise(*args, **kwargs): - mock_vmedia_cd.insert_media.side_effect = None - raise sushy.exceptions.ServerSideError( - "POST", 'img-url', mock.MagicMock()) - mock_vmedia_cd.insert_media.side_effect = clear_and_raise + mock_response = mock.MagicMock() + mock_response.json.return_value = { + "error": {"message": "Device is ejecting"} + } + + mock_vmedia_cd.insert_media.side_effect = mock_response mock_system.return_value.virtual_media.get_members.return_value = [ mock_vmedia_cd] redfish_boot._insert_vmedia( task, [mock_manager], 'img-url', sushy.VIRTUAL_MEDIA_CD) - self.assertEqual(mock_vmedia_cd.insert_media.call_count, 2) + self.assertEqual(mock_vmedia_cd.insert_media.call_count, 1) @mock.patch('time.sleep', lambda *args, **kwargs: None) @mock.patch.object(redfish_boot, '_has_vmedia_via_systems', autospec=True)