From 79523c5911e5582f0b29ddf9b7a327f8e82f957d Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 7 Mar 2024 18:48:13 +0100 Subject: [PATCH] Fix error handling in the virtual media attach API Currently, if the image download fails, there are no traces of the error. This change adds logging and populates last_error. Change-Id: I73ea2f94fb910daf21a5d4f52d6839aac3bad579 --- ironic/conductor/manager.py | 41 +++++++++------ ironic/tests/unit/conductor/test_manager.py | 50 +++++++++++++++++++ .../notes/vmedia-error-ef4eac3d08761d5c.yaml | 6 +++ 3 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/vmedia-error-ef4eac3d08761d5c.yaml diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 4badfb517b..3e5dfb059b 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -4080,21 +4080,32 @@ def do_sync_power_state(task, count): def do_attach_virtual_media(task, device_type, image_url, image_download_source): - assert device_type in boot_devices.VMEDIA_DEVICES - file_name = "%s.%s" % ( - device_type.lower(), - 'iso' if device_type == boot_devices.CDROM else 'img' - ) - image_url = image_utils.prepare_remote_image( - task, image_url, file_name=file_name, - download_source=image_download_source) - utils.run_node_action( - task, task.driver.management.attach_virtual_media, - success_msg="Device %(device_type)s attached to node %(node)s", - error_msg="Could not attach device %(device_type)s " - "to node %(node)s: %(exc)s", - device_type=device_type, - image_url=image_url) + success_msg = "Device %(device_type)s attached to node %(node)s", + error_msg = ("Could not attach device %(device_type)s " + "to node %(node)s: %(exc)s") + try: + assert device_type in boot_devices.VMEDIA_DEVICES + file_name = "%s.%s" % ( + device_type.lower(), + 'iso' if device_type == boot_devices.CDROM else 'img' + ) + image_url = image_utils.prepare_remote_image( + task, image_url, file_name=file_name, + download_source=image_download_source) + utils.run_node_action( + task, task.driver.management.attach_virtual_media, + success_msg=success_msg, + error_msg=error_msg, + device_type=device_type, + image_url=image_url) + except Exception as exc: + error = error_msg % {'node': task.node.uuid, + 'device_type': device_type, + 'exc': exc} + LOG.error( + error, exc_info=not isinstance(exc, exception.IronicException)) + utils.node_history_record(task.node, event=error, error=True) + task.node.save() def do_detach_virtual_media(task, device_types): diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 2e455b3ba3..00725d2412 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -8740,3 +8740,53 @@ class VirtualMediaTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): image_url='https://url', image_download_source='http') self.node.refresh() self.assertIsNone(self.node.last_error) + + @mock.patch.object(redfish.management.RedfishManagement, + 'attach_virtual_media', autospec=True) + @mock.patch.object(image_utils, 'prepare_remote_image', autospec=True) + def test_do_attach_virtual_media(self, mock_prepare_image, mock_attach): + with task_manager.acquire(self.context, self.node.id) as task: + manager.do_attach_virtual_media(task, boot_devices.CDROM, + "https://url", "local") + mock_prepare_image.assert_called_once_with( + task, "https://url", file_name="cdrom.iso", + download_source="local") + mock_attach.assert_called_once_with( + task.driver.management, task, device_type=boot_devices.CDROM, + image_url=mock_prepare_image.return_value) + + @mock.patch.object(redfish.management.RedfishManagement, + 'attach_virtual_media', autospec=True) + @mock.patch.object(image_utils, 'prepare_remote_image', autospec=True) + def test_do_attach_virtual_media_fails_on_prepare(self, mock_prepare_image, + mock_attach): + mock_prepare_image.side_effect = exception.InvalidImageRef + with task_manager.acquire(self.context, self.node.id) as task: + manager.do_attach_virtual_media(task, boot_devices.CDROM, + "https://url", "local") + mock_prepare_image.assert_called_once_with( + task, "https://url", file_name="cdrom.iso", + download_source="local") + mock_attach.assert_not_called() + self.node.refresh() + self.assertIn("Could not attach device cdrom", self.node.last_error) + self.assertIn("Invalid image href", self.node.last_error) + + @mock.patch.object(redfish.management.RedfishManagement, + 'attach_virtual_media', autospec=True) + @mock.patch.object(image_utils, 'prepare_remote_image', autospec=True) + def test_do_attach_virtual_media_fails_on_attach(self, mock_prepare_image, + mock_attach): + mock_attach.side_effect = exception.UnsupportedDriverExtension + with task_manager.acquire(self.context, self.node.id) as task: + manager.do_attach_virtual_media(task, boot_devices.CDROM, + "https://url", "local") + mock_prepare_image.assert_called_once_with( + task, "https://url", file_name="cdrom.iso", + download_source="local") + mock_attach.assert_called_once_with( + task.driver.management, task, device_type=boot_devices.CDROM, + image_url=mock_prepare_image.return_value) + self.node.refresh() + self.assertIn("Could not attach device cdrom", self.node.last_error) + self.assertIn("disabled or not implemented", self.node.last_error) diff --git a/releasenotes/notes/vmedia-error-ef4eac3d08761d5c.yaml b/releasenotes/notes/vmedia-error-ef4eac3d08761d5c.yaml new file mode 100644 index 0000000000..90b409a143 --- /dev/null +++ b/releasenotes/notes/vmedia-error-ef4eac3d08761d5c.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes error handling in the virtual media attachment API when the image + downloading fails. Now the ``last_error`` field is populated correctly + and the error is logged.