Merge "Make anaconda non-image deploys sane"

doc/source/admin/anaconda-deploy-interface.rst

@@ -153,6 +153,93 @@ ironic node:
         openstack baremetal node set <node> \
             --instance_info ks_template=glance://uuid
 
+.. warning::
+   In the Ironic Project terminology, the word ``template`` often refers to
+   a file which is supplied to the deployment, which Ironic supplies
+   parameters to render a specific output. One critical example of this in
+   the Ironic workflow, specifically with this driver, is that the generated
+   ``agent token`` is conveyed to the booting ramdisk, facilitating it to call
+   back to Ironic and indicate the state. This token is randomly generated
+   for every deploy, and is required. Specifically this is leveraged in the
+   template's ``pre``, ``onerror``, and ``post`` steps.
+   For more infomation on Agent Token, please see :doc:`/admin/agent-token`.
+
+Standalone deployments
+----------------------
+
+While this deployment interface driver was developed around the use of other
+OpenStack services, it is not explicitly required. For example HTTP(S) URLs
+can be supplied by the API user to explictly set the expected baremetal node
+``instance_info`` fields
+
+.. code-block:: shell
+
+        baremetal node set <node> \
+           --instance_info image_source=<Mirror URL> \
+           --instance_info kernel=<Kernel URL> \
+           --instance_info ramdisk=<Initial Ramdisk URL> \
+           --instance_info stage2=<Installer Stage2 Ramdisk URL>
+
+When doing so, you may wish to also utilize a customized kickstart template,
+which can also be a URL. Please reference the ironic community provided
+template *ks.cfg.template* and use it as a basis of your own kickstart
+as it accounts for the particular stages and appropriate callbacks to
+Ironic.
+
+.. warning::
+   The default template expects a ``instance_info\liveimg_url`` setting to
+   be provided by the user, which serves as a base operating system image.
+   In the context of the anaconda driver, it should be thought of almost
+   like "stage3". If you're using a custom template, it may not be required,
+   but proceed with caution.
+   See `pykickstart documentation <https://pykickstart.readthedocs.io/en/latest/kickstart-docs.html#liveimg>`_
+   for more information on liveimg file format, structure, and use.
+
+.. code-block:: shell
+
+        baremetal node set <node> \
+            --instance_info ks_template=<URL>
+
+If you do choose to use a liveimg with a customized template, or if you wish
+to use the stock template with a liveimg, you will need to provide parameter.
+
+.. code-block:: shell
+
+        baremetal node set <node> \
+            --instance_info liveimg_url=<URL>
+
+.. warning::
+   This is required if you do *not* utilize a customised template. As in use
+   Ironic's stock template.
+
+The pattern of deployment in this case is identical to a deployment case
+where Ironic is integrated with OpenStack, however in this case Ironic
+collects the files, and stages them appropriately.
+
+At this point, you should be able to request the baremetal node to deploy.
+
+Deployment Process
+------------------
+
+At a high level, the mechanics of the anaconda driver works in the following
+flow, where we also note the stages and purpose of each part for informational
+purposes.
+
+#. Network Boot Program (Such as iPXE) downloads the kernel, and initial
+   ramdisk.
+#. Kernel launches, uncompresses initial ramdisk, and executes init inside
+   of the ramdisk.
+#. The initial ramdisk boot scripts, such as Dracut, recognize the kernel
+   command line parameters Ironic supplied with the boot configuration,
+   and downloads the second stage artifacts, in this case called the
+   ``stage2`` image. This image contains Anaconda and base dependencies.
+#. Anaconda downloads and parses the kickstart configuration which was
+   also supplied on the kernel command line, and executes the commands
+   as defined in the kickstart template.
+#. The kickstart template, if specified in its contents, downloads a
+   ``liveimg`` which is used as the base operating system image to
+   start with.
+
 Limitations
 -----------
 

ironic/common/exception.py

@@ -835,3 +835,19 @@ class IncorrectConfiguration(IronicException):
 
 class NodeVerifyFailure(IronicException):
     _msg_fmt = _("Failed to verify node %(node)s: %(reason)s")
+
+
+class ImageRefIsARedirect(IronicException):
+    _msg_fmt = _("Received a URL redirect when attempting to evaluate "
+                 "image reference %(image_ref)s pointing to "
+                 "%(redirect_url)s. This may, or may not be recoverable.")
+    redirect_url = None
+
+    def __init__(self, image_ref=None, redirect_url=None, msg=None):
+        self.redirect_url = redirect_url
+        # Kwargs are expected by ironic_lib's IronicException to convert
+        # the message.
+        super(ImageRefIsARedirect, self).__init__(
+            message=msg,
+            image_ref=image_ref,
+            redirect_url=redirect_url)

ironic/common/image_service.py

