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 c144d5d0b5..8fe1fd5279 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.