openstack
/
ironic
Code Issues Proposed changes

Add `filename` parameter to Redfish virtual media boot URL 

Some BMCs seem to validate virtual media boot image URL requiring
that URL to end with something resembling ISO image file name
(perhaps, its suffix i.e. extension).

This patch tries to add, hopefully, meaningless `filename`
parameter to boot URL's query string in hope to make the entire boot
image URL looking more convincing to the BMC.

Change-Id: I316712b38d10c0e801f7fd96a584074c8918b46b
Story: 1526753
Task: 10389
This commit is contained in:
Ilya Etingof 2019-07-16 16:44:25 +02:00
parent b73420ec53
commit 046399ee63
2 changed files with 70 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
ironic/drivers/modules/redfish/boot.py
View File

@ -16,6 +16,7 @@ import tempfile
from oslo_log import log
from oslo_utils import importutils
from six.moves.urllib import parse as urlparse
from ironic.common import boot_devices
from ironic.common import exception
@ -218,6 +219,36 @@ class RedfishVirtualMediaBoot(base.BootInterface):
{'node': task.node.uuid, 'image': object_name,
'error': e})
@staticmethod
def _append_filename_param(url, filename):
"""Append 'filename=<file>' parameter to given URL.
Some BMCs seem to validate boot image URL requiring the URL to end
with something resembling ISO image file name.
This function tries to add, hopefully, meaningless 'filename'
parameter to URL's query string in hope to make the entire boot image
URL looking more convincing to the BMC.
However, `url` with fragments might not get cured by this hack.
:param url: a URL to work on
:param filename: name of the file to append to the URL
:returns: original URL with 'filename' parameter appended
"""
parsed_url = urlparse.urlparse(url)
parsed_qs = urlparse.parse_qsl(parsed_url.query)
has_filename = [x for x in parsed_qs if x[0].lower() == 'filename']
if has_filename:
return url
parsed_qs.append(('filename', filename))
parsed_url = list(parsed_url)
parsed_url[4] = urlparse.urlencode(parsed_qs)
return urlparse.urlunparse(parsed_url)
@staticmethod
def _get_floppy_image_name(node):
"""Returns the floppy image name for a given node.
@ -279,6 +310,8 @@ class RedfishVirtualMediaBoot(base.BootInterface):
image_url = swift_api.get_temp_url(container, object_name, timeout)
image_url = cls._append_filename_param(image_url, 'bootme.img')
LOG.debug("Created floppy image %(name)s in Swift for node %(node)s, "
"exposed as temporary URL "
"%(url)s", {'node': task.node.uuid,
@ -388,6 +421,9 @@ class RedfishVirtualMediaBoot(base.BootInterface):
boot_iso_url = swift_api.get_temp_url(
container, iso_object_name, timeout)
boot_iso_url = cls._append_filename_param(
boot_iso_url, 'bootme.iso')
LOG.debug("Created ISO %(name)s in Swift for node %(node)s, exposed "
"as temporary URL %(url)s", {'node': task.node.uuid,
'name': iso_object_name,

40
ironic/tests/unit/drivers/modules/redfish/test_boot.py
View File

@ -175,6 +175,30 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
task.driver.boot._parse_deploy_info,
task.node)
def test__append_filename_param_without_qs(self):
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
res = task.driver.boot._append_filename_param(
'http://a.b/c', 'b.img')
expected = 'http://a.b/c?filename=b.img'
self.assertEqual(expected, res)
def test__append_filename_param_with_qs(self):
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
res = task.driver.boot._append_filename_param(
'http://a.b/c?d=e&f=g', 'b.img')
expected = 'http://a.b/c?d=e&f=g&filename=b.img'
self.assertEqual(expected, res)
def test__append_filename_param_with_filename(self):
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
res = task.driver.boot._append_filename_param(
'http://a.b/c?filename=bootme.img', 'b.img')
expected = 'http://a.b/c?filename=bootme.img'
self.assertEqual(expected, res)
@mock.patch.object(redfish_boot, 'swift', autospec=True)
def test__cleanup_floppy_image(self, mock_swift):
with task_manager.acquire(self.context, self.node.uuid,
@ -193,14 +217,18 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
def test__prepare_floppy_image(self, mock_create_vfat_image, mock_swift):
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
task.driver.boot._prepare_floppy_image(task)
mock_swift_api = mock_swift.SwiftAPI.return_value
mock_swift_api.get_temp_url.return_value = 'https://a.b/c.f?e=f'
url = task.driver.boot._prepare_floppy_image(task)
self.assertIn('filename=bootme.img', url)
mock_swift.SwiftAPI.assert_called_once_with()
mock_create_vfat_image.assert_called_once_with(
mock.ANY, parameters=mock.ANY)
mock_swift.SwiftAPI.assert_called_once_with()
mock_swift_api = mock_swift.SwiftAPI.return_value
mock_swift_api.create_object.assert_called_once_with(
mock.ANY, mock.ANY, mock.ANY, mock.ANY)
@ -234,7 +262,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
task, 'http://kernel/img', 'http://ramdisk/img',
'http://bootloader/img', root_uuid=task.node.uuid)
self.assertTrue(url)
self.assertIn('filename=bootme.iso', url)
mock_create_boot_iso.assert_called_once_with(
mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img',
@ -263,7 +291,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
task, 'http://kernel/img', 'http://ramdisk/img',
bootloader_href=None, root_uuid=task.node.uuid)
self.assertTrue(url)
self.assertIn('filename=bootme.iso', url)
mock_create_boot_iso.assert_called_once_with(
mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img',