Merge "Refactor publishing images into a new module"

ironic/common/image_publisher.py

@@ -0,0 +1,187 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    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 abc
+import os.path
+import shutil
+from urllib import parse as urlparse
+
+from ironic_lib import utils as ironic_utils
+from oslo_log import log
+
+from ironic.common import exception
+from ironic.common import swift
+from ironic.common import utils
+from ironic.conf import CONF
+
+LOG = log.getLogger(__name__)
+
+
+class AbstractPublisher(metaclass=abc.ABCMeta):
+    """Abstract base class for publishing images via HTTP."""
+
+    @abc.abstractmethod
+    def publish(self, source_path, file_name=None):
+        """Publish an image.
+
+        :param source_path: Path to the source file.
+        :param file_name: Destination file name. If None, the file component
+            of source_path is used.
+        :return: The HTTP URL of the published image.
+        """
+
+    @abc.abstractmethod
+    def unpublish(self, file_name):
+        """Unpublish the image.
+
+        :param file_name: File name to unpublish.
+        """
+
+
+class LocalPublisher(AbstractPublisher):
+    """Image publisher using a local web server."""
+
+    def __init__(self, image_subdir=None, file_permission=0o644,
+                 dir_permission=0o755, root_url=None):
+        """Create a local publisher.
+
+        :param image_subdir: A subdirectory to put the image to.
+            Using an empty directory may cause name conflicts.
+        :param file_permission: Permissions for copied files.
+        :param dir_permission: Permissions for created directories.
+        :param root_url: Public URL of the web server. If empty, determined
+            from the configuration.
+        """
+        self.image_subdir = image_subdir
+        self.root_url = (root_url or CONF.deploy.external_http_url
+                         or CONF.deploy.http_url)
+        self.file_permission = file_permission
+        self.dir_permission = dir_permission
+
+    def publish(self, source_path, file_name=None):
+        if not file_name:
+            file_name = os.path.basename(source_path)
+
+        if self.image_subdir:
+            public_dir = os.path.join(CONF.deploy.http_root, self.image_subdir)
+        else:
+            public_dir = CONF.deploy.http_root
+
+        if not os.path.exists(public_dir):
+            os.mkdir(public_dir, self.dir_permission)
+
+        published_file = os.path.join(public_dir, file_name)
+
+        try:
+            os.link(source_path, published_file)
+            os.chmod(source_path, self.file_permission)
+            try:
+                utils.execute(
+                    '/usr/sbin/restorecon', '-i', '-R', 'v', public_dir)
+            except FileNotFoundError as exc:
+                LOG.debug(
+                    "Could not restore SELinux context on "
+                    "%(public_dir)s, restorecon command not found.\n"
+                    "Error: %(error)s",
+                    {'public_dir': public_dir,
+                        'error': exc})
+
+        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': source_path,
+                              'public': published_file,
+                              'error': exc})
+
+            shutil.copyfile(source_path, published_file)
+            os.chmod(published_file, self.file_permission)
+
+        if self.image_subdir:
+            return os.path.join(self.root_url, self.image_subdir, file_name)
+        else:
+            return os.path.join(self.root_url, file_name)
+
+    def unpublish(self, file_name):
+        published_file = os.path.join(
+            CONF.deploy.http_root, self.image_subdir, file_name)
+        ironic_utils.unlink_without_raise(published_file)
+
+
+class SwiftPublisher(AbstractPublisher):
+    """Image publisher using OpenStack Swift."""
+
+    def __init__(self, container, delete_after):
+        """Create a Swift publisher.
+
+        :param container: Swift container to use.
+        :param delete_after: Number of seconds after which the link will
+            no longer be valid.
+        """
+        self.container = container
+        self.delete_after = delete_after
+
+    def _append_filename_param(self, 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)
+
+    def publish(self, source_path, file_name=None):
+        api = swift.SwiftAPI()
+        if not file_name:
+            file_name = os.path.basename(source_path)
+
+        object_headers = {'X-Delete-After': str(self.delete_after)}
+        api.create_object(self.container, file_name, source_path,
+                          object_headers=object_headers)
+
+        image_url = api.get_temp_url(self.container, file_name,
+                                     self.delete_after)
+        return self._append_filename_param(
+            image_url, os.path.basename(source_path))
+
+    def unpublish(self, file_name):
+        api = swift.SwiftAPI()
+        LOG.debug("Cleaning up image %(name)s from Swift container "
+                  "%(container)s", {'name': file_name,
+                                    'container': self.container})
+
+        try:
+            api.delete_object(self.container, file_name)
+
+        except exception.SwiftOperationError as exc:
+            LOG.warning("Failed to clean up image %(image)s. Error: "
+                        "%(error)s.", {'image': file_name, 'error': exc})