@@ -86,6 +86,9 @@ class HttpImageService(BaseImageService):
             shown in exception message.
         :raises: exception.ImageRefValidationFailed if HEAD request failed or
             returned response code not equal to 200.
+        :raises: exception.ImageRefIsARedirect if the supplied URL is a
+            redirect to a different URL. The caller may be able to handle
+            this.
         :returns: Response to HEAD request.
         """
         output_url = 'secreturl' if secret else image_href
@@ -97,8 +100,46 @@ class HttpImageService(BaseImageService):
             verify = CONF.webserver_verify_ca
 
         try:
+            # NOTE(TheJulia): Head requests do not work on things that are not
+            # files, but they can be responded with redirects or a 200 OK....
+            # We don't want to permit endless redirects either, thus not
+            # request an override to the requests default to try and resolve
+            # redirects as otherwise we might end up with something like
+            # HTTPForbidden or a list of files. Both should be okay to at
+            # least know things are okay in a limited fashion.
             response = requests.head(image_href, verify=verify,
                                      timeout=CONF.webserver_connection_timeout)
+
+            if response.status_code == http_client.MOVED_PERMANENTLY:
+                # NOTE(TheJulia): In the event we receive a redirect, we need
+                # to notify the caller. Before this we would just fail,
+                # but a url which is missing a trailing slash results in a
+                # redirect to a target path, and the caller *may* actually
+                # care about that.
+                redirect = requests.Session().get_redirect_target(response)
+
+                # Extra guard because this is pointless if there is no
+                # location in the field. Requests also properly formats
+                # our string for us, or gives us None.
+                if redirect:
+                    raise exception.ImageRefIsARedirect(
+                        image_ref=image_href,
+                        redirect_url=redirect)
+
+            if (response.status_code == http_client.FORBIDDEN
+                    and str(image_href).endswith('/')):
+                LOG.warning('Attempted to validate a URL %s, however we '
+                            'received an HTTP Forbidden response and the '
+                            'url ends with trailing slash (/), suggesting '
+                            'non-image deploy may be in progress with '
+                            'a webserver which is not permitting an index '
+                            'to be generated. We will treat this as valid, '
+                            'but return the response.', image_href)
+                return response
+
+            # NOTE(TheJulia): Any file list reply will proceed past here just
+            # fine as they are conveyed as an HTTP 200 OK response with a
+            # server rendered HTML document payload.
             if response.status_code != http_client.OK:
                 raise exception.ImageRefValidationFailed(
                     image_href=output_url,

ironic/common/images.py

@@ -580,7 +580,8 @@ def is_whole_disk_image(ctx, instance_info):
     :param instance_info: a node's instance info dict
 
     :returns: True for whole disk images and False for partition images
-        and None on no image_source or Error.
+        and None on no image_source, the source being a path, or upon an
+        Error.
     """
     image_source = instance_info.get('image_source')
     if not image_source:
@@ -606,6 +607,11 @@ def is_whole_disk_image(ctx, instance_info):
                                and not iproperties.get('ramdisk_id'))
     else:
         # Non glance image ref
+        if is_source_a_path(ctx, instance_info.get('image_source')):
+            # Nothing is returned if not valid or there was an error.
+            # A third possibility is it is not a disk image, which would
+            # still be None.
+            return
         if (not instance_info.get('kernel')
             and not instance_info.get('ramdisk')):
             is_whole_disk_image = True
@@ -613,6 +619,73 @@ def is_whole_disk_image(ctx, instance_info):
     return is_whole_disk_image
 
 
+def is_source_a_path(ctx, image_source):
+    """Determine if the image source is a path.
+
+    This method determines if a supplied URL is a path.
+
+    :param ctx: an admin/process context.
+    :param image_source: The supplied image source, expected to be a
+                         URL, which can be used to attempt to determine
+                         if the source is a path.
+    :returns: True if the image_source appears to be a path as opposed
+              to an image to be downloaded. If the image source is not
+              a path, False is returned. If any error is detected,
+              None is returned.
+    """
+    if not image_source:
+        return
+    image_service = service.get_image_service(image_source,
+                                              context=ctx)
+    try:
+        res = image_service.validate_href(image_source)
+        if 'headers' in dir(res):
+            # response/result is from the HTTP check path.
+            headers = res.headers
+        else:
+            # We have no headers.
+            headers = {}
+    except exception.ImageRefIsARedirect as e:
+        # Our exception handling formats this for us in this
+        # case. \o/
+        LOG.debug(str(e))
+        # Servers redirect to a proper folder ending in a slash if
+        # not supplied originally.
+        if e.redirect_url and e.redirect_url.endswith('/'):
+            return True
+    except Exception:
+        # NOTE(TheJulia): I don't really like this pattern, *but*
+        # the wholedisk image support is similar.
+        return
+    # When issuing a head request, folders have no length
+    # A list can be generated by the server.. This is a solid
+    # hint.
+    if 'Content-Length' in headers:
+        LOG.debug('Evaluated %(url)s to determine if it is a URL to a path '
+                  'or a file. A Content-Length header was returned '
+                  'suggesting file.',
+                  {'url': image_source})
+        # NOTE(TheJulia): Files on a webserver have a length which is returned
+        # when headres are queried.
+        return False
+    if ('Content-Type' in headers
+        and str(headers['Content-Type']).startswith('text')
+        and 'Content-Length' not in headers):
+        LOG.debug('Evaluated %(url)s to determine if it is a URL to a path '
+                  'or a file. A Content-Type header was returned with a text '
+                  'content, which suggests a file list was returned.',
+                  {'url': image_source})
+        return True
+        # NOTE(TheJulia): Files should have been caught almost exclusively
+        # before with the Content-Length check.
+    if image_source.endswith('/'):
+        # If all else fails, looks like a URL, and the server didn't give
+        # us any hints.
+        return True
+    # We were unable to determine if this was a folder or a file.
+    return False
+
+
 def _extract_iso(extract_iso, extract_dir):
     # NOTE(rpittau): we could probably just extract the files we need
     # if we find them. Also we probably need to detect the correct iso

