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

.gitreview

@@ -2,3 +2,4 @@
 host=review.opendev.org
 port=29418
 project=openstack/ironic.git
+defaultbranch=bugfix/22.1

ironic/common/exception.py

@@ -857,7 +857,7 @@ class ImageRefIsARedirect(IronicException):
             redirect_url=redirect_url)
 
 
-class ConcurrentActionLimit(IronicException):
+class ConcurrentActionLimit(TemporaryFailure):
     # NOTE(TheJulia): We explicitly don't report the concurrent
     # action limit configuration value as a security guard since
     # if informed of the limit, an attacker can tailor their attack.

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:

ironic/tests/unit/drivers/modules/redfish/test_boot.py

@@ -1373,11 +1373,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]
@@ -1386,6 +1389,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')
@@ -1420,6 +1424,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):
 

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 <https://bugs.launchpad.net/ironic/+bug/2039042>`_
+    for details.

releasenotes/notes/max_concurrent_deploy-7a31ba142bf5ad5c.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    When Ironic hits the limit on the number of the concurrent deploys
+    (specified in the ``[conductor]max_concurrent_deploy`` option), the
+    resulting HTTP code is now 503 instead of the more generic 500.