From 766d2804a1743e238a37762c946dec0984721167 Mon Sep 17 00:00:00 2001 From: Iury Gregory Melo Ferreira Date: Wed, 11 Oct 2023 13:51:02 -0300 Subject: [PATCH] Make sure we eject media from DVD when CD is requested It's possible to use virtual media based provisioning on servers that only support DVD MediaTypes and do not support CD MediaTypes. The problem in this scenario is that Ironic will keep the media attached since it will only eject the ones matching the CD device, now we check if there is any DVD device with media inserted when looking for CD devices. Closes-Bug: 2039042 Change-Id: I7a5e871133300fea8a77ad5bfd9a0b045c24c201 --- ironic/drivers/modules/redfish/boot.py | 19 +++++++- .../unit/drivers/modules/redfish/test_boot.py | 44 ++++++++++++++++++- .../fix-eject-media-dvd-b1994446ea71be9c.yaml | 8 ++++ 3 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/fix-eject-media-dvd-b1994446ea71be9c.yaml diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index a169efd4a5..8b16be9f1a 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -266,8 +266,23 @@ def _eject_vmedia(task, managers, boot_device=None): 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: - continue - + # NOTE(iurygregory): this conditional allows v_media that only + # support DVD MediaType and NOT CD to also be used. + # if v_media.media_types contains sushy.VIRTUAL_MEDIA_DVD + # we follow the usual steps of checking if v_media is inserted + # and eject it. Otherwise we skip to the + # next v_media device, if any. + # This is needed to add support to Cisco UCSB and UCSX blades + # reference: https://bugs.launchpad.net/ironic/+bug/2039042 + if (boot_device == sushy.VIRTUAL_MEDIA_CD + and sushy.VIRTUAL_MEDIA_DVD in v_media.media_types): + LOG.debug('While looking for %(requested_device)s virtual ' + 'media device, found %(available_device)s ' + 'instead. Attempting to use it to eject media.', + {'requested_device': sushy.VIRTUAL_MEDIA_CD, + 'available_device': sushy.VIRTUAL_MEDIA_DVD}) + else: + continue inserted = v_media.inserted if inserted: diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index 9e4b9c8b72..5d873b27a9 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -1493,11 +1493,14 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_vmedia_floppy = mock.MagicMock( inserted=True, media_types=[sushy.VIRTUAL_MEDIA_FLOPPY]) + mock_vmedia_dvd = mock.MagicMock( + inserted=True, + media_types=[sushy.VIRTUAL_MEDIA_DVD]) mock_manager = mock.MagicMock() mock_manager.virtual_media.get_members.return_value = [ - mock_vmedia_cd, mock_vmedia_floppy] + mock_vmedia_cd, mock_vmedia_floppy, mock_vmedia_dvd] mock_redfish_utils.get_system.return_value.managers = [ mock_manager] @@ -1506,6 +1509,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_vmedia_cd.eject_media.assert_called_once_with() mock_vmedia_floppy.eject_media.assert_called_once_with() + mock_vmedia_dvd.eject_media.assert_called_once_with() mock_cleanup_iso.assert_called_once_with(task) mock_cleanup_disk.assert_called_once_with(task, prefix='configdrive') @@ -1540,6 +1544,44 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_cleanup_iso.assert_called_once_with(task) mock_cleanup_disk.assert_not_called() + @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) + @mock.patch.object(redfish_boot.LOG, 'debug', autospec=True) + @mock.patch.object(redfish_boot.LOG, 'info', autospec=True) + def test_eject_vmedia_with_dvd_cisco_ucs(self, mock_log_info, + mock_log_debug, + mock_redfish_utils, + mock_cleanup_iso, + mock_cleanup_disk): + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + mock_vmedia_dvd_1 = mock.MagicMock( + inserted=True, + media_types=[sushy.VIRTUAL_MEDIA_DVD]) + mock_vmedia_dvd_2 = mock.MagicMock( + inserted=True, + media_types=[sushy.VIRTUAL_MEDIA_DVD]) + + mock_manager = mock.MagicMock() + + mock_manager.virtual_media.get_members.return_value = [ + mock_vmedia_dvd_1, mock_vmedia_dvd_2] + + mock_redfish_utils.get_system.return_value.managers = [ + mock_manager] + + redfish_boot.eject_vmedia(task, sushy.VIRTUAL_MEDIA_CD) + + mock_vmedia_dvd_1.eject_media.assert_called_once_with() + mock_vmedia_dvd_2.eject_media.assert_called_once_with() + + self.assertEqual(mock_log_info.call_count, 2) + self.assertEqual(mock_log_debug.call_count, 3) + 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/releasenotes/notes/fix-eject-media-dvd-b1994446ea71be9c.yaml b/releasenotes/notes/fix-eject-media-dvd-b1994446ea71be9c.yaml new file mode 100644 index 0000000000..d9ef10117e --- /dev/null +++ b/releasenotes/notes/fix-eject-media-dvd-b1994446ea71be9c.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Properly eject the virtual media from a DVD device in case this is the + only MediaType available from the Hardware, and Ironic requested CD as + the device to be used. + See `bug 2039042 `_ + for details.