openstack/ironic
Code Issues Proposed changes

Merge "Fix generating local paths when connecting virtual media"

Browse Source
This commit is contained in:
Zuul 2024-04-05 17:44:09 +00:00 committed by Gerrit Code Review
commit 243e14359a
4 changed files with 35 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
ironic/conductor/manager.py
View File

@ -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)

2
ironic/drivers/modules/image_utils.py
View File

@ -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.

19
ironic/tests/unit/conductor/test_manager.py
View File

@ -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,

6
releasenotes/notes/vmedia-path-648cfa258708e0bb.yaml Normal file
View File

@ -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.