ironic/conductor/manager.py

@@ -1096,6 +1096,7 @@ class ConductorManager(base_manager.BaseConductorManager):
             node.del_driver_internal_info('instance')
             node.del_driver_internal_info('root_uuid_or_disk_id')
             node.del_driver_internal_info('is_whole_disk_image')
+            node.del_driver_internal_info('is_source_a_path')
             node.del_driver_internal_info('deploy_boot_mode')
             if node.driver_internal_info.get('automatic_lessee'):
                 # Remove lessee, as it was automatically added

ironic/conductor/utils.py

@@ -1658,7 +1658,21 @@ def update_image_type(context, node):
     """
     iwdi = images.is_whole_disk_image(context, node.instance_info)
     if iwdi is None:
-        return False
+        isap = images.is_source_a_path(
+            context,
+            node.instance_info.get('image_source')
+        )
+        if isap is None:
+            return False
+        node.set_driver_internal_info('is_source_a_path', isap)
+        # TBD(TheJulia): should we need to set image_type back?
+        # rloo doesn't believe we should. I'm kind of on board with that
+        # idea since it is also user-settable, but laregely is just geared
+        # to take what is in glance. Line below should we wish to uncomment.
+        # node.set_instance_info('image_type', images.IMAGE_TYPE_DIRECTORY)
+        # An alternative is to explictly allow it to be configured by the
+        # caller/requester.
+        return True
 
     node.set_driver_internal_info('is_whole_disk_image', iwdi)
     # We need to gradually phase out is_whole_disk_image in favour of

ironic/drivers/modules/deploy_utils.py

@@ -561,8 +561,10 @@ def validate_image_properties(task, deploy_info):
     :raises: MissingParameterValue if the image doesn't contain
         the mentioned properties.
     """
+    node = task.node
     image_href = deploy_info.get('image_source')
     boot_iso = deploy_info.get('boot_iso')
