From d7d1b6821f48a1a0fcc56e85496c9d0a0c73aedb Mon Sep 17 00:00:00 2001 From: Kien Nguyen Date: Fri, 7 Jul 2017 17:22:20 +0700 Subject: [PATCH] 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 | 27 +++++++-------- zun/image/glance/utils.py | 32 +++++++++--------- zun/tests/unit/image/glance/test_driver.py | 38 ++++++++++------------ 3 files changed, 44 insertions(+), 53 deletions(-) diff --git a/zun/image/glance/driver.py b/zun/image/glance/driver.py index f6d872714..bb3245fc0 100644 --- a/zun/image/glance/driver.py +++ b/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 images + return utils.find_images(context, repo, exact_match) 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 img + # Return a created image + 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: - tags = [] - tags.append(tag) - img = utils.update_image_tags(context, img_id, - tags) - img = utils.update_image_format(context, img_id, disk_format, - container_format) - return img + # NOTE(kiennt): Tags will be an empty list if no tag is defined. + tags = [tag] if tag else [] + # Return the updated image + return utils.update_image(context, img_id, disk_format, + container_format, tags=tags) 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 img + return utils.upload_image_data(context, img_id, data) except Exception as e: raise exception.ZunException(six.text_type(e)) diff --git a/zun/image/glance/utils.py b/zun/image/glance/utils.py index 558742f8a..4bedde2aa 100644 --- a/zun/image/glance/utils.py +++ b/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 img + return glance.images.create(name=image_name) -def update_image_format(context, img_id, disk_format, - container_format): - """Update container format of an image.""" +def update_image(context, img_id, disk_format, + container_format, tags): + """Update an image (container format, disk format & tags)""" glance = create_glanceclient(context) - img = glance.images.update(img_id, disk_format=disk_format, - container_format=container_format) - 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 + return glance.images.update(img_id, disk_format=disk_format, + container_format=container_format, tags=tags) 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 img + return glance.images.upload(img_id, data) + + +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) diff --git a/zun/tests/unit/image/glance/test_driver.py b/zun/tests/unit/image/glance/test_driver.py index 8f794e908..3b67c1dfe 100644 --- a/zun/tests/unit/image/glance/test_driver.py +++ b/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_search, mock_glance): + @mock.patch('zun.image.glance.utils.find_image') + 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, - mock_should_pull_image, mock_search): + def test_pull_image_success(self, mock_find_image, mock_download_image, + mock_should_pull_image, mock_search_on_host): mock_should_pull_image.return_value = True - mock_search.return_value = {'image': 'nginx', 'path': 'xyz', - 'checksum': 'xxx'} - mock_glance.images.data = mock.MagicMock(return_value='content') + mock_search_on_host.return_value = {'image': 'nginx', 'path': 'xyz', + 'checksum': 'xxx'} 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, - mock_glance): + def test_pull_image_not_found(self, mock_should_pull_image): 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_tags') - @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): + @mock.patch('zun.image.glance.utils.update_image') + def test_update_image(self, 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))