Merge "Add a delay/retry is vmedia insert fails"

4 files changed, 59 insertions, 2 deletions

diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py
index e145b1925..7305dace0 100644
--- a/ironic/drivers/modules/redfish/boot.py
+++ b/ironic/drivers/modules/redfish/boot.py
@@ -15,6 +15,7 @@
 
 from oslo_log import log
 from oslo_utils import importutils
+import retrying
 
 from ironic.common import boot_devices
 from ironic.common import exception
@@ -141,6 +142,19 @@ def _parse_instance_info(node):
     return deploy_info
 
 
+def _test_retry(exception):
+    if isinstance(exception, sushy.exceptions.ServerSideError):
+        # On some Dell hw, the eject media may still be in progress
+        # https://storyboard.openstack.org/#!/story/2008504
+        LOG.warning("Boot media insert failed for node %(node)s, "
+                    "will retry after 3 seconds",
+                    {'node': exception.node_uuid})
+        return True
+    return False
+
+
+@retrying.retry(wait_fixed=3000, stop_max_attempt_number=3,
+                retry_on_exception=_test_retry)
 def _insert_vmedia(task, boot_url, boot_device):
     """Insert bootable ISO image into virtual CD or DVD
 
@@ -169,8 +183,12 @@ def _insert_vmedia(task, boot_url, boot_device):
 
                 continue
 
-            v_media.insert_media(boot_url, inserted=True,
-                                 write_protected=True)
+            try:
+                v_media.insert_media(boot_url, inserted=True,
+                                     write_protected=True)
+            except sushy.exceptions.ServerSideError as e:
+                e.node_uuid = task.node.uuid
+                raise
 
             LOG.info("Inserted boot media %(boot_url)s into "
                      "%(boot_device)s for node "
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
index 110c1b7f6..c632d8615 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
@@ -846,6 +846,36 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
             self.assertFalse(mock_vmedia_cd.insert_media.call_count)
 
     @mock.patch.object(redfish_boot, 'redfish_utils', autospec=True)
+    @mock.patch('time.sleep', lambda *args, **kwargs: None)
+    def test__insert_vmedia_while_ejecting(self, mock_redfish_utils):
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            mock_vmedia_cd = mock.MagicMock(
+                inserted=False,
+                image='img-url',
+                media_types=[sushy.VIRTUAL_MEDIA_CD],
+            )
+            mock_manager = mock.MagicMock()
+
+            def clear_and_raise(*args, **kwargs):
+                mock_vmedia_cd.insert_media.side_effect = None
+                raise sushy.exceptions.ServerSideError(
+                    "POST", 'img-url', mock.MagicMock())
+            mock_vmedia_cd.insert_media.side_effect = clear_and_raise
+            mock_manager.virtual_media.get_members.return_value = [
+                mock_vmedia_cd]
+
+            mock_redfish_utils.get_system.return_value.managers = [
+                mock_manager]
+
+            redfish_boot._insert_vmedia(
+                task, 'img-url', sushy.VIRTUAL_MEDIA_CD)
+
+            self.assertEqual(mock_vmedia_cd.insert_media.call_count, 2)
+
+    @mock.patch.object(redfish_boot, 'redfish_utils', autospec=True)
+    @mock.patch('time.sleep', lambda *args, **kwargs: None)
     def test__insert_vmedia_bad_device(self, mock_redfish_utils):
 
         with task_manager.acquire(self.context, self.node.uuid,
@@ -865,6 +895,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
                 exception.InvalidParameterValue,
                 redfish_boot._insert_vmedia,
                 task, 'img-url', sushy.VIRTUAL_MEDIA_CD)
+            self.assertEqual(mock_redfish_utils.get_system.call_count, 1)
 
     @mock.patch.object(redfish_boot, 'redfish_utils', autospec=True)
     def test_eject_vmedia_everything(self, mock_redfish_utils):
diff --git a/ironic/tests/unit/drivers/third_party_driver_mocks.py b/ironic/tests/unit/drivers/third_party_driver_mocks.py
index 2d672a248..34eb41615 100644
--- a/ironic/tests/unit/drivers/third_party_driver_mocks.py
+++ b/ironic/tests/unit/drivers/third_party_driver_mocks.py
@@ -236,6 +236,8 @@ if not sushy:
         type('MissingAttributeError', (sushy.exceptions.SushyError,), {}))
     sushy.exceptions.OEMExtensionNotFoundError = (
         type('OEMExtensionNotFoundError', (sushy.exceptions.SushyError,), {}))
+    sushy.exceptions.ServerSideError = (
+        type('ServerSideError', (sushy.exceptions.SushyError,), {}))
     sushy.auth = mock.MagicMock(spec_set=mock_specs.SUSHY_AUTH_SPEC)
     sys.modules['sushy.auth'] = sushy.auth
 
diff --git a/releasenotes/notes/retry-vmedia-1999742c84f11103.yaml b/releasenotes/notes/retry-vmedia-1999742c84f11103.yaml
new file mode 100644
index 000000000..d678c2b9b
--- /dev/null
+++ b/releasenotes/notes/retry-vmedia-1999742c84f11103.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Retries virtual media insert on failure to allow
+    for an eject that may not have finished.
+    https://storyboard.openstack.org/#!/story/2008504