Clean up images when ejecting an ISO with Redfish

There is no point in keeping the disconnected ISO files around.

Change-Id: I371b0f6ab74de7aa26ecc4a65e825303e3d94085

4 files changed, 40 insertions, 44 deletions

diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py
index f329b06d4..dfc0d5d38 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 3ffdd8ff2..e69c29dca 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 156b3defb..0ca487c1e 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 000000000..0b83ddbee
--- /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.