+    isap = task.node.driver_internal_info.get('is_source_a_path')
     if image_href and boot_iso:
         raise exception.InvalidParameterValue(_(
             "An 'image_source' and 'boot_iso' parameter may not be "
@@ -573,15 +575,22 @@ def validate_image_properties(task, deploy_info):
     boot_option = get_boot_option(task.node)
 
     if (boot_iso
-            or task.node.driver_internal_info.get('is_whole_disk_image')
-            or boot_option == 'local'):
+            or node.driver_internal_info.get('is_whole_disk_image')
+            or boot_option == 'local'
+            or isap):
         # No image properties are required in this case, but validate that the
         # image at least looks reasonable.
         try:
+            # This doesn't actually test *getting* the defined url or file
+            # but actually validates we can parse the data *to* connect.
             image_service.get_image_service(image_href, context=task.context)
         except exception.ImageRefValidationFailed as e:
             raise exception.InvalidParameterValue(err=e)
-        return
+        if not isap:
+            # If the source is not a path, but otherwise matches, we need to
+            # exit validation here. Deployments, such as ramdisk or anaconda
+            # need further parameter validation and this supplies it.
+            return
 
     if service_utils.is_glance_image(image_href):
         properties = ['kernel_id', 'ramdisk_id']
@@ -589,6 +598,7 @@ def validate_image_properties(task, deploy_info):
             properties.append('stage2_id')
         image_props = get_image_properties(task.context, image_href)
     else:
+        # We are likely netbooting in this case...
         properties = ['kernel', 'ramdisk']
         image_props = {}
 
@@ -829,7 +839,7 @@ def get_image_instance_info(node):
 _ERR_MSG_INVALID_DEPLOY = _("Invalid parameter %(param)s: %(reason)s")
 
 
-def parse_instance_info(node):
+def parse_instance_info(node, image_deploy=True):
     """Gets the instance specific Node deployment info.
 
     This method validates whether the 'instance_info' property of the
@@ -837,6 +847,9 @@ def parse_instance_info(node):
     deploy images to the node.
 
     :param node: a single Node.
+    :param image_deploy: If the deployment interface is aware this
+                         is an image based deployment, default
+                         True.
     :returns: A dict with the instance_info values.
     :raises: MissingParameterValue, if any of the required parameters are
         missing.
@@ -856,7 +869,12 @@ def parse_instance_info(node):
                     i_info['image_source'])):
             i_info['kernel'] = info.get('kernel')
             i_info['ramdisk'] = info.get('ramdisk')
-        i_info['root_gb'] = info.get('root_gb')
+        if image_deploy:
+            # root_gb is expected with partition images.
+            i_info['root_gb'] = info.get('root_gb')
+
+    # NOTE(TheJulia): Kernel/ramdisk may be optional and originated
+    # with pure workload network booting.
 
     error_msg = _("Some parameters were missing in node's instance_info")
     check_for_missing_params(i_info, error_msg)
@@ -879,7 +897,8 @@ def parse_instance_info(node):
             err_msg_invalid = _("Cannot deploy whole disk image with "
                                 "swap or ephemeral size set")
             raise exception.InvalidParameterValue(err_msg_invalid)
-    else:
+    elif image_deploy:
+        # NOTE(TheJulia): This only applies to partition image deploys.
         _validate_layout_properties(node, info, i_info)
 
     i_info['configdrive'] = info.get('configdrive')
@@ -1066,6 +1085,10 @@ def _validate_image_url(node, url, secret=False):
         it will not be shown in logs.
     """
     try:
+        # NOTE(TheJulia): This method only validates that an exception
+        # is NOT raised. In other words, that the endpoint does not
+        # return a 200. If we're fed a folder list, this will still
+        # work, which is a good and bad thing at the same time. :/
         image_service.HttpImageService().validate_href(url, secret)
     except exception.ImageRefValidationFailed as e:
         with excutils.save_and_reraise_exception():
@@ -1178,6 +1201,10 @@ def build_instance_info_for_deploy(task):
     instance_info = node.instance_info
     iwdi = node.driver_internal_info.get('is_whole_disk_image')
     image_source = instance_info['image_source']
+    isap = node.driver_internal_info.get('is_source_a_path')
+    # If our url ends with a /, i.e. we have been supplied with a path,
+    # we can only deploy this in limited cases for drivers and tools
+    # which are aware of such. i.e. anaconda.
     image_download_source = get_image_download_source(node)
     boot_option = get_boot_option(task.node)
 
@@ -1207,17 +1234,35 @@ def build_instance_info_for_deploy(task):
             instance_info['ramdisk'] = image_info['properties']['ramdisk_id']
     elif (image_source.startswith('file://')
           or image_download_source == 'local'):
+        # NOTE(TheJulia): Intentionally only supporting file:/// as image
+        # based deploy source since we don't want to, nor should we be in
+        # in the business of copying large numbers of files as it is a
+        # huge performance impact.
         _cache_and_convert_image(task, instance_info)
     else:
-        _validate_image_url(node, image_source)
-        instance_info['image_url'] = image_source
+        try:
+            _validate_image_url(node, image_source)
+            # image_url is internal, and used by IPA and some boot templates.
+            # in most cases, it needs to come from image_source explicitly.
+            instance_info['image_url'] = image_source
+        except exception.ImageRefIsARedirect as e:
+            if e.redirect_url:
+                instance_info['image_url'] = e.redirect_url
+            else:
+                raise
 
-    if not iwdi:
-        instance_info['image_type'] = images.IMAGE_TYPE_PARTITION
-        i_info = parse_instance_info(node)
-        instance_info.update(i_info)
+    if not isap:
+        if not iwdi:
+            instance_info['image_type'] = images.IMAGE_TYPE_PARTITION
+            i_info = parse_instance_info(node)
+            instance_info.update(i_info)
+        else:
+            instance_info['image_type'] = images.IMAGE_TYPE_WHOLE_DISK
     else:
-        instance_info['image_type'] = images.IMAGE_TYPE_WHOLE_DISK
+        # Call central parsing so we retain things like config drives.
+        i_info = parse_instance_info(node, image_deploy=False)
+        instance_info.update(i_info)
+
     return instance_info
 
 

ironic/drivers/modules/ks.cfg.template

@@ -16,6 +16,9 @@ clearpart --all --initlabel
 autopart
 
 # Downloading and installing OS image using liveimg section is mandatory
+# in a *default* ironic configuration. Users (infrastructure operators)
+# may choose to customize this pattern, or use release specific kickstart
+# configurations which may already point to a mirror.
 liveimg --url {{ ks_options.liveimg_url }}
 
 # Following %pre and %onerror sections are mandatory

ironic/tests/unit/common/test_image_service.py

@@ -194,6 +194,34 @@ class HttpImageServiceTestCase(base.TestCase):
         head_mock.assert_called_once_with(self.href, verify=False,
                                           timeout=60)
 
+    @mock.patch.object(requests, 'head', autospec=True)
+    def test_validate_href_path_forbidden(self, head_mock):
+        cfg.CONF.set_override('webserver_verify_ca', 'True')
+
+        response = head_mock.return_value
+        response.status_code = http_client.FORBIDDEN
+        url = self.href + '/'
+        resp = self.service.validate_href(url)
+        head_mock.assert_called_once_with(url, verify=True,
+                                          timeout=60)
+        self.assertEqual(http_client.FORBIDDEN, resp.status_code)
+
+    @mock.patch.object(requests, 'head', autospec=True)
+    def test_validate_href_path_redirected(self, head_mock):
+        cfg.CONF.set_override('webserver_verify_ca', 'True')
+
+        response = head_mock.return_value
+        response.status_code = http_client.MOVED_PERMANENTLY
+        url = self.href + '/'
+        new_url = 'http://new-url'
+        response.headers = {'location': new_url}
+        exc = self.assertRaises(exception.ImageRefIsARedirect,
+                                self.service.validate_href,
+                                url)
+        self.assertEqual(new_url, exc.redirect_url)
+        head_mock.assert_called_once_with(url, verify=True,
+                                          timeout=60)
+
     @mock.patch.object(requests, 'head', autospec=True)
     def _test_show(self, head_mock, mtime, mtime_date):
         head_mock.return_value.status_code = http_client.OK

ironic/tests/unit/common/test_images.py

@@ -274,26 +274,86 @@ class IronicImagesTestCase(base.TestCase):
     def test_is_whole_disk_image_partition_non_glance(self, mock_igi,
                                                       mock_gip):
         mock_igi.return_value = False
-        instance_info = {'image_source': 'partition_image',
-                         'kernel': 'kernel',
-                         'ramdisk': 'ramdisk'}
+        instance_info = {
+            'image_source': 'fcf5a777-d9d2-4b86-b3da-bb0b61d5a291',
+            'kernel': 'kernel',
+            'ramdisk': 'ramdisk'}
         is_whole_disk_image = images.is_whole_disk_image('context',
                                                          instance_info)
         self.assertFalse(is_whole_disk_image)
         self.assertFalse(mock_gip.called)
         mock_igi.assert_called_once_with(instance_info['image_source'])
 
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
     @mock.patch.object(images, 'get_image_properties', autospec=True)
     @mock.patch.object(glance_utils, 'is_glance_image', autospec=True)
     def test_is_whole_disk_image_whole_disk_non_glance(self, mock_igi,
-                                                       mock_gip):
+                                                       mock_gip,
+                                                       mock_validate):
         mock_igi.return_value = False
