Fix generating local paths when connecting virtual media

The generate path does not contain the node UUID, causing conflicts.

Also make sure to always clean up any existing files first.

Change-Id: I30f948d64e7b87f33841dc22828db60338a62dd8

ironic/conductor/manager.py

@@ -4081,6 +4081,14 @@ def do_sync_power_state(task, count):
     return count
 
 
+def _virtual_media_file_name(task, device_type):
+    return "%s-%s.%s" % (
+        device_type.lower(),
+        task.node.uuid,
+        'iso' if device_type == boot_devices.CDROM else 'img'
+    )
+
+
 def do_attach_virtual_media(task, device_type, image_url,
                             image_download_source):
     success_msg = "Device %(device_type)s attached to node %(node)s",
@@ -4088,10 +4096,8 @@ def do_attach_virtual_media(task, device_type, image_url,
                  "to node %(node)s: %(exc)s")
     try:
         assert device_type in boot_devices.VMEDIA_DEVICES
-        file_name = "%s.%s" % (
+        file_name = _virtual_media_file_name(task, device_type)
-            device_type.lower(),
+        image_utils.cleanup_remote_image(task, file_name)
-            'iso' if device_type == boot_devices.CDROM else 'img'
-        )
         image_url = image_utils.prepare_remote_image(
             task, image_url, file_name=file_name,
             download_source=image_download_source)
@@ -4119,6 +4125,5 @@ def do_detach_virtual_media(task, device_types):
                           "%(device_types)s from node %(node)s: %(exc)s",
                           device_types=device_types)
     for device_type in device_types:
-        suffix = '.iso' if device_type == boot_devices.CDROM else '.img'
+        file_name = _virtual_media_file_name(task, device_type)
-        image_utils.ImageHandler.unpublish_image_for_node(
+        image_utils.cleanup_remote_image(task, file_name)
-            task.node, prefix=device_type.lower(), suffix=suffix)

ironic/drivers/modules/image_utils.py

@@ -326,6 +326,8 @@ def cleanup_disk_image(task, prefix=None):
     ImageHandler.unpublish_image_for_node(task.node, prefix=prefix)
 
 
+# FIXME(dtantsur): file_name is not node-specific, we should probably replace
+# it with a prefix/suffix pair and pass to _get_name
 def prepare_remote_image(task, image_url, file_name='boot.iso',
                          download_source='local', cache=None):
     """Generic function for publishing remote images.

ironic/tests/unit/conductor/test_manager.py

@@ -8743,30 +8743,38 @@ class VirtualMediaTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
 
     @mock.patch.object(redfish.management.RedfishManagement,
                        'attach_virtual_media', autospec=True)
+    @mock.patch.object(image_utils, 'cleanup_remote_image', autospec=True)
     @mock.patch.object(image_utils, 'prepare_remote_image', autospec=True)
-    def test_do_attach_virtual_media(self, mock_prepare_image, mock_attach):
+    def test_do_attach_virtual_media(self, mock_prepare_image,
+                                     mock_cleanup_image, mock_attach):
         with task_manager.acquire(self.context, self.node.id) as task:
             manager.do_attach_virtual_media(task, boot_devices.CDROM,
                                             "https://url", "local")
+            file_name = f"cdrom-{self.node.uuid}.iso"
             mock_prepare_image.assert_called_once_with(
-                task, "https://url", file_name="cdrom.iso",
+                task, "https://url", file_name=file_name,
                 download_source="local")
+            mock_cleanup_image.assert_called_once_with(task, file_name)
             mock_attach.assert_called_once_with(
                 task.driver.management, task, device_type=boot_devices.CDROM,
                 image_url=mock_prepare_image.return_value)
 
     @mock.patch.object(redfish.management.RedfishManagement,
                        'attach_virtual_media', autospec=True)
+    @mock.patch.object(image_utils, 'cleanup_remote_image', autospec=True)
     @mock.patch.object(image_utils, 'prepare_remote_image', autospec=True)
     def test_do_attach_virtual_media_fails_on_prepare(self, mock_prepare_image,
+                                                      mock_cleanup_image,
                                                       mock_attach):
         mock_prepare_image.side_effect = exception.InvalidImageRef
         with task_manager.acquire(self.context, self.node.id) as task:
             manager.do_attach_virtual_media(task, boot_devices.CDROM,
                                             "https://url", "local")
+            file_name = f"cdrom-{self.node.uuid}.iso"
             mock_prepare_image.assert_called_once_with(
-                task, "https://url", file_name="cdrom.iso",
+                task, "https://url", file_name=file_name,
                 download_source="local")
+            mock_cleanup_image.assert_called_once_with(task, file_name)
             mock_attach.assert_not_called()
         self.node.refresh()
         self.assertIn("Could not attach device cdrom", self.node.last_error)
@@ -8774,15 +8782,18 @@ class VirtualMediaTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
 
     @mock.patch.object(redfish.management.RedfishManagement,
                        'attach_virtual_media', autospec=True)
+    @mock.patch.object(image_utils, 'cleanup_remote_image', autospec=True)
     @mock.patch.object(image_utils, 'prepare_remote_image', autospec=True)
     def test_do_attach_virtual_media_fails_on_attach(self, mock_prepare_image,
+                                                     mock_cleanup_image,
                                                      mock_attach):
         mock_attach.side_effect = exception.UnsupportedDriverExtension
         with task_manager.acquire(self.context, self.node.id) as task:
             manager.do_attach_virtual_media(task, boot_devices.CDROM,
                                             "https://url", "local")
+            file_name = f"cdrom-{self.node.uuid}.iso"
             mock_prepare_image.assert_called_once_with(
-                task, "https://url", file_name="cdrom.iso",
+                task, "https://url", file_name=file_name,
                 download_source="local")
             mock_attach.assert_called_once_with(
                 task.driver.management, task, device_type=boot_devices.CDROM,

releasenotes/notes/vmedia-path-648cfa258708e0bb.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes generated URL when using the virtual media attachment API.
+    Previously, it missed the node UUID, causing conflicts between different
+    nodes.