Correct Image properties lookup for paths

The image lookup process, when handed a path attempts
to issue a HEAD request against the path and gets a
response which is devoid of details like a content length
or any properties. This is expected behavior, however if
we have a path, we also know we don't need to explicitly
attempt to make an HTTP HEAD request in an attempt to
match the glance ``kernel_id`` -> ``kernel`` and similar
value population behavior.

Also removes an invalid test which was written before the
overall method was fully understood.

And fixes the default fallback for kickstart template
configuration, so that it uses a URL instead of a
direct file path.

And fix logic in the handling of image property result
set, where the code previously assumed a ``stage2``
ramdisk was always required, and based other cleanup
upon that.

Change-Id: I589e9586d1279604a743746952aeabbc483825df
(cherry picked from commit 4d653ac225a26dd83e81477f9d1152fe385c64aa)

3 files changed, 105 insertions, 92 deletions

diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py
index 40ad98217..ccd5f3963 100644
--- a/ironic/common/pxe_utils.py
+++ b/ironic/common/pxe_utils.py
@@ -674,20 +674,33 @@ def get_instance_image_info(task, ipxe_enabled=False):
             os.path.join(root_dir, node.uuid, 'boot_iso'))
 
         return image_info
-
     image_properties = None
     d_info = deploy_utils.get_image_instance_info(node)
+    isap = node.driver_internal_info.get('is_source_a_path')
 
     def _get_image_properties():
-        nonlocal image_properties
-        if not image_properties:
+        nonlocal image_properties, isap
+        if not image_properties and not isap:
             i_service = service.get_image_service(
                 d_info['image_source'],
                 context=ctx)
             image_properties = i_service.show(
                 d_info['image_source'])['properties']
+        # TODO(TheJulia): At some point, we should teach this code
+        # to understand that with a path, it *can* retrieve the
+        # manifest from the HTTP(S) endpoint, which can populate
+        # image_properties, and drive path to variable population
+        # like is done with basically Glance.
 
     labels = ('kernel', 'ramdisk')
+    if not isap:
+        anaconda_labels = ('stage2', 'ks_template', 'ks_cfg')
+    else:
+        # When a path is used, a stage2 ramdisk can be determiend
+        # automatically by anaconda, so it is not an explicit
+        # requirement.
+        anaconda_labels = ('ks_template', 'ks_cfg')
+
     if not (i_info.get('kernel') and i_info.get('ramdisk')):
         # NOTE(rloo): If both are not specified in instance_info
         # we won't use any of them. We'll use the values specified
@@ -700,20 +713,13 @@ def get_instance_image_info(task, ipxe_enabled=False):
                 i_info[label] = str(image_properties[label + '_id'])
             node.instance_info = i_info
             node.save()
+        # TODO(TheJulia): Add functionality to look/grab the hints file
+        # for anaconda and just run with the entire path.
 
-    anaconda_labels = ()
-    if deploy_utils.get_boot_option(node) == 'kickstart':
-        isap = node.driver_internal_info.get('is_source_a_path')
         # stage2: installer stage2 squashfs image
         # ks_template: anaconda kickstart template
         # ks_cfg - rendered ks_template
-        if not isap:
-            anaconda_labels = ('stage2', 'ks_template', 'ks_cfg')
-        else:
-            # When a path is used, a stage2 ramdisk can be determiend
-            # automatically by anaconda, so it is not an explicit
-            # requirement.
-            anaconda_labels = ('ks_template', 'ks_cfg')
+
         # NOTE(rloo): We save stage2 & ks_template values in case they
         # are changed by the user after we start using them and to
         # prevent re-computing them again.
@@ -733,26 +739,31 @@ def get_instance_image_info(task, ipxe_enabled=False):
                 else:
                     node.set_driver_internal_info(
                         'stage2', str(image_properties['stage2_id']))
-        # NOTE(TheJulia): A kickstart template is entirely independent
-        # of the stage2 ramdisk. In the end, it was the configuration which
-        # told anaconda how to execute.
-        if i_info.get('ks_template'):
-            # If the value is set, we always overwrite it, in the event
-            # a rebuild is occuring or something along those lines.
-            node.set_driver_internal_info('ks_template',
-                                          i_info['ks_template'])
+    # NOTE(TheJulia): A kickstart template is entirely independent
+    # of the stage2 ramdisk. In the end, it was the configuration which
+    # told anaconda how to execute.
+    if i_info.get('ks_template'):
+        # If the value is set, we always overwrite it, in the event
+        # a rebuild is occuring or something along those lines.
+        node.set_driver_internal_info('ks_template',
+                                      i_info['ks_template'])
+    else:
+        _get_image_properties()
+        # ks_template is an optional property on the image
+        if image_properties and 'ks_template' in image_properties:
+            node.set_driver_internal_info(
+                'ks_template', str(image_properties['ks_template']))
         else:
