diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 57c97a30a5..506454173c 100644 --- a/ironic/conductor/manager.py +++ b/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" % ( - device_type.lower(), - 'iso' if device_type == boot_devices.CDROM else 'img' - ) + file_name = _virtual_media_file_name(task, device_type) + image_utils.cleanup_remote_image(task, file_name) 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' - image_utils.ImageHandler.unpublish_image_for_node( - task.node, prefix=device_type.lower(), suffix=suffix) + file_name = _virtual_media_file_name(task, device_type) + image_utils.cleanup_remote_image(task, file_name) diff --git a/ironic/drivers/modules/image_utils.py b/ironic/drivers/modules/image_utils.py index 5ce4303514..021cad8508 100644 --- a/ironic/drivers/modules/image_utils.py +++ b/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. diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index db2dbc9d30..2917ef8e0d 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/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, diff --git a/releasenotes/notes/vmedia-path-648cfa258708e0bb.yaml b/releasenotes/notes/vmedia-path-648cfa258708e0bb.yaml new file mode 100644 index 0000000000..f4a6124f1e --- /dev/null +++ b/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.