-        instance_info = {'image_source': 'whole_disk_image'}
+        instance_info = {
+            'image_source': 'http://whole-disk-image'}
         is_whole_disk_image = images.is_whole_disk_image('context',
                                                          instance_info)
         self.assertTrue(is_whole_disk_image)
         self.assertFalse(mock_gip.called)
         mock_igi.assert_called_once_with(instance_info['image_source'])
+        mock_validate.assert_called_once_with(mock.ANY,
+                                              'http://whole-disk-image')
+
+    def test_is_source_a_path_returns_none(self):
+        self.assertIsNone(images.is_source_a_path('context', {}))
+
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
+    def test_is_source_a_path_simple(self, validate_mock):
+        mock_response = mock.Mock()
+        mock_response.headers = {}
+        validate_mock.return_value = mock_response
+        self.assertTrue(images.is_source_a_path('context', 'http://foo/bar/'))
+        validate_mock.assert_called_once_with(mock.ANY, 'http://foo/bar/')
+
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
+    def test_is_source_a_path_content_length(self, validate_mock):
+        mock_response = mock.Mock()
+        mock_response.headers = {'Content-Length': 1}
+        validate_mock.return_value = mock_response
+        self.assertFalse(images.is_source_a_path('context', 'http://foo/bar/'))
+        validate_mock.assert_called_once_with(mock.ANY, 'http://foo/bar/')
+
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
+    def test_is_source_a_path_content_type(self, validate_mock):
+        mock_response = mock.Mock()
+        mock_response.headers = {'Content-Type': 'text/html'}
+        validate_mock.return_value = mock_response
+        self.assertTrue(images.is_source_a_path('context', 'http://foo/bar'))
+        validate_mock.assert_called_once_with(mock.ANY, 'http://foo/bar')
+
+    @mock.patch.object(images, 'LOG', autospec=True)
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
+    def test_is_source_a_path_redirect(self, validate_mock, log_mock):
+        url = 'http://foo/bar'
+        redirect_url = url + '/'
+        validate_mock.side_effect = exception.ImageRefIsARedirect(
+            url, redirect_url)
+        self.assertTrue(images.is_source_a_path('context', url))
+        log_mock.debug.assert_called_once_with('Received a URL redirect when '
+                                               'attempting to evaluate image '
+                                               'reference http://foo/bar '
+                                               'pointing to http://foo/bar/. '
+                                               'This may, or may not be '
+                                               'recoverable.')
+
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
+    def test_is_source_a_path_other_error(self, validate_mock):
+        url = 'http://foo/bar'
+        validate_mock.side_effect = OSError
+        self.assertIsNone(images.is_source_a_path('context', url))
 
 
 class FsImageTestCase(base.TestCase):

