From da27e370b6fdf12330b3ccdc4404c56896862939 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 7 Jul 2021 15:09:46 +0200 Subject: [PATCH] Clean up images when ejecting an ISO with Redfish There is no point in keeping the disconnected ISO files around. Change-Id: I371b0f6ab74de7aa26ecc4a65e825303e3d94085 --- ironic/drivers/modules/redfish/boot.py | 14 +++++- .../unit/drivers/modules/redfish/test_boot.py | 15 +++++- .../drivers/modules/redfish/test_vendor.py | 49 +++---------------- .../redfish-eject-iso-9875388ae09bc8f6.yaml | 6 +++ 4 files changed, 40 insertions(+), 44 deletions(-) create mode 100644 releasenotes/notes/redfish-eject-iso-9875388ae09bc8f6.yaml diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index f329b06d49..dfc0d5d38b 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -221,9 +221,11 @@ def _eject_vmedia(task, managers, boot_device=None): :param boot_device: sushy boot device e.g. `VIRTUAL_MEDIA_CD`, `VIRTUAL_MEDIA_DVD` or `VIRTUAL_MEDIA_FLOPPY` or `None` to eject everything (default). + :return: True if any device was ejected, else False :raises: InvalidParameterValue, if no suitable virtual CD or DVD is found on the node. """ + found = False for manager in managers: for v_media in manager.virtual_media.get_members(): if boot_device and boot_device not in v_media.media_types: @@ -233,12 +235,14 @@ def _eject_vmedia(task, managers, boot_device=None): if inserted: v_media.eject_media() + found = True LOG.info("Boot media is%(already)s ejected from " "%(boot_device)s for node %(node)s" "", {'node': task.node.uuid, 'already': '' if inserted else ' already', 'boot_device': v_media.name}) + return found def eject_vmedia(task, boot_device=None): @@ -252,7 +256,15 @@ def eject_vmedia(task, boot_device=None): found on the node. """ system = redfish_utils.get_system(task.node) - _eject_vmedia(task, system.managers, boot_device=boot_device) + if _eject_vmedia(task, system.managers, boot_device=boot_device): + LOG.debug('Cleaning up unused files after ejecting %(dev)s for node ' + '%(node)s', {'dev': boot_device or 'all devices', + 'node': task.node.uuid}) + if (boot_device is None + or boot_device == sushy.VIRTUAL_MEDIA_USBSTICK): + image_utils.cleanup_disk_image(task, prefix='configdrive') + if boot_device is None or boot_device == sushy.VIRTUAL_MEDIA_CD: + image_utils.cleanup_iso_image(task) def _has_vmedia_device(managers, boot_device, inserted=None): diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index 3ffdd8ff27..e69c29dcad 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -1277,8 +1277,11 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): redfish_boot._insert_vmedia, task, [mock_manager], 'img-url', sushy.VIRTUAL_MEDIA_CD) + @mock.patch.object(image_utils, 'cleanup_disk_image', autospec=True) + @mock.patch.object(image_utils, 'cleanup_iso_image', autospec=True) @mock.patch.object(redfish_boot, 'redfish_utils', autospec=True) - def test_eject_vmedia_everything(self, mock_redfish_utils): + def test_eject_vmedia_everything(self, mock_redfish_utils, + mock_cleanup_iso, mock_cleanup_disk): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -1301,9 +1304,15 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_vmedia_cd.eject_media.assert_called_once_with() mock_vmedia_floppy.eject_media.assert_called_once_with() + mock_cleanup_iso.assert_called_once_with(task) + mock_cleanup_disk.assert_called_once_with(task, + prefix='configdrive') + @mock.patch.object(image_utils, 'cleanup_disk_image', autospec=True) + @mock.patch.object(image_utils, 'cleanup_iso_image', autospec=True) @mock.patch.object(redfish_boot, 'redfish_utils', autospec=True) - def test_eject_vmedia_specific(self, mock_redfish_utils): + def test_eject_vmedia_specific(self, mock_redfish_utils, + mock_cleanup_iso, mock_cleanup_disk): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -1326,6 +1335,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_vmedia_cd.eject_media.assert_called_once_with() self.assertFalse(mock_vmedia_floppy.eject_media.call_count) + mock_cleanup_iso.assert_called_once_with(task) + mock_cleanup_disk.assert_not_called() @mock.patch.object(redfish_boot, 'redfish_utils', autospec=True) def test_eject_vmedia_not_inserted(self, mock_redfish_utils): diff --git a/ironic/tests/unit/drivers/modules/redfish/test_vendor.py b/ironic/tests/unit/drivers/modules/redfish/test_vendor.py index 156b3defbc..0ca487c1e8 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_vendor.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_vendor.py @@ -42,59 +42,26 @@ class RedfishVendorPassthruTestCase(db_base.DbTestCase): self.node = obj_utils.create_test_node( self.context, driver='redfish', driver_info=INFO_DICT) - @mock.patch.object(redfish_boot, 'redfish_utils', autospec=True) - def test_eject_vmedia_all(self, mock_redfish_utils): + @mock.patch.object(redfish_boot, 'eject_vmedia', autospec=True) + def test_eject_vmedia_all(self, mock_eject_vmedia): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - mock_vmedia_cd = mock.MagicMock( - inserted=True, - media_types=[sushy.VIRTUAL_MEDIA_CD]) - mock_vmedia_floppy = mock.MagicMock( - inserted=True, - media_types=[sushy.VIRTUAL_MEDIA_FLOPPY]) - - mock_manager = mock.MagicMock() - - mock_manager.virtual_media.get_members.return_value = [ - mock_vmedia_cd, mock_vmedia_floppy] - - mock_redfish_utils.get_system.return_value.managers = [ - mock_manager] - task.driver.vendor.eject_vmedia(task) + mock_eject_vmedia.assert_called_once_with(task, None) - mock_vmedia_cd.eject_media.assert_called_once_with() - mock_vmedia_floppy.eject_media.assert_called_once_with() - - @mock.patch.object(redfish_boot, 'redfish_utils', autospec=True) - def test_eject_vmedia_cd(self, mock_redfish_utils): + @mock.patch.object(redfish_boot, 'eject_vmedia', autospec=True) + def test_eject_vmedia_cd(self, mock_eject_vmedia): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - mock_vmedia_cd = mock.MagicMock( - inserted=True, - media_types=[sushy.VIRTUAL_MEDIA_CD]) - mock_vmedia_floppy = mock.MagicMock( - inserted=True, - media_types=[sushy.VIRTUAL_MEDIA_FLOPPY]) - - mock_manager = mock.MagicMock() - - mock_manager.virtual_media.get_members.return_value = [ - mock_vmedia_cd, mock_vmedia_floppy] - - mock_redfish_utils.get_system.return_value.managers = [ - mock_manager] - task.driver.vendor.eject_vmedia(task, boot_device=sushy.VIRTUAL_MEDIA_CD) - - mock_vmedia_cd.eject_media.assert_called_once_with() - mock_vmedia_floppy.eject_media.assert_not_called() + mock_eject_vmedia.assert_called_once_with(task, + sushy.VIRTUAL_MEDIA_CD) @mock.patch.object(redfish_vendor, 'redfish_utils', autospec=True) - def test_eject_vmedia_invalid_dev(self, mock_redfish_utils): + def test_validate_invalid_dev(self, mock_redfish_utils): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: diff --git a/releasenotes/notes/redfish-eject-iso-9875388ae09bc8f6.yaml b/releasenotes/notes/redfish-eject-iso-9875388ae09bc8f6.yaml new file mode 100644 index 0000000000..0b83ddbee5 --- /dev/null +++ b/releasenotes/notes/redfish-eject-iso-9875388ae09bc8f6.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Removes unused local images after ejecting a virtual media device via + the ``eject_vmedia`` vendor passthru call of the ``redfish`` vendor + interface.