From ffbe3a0fe49e24c47e0f39b738c512ac08073b82 Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Wed, 8 Mar 2017 17:55:53 -0600 Subject: [PATCH] Avoid re-downloading the same image from Glance Each glance image has a checksum that is a hash of image data. This commit leverage the checksum to verify if the same image was already downloaded in before. Change-Id: Ied44f014d9339ed6344ee3c460c6b1d60d4279d3 --- tox.ini | 4 ++- zun/image/glance/driver.py | 22 +++++++++++- zun/tests/unit/image/glance/test_driver.py | 40 ++++++++++++++-------- 3 files changed, 49 insertions(+), 17 deletions(-) diff --git a/tox.ini b/tox.ini index 5dcd581a3..ddef04826 100644 --- a/tox.ini +++ b/tox.ini @@ -25,7 +25,9 @@ commands = commands = doc8 -e .rst doc/source/ CONTRIBUTING.rst HACKING.rst README.rst bash tools/flake8wrap.sh {posargs} - bandit -r zun -x tests -n5 -ll + # The following bandit tests are being skipped: + # B303 - Use of insecure MD2, MD4, or MD5 hash function. + bandit -r zun -x tests -n5 -ll --skip B303 [testenv:venv] commands = {posargs} diff --git a/zun/image/glance/driver.py b/zun/image/glance/driver.py index dccb55e5c..8995a4980 100644 --- a/zun/image/glance/driver.py +++ b/zun/image/glance/driver.py @@ -11,6 +11,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import hashlib import os import six @@ -47,7 +48,10 @@ class GlanceDriver(driver.ContainerImageDriver): out_path = os.path.join(images_directory, image_meta.id + '.tar') if os.path.isfile(out_path): - return {'image': repo, 'path': out_path} + return { + 'image': repo, + 'path': out_path, + 'checksum': image_meta.checksum} else: return None @@ -56,6 +60,22 @@ class GlanceDriver(driver.ContainerImageDriver): # once metadata is stored in db then handle tags image_loaded = False image = self._search_image_on_host(context, repo) + if image: + image_path = image['path'] + image_checksum = image['checksum'] + md5sum = hashlib.md5() + with open(image_path, 'rb') as fd: + while True: + # read 10MB of data each time + data = fd.read(10 * 1024 * 1024) + if not data: + break + md5sum.update(data) + md5sum = md5sum.hexdigest() + if md5sum == image_checksum: + image_loaded = True + return image, image_loaded + if not common_utils.should_pull_image(image_pull_policy, bool(image)): if image: LOG.debug('Image %s present locally' % repo) diff --git a/zun/tests/unit/image/glance/test_driver.py b/zun/tests/unit/image/glance/test_driver.py index 305d70338..a4b0c1ed9 100644 --- a/zun/tests/unit/image/glance/test_driver.py +++ b/zun/tests/unit/image/glance/test_driver.py @@ -52,10 +52,15 @@ class TestDriver(base.BaseTestCase): def test_pull_image_should_pull_no_image_present_locally( self, mock_should_pull_image, mock_search): mock_should_pull_image.return_value = False - mock_search.return_value = {'image': 'nginx', 'path': 'xyz'} - self.assertEqual(({'image': 'nginx', 'path': 'xyz'}, True), - self.driver.pull_image(None, 'nonexisting', - 'tag', 'never')) + 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): + self.assertEqual(({'image': 'nginx', 'path': 'xyz', + 'checksum': 'xxx'}, True), + self.driver.pull_image(None, 'nonexisting', + 'tag', 'never')) + mock_open_file.assert_any_call('xyz', 'rb') @mock.patch('zun.image.glance.utils.create_glanceclient') @mock.patch.object(driver.GlanceDriver, @@ -64,10 +69,14 @@ class TestDriver(base.BaseTestCase): def test_pull_image_failure(self, mock_should_pull_image, mock_search, mock_glance): mock_should_pull_image.return_value = True - mock_search.return_value = {'image': 'nginx', 'path': 'xyz'} - mock_glance.side_effect = Exception - self.assertRaises(exception.ZunException, self.driver.pull_image, - None, 'nonexisting', 'tag', 'always') + 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 + self.assertRaises(exception.ZunException, self.driver.pull_image, + None, 'nonexisting', 'tag', 'always') + mock_open_file.assert_any_call('xyz', 'rb') @mock.patch.object(driver.GlanceDriver, '_search_image_on_host') @@ -77,25 +86,26 @@ class TestDriver(base.BaseTestCase): def test_pull_image(self, mock_find_image, mock_glance, mock_should_pull_image, mock_search): mock_should_pull_image.return_value = True - mock_search.return_value = {'image': 'nginx', 'path': 'xyz'} + mock_search.return_value = {'image': 'nginx', 'path': 'xyz', + '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 CONF.set_override('images_directory', self.test_dir, group='glance') out_path = os.path.join(self.test_dir, '1234' + '.tar') - ret = self.driver.pull_image(None, 'image', 'latest', 'always') + mock_open_file = mock.mock_open() + with mock.patch('zun.image.glance.driver.open', mock_open_file): + 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.assertEqual(({'image': 'image', 'path': out_path}, False), ret) - self.assertTrue(os.path.isfile(ret[0]['path'])) @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_not_found(self, mock_should_pull_image, - mock_search, mock_glance): + mock_glance): mock_should_pull_image.return_value = True - mock_search.return_value = {'image': 'nginx', 'path': 'xyz'} with mock.patch('zun.image.glance.utils.find_image') as mock_find: mock_find.side_effect = exception.ImageNotFound self.assertRaises(exception.ImageNotFound, self.driver.pull_image,