Do not strip 'glance://' prefix from image hrefs

pxe_utils.get_deploy_kr_info and drivers.modules.pxe._get_image_info
functions strip 'glance://' prefix from image references while it's
not necessary as Glance service can download images by such refs.
Also some refactoring done as these references are not just uuids.

Partially implements: blueprint non-glance-image-refs

Change-Id: Ie0d87c96c80dab23ea81011413600d9dbbf50177

ironic/common/pxe_utils.py

@@ -138,7 +138,7 @@ def _get_pxe_ip_address_path(ip_address):
 
 
 def get_deploy_kr_info(node_uuid, driver_info):
-    """Get uuid and tftp path for deploy kernel and ramdisk.
+    """Get href and tftp path for deploy kernel and ramdisk.
 
     Note: driver_info should be validated outside of this method.
     """
@@ -147,7 +147,7 @@ def get_deploy_kr_info(node_uuid, driver_info):
     for label in ('deploy_kernel', 'deploy_ramdisk'):
         # the values for these keys will look like "glance://image-uuid"
         image_info[label] = (
-            str(driver_info[label]).split('/')[-1],
+            str(driver_info[label]),
             os.path.join(root_dir, node_uuid, label)
         )
     return image_info

ironic/drivers/modules/deploy_utils.py

@@ -424,7 +424,7 @@ def fetch_images(ctx, cache, images_info, force_raw=True):
 
     :param ctx: context
     :param cache: ImageCache instance to use for fetching
-    :param images_info: list of tuples (image uuid, destination path)
+    :param images_info: list of tuples (image href, destination path)
     :param force_raw: boolean value, whether to convert the image to raw
                       format
     :raises: InstanceDeployFailure if unable to find enough disk space
@@ -439,5 +439,5 @@ def fetch_images(ctx, cache, images_info, force_raw=True):
     # if disk space is used between the check and actual download.
     # This is probably unavoidable, as we can't control other
     # (probably unrelated) processes
-    for uuid, path in images_info:
-        cache.fetch_image(uuid, path, ctx=ctx, force_raw=force_raw)
+    for href, path in images_info:
+        cache.fetch_image(href, path, ctx=ctx, force_raw=force_raw)

ironic/drivers/modules/ilo/deploy.py

@@ -89,9 +89,9 @@ def _get_boot_iso(task, root_uuid):
     # Option 1 - Check if user has provided a boot_iso in Glance.
     LOG.debug("Trying to get a boot ISO to boot the baremetal node")
     deploy_info = _parse_deploy_info(task.node)
-    image_uuid = deploy_info['image_source']
+    image_href = deploy_info['image_source']
     boot_iso_uuid = images.get_glance_image_property(task.context,
-            image_uuid, 'boot_iso')
+            image_href, 'boot_iso')
     if boot_iso_uuid:
         LOG.debug("Found boot_iso %s in Glance", boot_iso_uuid)
         return 'glance:%s' % boot_iso_uuid
@@ -105,13 +105,13 @@ def _get_boot_iso(task, root_uuid):
         return
 
     kernel_uuid = images.get_glance_image_property(task.context,
-            image_uuid, 'kernel_id')
+            image_href, 'kernel_id')
     ramdisk_uuid = images.get_glance_image_property(task.context,
-            image_uuid, 'ramdisk_id')
+            image_href, 'ramdisk_id')
     if not kernel_uuid or not ramdisk_uuid:
         LOG.error(_LE("Unable to find 'kernel_id' and 'ramdisk_id' in Glance "
                       "image %(image)s for generating boot ISO for %(node)s"),
-                  {'image': image_uuid, 'node': task.node.uuid})
+                  {'image': image_href, 'node': task.node.uuid})
         return
 
     # NOTE(rameshg87): Functionality to share the boot ISOs created for

ironic/drivers/modules/image_cache.py

@@ -72,14 +72,14 @@ class ImageCache(object):
         if master_dir is not None:
             fileutils.ensure_tree(master_dir)
 
-    def fetch_image(self, uuid, dest_path, ctx=None, force_raw=True):
-        """Fetch image with given uuid to the destination path.
+    def fetch_image(self, href, dest_path, ctx=None, force_raw=True):
+        """Fetch image by given href to the destination path.
 
         Does nothing if destination path exists.
         Only creates a link if master image for this UUID is already in cache.
         Otherwise downloads an image and also stores it in cache.
 
-        :param uuid: image UUID or href to fetch
+        :param href: image UUID or href to fetch
         :param dest_path: destination file path
         :param ctx: context
         :param force_raw: boolean value, whether to convert the image to raw
@@ -90,15 +90,15 @@ class ImageCache(object):
             # NOTE(ghe): We don't share images between instances/hosts
             if not CONF.parallel_image_downloads:
                 with lockutils.lock(img_download_lock_name, 'ironic-'):
-                    _fetch(ctx, uuid, dest_path, self._image_service,
+                    _fetch(ctx, href, dest_path, self._image_service,
                            force_raw)
             else:
-                _fetch(ctx, uuid, dest_path, self._image_service, force_raw)
+                _fetch(ctx, href, dest_path, self._image_service, force_raw)
             return
 
         # TODO(ghe): have hard links and counts the same behaviour in all fs
 
-        master_file_name = service_utils.parse_image_ref(uuid)[0]
+        master_file_name = service_utils.parse_image_ref(href)[0]
         master_path = os.path.join(self.master_dir, master_file_name)
 
         if CONF.parallel_image_downloads:
@@ -109,7 +109,7 @@ class ImageCache(object):
             if os.path.exists(dest_path):
                 LOG.debug("Destination %(dest)s already exists for "
                             "image %(uuid)s" %
-                          {'uuid': uuid,
+                          {'uuid': href,
                            'dest': dest_path})
                 return
 
@@ -120,25 +120,25 @@ class ImageCache(object):
             except OSError:
                 LOG.info(_LI("Master cache miss for image %(uuid)s, "
                              "starting download"),
-                         {'uuid': uuid})
+                         {'uuid': href})
             else:
                 LOG.debug("Master cache hit for image %(uuid)s",
-                          {'uuid': uuid})
+                          {'uuid': href})
                 return
 
             self._download_image(
-                uuid, master_path, dest_path, ctx=ctx, force_raw=force_raw)
+                href, master_path, dest_path, ctx=ctx, force_raw=force_raw)
 
         # NOTE(dtantsur): we increased cache size - time to clean up
         self.clean_up()
 
-    def _download_image(self, uuid, master_path, dest_path, ctx=None,
+    def _download_image(self, href, master_path, dest_path, ctx=None,
                         force_raw=True):
-        """Download image from Glance and store at a given path.
+        """Download image by href and store at a given path.
 
         This method should be called with uuid-specific lock taken.
 
-        :param uuid: image UUID or href to fetch
+        :param href: image UUID or href to fetch
         :param master_path: destination master path
         :param dest_path: destination file path
         :param ctx: context
@@ -148,9 +148,10 @@ class ImageCache(object):
         # TODO(ghe): timeout and retry for downloads
         # TODO(ghe): logging when image cannot be created
         tmp_dir = tempfile.mkdtemp(dir=self.master_dir)
-        tmp_path = os.path.join(tmp_dir, uuid)
+        tmp_path = os.path.join(tmp_dir, href.split('/')[-1])
+
         try:
-            _fetch(ctx, uuid, tmp_path, self._image_service, force_raw)
+            _fetch(ctx, href, tmp_path, self._image_service, force_raw)
             # NOTE(dtantsur): no need for global lock here - master_path
             # will have link count >1 at any moment, so won't be cleaned up
             os.link(tmp_path, master_path)

ironic/drivers/modules/pxe.py

@@ -235,7 +235,7 @@ def _get_image_info(node, ctx):
         glance_service = service.Service(version=1, context=ctx)
         iproperties = glance_service.show(d_info['image_source'])['properties']
         for label in labels:
-            i_info[label] = str(iproperties[label + '_id']).split('/')[-1]
+            i_info[label] = str(iproperties[label + '_id'])
         node.instance_info = i_info
         node.save()
 

ironic/tests/drivers/test_pxe.py

@@ -120,12 +120,12 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
                                        self.node.uuid,
                                        'kernel')),
                          'deploy_ramdisk':
-                         ('deploy_ramdisk_uuid',
+                         (DRV_INFO_DICT['pxe_deploy_ramdisk'],
                            os.path.join(CONF.pxe.tftp_root,
                                         self.node.uuid,
                                         'deploy_ramdisk')),
                          'deploy_kernel':
-                         ('deploy_kernel_uuid',
+                         (DRV_INFO_DICT['pxe_deploy_kernel'],
                           os.path.join(CONF.pxe.tftp_root,
                                        self.node.uuid,
                                        'deploy_kernel'))}

ironic/tests/test_pxe_utils.py

@@ -222,9 +222,9 @@ class TestPXEUtils(db_base.DbTestCase):
         }
 
         expected = {
-            'deploy_kernel': ('deploy-kernel',
+            'deploy_kernel': ('glance://deploy-kernel',
                               expected_dir + '/fake-node/deploy_kernel'),
-            'deploy_ramdisk': ('deploy-ramdisk',
+            'deploy_ramdisk': ('glance://deploy-ramdisk',
                                expected_dir + '/fake-node/deploy_ramdisk'),
         }