Refactor Glance Image driver

- In update_image method, has tag arg or not, always return
  update_image_format. The called method, update_image_tags,
  doesn't do any works.
- Add new method download_image_in_chunks like another methods.
  Init glanceclient in utils module.
- Update testcases.

Change-Id: I3a3e9ec38c39ae4abc12196d907b1f32096ad9b6
Partial-Bug: #1702587

zun/image/glance/driver.py

@@ -88,10 +88,10 @@ class GlanceDriver(driver.ContainerImageDriver):
 
         LOG.debug('Pulling image from glance %s', repo)
         try:
-            glance = utils.create_glanceclient(context)
             image_meta = utils.find_image(context, repo)
             LOG.debug('Image %s was found in glance, downloading now...', repo)
-            image_chunks = glance.images.data(image_meta.id)
+            image_chunks = utils.download_image_in_chunks(context,
+                                                          image_meta.id)
         except exception.ImageNotFound:
             LOG.error('Image %s was not found in glance', repo)
             raise
@@ -118,8 +118,7 @@ class GlanceDriver(driver.ContainerImageDriver):
         LOG.debug('Searching image in glance %s', repo)
         try:
             # TODO(hongbin): find image by both repo and tag
-            images = utils.find_images(context, repo, exact_match)
+            return utils.find_images(context, repo, exact_match)
-            return images
         except Exception as e:
             raise exception.ZunException(six.text_type(e))
 
@@ -127,8 +126,8 @@ class GlanceDriver(driver.ContainerImageDriver):
         """Create an image."""
         LOG.debug('Creating a new image in glance %s', image_name)
         try:
-            img = utils.create_image(context, image_name)
+            # Return a created image
-            return img
+            return utils.create_image(context, image_name)
         except Exception as e:
             raise exception.ZunException(six.text_type(e))
 
@@ -137,14 +136,11 @@ class GlanceDriver(driver.ContainerImageDriver):
         """Update an image."""
         LOG.debug('Updating an image %s in glance', img_id)
         try:
-            if tag is not None:
+            # NOTE(kiennt): Tags will be an empty list if no tag is defined.
-                tags = []
+            tags = [tag] if tag else []
-                tags.append(tag)
+            # Return the updated image
-                img = utils.update_image_tags(context, img_id,
+            return utils.update_image(context, img_id, disk_format,
-                                              tags)
+                                      container_format, tags=tags)
-            img = utils.update_image_format(context, img_id, disk_format,
-                                            container_format)
-            return img
         except Exception as e:
             raise exception.ZunException(six.text_type(e))
 
@@ -152,7 +148,6 @@ class GlanceDriver(driver.ContainerImageDriver):
         """Update an image."""
         LOG.debug('Uploading an image to glance %s', img_id)
         try:
-            img = utils.upload_image_data(context, img_id, data)
+            return utils.upload_image_data(context, img_id, data)
-            return img
         except Exception as e:
             raise exception.ZunException(six.text_type(e))

zun/image/glance/utils.py

@@ -20,6 +20,7 @@ from zun.common import clients
 from zun.common import exception
 
 from oslo_log import log as logging
+
 LOG = logging.getLogger(__name__)
 
 
@@ -71,29 +72,26 @@ def find_images(context, image_ident, exact_match):
 def create_image(context, image_name):
     """Create an image."""
     glance = create_glanceclient(context)
-    img = glance.images.create(name=image_name)
+    return glance.images.create(name=image_name)
-    return img
 
 
-def update_image_format(context, img_id, disk_format,
+def update_image(context, img_id, disk_format,
-                        container_format):
+                 container_format, tags):
-    """Update container format of an image."""
+    """Update an image (container format, disk format & tags)"""
     glance = create_glanceclient(context)
-    img = glance.images.update(img_id, disk_format=disk_format,
+    return glance.images.update(img_id, disk_format=disk_format,
-                               container_format=container_format)
+                                container_format=container_format, tags=tags)
-    return img
-
-
-def update_image_tags(context, img_id, tags):
-    """Adding new tags to the tag list of an image."""
-    glance = create_glanceclient(context)
-    img = glance.images.update(img_id, tags=tags)
-    return img
 
 
 def upload_image_data(context, img_id, data):
     """Upload an image."""
     LOG.debug('Upload image %s ', img_id)
     glance = create_glanceclient(context)
-    img = glance.images.upload(img_id, data)
+    return glance.images.upload(img_id, data)
-    return img
+
+
+def download_image_in_chunks(context, img_id):
+    """Download image in chunks."""
+    LOG.debug('Download image %s', img_id)
+    glance = create_glanceclient(context)
+    return glance.images.data(img_id)

zun/tests/unit/image/glance/test_driver.py