-            _get_image_properties()
-            # ks_template is an optional property on the image
-            if 'ks_template' not in image_properties:
-                # If not defined, default to the overall system default
-                # kickstart template, as opposed to a user supplied
-                # template.
-                node.set_driver_internal_info(
-                    'ks_template', CONF.anaconda.default_ks_template)
-            else:
-                node.set_driver_internal_info(
-                    'ks_template', str(image_properties['ks_template']))
+            # If not defined, default to the overall system default
+            # kickstart template, as opposed to a user supplied
+            # template.
+            node.set_driver_internal_info(
+                'ks_template',
+                'file://' + os.path.abspath(
+                    CONF.anaconda.default_ks_template
+                )
+            )
+
     node.save()
 
     for label in labels + anaconda_labels:
@@ -1249,6 +1260,8 @@ def cache_ramdisk_kernel(task, pxe_info, ipxe_enabled=False):
                                              CONF.deploy.http_root,
                                              'stage2')
         ensure_tree(os.path.dirname(file_path))
+
+    if 'ks_cfg' in pxe_info:
         # ks_cfg is rendered later by the driver using ks_template. It cannot
         # be fetched and cached.
         t_pxe_info.pop('ks_cfg')
diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py
index 037ad7449..e10af1c2d 100644
--- a/ironic/tests/unit/common/test_pxe_utils.py
+++ b/ironic/tests/unit/common/test_pxe_utils.py
@@ -1357,7 +1357,7 @@ class PXEInterfacesTestCase(db_base.DbTestCase):
                                        'LiveOS',
                                        'squashfs.img')),
                          'ks_template':
-                         (CONF.anaconda.default_ks_template,
+                         ('file://' + CONF.anaconda.default_ks_template,
                           os.path.join(CONF.deploy.http_root,
                                        self.node.uuid,
                                        'ks.cfg.template')),
@@ -1375,63 +1375,7 @@ class PXEInterfacesTestCase(db_base.DbTestCase):
             self.assertEqual(expected_info, image_info)
             # In the absense of kickstart template in both instance_info and
             # image default kickstart template is used
-            self.assertEqual(CONF.anaconda.default_ks_template,
-                             image_info['ks_template'][0])
-            calls = [mock.call(task.node), mock.call(task.node)]
-            boot_opt_mock.assert_has_calls(calls)
-            # Instance info gets presedence over kickstart template on the
-            # image
-            properties['properties'] = {'ks_template': 'glance://template_id'}
-            task.node.instance_info['ks_template'] = 'https://server/fake.tmpl'
-            image_show_mock.return_value = properties
-            image_info = pxe_utils.get_instance_image_info(
-                task, ipxe_enabled=False)
-            self.assertEqual('https://server/fake.tmpl',
-                             image_info['ks_template'][0])
-
-    @mock.patch('ironic.drivers.modules.deploy_utils.get_boot_option',
-                return_value='kickstart', autospec=True)
-    @mock.patch.object(image_service.GlanceImageService, 'show', autospec=True)
-    def test_get_instance_image_info_with_kickstart_url(
-            self, image_show_mock, boot_opt_mock):
-        properties = {'properties': {u'kernel_id': u'instance_kernel_uuid',
-                                     u'ramdisk_id': u'instance_ramdisk_uuid',
-                                     u'image_source': u'http://path/to/os/'}}
-
-        expected_info = {'ramdisk':
-                         ('instance_ramdisk_uuid',
-                          os.path.join(CONF.pxe.tftp_root,
-                                       self.node.uuid,
-                                       'ramdisk')),
-                         'kernel':
-                         ('instance_kernel_uuid',
-                          os.path.join(CONF.pxe.tftp_root,
-                                       self.node.uuid,
-                                       'kernel')),
-                         'ks_template':
-                         (CONF.anaconda.default_ks_template,
-                          os.path.join(CONF.deploy.http_root,
-                                       self.node.uuid,
-                                       'ks.cfg.template')),
-                         'ks_cfg':
-                         ('',
-                          os.path.join(CONF.deploy.http_root,
-                                       self.node.uuid,
-                                       'ks.cfg'))}
-        image_show_mock.return_value = properties
-        self.context.auth_token = 'fake'
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=True) as task:
-            dii = task.node.driver_internal_info
-            dii['is_source_a_path'] = True
-            task.node.driver_internal_info = dii
-            task.node.save()
-            image_info = pxe_utils.get_instance_image_info(
-                task, ipxe_enabled=False)
-            self.assertEqual(expected_info, image_info)
-            # In the absense of kickstart template in both instance_info and
-            # image default kickstart template is used
-            self.assertEqual(CONF.anaconda.default_ks_template,
+            self.assertEqual('file://' + CONF.anaconda.default_ks_template,
                              image_info['ks_template'][0])
             calls = [mock.call(task.node), mock.call(task.node)]
             boot_opt_mock.assert_has_calls(calls)
@@ -1463,7 +1407,7 @@ class PXEInterfacesTestCase(db_base.DbTestCase):
                                        self.node.uuid,
                                        'kernel')),
                          'ks_template':
