Fixed potential problems with image creation exceptions.

With Glance client v2, it appears that rolling back on image
creation error was sending in an OpenStack image object into
the delete_image() function where it should have been expecting
a SNAPS-OO Image domain object.
Also added log messages to help expose other potential issues
if this patch does not address the root problem at hand.

JIRA: SNAPS-139

Change-Id: I52591ff1d055ddf4eacbbc49b3bc8718285edcfa
Signed-off-by: spisarski <s.pisarski@cablelabs.com>

2 files changed, 43 insertions, 16 deletions

diff --git a/snaps/openstack/tests/create_image_tests.py b/snaps/openstack/tests/create_image_tests.py
index 6c9b175..5fb39dc 100644
--- a/snaps/openstack/tests/create_image_tests.py
+++ b/snaps/openstack/tests/create_image_tests.py
@@ -524,6 +524,22 @@ class CreateImageNegativeTests(OSIntegrationTestCase):
         except Exception as e:
             self.fail('Invalid Exception ' + str(e))
 
+    def test_bad_image_image_type(self):
+        """
+        Expect an ImageCreationError when the image type bad
+        """
+        os_image_settings = openstack_tests.cirros_image_settings(
+            name=self.image_name)
+        self.image_creator = create_image.OpenStackImage(
+            self.os_creds,
+            create_image.ImageSettings(name=os_image_settings.name,
+                                       image_user=os_image_settings.image_user,
+                                       img_format='foo',
+                                       url=os_image_settings.url))
+
+        with self.assertRaises(Exception):
+            self.image_creator.create()
+
     def test_bad_image_file(self):
         """
         Expect an ImageCreationError when the image file does not exist
diff --git a/snaps/openstack/utils/glance_utils.py b/snaps/openstack/utils/glance_utils.py
index 090846c..8479be3 100644
--- a/snaps/openstack/utils/glance_utils.py
+++ b/snaps/openstack/utils/glance_utils.py
@@ -65,6 +65,19 @@ def get_image(glance, image_name=None):
     return None
 
 
+def get_image_by_id(glance, image_id):
+    """
+    Returns an OpenStack image object for a given name
+    :param glance: the Glance client
+    :param image_id: the image ID to lookup
+    :return: the SNAPS-OO Domain Image object or None
+    """
+    image = glance.images.get(image_id)
+    return Image(
+        name=image['name'], image_id=image['id'],
+        size=image['size'], properties=image.get('properties'))
+
+
 def get_image_status(glance, image):
     """
     Returns a new OpenStack Image object for a given OpenStack image object
@@ -147,14 +160,15 @@ def __create_image_v2(glance, image_settings):
                 image_settings.url, './tmp', file_name)
             image_filename = image_file.name
         except:
-            os.remove('./tmp/' + file_name)
+            if image_file:
+                os.remove(image_file.name)
             raise
 
         cleanup_temp_file = True
     else:
         raise GlanceException('Filename or URL of image not configured')
 
-    created_image = None
+    os_image = None
     try:
         kwargs = dict()
         kwargs['name'] = image_settings.name
@@ -167,24 +181,25 @@ def __create_image_v2(glance, image_settings):
         if image_settings.extra_properties:
             kwargs.update(image_settings.extra_properties)
 
-        created_image = glance.images.create(**kwargs)
+        os_image = glance.images.create(**kwargs)
         image_file = open(image_filename, 'rb')
-        glance.images.upload(created_image['id'], image_file)
+        glance.images.upload(os_image['id'], image_file)
     except:
         logger.error('Unexpected exception creating image. Rolling back')
-        if created_image:
-            delete_image(glance, created_image)
+        if os_image:
+            delete_image(glance, Image(
+                name=os_image['name'], image_id=os_image['id'],
+                size=os_image['size'], properties=os_image.get('properties')))
         raise
     finally:
         if image_file:
+            logger.debug('Closing file %s', image_file.name)
             image_file.close()
         if cleanup_temp_file:
+            logger.info('Removing file %s', image_file.name)
             os.remove(image_filename)
 
-    updated_image = glance.images.get(created_image['id'])
-    return Image(
-        name=updated_image['name'], image_id=updated_image['id'],
-        size=updated_image['size'], properties=updated_image.get('properties'))
+    return get_image_by_id(glance, os_image['id'])
 
 
 def delete_image(glance, image):
@@ -193,12 +208,8 @@ def delete_image(glance, image):
     :param glance: the glance client
     :param image: the image to delete
     """
-    if glance.version == VERSION_1:
-        glance.images.delete(image)
-    elif glance.version == VERSION_2:
-        glance.images.delete(image.id)
-    else:
-        raise GlanceException('Unsupported glance client version')
+    logger.info('Deleting image named - %s', image.name)
+    glance.images.delete(image.id)
 
 
 class GlanceException(Exception):