ironic/tests/unit/conductor/test_deployments.py

@@ -385,6 +385,104 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
     def test_start_deploy_records_lessee(self):
         self._test_start_deploy(automatic_lessee=True)
 
+    @mock.patch.object(images, 'is_source_a_path', autospec=True)
+    @mock.patch.object(task_manager.TaskManager, 'process_event',
+                       autospec=True)
+    @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
+                autospec=True)
+    @mock.patch('ironic.drivers.modules.fake.FakeDeploy.validate',
+                autospec=True)
+    @mock.patch.object(conductor_steps,
+                       'validate_user_deploy_steps_and_templates',
+                       autospec=True)
+    @mock.patch.object(conductor_utils, 'validate_instance_info_traits',
+                       autospec=True)
+    @mock.patch.object(images, 'is_whole_disk_image', autospec=True)
+    def test_start_deploy_source_path(
+            self, mock_iwdi, mock_validate_traits,
+            mock_validate_deploy_user_steps_and_templates,
+            mock_deploy_validate, mock_power_validate,
+            mock_process_event, mock_is_source_a_path):
+        self._start_service()
+        mock_iwdi.return_value = None
+        mock_is_source_a_path.return_value = True
+        deploy_steps = [{"interface": "bios", "step": "factory_reset",
+                         "priority": 95}]
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          provision_state=states.AVAILABLE,
+                                          target_provision_state=states.ACTIVE)
+        task = task_manager.TaskManager(self.context, node.uuid)
+
+        deployments.start_deploy(task, self.service, configdrive=None,
+                                 event='deploy', deploy_steps=deploy_steps)
+        node.refresh()
+        mock_iwdi.assert_called_once_with(task.context,
+                                          task.node.instance_info)
+        mock_is_source_a_path.assert_called_once_with(
+            task.context,
+            task.node.instance_info.get('image_source')
+        )
+        self.assertNotIn('is_whole_disk_iamge', node.driver_internal_info)
+        self.assertTrue(node.driver_internal_info['is_source_a_path'])
+        mock_power_validate.assert_called_once_with(task.driver.power, task)
+        mock_deploy_validate.assert_called_once_with(task.driver.deploy, task)
+        mock_validate_traits.assert_called_once_with(task.node)
+        mock_validate_deploy_user_steps_and_templates.assert_called_once_with(
+            task, deploy_steps, skip_missing=True)
+        mock_process_event.assert_called_with(
+            mock.ANY, 'deploy', call_args=(
+                deployments.do_node_deploy, task, 1, None, deploy_steps),
+            callback=mock.ANY, err_handler=mock.ANY)
+
+    @mock.patch.object(images, 'is_source_a_path', autospec=True)
+    @mock.patch.object(task_manager.TaskManager, 'process_event',
+                       autospec=True)
+    @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
+                autospec=True)
+    @mock.patch('ironic.drivers.modules.fake.FakeDeploy.validate',
+                autospec=True)
+    @mock.patch.object(conductor_steps,
+                       'validate_user_deploy_steps_and_templates',
+                       autospec=True)
+    @mock.patch.object(conductor_utils, 'validate_instance_info_traits',
+                       autospec=True)
+    @mock.patch.object(images, 'is_whole_disk_image', autospec=True)
+    def test_start_deploy_source_path_none(
+            self, mock_iwdi, mock_validate_traits,
+            mock_validate_deploy_user_steps_and_templates,
+            mock_deploy_validate, mock_power_validate,
+            mock_process_event, mock_is_source_a_path):
+        self._start_service()
+        mock_iwdi.return_value = None
+        mock_is_source_a_path.return_value = None
+        deploy_steps = [{"interface": "bios", "step": "factory_reset",
+                         "priority": 95}]
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          provision_state=states.AVAILABLE,
+                                          target_provision_state=states.ACTIVE)
+        task = task_manager.TaskManager(self.context, node.uuid)
+
+        deployments.start_deploy(task, self.service, configdrive=None,
+                                 event='deploy', deploy_steps=deploy_steps)
+        node.refresh()
+        mock_iwdi.assert_called_once_with(task.context,
+                                          task.node.instance_info)
+        mock_is_source_a_path.assert_called_once_with(
+            task.context,
+            task.node.instance_info.get('image_source')
+        )
+        self.assertNotIn('is_whole_disk_iamge', node.driver_internal_info)
+        self.assertNotIn('is_source_a_path', node.driver_internal_info)
+        mock_power_validate.assert_called_once_with(task.driver.power, task)
+        mock_deploy_validate.assert_called_once_with(task.driver.deploy, task)
+        mock_validate_traits.assert_called_once_with(task.node)
+        mock_validate_deploy_user_steps_and_templates.assert_called_once_with(
+            task, deploy_steps, skip_missing=True)
+        mock_process_event.assert_called_with(
+            mock.ANY, 'deploy', call_args=(
+                deployments.do_node_deploy, task, 1, None, deploy_steps),
+            callback=mock.ANY, err_handler=mock.ANY)
+
 
 @mgr_utils.mock_record_keepalive
 class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin,

