Merge "Correct Image properties lookup for paths"

3 files changed, 105 insertions, 92 deletions

diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py
index ec0719b75..33a24ecb6 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:
@@ -1253,6 +1264,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 3e57b83ed..4d4fbb5b5 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.