From e33289816409f712c66d9b98755b3f3658121569 Mon Sep 17 00:00:00 2001 From: Jacob Anders Date: Tue, 18 Jul 2023 11:17:15 +1000 Subject: [PATCH] Retry connecting vmedia through a DVD device if available Currently, it is not possible to use virtual media based provisioning on servers that only support DVD MediaTypes and do not support CD MediaTypes. This change adds support for DVD-only virtual media. In addition to this, it adds hanling of BadRequestException on attempt to insert virtual media, so that instead of giving up on the first failure encountered, the code continues to iterate through the list of other virtual media devices available on the system and attempts to insert media into the next suitable device till it either suceeds or runs out of devices to try. Change-Id: I0bb0ad5df613df86cd8a7686f9c32e902826cd20 Closes-Bug: #2031595 --- ironic/drivers/modules/redfish/boot.py | 41 ++++++++++++++-- .../unit/drivers/modules/redfish/test_boot.py | 47 +++++++++++++++++++ ...ndle-dvd-only-vmedia-f4971a013a8aafd0.yaml | 8 ++++ 3 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/handle-dvd-only-vmedia-f4971a013a8aafd0.yaml diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index a321c08ec5..a169efd4a5 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -183,10 +183,27 @@ def _insert_vmedia(task, managers, boot_url, boot_device): :raises: InvalidParameterValue, if no suitable virtual CD or DVD is found on the node. """ + err_msg = None for manager in managers: for v_media in manager.virtual_media.get_members(): if boot_device not in v_media.media_types: - continue + # NOTE(janders): 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 if not, attempt to insert 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/2031595 + 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 configure it.", + {'requested_device': sushy.VIRTUAL_MEDIA_CD, + 'available_device': sushy.VIRTUAL_MEDIA_DVD}) + else: + continue if v_media.inserted: if v_media.image == boot_url: @@ -202,6 +219,19 @@ def _insert_vmedia(task, managers, boot_url, boot_device): try: v_media.insert_media(boot_url, inserted=True, write_protected=True) + # NOTE(janders): On Cisco UCSB and UCSX blades there are several + # vMedia devices. Some of those are only meant for internal use + # by CIMC vKVM - attempts to InsertMedia into those will result + # in BadRequestError. We catch the exception here so that we don't + # fail out and try the next available device instead, if available. + except sushy.exceptions.BadRequestError: + err_msg = ("Inserting virtual media into %(boot_device)s " + "failed for node %(node)s, moving to next virtual " + "media device, if available", + {'node': task.node.uuid, + 'boot_device': boot_device}) + LOG.warning(err_msg) + continue except sushy.exceptions.ServerSideError as e: e.node_uuid = task.node.uuid raise @@ -212,9 +242,12 @@ def _insert_vmedia(task, managers, boot_url, boot_device): 'boot_url': boot_url, 'boot_device': boot_device}) return - - raise exception.InvalidParameterValue( - _('No suitable virtual media device found')) + if (err_msg is not None): + exc_msg = ("All virtual media mount attempts failed. " + "Most recent error: ", err_msg) + else: + exc_msg = 'No suitable virtual media device found' + raise exception.InvalidParameterValue(exc_msg) def _eject_vmedia(task, managers, boot_device=None): diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index d74f98bf2e..fe2def6f70 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -1251,6 +1251,53 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): self.assertFalse(mock_vmedia_floppy.insert_media.call_count) + def test__insert_vmedia_anew_dvd(self): + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + mock_vmedia_dvd = mock.MagicMock( + inserted=False, + media_types=[sushy.VIRTUAL_MEDIA_DVD]) + + mock_manager = mock.MagicMock() + + mock_manager.virtual_media.get_members.return_value = [ + mock_vmedia_dvd] + + redfish_boot._insert_vmedia( + task, [mock_manager], 'img-url', sushy.VIRTUAL_MEDIA_CD) + + mock_vmedia_dvd.insert_media.assert_called_once_with( + 'img-url', inserted=True, write_protected=True) + + @mock.patch('time.sleep', lambda *args, **kwargs: None) + def test__insert_vmedia_anew_dvd_retry(self): + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + mock_vmedia_dvd_1 = mock.MagicMock( + inserted=False, + media_types=[sushy.VIRTUAL_MEDIA_DVD]) + + mock_vmedia_dvd_2 = mock.MagicMock( + inserted=False, + media_types=[sushy.VIRTUAL_MEDIA_DVD]) + + mock_manager = mock.MagicMock() + + def clear_and_raise(*args, **kwargs): + mock_vmedia_dvd_1.insert_media.side_effect = None + raise sushy.exceptions.BadRequestError( + "POST", 'img-url', mock.MagicMock()) + mock_vmedia_dvd_1.insert_media.side_effect = clear_and_raise + mock_manager.virtual_media.get_members.return_value = [ + mock_vmedia_dvd_1, mock_vmedia_dvd_2] + + redfish_boot._insert_vmedia( + task, [mock_manager], 'img-url', sushy.VIRTUAL_MEDIA_CD) + + self.assertEqual(mock_vmedia_dvd_2.insert_media.call_count, 1) + def test__insert_vmedia_already_inserted(self): with task_manager.acquire(self.context, self.node.uuid, diff --git a/releasenotes/notes/handle-dvd-only-vmedia-f4971a013a8aafd0.yaml b/releasenotes/notes/handle-dvd-only-vmedia-f4971a013a8aafd0.yaml new file mode 100644 index 0000000000..af1d87366b --- /dev/null +++ b/releasenotes/notes/handle-dvd-only-vmedia-f4971a013a8aafd0.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes the bug where provisioning a Redfish managed node fails if the BMC + only supports virtual media devices limited to MediaType of DVD (and not + CD). Also ddds handling of BadRequest exceptions while iterating through + the list of virtual media devices. This fix is needed to successfully + provision machines such as Cisco UCSB and UCSX.