From 199a218f5c2436ce822da8f4d2719f3d012cfb34 Mon Sep 17 00:00:00 2001 From: Ilya Etingof Date: Tue, 17 Sep 2019 19:13:50 +0200 Subject: [PATCH] Serve virtual media boot images from ironic conductor Add the option to ironic to place bootable ISO images into local HTTP server's document root so that BMC could boot from there. This is an alternative to Swift-based boot controllable via the ``[redfish]use_swift`` ironic configuration option. Change-Id: I390fbb3a27f1b7d0170caff84f54d4ca40e6bcb2 Story: 2006569 Task: 36674 --- ironic/conf/redfish.py | 13 +- ironic/drivers/modules/redfish/boot.py | 154 ++++++++++----- .../unit/drivers/modules/redfish/test_boot.py | 186 +++++++++++++----- ...dfish-boot-interface-e7e05bdd2c894d80.yaml | 5 +- 4 files changed, 253 insertions(+), 105 deletions(-) diff --git a/ironic/conf/redfish.py b/ironic/conf/redfish.py index 20cd3af5e0..b67f51315a 100644 --- a/ironic/conf/redfish.py +++ b/ironic/conf/redfish.py @@ -44,13 +44,22 @@ opts = [ 'fall back to basic HTTP authentication'))], default='auto', help=_('Redfish HTTP client authentication method.')), + cfg.BoolOpt('use_swift', + default=True, + help=_('Upload generated ISO images for virtual media boot to ' + 'Swift, then pass temporary URL to BMC for booting the ' + 'node. If set to false, images are are placed on the ' + 'ironic-conductor node and served over its ' + 'local HTTP server.')), cfg.StrOpt('swift_container', default='ironic_redfish_container', - help=_('The Swift container to store Redfish driver data.')), + help=_('The Swift container to store Redfish driver data. ' + 'Applies only when `use_swift` is enabled.')), cfg.IntOpt('swift_object_expiry_timeout', default=900, help=_('Amount of time in seconds for Swift objects to ' - 'auto-expire.')), + 'auto-expire. Applies only when `use_swift` is ' + 'enabled.')), cfg.StrOpt('kernel_append_params', default='nofb nomodeset vga=normal', help=_('Additional kernel parameters for baremetal ' diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index a6c7162953..4b8175cbf8 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -12,8 +12,12 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. + +import os +import shutil import tempfile +from ironic_lib import utils as ironic_utils from oslo_log import log from oslo_utils import importutils from six.moves.urllib import parse as urlparse @@ -103,6 +107,8 @@ class RedfishVirtualMediaBoot(base.BootInterface): `[instance_info]image_source` node property. """ + IMAGE_SUBDIR = 'redfish' + capabilities = ['iscsi_volume_boot', 'ramdisk_boot'] def __init__(self): @@ -200,25 +206,6 @@ class RedfishVirtualMediaBoot(base.BootInterface): return deploy_info - @staticmethod - def _delete_from_swift(task, container, object_name): - LOG.debug("Cleaning up image %(name)s from Swift container " - "%(container)s for node " - "%(node)s", {'node': task.node.uuid, - 'name': object_name, - 'container': container}) - - swift_api = swift.SwiftAPI() - - try: - swift_api.delete_object(container, object_name) - - except exception.SwiftOperationError as e: - LOG.warning("Failed to clean up image %(image)s for node " - "%(node)s. Error: %(error)s.", - {'node': task.node.uuid, 'image': object_name, - 'error': e}) - @staticmethod def _append_filename_param(url, filename): """Append 'filename=' parameter to given URL. @@ -249,6 +236,94 @@ class RedfishVirtualMediaBoot(base.BootInterface): return urlparse.urlunparse(parsed_url) + @classmethod + def _publish_image(cls, image_file, object_name): + """Make image file downloadable. + + Depending on ironic settings, pushes given file into Swift or copies + it over to local HTTP server's document root and returns publicly + accessible URL leading to the given file. + + :param image_file: path to file to publish + :param object_name: name of the published file + :return: a URL to download published file + """ + + if CONF.redfish.use_swift: + container = CONF.redfish.swift_container + timeout = CONF.redfish.swift_object_expiry_timeout + + object_headers = {'X-Delete-After': str(timeout)} + + swift_api = swift.SwiftAPI() + + swift_api.create_object(container, object_name, image_file, + object_headers=object_headers) + + image_url = swift_api.get_temp_url(container, object_name, timeout) + + else: + public_dir = os.path.join(CONF.deploy.http_root, cls.IMAGE_SUBDIR) + + if not os.path.exists(public_dir): + os.mkdir(public_dir, 0x755) + + published_file = os.path.join(public_dir, object_name) + + try: + os.link(image_file, published_file) + + except OSError as exc: + LOG.debug( + "Could not hardlink image file %(image)s to public " + "location %(public)s (will copy it over): " + "%(error)s", {'image': image_file, + 'public': published_file, + 'error': exc}) + + shutil.copyfile(image_file, published_file) + + image_url = urlparse.urljoin( + CONF.deploy.http_url, cls.IMAGE_SUBDIR, object_name) + + image_url = cls._append_filename_param( + image_url, os.path.basename(image_file)) + + return image_url + + @classmethod + def _unpublish_image(cls, object_name): + """Withdraw the image previously made downloadable. + + Depending on ironic settings, removes previously published file + from where it has been published - Swift or local HTTP server's + document root. + + :param object_name: name of the published file (optional) + """ + if CONF.redfish.use_swift: + container = CONF.redfish.swift_container + + swift_api = swift.SwiftAPI() + + LOG.debug("Cleaning up image %(name)s from Swift container " + "%(container)s", {'name': object_name, + 'container': container}) + + try: + swift_api.delete_object(container, object_name) + + except exception.SwiftOperationError as exc: + LOG.warning("Failed to clean up image %(image)s. Error: " + "%(error)s.", {'image': object_name, + 'error': exc}) + + else: + published_file = os.path.join( + CONF.deploy.http_root, cls.IMAGE_SUBDIR, object_name) + + ironic_utils.unlink_without_raise(published_file) + @staticmethod def _get_floppy_image_name(node): """Returns the floppy image name for a given node. @@ -265,8 +340,7 @@ class RedfishVirtualMediaBoot(base.BootInterface): """ floppy_object_name = cls._get_floppy_image_name(task.node) - cls._delete_from_swift( - task, CONF.redfish.swift_container, floppy_object_name) + cls._unpublish_image(floppy_object_name) @classmethod def _prepare_floppy_image(cls, task, params=None): @@ -290,12 +364,6 @@ class RedfishVirtualMediaBoot(base.BootInterface): """ object_name = cls._get_floppy_image_name(task.node) - container = CONF.redfish.swift_container - timeout = CONF.redfish.swift_object_expiry_timeout - - object_headers = {'X-Delete-After': str(timeout)} - swift_api = swift.SwiftAPI() - LOG.debug("Trying to create floppy image for node " "%(node)s", {'node': task.node.uuid}) @@ -305,12 +373,7 @@ class RedfishVirtualMediaBoot(base.BootInterface): vfat_image_tmpfile = vfat_image_tmpfile_obj.name images.create_vfat_image(vfat_image_tmpfile, parameters=params) - swift_api.create_object(container, object_name, vfat_image_tmpfile, - object_headers=object_headers) - - image_url = swift_api.get_temp_url(container, object_name, timeout) - - image_url = cls._append_filename_param(image_url, 'bootme.img') + image_url = cls._publish_image(vfat_image_tmpfile, object_name) LOG.debug("Created floppy image %(name)s in Swift for node %(node)s, " "exposed as temporary URL " @@ -336,8 +399,7 @@ class RedfishVirtualMediaBoot(base.BootInterface): """ iso_object_name = cls._get_iso_image_name(task.node) - cls._delete_from_swift( - task, CONF.redfish.swift_container, iso_object_name) + cls._unpublish_image(iso_object_name) @classmethod def _prepare_iso_image(cls, task, kernel_href, ramdisk_href, @@ -406,30 +468,14 @@ class RedfishVirtualMediaBoot(base.BootInterface): iso_object_name = cls._get_iso_image_name(task.node) - container = CONF.redfish.swift_container - - timeout = CONF.redfish.swift_object_expiry_timeout - - object_headers = {'X-Delete-After': str(timeout)} - - swift_api = swift.SwiftAPI() - - swift_api.create_object(container, iso_object_name, - boot_iso_tmp_file, - object_headers=object_headers) - - 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') + image_url = cls._publish_image(boot_iso_tmp_file, iso_object_name) 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, - 'url': boot_iso_url}) + 'url': image_url}) - return boot_iso_url + return image_url @classmethod def _prepare_deploy_iso(cls, task, params, mode): diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index 61033c5c64..44d337c1ae 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import os + import mock from oslo_utils import importutils @@ -200,35 +202,19 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): 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, - shared=True) as task: - task.driver.boot._cleanup_floppy_image(task) - - mock_swift.SwiftAPI.assert_called_once_with() - mock_swift_api = mock_swift.SwiftAPI.return_value - - mock_swift_api.delete_object.assert_called_once_with( - 'ironic_redfish_container', 'image-%s' % task.node.uuid - ) - - @mock.patch.object(redfish_boot, 'swift', autospec=True) - @mock.patch.object(images, 'create_vfat_image', autospec=True) - def test__prepare_floppy_image(self, mock_create_vfat_image, mock_swift): + def test__publish_image_swift(self, mock_swift): with task_manager.acquire(self.context, self.node.uuid, shared=True) as 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) + url = task.driver.boot._publish_image('file.iso', 'boot.iso') - self.assertIn('filename=bootme.img', url) + self.assertEqual( + 'https://a.b/c.f?e=f&filename=file.iso', url) mock_swift.SwiftAPI.assert_called_once_with() - mock_create_vfat_image.assert_called_once_with( - mock.ANY, parameters=mock.ANY) - mock_swift_api.create_object.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, mock.ANY) @@ -236,33 +222,143 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock.ANY, mock.ANY, mock.ANY) @mock.patch.object(redfish_boot, 'swift', autospec=True) - def test__cleanup_iso_image(self, mock_swift): + def test__unpublish_image_swift(self, mock_swift): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - task.driver.boot._cleanup_iso_image(task) + object_name = 'image-%s' % task.node.uuid + + task.driver.boot._unpublish_image(object_name) mock_swift.SwiftAPI.assert_called_once_with() mock_swift_api = mock_swift.SwiftAPI.return_value mock_swift_api.delete_object.assert_called_once_with( - 'ironic_redfish_container', 'boot-%s' % task.node.uuid - ) + 'ironic_redfish_container', object_name) - @mock.patch.object(redfish_boot, 'swift', autospec=True) + @mock.patch.object(redfish_boot, 'shutil', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'mkdir', autospec=True) + def test__publish_image_local_link( + self, mock_mkdir, mock_link, mock_shutil): + self.config(use_swift=False, group='redfish') + self.config(http_url='http://localhost', group='deploy') + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + + url = task.driver.boot._publish_image('file.iso', 'boot.iso') + + self.assertEqual( + 'http://localhost/redfish?filename=file.iso', url) + + mock_mkdir.assert_called_once_with('/httpboot/redfish', 0x755) + mock_link.assert_called_once_with( + 'file.iso', '/httpboot/redfish/boot.iso') + + @mock.patch.object(redfish_boot, 'shutil', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'mkdir', autospec=True) + def test__publish_image_local_copy( + self, mock_mkdir, mock_link, mock_shutil): + self.config(use_swift=False, group='redfish') + self.config(http_url='http://localhost', group='deploy') + + mock_link.side_effect = OSError() + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + + url = task.driver.boot._publish_image('file.iso', 'boot.iso') + + self.assertEqual( + 'http://localhost/redfish?filename=file.iso', url) + + mock_mkdir.assert_called_once_with('/httpboot/redfish', 0x755) + + mock_shutil.copyfile.assert_called_once_with( + 'file.iso', '/httpboot/redfish/boot.iso') + + @mock.patch.object(redfish_boot, 'ironic_utils', autospec=True) + def test__unpublish_image_local(self, mock_ironic_utils): + self.config(use_swift=False, group='redfish') + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + object_name = 'image-%s' % task.node.uuid + + expected_file = '/httpboot/redfish/' + object_name + + task.driver.boot._unpublish_image(object_name) + + mock_ironic_utils.unlink_without_raise.assert_called_once_with( + expected_file) + + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_unpublish_image', autospec=True) + def test__cleanup_floppy_image(self, mock_unpublish): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.driver.boot._cleanup_floppy_image(task) + + object_name = 'image-%s' % task.node.uuid + + mock_unpublish.assert_called_once_with(object_name) + + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_publish_image', autospec=True) + @mock.patch.object(images, 'create_vfat_image', autospec=True) + def test__prepare_floppy_image( + self, mock_create_vfat_image, mock__publish_image): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + expected_url = 'https://a.b/c.f?e=f' + + mock__publish_image.return_value = expected_url + + url = task.driver.boot._prepare_floppy_image(task) + + object_name = 'image-%s' % task.node.uuid + + mock__publish_image.assert_called_once_with( + mock.ANY, object_name) + + mock_create_vfat_image.assert_called_once_with( + mock.ANY, parameters=mock.ANY) + + self.assertEqual(expected_url, url) + + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_unpublish_image', autospec=True) + def test__cleanup_iso_image(self, mock_unpublish): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.driver.boot._cleanup_iso_image(task) + + object_name = 'boot-%s' % task.node.uuid + + mock_unpublish.assert_called_once_with(object_name) + + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_publish_image', autospec=True) @mock.patch.object(images, 'create_boot_iso', autospec=True) - def test__prepare_iso_image_uefi(self, mock_create_boot_iso, mock_swift): + def test__prepare_iso_image_uefi( + self, mock_create_boot_iso, mock__publish_image): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.instance_info.update(deploy_boot_mode='uefi') - mock_swift_api = mock_swift.SwiftAPI.return_value - mock_swift_api.get_temp_url.return_value = 'https://a.b/c.f?e=f' + expected_url = 'https://a.b/c.f?e=f' + + mock__publish_image.return_value = expected_url url = task.driver.boot._prepare_iso_image( task, 'http://kernel/img', 'http://ramdisk/img', 'http://bootloader/img', root_uuid=task.node.uuid) - self.assertIn('filename=bootme.iso', url) + object_name = 'boot-%s' % task.node.uuid + + mock__publish_image.assert_called_once_with( + mock.ANY, object_name) mock_create_boot_iso.assert_called_once_with( mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', @@ -270,28 +366,28 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): kernel_params='nofb nomodeset vga=normal', root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123') - mock_swift.SwiftAPI.assert_called_once_with() + self.assertEqual(expected_url, url) - mock_swift_api.create_object.assert_called_once_with( - mock.ANY, mock.ANY, mock.ANY, mock.ANY) - - mock_swift_api.get_temp_url.assert_called_once_with( - mock.ANY, mock.ANY, mock.ANY) - - @mock.patch.object(redfish_boot, 'swift', autospec=True) + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_publish_image', autospec=True) @mock.patch.object(images, 'create_boot_iso', autospec=True) - def test__prepare_iso_image_bios(self, mock_create_boot_iso, mock_swift): + def test__prepare_iso_image_bios( + self, mock_create_boot_iso, mock__publish_image): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - mock_swift_api = mock_swift.SwiftAPI.return_value - mock_swift_api.get_temp_url.return_value = 'https://a.b/c.f?e=f' + expected_url = 'https://a.b/c.f?e=f' + + mock__publish_image.return_value = expected_url url = task.driver.boot._prepare_iso_image( task, 'http://kernel/img', 'http://ramdisk/img', bootloader_href=None, root_uuid=task.node.uuid) - self.assertIn('filename=bootme.iso', url) + object_name = 'boot-%s' % task.node.uuid + + mock__publish_image.assert_called_once_with( + mock.ANY, object_name) mock_create_boot_iso.assert_called_once_with( mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', @@ -299,13 +395,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): kernel_params='nofb nomodeset vga=normal', root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123') - mock_swift.SwiftAPI.assert_called_once_with() - - mock_swift_api.create_object.assert_called_once_with( - mock.ANY, mock.ANY, mock.ANY, mock.ANY) - - mock_swift_api.get_temp_url.assert_called_once_with( - mock.ANY, mock.ANY, mock.ANY) + self.assertEqual(expected_url, url) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_prepare_iso_image', autospec=True) diff --git a/releasenotes/notes/add-redfish-boot-interface-e7e05bdd2c894d80.yaml b/releasenotes/notes/add-redfish-boot-interface-e7e05bdd2c894d80.yaml index eb1f1388dd..643ec0561d 100644 --- a/releasenotes/notes/add-redfish-boot-interface-e7e05bdd2c894d80.yaml +++ b/releasenotes/notes/add-redfish-boot-interface-e7e05bdd2c894d80.yaml @@ -7,4 +7,7 @@ features: ``redfish-virtual-media`` boot interface can additionally require EFI system partition image (ESP) when performing UEFI boot. New configuration option ``bootloader`` or ``[driver_info]/bootloader`` property can be used - to convey ESP location to ironic. + to convey ESP location to ironic. Bootable ISO images can be served to + BMC either from Swift or from a HTTP server running on ironic conductor + machine. This is controlled by the ``[redfish]use_swift`` ironic + configuration option.