Merge "Allow overriding an external URL for virtual media"

8 files changed, 140 insertions, 4 deletions

diff --git a/doc/source/admin/dhcp-less.rst b/doc/source/admin/dhcp-less.rst
index c9a0d9608..fe3aa055f 100644
--- a/doc/source/admin/dhcp-less.rst
+++ b/doc/source/admin/dhcp-less.rst
@@ -87,3 +87,26 @@ An example network data:
 .. _Glean: https://docs.openstack.org/infra/glean/
 .. _simple-init: https://docs.openstack.org/diskimage-builder/latest/elements/simple-init/README.html
 .. _network_data: https://specs.openstack.org/openstack/nova-specs/specs/liberty/implemented/metadata-service-network-info.html
+
+.. _l3-external-ip:
+
+Deploying outside of the provisioning network
+---------------------------------------------
+
+If you need to combine traditional deployments using a provisioning network
+with virtual media deployments over L3, you may need to provide an alternative
+IP address for the remote nodes to connect to:
+
+.. code-block:: ini
+
+   [deploy]
+   http_url = <HTTP server URL internal to the provisioning network>
+   external_http_url = <HTTP server URL with a routable IP address>
+
+You may also need to override the callback URL, which is normally fetched from
+the service catalog or configured in the ``[service_catalog]`` section:
+
+.. code-block:: ini
+
+   [deploy]
+   external_callback_url = <Bare Metal API URL with a routable IP address>
diff --git a/doc/source/admin/interfaces/deploy.rst b/doc/source/admin/interfaces/deploy.rst
index d7884d816..f8fbb3916 100644
--- a/doc/source/admin/interfaces/deploy.rst
+++ b/doc/source/admin/interfaces/deploy.rst
@@ -71,6 +71,9 @@ ironic configuration file to match the HTTP server configurations.
    http_url = http://example.com
    http_root = /httpboot
 
+.. note::
+   See also: :ref:`l3-external-ip`.
+
 Each HTTP server should be configured to follow symlinks for images
 accessible from HTTP service. Please refer to configuration option
 ``FollowSymLinks`` if you are using Apache HTTP server, or
diff --git a/doc/source/install/configure-pxe.rst b/doc/source/install/configure-pxe.rst
index 6c51a24f3..0f5f392a2 100644
--- a/doc/source/install/configure-pxe.rst
+++ b/doc/source/install/configure-pxe.rst
@@ -332,6 +332,8 @@ on the Bare Metal service node(s) where ``ironic-conductor`` is running.
       # http://192.1.2.3:8080 (string value)
       http_url=http://192.168.0.2:8080
 
+   See also: :ref:`l3-external-ip`.
+
 #. Install the iPXE package with the boot images:
 
    Ubuntu::
diff --git a/ironic/conf/deploy.py b/ironic/conf/deploy.py
index 49975595c..69d5fa537 100644
--- a/ironic/conf/deploy.py
+++ b/ironic/conf/deploy.py
@@ -27,6 +27,17 @@ opts = [
     cfg.StrOpt('http_root',
                default='/httpboot',
                help=_("ironic-conductor node's HTTP root path.")),
+    cfg.StrOpt('external_http_url',
+               help=_("URL of the ironic-conductor node's HTTP server for "
+                      "boot methods such as virtual media, "
+                      "where images could be served outside of the "
+                      "provisioning network. Does not apply when Swift is "
+                      "used. Defaults to http_url.")),
+    cfg.StrOpt('external_callback_url',
+               help=_("Agent callback URL of the bare metal API for boot "
+                      "methods such as virtual media, where images could be "
+                      "served outside of the provisioning network. Defaults "
+                      "to the configuration from [service_catalog].")),
     cfg.BoolOpt('enable_ata_secure_erase',
                 default=True,
                 mutable=True,
diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index ba8ebbebd..e3b7fa3fb 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -622,6 +622,9 @@ def is_software_raid(node):
     return software_raid
 
 
+IPA_URL_PARAM_NAME = 'ipa-api-url'
+
+
 def build_agent_options(node):
     """Build the options to be passed to the agent ramdisk.
 
@@ -630,7 +633,7 @@ def build_agent_options(node):
         agent ramdisk.
     """
     agent_config_opts = {
-        'ipa-api-url': get_ironic_api_url(),
+        IPA_URL_PARAM_NAME: get_ironic_api_url(),
     }
     return agent_config_opts
 
diff --git a/ironic/drivers/modules/image_utils.py b/ironic/drivers/modules/image_utils.py
index e26ecddf1..f1283b092 100644
--- a/ironic/drivers/modules/image_utils.py
+++ b/ironic/drivers/modules/image_utils.py
@@ -214,8 +214,8 @@ class ImageHandler(object):
                 shutil.copyfile(image_file, published_file)
                 os.chmod(published_file, self._file_permission)
 
-            image_url = os.path.join(
-                CONF.deploy.http_url, self._image_subdir, object_name)
+            http_url = CONF.deploy.external_http_url or CONF.deploy.http_url
+            image_url = os.path.join(http_url, self._image_subdir, object_name)
 
         image_url = self._append_filename_param(
             image_url, os.path.basename(image_file))
@@ -244,6 +244,16 @@ def cleanup_iso_image(task):
                                           suffix='.iso')
 
 
+def override_api_url(params):
+    if not CONF.deploy.external_callback_url:
+        return params
+
+    params = params or {}
+    params[deploy_utils.IPA_URL_PARAM_NAME] = \
+        CONF.deploy.external_callback_url.rstrip('/')
+    return params
+
+
 def prepare_floppy_image(task, params=None):
     """Prepares the floppy image for passing the parameters.
 
@@ -264,6 +274,7 @@ def prepare_floppy_image(task, params=None):
     :returns: image URL for the floppy image.
     """
     object_name = _get_name(task.node, prefix='image')
+    params = override_api_url(params)
 
     LOG.debug("Trying to create floppy image for node "
               "%(node)s", {'node': task.node.uuid})
@@ -533,6 +544,8 @@ def prepare_deploy_iso(task, params, mode, d_info):
     iso_href = _find_param(iso_str, d_info)
     bootloader_href = _find_param(bootloader_str, d_info)
 
+    params = override_api_url(params)
+
     # TODO(TheJulia): At some point we should support something like
     # boot_iso for the deploy interface, perhaps when we support config
     # injection.
diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py
index a8305d023..cb41db3eb 100644
--- a/ironic/tests/unit/drivers/modules/test_image_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_image_utils.py
@@ -125,6 +125,28 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase):
     @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):