ironic/drivers/modules/image_utils.py

@@ -22,15 +22,14 @@ import shutil
 import tempfile
 from urllib import parse as urlparse
 
-from ironic_lib import utils as ironic_utils
 from oslo_log import log
 
 from ironic.common import exception
 from ironic.common.glance_service import service_utils
 from ironic.common.i18n import _
+from ironic.common import image_publisher
 from ironic.common import images
 from ironic.common import states
-from ironic.common import swift
 from ironic.common import utils
 from ironic.conf import CONF
 from ironic.drivers.modules import boot_mode_utils
@@ -82,12 +81,15 @@ class ImageHandler(object):
             },
         }
 
-        self._driver = driver
+        if _SWIFT_MAP[driver].get("swift_enabled"):
-        self.swift_enabled = _SWIFT_MAP[driver].get("swift_enabled")
+            self._publisher = image_publisher.SwiftPublisher(
-        self._container = _SWIFT_MAP[driver].get("container")
+                container=_SWIFT_MAP[driver].get("container"),
-        self._timeout = _SWIFT_MAP[driver].get("timeout")
+                delete_after=_SWIFT_MAP[driver].get("timeout"))
-        self._image_subdir = _SWIFT_MAP[driver].get("image_subdir")
+        else:
-        self._file_permission = _SWIFT_MAP[driver].get("file_permission")
+            self._publisher = image_publisher.LocalPublisher(
+                image_subdir=_SWIFT_MAP[driver].get("image_subdir"),
+                file_permission=_SWIFT_MAP[driver].get("file_permission"))
+
         # To get the kernel parameters
         self.kernel_params = _SWIFT_MAP[driver].get("kernel_params")
 
@@ -100,28 +102,7 @@ class ImageHandler(object):
 
         :param object_name: name of the published file (optional)
         """
-        if self.swift_enabled:
+        self._publisher.unpublish(object_name)
-            container = self._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, self._image_subdir, object_name)
-
-            ironic_utils.unlink_without_raise(published_file)
 
     @classmethod
     def unpublish_image_for_node(cls, node, prefix='', suffix=''):
@@ -140,35 +121,6 @@ class ImageHandler(object):
         LOG.debug('Removed image %(name)s for node %(node)s',
                   {'node': node.uuid, 'name': name})
 
-    def _append_filename_param(self, 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)
-
     def publish_image(self, image_file, object_name, node_http_url=None):
         """Make image file downloadable.
 
