diff options
Diffstat (limited to 'nova')
| -rw-r--r-- | nova/tests/test_imagecache.py | 86 | ||||
| -rw-r--r-- | nova/virt/libvirt/imagecache.py | 87 |
2 files changed, 64 insertions, 109 deletions
diff --git a/nova/tests/test_imagecache.py b/nova/tests/test_imagecache.py index 242f9c010..c748abbae 100644 --- a/nova/tests/test_imagecache.py +++ b/nova/tests/test_imagecache.py @@ -117,15 +117,15 @@ class ImageCacheManagerTestCase(test.TestCase): def test_list_running_instances(self): self.stubs.Set(db, 'instance_get_all', - lambda x: [{'image_ref': 'image-1', + lambda x: [{'image_ref': '1', 'host': FLAGS.host, 'name': 'inst-1', 'uuid': '123'}, - {'image_ref': 'image-2', + {'image_ref': '2', 'host': FLAGS.host, 'name': 'inst-2', 'uuid': '456'}, - {'image_ref': 'image-2', + {'image_ref': '2', 'host': 'remotehost', 'name': 'inst-3', 'uuid': '789'}]) @@ -136,14 +136,14 @@ class ImageCacheManagerTestCase(test.TestCase): image_cache_manager._list_running_instances(None) self.assertEqual(len(image_cache_manager.used_images), 2) - self.assertTrue(image_cache_manager.used_images['image-1'] == + self.assertTrue(image_cache_manager.used_images['1'] == (1, 0, ['inst-1'])) - self.assertTrue(image_cache_manager.used_images['image-2'] == + self.assertTrue(image_cache_manager.used_images['2'] == (1, 1, ['inst-2', 'inst-3'])) self.assertEqual(len(image_cache_manager.image_popularity), 2) - self.assertEqual(image_cache_manager.image_popularity['image-1'], 1) - self.assertEqual(image_cache_manager.image_popularity['image-2'], 2) + self.assertEqual(image_cache_manager.image_popularity['1'], 1) + self.assertEqual(image_cache_manager.image_popularity['2'], 2) def test_list_backing_images_small(self): self.stubs.Set(os, 'listdir', @@ -420,9 +420,7 @@ class ImageCacheManagerTestCase(test.TestCase): -1) def test_handle_base_image_unused(self): - img = {'container_format': 'ami', - 'id': '123', - 'uuid': '1234-4567-2378'} + img = '123' with self._make_base_file() as fname: os.utime(fname, (-1, time.time() - 3601)) @@ -437,9 +435,7 @@ class ImageCacheManagerTestCase(test.TestCase): self.assertEquals(image_cache_manager.corrupt_base_files, []) def test_handle_base_image_used(self): - img = {'container_format': 'ami', - 'id': '123', - 'uuid': '1234-4567-2378'} + img = '123' with self._make_base_file() as fname: os.utime(fname, (-1, time.time() - 3601)) @@ -454,9 +450,7 @@ class ImageCacheManagerTestCase(test.TestCase): self.assertEquals(image_cache_manager.corrupt_base_files, []) def test_handle_base_image_used_remotely(self): - img = {'container_format': 'ami', - 'id': '123', - 'uuid': '1234-4567-2378'} + img = '123' with self._make_base_file() as fname: os.utime(fname, (-1, time.time() - 3601)) @@ -472,9 +466,7 @@ class ImageCacheManagerTestCase(test.TestCase): def test_handle_base_image_absent(self): """Ensure we warn for use of a missing base image.""" - img = {'container_format': 'ami', - 'id': '123', - 'uuid': '1234-4567-2378'} + img = '123' with self._intercept_log_messages() as stream: image_cache_manager = imagecache.ImageCacheManager() @@ -488,9 +480,7 @@ class ImageCacheManagerTestCase(test.TestCase): -1) def test_handle_base_image_used_missing(self): - img = {'container_format': 'ami', - 'id': '123', - 'uuid': '1234-4567-2378'} + img = '123' with utils.tempdir() as tmpdir: fname = os.path.join(tmpdir, 'aaa') @@ -505,9 +495,7 @@ class ImageCacheManagerTestCase(test.TestCase): self.assertEquals(image_cache_manager.corrupt_base_files, []) def test_handle_base_image_checksum_fails(self): - img = {'container_format': 'ami', - 'id': '123', - 'uuid': '1234-4567-2378'} + img = '123' with self._make_base_file() as fname: f = open(fname, 'w') @@ -525,6 +513,9 @@ class ImageCacheManagerTestCase(test.TestCase): [fname]) def test_verify_base_images(self): + hashed_1 = '356a192b7913b04c54574d18c28d46e6395428ab' + hashed_42 = '92cfceb39d57d914ed8b14d0e37643de0797ae56' + self.flags(instances_path='/instance_path') self.flags(remove_unused_base_images=True) @@ -532,12 +523,10 @@ class ImageCacheManagerTestCase(test.TestCase): 'ephemeral_0_20_None', 'e97222e91fc4241f49a7f520d1dcf446751129b3_sm', 'e09c675c2d1cfac32dae3c2d83689c8c94bc693b_sm', - '92cfceb39d57d914ed8b14d0e37643de0797ae56', - '17d1b00b81642842e514494a78e804e9a511637c', - ('17d1b00b81642842e514494a78e804e9a511637c_' - '5368709120'), - ('17d1b00b81642842e514494a78e804e9a511637c_' - '10737418240'), + hashed_42, + hashed_1, + '%s_5368709120' % hashed_1, + '%s_10737418240' % hashed_1, '00000004'] def fq_path(path): @@ -556,8 +545,7 @@ class ImageCacheManagerTestCase(test.TestCase): '/instance_path/instance-1/disk', '/instance_path/instance-2/disk', '/instance_path/instance-3/disk', - ('/instance_path/_base/' - '92cfceb39d57d914ed8b14d0e37643de0797ae56.sha1')]: + '/instance_path/_base/%s.sha1' % hashed_42]: return True for p in base_file_list: @@ -566,8 +554,8 @@ class ImageCacheManagerTestCase(test.TestCase): if path == fq_path(p) + '.sha1': return False - if path in [('/instance_path/_base/' - '92cfceb39d57d914ed8b14d0e37643de0797ae56_sm')]: + if path in ['/instance_path/_base/%s_sm' % hashed_1, + '/instance_path/_base/%s_sm' % hashed_42]: return False self.fail('Unexpected path existance check: %s' % path) @@ -610,32 +598,22 @@ class ImageCacheManagerTestCase(test.TestCase): # Fake the database call which lists running instances self.stubs.Set(db, 'instance_get_all', - lambda x: [{'image_ref': 'image-1', + lambda x: [{'image_ref': '1', 'host': FLAGS.host, 'name': 'instance-1', 'uuid': '123'}, - {'image_ref': 'image-2', + {'image_ref': '1', 'host': FLAGS.host, 'name': 'instance-2', 'uuid': '456'}]) image_cache_manager = imagecache.ImageCacheManager() - # Fake the image service call which lists all registered images - class FakeImageService(object): - def detail(self, _context): - return [{'container_format': 'ami', 'id': '42'}, - {'container_format': 'amk', 'id': '43'}] - - self.stubs.Set(image, 'get_default_image_service', - lambda: FakeImageService()) - # Fake the utils call which finds the backing image def get_disk_backing_file(path): if path in ['/instance_path/instance-1/disk', '/instance_path/instance-2/disk']: - return fq_path('17d1b00b81642842e514494a78e804e9a511637c_' - '5368709120') + return fq_path('%s_5368709120' % hashed_1) self.fail('Unexpected backing file lookup: %s' % path) self.stubs.Set(virtutils, 'get_disk_backing_file', @@ -643,8 +621,7 @@ class ImageCacheManagerTestCase(test.TestCase): # Fake out verifying checksums, as that is tested elsewhere self.stubs.Set(image_cache_manager, '_verify_checksum', - lambda x, y: - y == '92cfceb39d57d914ed8b14d0e37643de0797ae56') + lambda x, y: y == hashed_42) # Fake getmtime as well orig_getmtime = os.path.getmtime @@ -674,16 +651,13 @@ class ImageCacheManagerTestCase(test.TestCase): image_cache_manager.verify_base_images(None) # Verify - active = [fq_path('17d1b00b81642842e514494a78e804e9a511637c_' - '5368709120')] + active = [fq_path(hashed_1), fq_path('%s_5368709120' % hashed_1)] self.assertEquals(image_cache_manager.active_base_files, active) for rem in [fq_path('e97222e91fc4241f49a7f520d1dcf446751129b3_sm'), fq_path('e09c675c2d1cfac32dae3c2d83689c8c94bc693b_sm'), - fq_path('92cfceb39d57d914ed8b14d0e37643de0797ae56'), - fq_path('17d1b00b81642842e514494a78e804e9a511637c'), - fq_path('17d1b00b81642842e514494a78e804e9a511637c_' - '10737418240')]: + fq_path(hashed_42), + fq_path('%s_10737418240' % hashed_1)]: self.assertTrue(rem in image_cache_manager.removable_base_files) def test_verify_base_images_no_base(self): diff --git a/nova/virt/libvirt/imagecache.py b/nova/virt/libvirt/imagecache.py index 22a4d1e8b..8b01c27c5 100644 --- a/nova/virt/libvirt/imagecache.py +++ b/nova/virt/libvirt/imagecache.py @@ -34,7 +34,6 @@ from nova import compute from nova import context as db_context from nova import db from nova import flags -from nova import image from nova import log as logging from nova.openstack.common import cfg from nova import utils @@ -220,7 +219,7 @@ class ImageCacheManager(object): if m: yield img, False, True - def _verify_checksum(self, img, base_file): + def _verify_checksum(self, img_id, base_file): """Compare the checksum stored on disk with the current file. Note that if the checksum fails to verify this is logged, but no actual @@ -235,11 +234,9 @@ class ImageCacheManager(object): f.close() if current_checksum != stored_checksum: - LOG.error(_('%(container_format)s-%(id)s ' - '(%(base_file)s): ' - 'image verification failed'), - {'container_format': img['container_format'], - 'id': img['id'], + LOG.error(_('%(id)s (%(base_file)s): image verification ' + 'failed'), + {'id': img_id, 'base_file': base_file}) return False @@ -247,10 +244,9 @@ class ImageCacheManager(object): return True else: - LOG.debug(_('%(container_format)s-%(id)s (%(base_file)s): ' - 'image verification skipped, no hash stored'), - {'container_format': img['container_format'], - 'id': img['id'], + LOG.debug(_('%(id)s (%(base_file)s): image verification skipped, ' + 'no hash stored'), + {'id': img_id, 'base_file': base_file}) # NOTE(mikal): If the checksum file is missing, then we should @@ -294,15 +290,14 @@ class ImageCacheManager(object): {'base_file': base_file, 'error': e}) - def _handle_base_image(self, img, base_file): + def _handle_base_image(self, img_id, base_file): """Handle the checks for a single base image.""" image_bad = False image_in_use = False - LOG.info(_('%(container_format)s-%(id)s (%(base_file)s): checking'), - {'container_format': img['container_format'], - 'id': img['id'], + LOG.info(_('%(id)s (%(base_file)s): checking'), + {'id': img_id, 'base_file': base_file}) if base_file in self.unexplained_images: @@ -311,17 +306,16 @@ class ImageCacheManager(object): if (base_file and os.path.exists(base_file) and os.path.isfile(base_file)): # _verify_checksum returns True if the checksum is ok. - image_bad = not self._verify_checksum(img, base_file) + image_bad = not self._verify_checksum(img_id, base_file) instances = [] - if str(img['id']) in self.used_images: - local, remote, instances = self.used_images[str(img['id'])] + if img_id in self.used_images: + local, remote, instances = self.used_images[img_id] if local > 0: - LOG.debug(_('%(container_format)s-%(id)s (%(base_file)s): ' + LOG.debug(_('%(id)s (%(base_file)s): ' 'in use: on this node %(local)d local, ' '%(remote)d on other nodes'), - {'container_format': img['container_format'], - 'id': img['id'], + {'id': img_id, 'base_file': base_file, 'local': local, 'remote': remote}) @@ -330,21 +324,17 @@ class ImageCacheManager(object): self.active_base_files.append(base_file) if not base_file: - LOG.warning(_('%(container_format)s-%(id)s ' - '(%(base_file)s): warning -- an absent ' - 'base file is in use! instances: ' + LOG.warning(_('%(id)s (%(base_file)s): warning -- an ' + 'absent base file is in use! instances: ' '%(instance_list)s'), - {'container_format': - img['container_format'], - 'id': img['id'], + {'id': img_id, 'base_file': base_file, 'instance_list': ' '.join(instances)}) else: - LOG.debug(_('%(container_format)s-%(id)s (%(base_file)s): ' - 'in use on (%(remote)d on other nodes)'), - {'container_format': img['container_format'], - 'id': img['id'], + LOG.debug(_('%(id)s (%(base_file)s): in use on (%(remote)d on ' + 'other nodes)'), + {'id': img_id, 'base_file': base_file, 'remote': remote}) if image_bad: @@ -352,24 +342,18 @@ class ImageCacheManager(object): if base_file: if not image_in_use: - LOG.debug(_('%(container_format)s-%(id)s (%(base_file)s): ' - 'image is not in use'), - {'container_format': img['container_format'], - 'id': img['id'], + LOG.debug(_('%(id)s (%(base_file)s): image is not in use'), + {'id': img_id, 'base_file': base_file}) self.removable_base_files.append(base_file) else: - LOG.debug(_('%(container_format)s-%(id)s (%(base_file)s): ' - 'image is in use'), - {'container_format': img['container_format'], - 'id': img['id'], + LOG.debug(_('%(id)s (%(base_file)s): image is in use'), + {'id': img_id, 'base_file': base_file}) def verify_base_images(self, context): """Verify that base images are in a reasonable state.""" - # TODO(mikal): Write a unit test for this method - # TODO(mikal): Handle "generated" images # NOTE(mikal): The new scheme for base images is as follows -- an # image is streamed from the image service to _base (filename is the @@ -392,14 +376,12 @@ class ImageCacheManager(object): self._list_base_images(base_dir) self._list_running_instances(context) - # Determine what images are in glance. GlanceImageService.detail uses - # _fetch_images which handles pagination for us - image_service = image.get_default_image_service() - for img in image_service.detail(context): - if img['container_format'] != 'ami': - continue - - fingerprint = hashlib.sha1(str(img['id'])).hexdigest() + # Determine what images are on disk because they're in use + for img in self.used_images: + fingerprint = hashlib.sha1(img).hexdigest() + LOG.debug(_('Image id %(id)s yields fingerprint %(fingerprint)s'), + {'id': img, + 'fingerprint': fingerprint}) for result in self._find_base_file(base_dir, fingerprint): base_file, image_small, image_resized = result self._handle_base_image(img, base_file) @@ -407,12 +389,11 @@ class ImageCacheManager(object): if not image_small and not image_resized: self.originals.append(base_file) - # Elements remaining in unexplained_images are not directly from - # glance. That might mean they're from a image that was removed from - # glance, but it might also mean that they're a resized image. + # Elements remaining in unexplained_images might be in use inuse_backing_images = self._list_backing_images() for backing_path in inuse_backing_images: - self.active_base_files.append(backing_path) + if not backing_path in self.active_base_files: + self.active_base_files.append(backing_path) # Anything left is an unknown base image for img in self.unexplained_images: |