+        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?filename=file.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.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')
@@ -248,7 +270,31 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase):
                                                        mock.ANY, object_name)
 
             mock_create_vfat_image.assert_called_once_with(
-                mock.ANY, parameters=mock.ANY)
+                mock.ANY, parameters=None)
+
+            self.assertEqual(expected_url, url)
+
+    @mock.patch.object(image_utils.ImageHandler, 'publish_image',
+                       autospec=True)
+    @mock.patch.object(images, 'create_vfat_image', autospec=True)
+    def test_prepare_floppy_image_with_external_ip(
+            self, mock_create_vfat_image, mock_publish_image):
+        self.config(external_callback_url='http://callback/', group='deploy')
+        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 = image_utils.prepare_floppy_image(task)
+
+            object_name = 'image-%s' % task.node.uuid
+
+            mock_publish_image.assert_called_once_with(mock.ANY,
+                                                       mock.ANY, object_name)
+
+            mock_create_vfat_image.assert_called_once_with(
+                mock.ANY, parameters={"ipa-api-url": "http://callback"})
 
             self.assertEqual(expected_url, url)
 
@@ -632,6 +678,28 @@ cafile = /etc/ironic-python-agent/ironic.crt
                 task, 'kernel', 'ramdisk', 'bootloader', params={},
                 inject_files=expected_files, base_iso=None)
 
+    @mock.patch.object(image_utils, '_prepare_iso_image', autospec=True)
+    def test_prepare_deploy_iso_external_ip(self, mock__prepare_iso_image):
+        self.config(external_callback_url='http://callback/', group='deploy')
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+
+            d_info = {
+                'ilo_deploy_kernel': 'kernel',
+                'ilo_deploy_ramdisk': 'ramdisk',
+                'ilo_bootloader': 'bootloader'
+            }
+            task.node.driver_info.update(d_info)
+
+            task.node.instance_info.update(deploy_boot_mode='uefi')
+
+            image_utils.prepare_deploy_iso(task, {}, 'deploy', d_info)
+
+            mock__prepare_iso_image.assert_called_once_with(
+                task, 'kernel', 'ramdisk', 'bootloader',
+                params={'ipa-api-url': 'http://callback'},
+                inject_files={}, base_iso=None)
+
     @mock.patch.object(image_utils, '_find_param', autospec=True)
     @mock.patch.object(image_utils, '_prepare_iso_image', autospec=True)
     @mock.patch.object(images, 'create_boot_iso', autospec=True)
diff --git a/releasenotes/notes/external-ip-5ec9b7b55a90cec4.yaml b/releasenotes/notes/external-ip-5ec9b7b55a90cec4.yaml
new file mode 100644
index 000000000..eea112ba3
--- /dev/null
+++ b/releasenotes/notes/external-ip-5ec9b7b55a90cec4.yaml
@@ -0,0 +1,13 @@
+---
+features:
+  - |
+    Provides operator ability to override URL settings required for
+    provisioning/cleaning in the event of virtual media based deployment.
+    These scenarios tend to require more delineation than more traditional
+    deployments as they often have a different environmental security
+    requirements. Set these two new configuration options using an IP
+    address that is available to these nodes (both the ramdisk and the BMCs)::
+
+      [deploy]
+      external_http_url = <routable URL of the HTTP server>
+      external_callback_url = <routable URL of bare metal API>