-                         (CONF.anaconda.default_ks_template,
+                         ('file://' + CONF.anaconda.default_ks_template,
                           os.path.join(CONF.deploy.http_root,
                                        self.node.uuid,
                                        'ks.cfg.template')),
@@ -1490,7 +1434,7 @@ class PXEInterfacesTestCase(db_base.DbTestCase):
             self.assertEqual(expected_info, image_info)
             # In the absense of kickstart template in both instance_info and
             # image default kickstart template is used
-            self.assertEqual(CONF.anaconda.default_ks_template,
+            self.assertEqual('file://' + CONF.anaconda.default_ks_template,
                              image_info['ks_template'][0])
             calls = [mock.call(task.node), mock.call(task.node)]
             boot_opt_mock.assert_has_calls(calls)
@@ -1577,6 +1521,46 @@ class PXEInterfacesTestCase(db_base.DbTestCase):
                                                  list(fake_pxe_info.values()),
                                                  True)
 
+    @mock.patch.object(os, 'chmod', autospec=True)
+    @mock.patch.object(pxe_utils, 'TFTPImageCache', lambda: None)
+    @mock.patch.object(pxe_utils, 'ensure_tree', autospec=True)
+    @mock.patch.object(deploy_utils, 'fetch_images', autospec=True)
+    def test_cache_ramdisk_kernel_ipxe_anaconda(self, mock_fetch_image,
+                                                mock_ensure_tree, mock_chmod):
+        expected_path = os.path.join(CONF.deploy.http_root,
+                                     self.node.uuid)
+        fake_pxe_info = {'ramdisk':
+                         ('instance_ramdisk_uuid',
+                          os.path.join(CONF.pxe.tftp_root,
+                                       self.node.uuid,
+                                       'ramdisk')),
+                         'kernel':
+                         ('instance_kernel_uuid',
+                          os.path.join(CONF.pxe.tftp_root,
+                                       self.node.uuid,
+                                       'kernel')),
+                         'ks_template':
+                         ('file://' + CONF.anaconda.default_ks_template,
+                          os.path.join(CONF.deploy.http_root,
+                                       self.node.uuid,
+                                       'ks.cfg.template')),
+                         'ks_cfg':
+                         ('',
+                          os.path.join(CONF.deploy.http_root,
+                                       self.node.uuid,
+                                       'ks.cfg'))}
+        expected = fake_pxe_info.copy()
+        expected.pop('ks_cfg')
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            pxe_utils.cache_ramdisk_kernel(task, fake_pxe_info,
+                                           ipxe_enabled=True)
+        mock_ensure_tree.assert_called_with(expected_path)
+        mock_fetch_image.assert_called_once_with(self.context, mock.ANY,
+                                                 list(expected.values()),
+                                                 True)
+
 
 @mock.patch.object(pxe.PXEBoot, '__init__', lambda self: None)
 class PXEBuildKickstartConfigOptionsTestCase(db_base.DbTestCase):
diff --git a/releasenotes/notes/correct-source-path-handling-lookups-4ce2023a56372f10.yaml b/releasenotes/notes/correct-source-path-handling-lookups-4ce2023a56372f10.yaml
new file mode 100644
index 000000000..10d270a45
--- /dev/null
+++ b/releasenotes/notes/correct-source-path-handling-lookups-4ce2023a56372f10.yaml
@@ -0,0 +1,16 @@
+---
+fixes:
+  - |
+    Fixes an issue where image information retrieval would fail when a
+    path was supplied when using the ``anaconda`` deploy interface,
+    as `HTTP` ``HEAD`` requests on a URL path have no ``Content-Length``.
+    We now consider if a path is used prior to attempting to collect
+    additional configuration data from what is normally expected to
+    be Glance.
+  - |
+    Fixes an issue where the fallback to a default kickstart template
+    value would result in error indicating
+    "Scheme-less image href is not a UUID".
+    This was becaues the handling code falling back to the default
+    did not explicitly indicate it was a file URL before saving the
+    value.