@@ -62,18 +62,19 @@ class TestDriver(base.BaseTestCase):
                                                     'tag', 'never'))
         mock_open_file.assert_any_call('xyz', 'rb')
 
-    @mock.patch('zun.image.glance.utils.create_glanceclient')
     @mock.patch.object(driver.GlanceDriver,
                        '_search_image_on_host')
     @mock.patch('zun.common.utils.should_pull_image')
-    def test_pull_image_failure(self, mock_should_pull_image,
+    @mock.patch('zun.image.glance.utils.find_image')
-                                mock_search, mock_glance):
+    def test_pull_image_failure(self, mock_find_image,
+                                mock_should_pull_image,
+                                mock_search):
         mock_should_pull_image.return_value = True
         mock_search.return_value = {'image': 'nginx', 'path': 'xyz',
                                     'checksum': 'xxx'}
         mock_open_file = mock.mock_open()
         with mock.patch('zun.image.glance.driver.open', mock_open_file):
-            mock_glance.side_effect = Exception
+            mock_find_image.side_effect = Exception
             self.assertRaises(exception.ZunException, self.driver.pull_image,
                               None, 'nonexisting', 'tag', 'always')
         mock_open_file.assert_any_call('xyz', 'rb')
@@ -81,17 +82,17 @@ class TestDriver(base.BaseTestCase):
     @mock.patch.object(driver.GlanceDriver,
                        '_search_image_on_host')
     @mock.patch('zun.common.utils.should_pull_image')
-    @mock.patch('zun.image.glance.utils.create_glanceclient')
+    @mock.patch('zun.image.glance.utils.download_image_in_chunks')
     @mock.patch('zun.image.glance.utils.find_image')
-    def test_pull_image(self, mock_find_image, mock_glance,
+    def test_pull_image_success(self, mock_find_image, mock_download_image,
-                        mock_should_pull_image, mock_search):
+                                mock_should_pull_image, mock_search_on_host):
         mock_should_pull_image.return_value = True
-        mock_search.return_value = {'image': 'nginx', 'path': 'xyz',
+        mock_search_on_host.return_value = {'image': 'nginx', 'path': 'xyz',
-                                    'checksum': 'xxx'}
+                                            'checksum': 'xxx'}
-        mock_glance.images.data = mock.MagicMock(return_value='content')
         image_meta = mock.MagicMock()
         image_meta.id = '1234'
         mock_find_image.return_value = image_meta
+        mock_download_image.return_value = 'content'
         CONF.set_override('images_directory', self.test_dir, group='glance')
         out_path = os.path.join(self.test_dir, '1234' + '.tar')
         mock_open_file = mock.mock_open()
@@ -99,12 +100,14 @@ class TestDriver(base.BaseTestCase):
             ret = self.driver.pull_image(None, 'image', 'latest', 'always')
         mock_open_file.assert_any_call('xyz', 'rb')
         mock_open_file.assert_any_call(out_path, 'wb')
+        self.assertTrue(mock_search_on_host.called)
+        self.assertTrue(mock_should_pull_image.called)
+        self.assertTrue(mock_find_image.called)
+        self.assertTrue(mock_download_image.called)
         self.assertEqual(({'image': 'image', 'path': out_path}, False), ret)
 
-    @mock.patch('zun.image.glance.utils.create_glanceclient')
     @mock.patch('zun.common.utils.should_pull_image')
-    def test_pull_image_not_found(self, mock_should_pull_image,
+    def test_pull_image_not_found(self, mock_should_pull_image):
-                                  mock_glance):
         mock_should_pull_image.return_value = True
         with mock.patch('zun.image.glance.utils.find_image') as mock_find:
             mock_find.side_effect = exception.ImageNotFound
@@ -145,15 +148,10 @@ class TestDriver(base.BaseTestCase):
         self.assertEqual(1, len(ret))
         self.assertTrue(mock_create_image.called)
 
-    @mock.patch.object(driver.GlanceDriver, 'update_image')
+    @mock.patch('zun.image.glance.utils.update_image')
-    @mock.patch('zun.image.glance.utils.update_image_tags')
+    def test_update_image(self, mock_update_image):
-    @mock.patch('zun.image.glance.utils.update_image_format')
-    def test_update_image(self, mock_update_image_format,
-                          mock_update_image_tags, mock_update_image):
         image_meta = mock.MagicMock()
         image_meta.id = '1234'
-        mock_update_image_tags.return_value = [image_meta]
-        mock_update_image_format.return_value = [image_meta]
         mock_update_image.return_value = [image_meta]
         ret = self.driver.update_image(None, 'id', container_format='docker')
         self.assertEqual(1, len(ret))