ironic/tests/unit/conductor/test_manager.py

@@ -2410,7 +2410,8 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
             provision_state=states.DELETING,
             target_provision_state=states.AVAILABLE,
             instance_info={'foo': 'bar'},
-            driver_internal_info={'is_whole_disk_image': False})
+            driver_internal_info={'is_whole_disk_image': False,
+                                  'is_source_a_path': None})
 
         task = task_manager.TaskManager(self.context, node.uuid)
         self._start_service()
@@ -2463,7 +2464,8 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
     def _test__do_node_tear_down_ok(self, mock_tear_down, mock_clean,
                                     mock_unbind, mock_console,
                                     enabled_console=False,
-                                    with_allocation=False):
+                                    with_allocation=False,
+                                    source_a_path=False):
         # test when driver.deploy.tear_down succeeds
         node = obj_utils.create_test_node(
             self.context, driver='fake-hardware',
@@ -2488,6 +2490,10 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
             alloc.node_id = node.id
             alloc.save()
             node.refresh()
+        if source_a_path:
+            d_ii = node.driver_internal_info
+            d_ii['is_source_a_path'] = True
+            node.driver_internal_info = d_ii
 
         task = task_manager.TaskManager(self.context, node.uuid)
         self._start_service()
@@ -2507,6 +2513,7 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self.assertNotIn('root_uuid_or_disk_id', node.driver_internal_info)
         self.assertNotIn('is_whole_disk_image', node.driver_internal_info)
         self.assertNotIn('automatic_lessee', node.driver_internal_info)
+        self.assertNotIn('is_source_a_path', node.driver_internal_info)
         mock_tear_down.assert_called_once_with(task.driver.deploy, task)
         mock_clean.assert_called_once_with(task)
         self.assertEqual({}, port.internal_info)
@@ -2529,6 +2536,9 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
     def test__do_node_tear_down_with_allocation(self):
         self._test__do_node_tear_down_ok(with_allocation=True)
 
+    def test__do_node_tear_down_with_source_path(self):
+        self._test__do_node_tear_down_ok(source_a_path=True)
+
     @mock.patch('ironic.drivers.modules.fake.FakeRescue.clean_up',
                 autospec=True)
     @mock.patch('ironic.conductor.cleaning.do_node_clean', autospec=True)
@@ -7333,7 +7343,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
             provision_state=states.ADOPTING,
             instance_info={
                 'capabilities': {'boot_option': 'netboot'},
-                'image_source': 'image',
+                'image_source': 'http://127.0.0.1/image',
             })
         task = task_manager.TaskManager(self.context, node.uuid)
 

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

@@ -1487,6 +1487,24 @@ class ValidateImagePropertiesTestCase(db_base.DbTestCase):
         utils.validate_image_properties(self.task, inst_info)
         image_service_show_mock.assert_not_called()
 
+    @mock.patch.object(utils, 'get_boot_option', autospec=True,
+                       return_value='kickstart')
+    @mock.patch.object(image_service.HttpImageService, 'show', autospec=True)
+    def test_validate_image_properties_anaconda_deploy_image_source(
+            self, image_service_show_mock, boot_options_mock):
+        d_ii = self.node.driver_internal_info
+        d_ii.pop('is_whole_disk_image')
+        d_ii['is_source_a_path'] = True
+        instance_info = {
+            'kernel': 'file://kernel',
+            'ramdisk': 'file://initrd',
+            'image_source': 'http://foo/bar/'
+        }
+        self.node.instance_info = instance_info
+        inst_info = utils.get_image_instance_info(self.node)
+        utils.validate_image_properties(self.task, inst_info)
+        image_service_show_mock.assert_not_called()
+
 
 class ValidateParametersTestCase(db_base.DbTestCase):
 