@@ -183,61 +135,9 @@ class ImageHandler(object):
                               from CONF.deploy won't be used.
         :return: a URL to download published file
         """
-
+        if node_http_url:
-        if self.swift_enabled:
+            self._publisher.root_url = node_http_url
-            container = self._container
+        return self._publisher.publish(image_file, object_name)
-            timeout = self._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)
-            image_url = self._append_filename_param(
-                image_url, os.path.basename(image_file))
-
-        else:
-            public_dir = os.path.join(CONF.deploy.http_root,
-                                      self._image_subdir)
-
-            if not os.path.exists(public_dir):
-                os.mkdir(public_dir, 0o755)
-
-            published_file = os.path.join(public_dir, object_name)
-
-            try:
-                os.link(image_file, published_file)
-                os.chmod(image_file, self._file_permission)
-                try:
-                    utils.execute(
-                        '/usr/sbin/restorecon', '-i', '-R', 'v', public_dir)
-                except FileNotFoundError as exc:
-                    LOG.debug(
-                        "Could not restore SELinux context on "
-                        "%(public_dir)s, restorecon command not found.\n"
-                        "Error: %(error)s",
-                        {'public_dir': public_dir,
-                         'error': exc})
-
-            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)
-                os.chmod(published_file, self._file_permission)
-
-            http_url = (node_http_url or CONF.deploy.external_http_url
-                        or CONF.deploy.http_url)
-            image_url = os.path.join(http_url, self._image_subdir, object_name)
-
-        return image_url
 
 
 @image_cache.cleanup(priority=75)

ironic/tests/unit/common/test_image_publisher.py

@@ -0,0 +1,184 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    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
+from unittest import mock
+
+from ironic_lib import utils as ironic_utils
+
+from ironic.common import image_publisher
+from ironic.common import utils
+from ironic.tests.unit.db import base as db_base
+
+
+class SwiftPublisherTestCase(db_base.DbTestCase):
+
+    container = "test"
+    publisher = image_publisher.SwiftPublisher(container, 42)
+
+    def test__append_filename_param_without_qs(self):
+        res = self.publisher._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):
+        res = self.publisher._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):
+        res = self.publisher._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(image_publisher, 'swift', autospec=True)
+    def test_publish(self, mock_swift):
+        mock_swift_api = mock_swift.SwiftAPI.return_value
+        mock_swift_api.get_temp_url.return_value = 'https://a.b/c.f?e=f'
+
+        url = self.publisher.publish('file.iso', 'boot.iso')
+
+        self.assertEqual(
+            'https://a.b/c.f?e=f&filename=file.iso', url)
+
+        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)
+
+    @mock.patch.object(image_publisher, 'swift', autospec=True)
+    def test_unpublish(self, mock_swift):
+        object_name = 'boot.iso'
+
+        self.publisher.unpublish(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(
+            self.container, object_name)
+
+
+class LocalPublisherTestCase(db_base.DbTestCase):
+
+    def setUp(self):
+        super().setUp()
+        self.config(http_url='http://localhost', group='deploy')
+        self.publisher = image_publisher.LocalPublisher('redfish')
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    @mock.patch.object(os, 'chmod', autospec=True)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(os, 'mkdir', autospec=True)
+    def test_publish_local_link(
+            self, mock_mkdir, mock_link, mock_chmod, mock_execute):
+        url = self.publisher.publish('file.iso', 'boot.iso')
+        self.assertEqual(
+            'http://localhost/redfish/boot.iso', url)
+        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
+        mock_link.assert_called_once_with(
+            'file.iso', '/httpboot/redfish/boot.iso')
+        mock_chmod.assert_called_once_with('file.iso', 0o644)
+        mock_execute.assert_called_once_with(
+            '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish')
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    @mock.patch.object(os, 'chmod', autospec=True)
+    @mock.patch.object(shutil, 'copyfile', autospec=True)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(os, 'mkdir', autospec=True)
+    def test_publish_local_link_no_restorecon(
+            self, mock_mkdir, mock_link, mock_copyfile, mock_chmod,
+            mock_execute):
+        url = self.publisher.publish('file.iso', 'boot.iso')
+        self.assertEqual(
+            'http://localhost/redfish/boot.iso', url)
+        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
+        mock_link.assert_called_once_with(
+            'file.iso', '/httpboot/redfish/boot.iso')
+        mock_chmod.assert_called_once_with('file.iso', 0o644)
+        mock_execute.return_value = FileNotFoundError
+        mock_copyfile.assert_not_called()
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    @mock.patch.object(os, 'chmod', autospec=True)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(os, 'mkdir', autospec=True)
+    def test_publish_external_ip(
+            self, mock_mkdir, mock_link, mock_chmod, mock_execute):
+        self.config(external_http_url='http://non-local.host', group='deploy')
+        self.publisher = image_publisher.LocalPublisher(image_subdir='redfish')
+        url = self.publisher.publish('file.iso', 'boot.iso')
+        self.assertEqual(
+            'http://non-local.host/redfish/boot.iso', url)
+        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
+        mock_link.assert_called_once_with(
+            'file.iso', '/httpboot/redfish/boot.iso')
+        mock_chmod.assert_called_once_with('file.iso', 0o644)
+        mock_execute.assert_called_once_with(
+            '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish')
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    @mock.patch.object(os, 'chmod', autospec=True)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(os, 'mkdir', autospec=True)
+    def test_publish_external_ip_node_override(
+            self, mock_mkdir, mock_link, mock_chmod, mock_execute):
+        self.config(external_http_url='http://non-local.host', group='deploy')
+        override_url = "http://node.override.url"
+        self.publisher = image_publisher.LocalPublisher(
+            image_subdir='redfish', root_url=override_url)
+        url = self.publisher.publish('file.iso', 'boot.iso')
+        self.assertEqual(
+            'http://node.override.url/redfish/boot.iso', url)
+        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
+        mock_link.assert_called_once_with(
+            'file.iso', '/httpboot/redfish/boot.iso')
+        mock_chmod.assert_called_once_with('file.iso', 0o644)
+        mock_execute.assert_called_once_with(
+            '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish')
+
+    @mock.patch.object(os, 'chmod', autospec=True)
+    @mock.patch.object(shutil, 'copyfile', autospec=True)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(os, 'mkdir', autospec=True)
+    def test_publish_local_copy(self, mock_mkdir, mock_link,
+                                mock_copyfile, mock_chmod):
+        mock_link.side_effect = OSError()
+
+        url = self.publisher.publish('file.iso', 'boot.iso')
+
+        self.assertEqual(
+            'http://localhost/redfish/boot.iso', url)
+
+        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
+
+        mock_copyfile.assert_called_once_with(
+            'file.iso', '/httpboot/redfish/boot.iso')
+        mock_chmod.assert_called_once_with('/httpboot/redfish/boot.iso',
+                                           0o644)
+
+    @mock.patch.object(ironic_utils, 'unlink_without_raise', autospec=True)
+    def test_unpublish_local(self, mock_unlink):
+        object_name = 'boot.iso'
+        expected_file = '/httpboot/redfish/' + object_name
+
+        self.publisher.unpublish(object_name)
+
+        mock_unlink.assert_called_once_with(expected_file)

ironic/tests/unit/drivers/modules/test_image_utils.py

@@ -50,190 +50,13 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase):
         self.node = obj_utils.create_test_node(
             self.context, driver='redfish', driver_info=INFO_DICT)
 
-    def test__append_filename_param_without_qs(self):
+    def test_redfish_kernel_param_config(self):
+        self.config(kernel_append_params="console=ttyS1", group='redfish')
         img_handler_obj = image_utils.ImageHandler(self.node.driver)
-        res = img_handler_obj._append_filename_param(
+        actual_k_param = img_handler_obj.kernel_params
-            'http://a.b/c', 'b.img')
+        expected_k_param = "console=ttyS1"
-        expected = 'http://a.b/c?filename=b.img'
-        self.assertEqual(expected, res)
 
-    def test__append_filename_param_with_qs(self):
+        self.assertEqual(expected_k_param, actual_k_param)
-        img_handler_obj = image_utils.ImageHandler(self.node.driver)
-        res = img_handler_obj._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):
-        img_handler_obj = image_utils.ImageHandler(self.node.driver)
-        res = img_handler_obj._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(image_utils, 'swift', autospec=True)
-    def test_publish_image_swift(self, mock_swift):
-        img_handler_obj = image_utils.ImageHandler(self.node.driver)
-        mock_swift_api = mock_swift.SwiftAPI.return_value
-        mock_swift_api.get_temp_url.return_value = 'https://a.b/c.f?e=f'
-
-        url = img_handler_obj.publish_image('file.iso', 'boot.iso')
-
-        self.assertEqual(
-            'https://a.b/c.f?e=f&filename=file.iso', url)
-
-        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)
-
-    @mock.patch.object(image_utils, 'swift', autospec=True)
-    def test_unpublish_image_swift(self, mock_swift):
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=True) as task:
-            img_handler_obj = image_utils.ImageHandler(self.node.driver)
-            object_name = 'image-%s' % task.node.uuid
-
-            img_handler_obj.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', object_name)
-
-    @mock.patch.object(utils, 'execute', autospec=True)
-    @mock.patch.object(os, 'chmod', autospec=True)
-    @mock.patch.object(image_utils, '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, mock_chmod,
-            mock_execute):
-        self.config(use_swift=False, group='redfish')
-        self.config(http_url='http://localhost', group='deploy')
-        img_handler_obj = image_utils.ImageHandler(self.node.driver)
-        url = img_handler_obj.publish_image('file.iso', 'boot.iso')
-        self.assertEqual(
-            'http://localhost/redfish/boot.iso', url)
-        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
-        mock_link.assert_called_once_with(
-            'file.iso', '/httpboot/redfish/boot.iso')
-        mock_chmod.assert_called_once_with('file.iso', 0o644)
-        mock_execute.assert_called_once_with(
-            '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish')
-
-    @mock.patch.object(utils, 'execute', autospec=True)
-    @mock.patch.object(os, 'chmod', autospec=True)
-    @mock.patch.object(image_utils, 'shutil', autospec=True)
-    @mock.patch.object(os, 'link', autospec=True)
-    @mock.patch.object(os, 'mkdir', autospec=True)
-    def test_publish_image_local_link_no_restorecon(
-            self, mock_mkdir, mock_link, mock_shutil, mock_chmod,
-            mock_execute):
-        self.config(use_swift=False, group='redfish')
-        self.config(http_url='http://localhost', group='deploy')
-        img_handler_obj = image_utils.ImageHandler(self.node.driver)
-        url = img_handler_obj.publish_image('file.iso', 'boot.iso')
-        self.assertEqual(
-            'http://localhost/redfish/boot.iso', url)
-        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
-        mock_link.assert_called_once_with(
-            'file.iso', '/httpboot/redfish/boot.iso')
-        mock_chmod.assert_called_once_with('file.iso', 0o644)
-        mock_execute.return_value = FileNotFoundError
-        mock_shutil.assert_not_called()
-
-    @mock.patch.object(utils, 'execute', autospec=True)
-    @mock.patch.object(os, 'chmod', autospec=True)
-    @mock.patch.object(image_utils, 'shutil', autospec=True)
-    @mock.patch.object(os, 'link', autospec=True)
-    @mock.patch.object(os, 'mkdir', autospec=True)
-    def test_publish_image_external_ip(
-            self, mock_mkdir, mock_link, mock_shutil, mock_chmod,
-            mock_execute):
-        self.config(use_swift=False, group='redfish')
-        self.config(http_url='http://localhost',
-                    external_http_url='http://non-local.host',
-                    group='deploy')
-        img_handler_obj = image_utils.ImageHandler(self.node.driver)
-        url = img_handler_obj.publish_image('file.iso', 'boot.iso')
-        self.assertEqual(
-            'http://non-local.host/redfish/boot.iso', url)
-        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
-        mock_link.assert_called_once_with(
-            'file.iso', '/httpboot/redfish/boot.iso')
-        mock_chmod.assert_called_once_with('file.iso', 0o644)
-        mock_execute.assert_called_once_with(
-            '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish')
-
-    @mock.patch.object(utils, 'execute', autospec=True)
-    @mock.patch.object(os, 'chmod', autospec=True)
-    @mock.patch.object(image_utils, 'shutil', autospec=True)
-    @mock.patch.object(os, 'link', autospec=True)
-    @mock.patch.object(os, 'mkdir', autospec=True)
-    def test_publish_image_external_ip_node_override(
-            self, mock_mkdir, mock_link, mock_shutil, mock_chmod,
-            mock_execute):
-        self.config(use_swift=False, group='redfish')
-        self.config(http_url='http://localhost',
-                    external_http_url='http://non-local.host',
-                    group='deploy')
-        img_handler_obj = image_utils.ImageHandler(self.node.driver)
-        self.node.driver_info["external_http_url"] = "http://node.override.url"
-        override_url = self.node.driver_info.get("external_http_url")
-        url = img_handler_obj.publish_image('file.iso', 'boot.iso',
-                                            override_url)
-        self.assertEqual(
-            'http://node.override.url/redfish/boot.iso', url)
-        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
-        mock_link.assert_called_once_with(
-            'file.iso', '/httpboot/redfish/boot.iso')
-        mock_chmod.assert_called_once_with('file.iso', 0o644)
-        mock_execute.assert_called_once_with(
-            '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish')
-
-    @mock.patch.object(os, 'chmod', autospec=True)
-    @mock.patch.object(image_utils, '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, mock_chmod):
-        self.config(use_swift=False, group='redfish')
-        self.config(http_url='http://localhost', group='deploy')
-        img_handler_obj = image_utils.ImageHandler(self.node.driver)
-
-        mock_link.side_effect = OSError()
-
-        url = img_handler_obj.publish_image('file.iso', 'boot.iso')
-
-        self.assertEqual(
-            'http://localhost/redfish/boot.iso', url)
-
-        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
-
-        mock_shutil.copyfile.assert_called_once_with(
-            'file.iso', '/httpboot/redfish/boot.iso')
-        mock_chmod.assert_called_once_with('/httpboot/redfish/boot.iso',
-                                           0o644)
-
-    @mock.patch.object(image_utils, '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:
-            img_handler_obj = image_utils.ImageHandler(self.node.driver)
-            object_name = 'image-%s' % task.node.uuid
-
-            expected_file = '/httpboot/redfish/' + object_name
-
-            img_handler_obj.unpublish_image(object_name)
-
-            mock_ironic_utils.unlink_without_raise.assert_called_once_with(
-                expected_file)
 
 
 class IloImageHandlerTestCase(db_base.DbTestCase):