Merge "Serve virtual media boot images from ironic conductor"

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 '

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=<file>' 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(
+        cls._unpublish_image(floppy_object_name)
-            task, CONF.redfish.swift_container, 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,
+            image_url = cls._publish_image(vfat_image_tmpfile, object_name)
-                                    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')
 
         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(
+        cls._unpublish_image(iso_object_name)
-            task, CONF.redfish.swift_container, 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
+            image_url = cls._publish_image(boot_iso_tmp_file, iso_object_name)
-
-            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')
 
         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):

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):
+    def test__publish_image_swift(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):
         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
+            expected_url = 'https://a.b/c.f?e=f'
-            mock_swift_api.get_temp_url.return_value = '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.patch.object(redfish_boot.RedfishVirtualMediaBoot,
-                mock.ANY, mock.ANY, mock.ANY, mock.ANY)
+                       '_publish_image', autospec=True)
-
-            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(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
+            expected_url = 'https://a.b/c.f?e=f'
-            mock_swift_api.get_temp_url.return_value = '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()
+            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.RedfishVirtualMediaBoot,
                        '_prepare_iso_image', autospec=True)

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.