@@ -1869,6 +1887,25 @@ class InstanceInfoTestCase(db_base.DbTestCase):
         self.assertNotIn('ephemeral_mb', instance_info)
         self.assertNotIn('swap_mb', instance_info)
 
+    def test_parse_instance_info_non_image_deploy(self):
+        driver_internal_info = dict(DRV_INTERNAL_INFO_DICT)
+        driver_internal_info['is_whole_disk_image'] = None
+        instance_info = {'image_source': 'http://cat/meow/',
+                         'kernel': 'corgi',
+                         'ramdisk': 'mushroom'}
+        node = obj_utils.create_test_node(
+            self.context, instance_info=instance_info,
+            driver_internal_info=driver_internal_info,
+            deploy_interface='anaconda'
+        )
+        instance_info = utils.parse_instance_info(node, image_deploy=False)
+        self.assertIsNotNone(instance_info['image_source'])
+        self.assertNotIn('root_mb', instance_info)
+        self.assertNotIn('ephemeral_mb', instance_info)
+        self.assertNotIn('swap_mb', instance_info)
+        self.assertIn('kernel', instance_info)
+        self.assertIn('ramdisk', instance_info)
+
 
 class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
     def setUp(self):
@@ -2133,6 +2170,59 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
             self.assertRaises(exception.ImageRefValidationFailed,
                               utils.build_instance_info_for_deploy, task)
 
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
+    def test_build_instance_info_for_deploy_source_is_a_path(
+            self, validate_href_mock):
+        i_info = self.node.instance_info
+        driver_internal_info = self.node.driver_internal_info
+        i_info['image_source'] = 'http://image-url/folder/'
+        driver_internal_info.pop('is_whole_disk_image', None)
+        driver_internal_info['is_source_a_path'] = True
+        self.node.instance_info = i_info
+        self.node.driver_internal_info = driver_internal_info
+        self.node.save()
+
+        with task_manager.acquire(
+                self.context, self.node.uuid, shared=False) as task:
+
+            info = utils.build_instance_info_for_deploy(task)
+
+            self.assertEqual(self.node.instance_info['image_source'],
+                             info['image_url'])
+            self.assertNotIn('root_gb', info)
+            validate_href_mock.assert_called_once_with(
+                mock.ANY, 'http://image-url/folder/', False)
+
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
+    def test_build_instance_info_for_deploy_source_redirect(
+            self, validate_href_mock):
+        i_info = self.node.instance_info
+        driver_internal_info = self.node.driver_internal_info
+        url = 'http://image-url/folder'
+        r_url = url + '/'
+        i_info['image_source'] = 'http://image-url/folder'
+        driver_internal_info.pop('is_whole_disk_image', None)
+        driver_internal_info['is_source_a_path'] = True
+        self.node.instance_info = i_info
+        self.node.driver_internal_info = driver_internal_info
+        self.node.save()
+        validate_href_mock.side_effect = exception.ImageRefIsARedirect(
+            image_ref=url,
+            redirect_url=r_url)
+
+        with task_manager.acquire(
+                self.context, self.node.uuid, shared=False) as task:
+
+            info = utils.build_instance_info_for_deploy(task)
+
+            self.assertNotEqual(self.node.instance_info['image_source'],
+                                info['image_url'])
+            self.assertEqual(r_url, info['image_url'])
+            validate_href_mock.assert_called_once_with(
+                mock.ANY, 'http://image-url/folder', False)
+
 
 class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
     def setUp(self):

releasenotes/notes/anaconda-based-deploy-option-sanity-b98fa138747c16d2.yaml

@@ -0,0 +1,21 @@
+---
+fixes:
+  - |
+    Fixes an issue where ``root_gb`` became a required field when using the
+    ``anaconda`` deployment interface, with a recent bug fix as the code path
+    largely expected all deployment operations to utilize images, which is
+    not the case. The case handling for non-image based deployments is now
+    explicitly in internal parameter validation code.
+  - |
+    Fixes handling of ``image_source`` parameters where internal validations
+    would not gracefully handle received redirects and treat it as a failure.
+    We now no longer explicitly fail when a redirect is received.
+  - |
+    Fixes an issue where an ``image_source`` could not be set to a mirror URL
+    to facilitate deployments using a mirror with the ``anaconda`` deployment
+    interface. Ironic still presently has an explicit requirement on a
+    ``stage2`` parameter to be explicitly defined.
+other:
+  - |
+    Adds documentation of standalone deployment use case with the ``anaconda``
+    deployment interface.