From e141bcdb25bcbb68267a4b26169a8c734e35c02e Mon Sep 17 00:00:00 2001 From: Mohammed Naser Date: Wed, 14 Aug 2019 22:13:10 -0400 Subject: [PATCH] config: remove deprecated checksum options This patch removes the legacy code for image checksumming as well as configuration values that are not longer being used. Change-Id: I9c552e33456bb862688beaabe69f2b72bb8ebcce --- nova/conf/libvirt.py | 27 -------- .../unit/virt/libvirt/test_imagecache.py | 66 ++---------------- nova/virt/libvirt/imagecache.py | 68 +------------------ ...e-cache-checksumming-1c9f1bebfbd00673.yaml | 5 ++ 4 files changed, 10 insertions(+), 156 deletions(-) create mode 100644 releasenotes/notes/remove-image-cache-checksumming-1c9f1bebfbd00673.yaml diff --git a/nova/conf/libvirt.py b/nova/conf/libvirt.py index c803986935b0..3d02a0e88768 100644 --- a/nova/conf/libvirt.py +++ b/nova/conf/libvirt.py @@ -901,37 +901,10 @@ Requires: ] libvirt_imagecache_opts = [ - cfg.StrOpt('image_info_filename_pattern', - default='$instances_path/$image_cache_subdirectory_name/' - '%(image)s.info', - deprecated_for_removal=True, - deprecated_since='14.0.0', - deprecated_reason='Image info files are no longer used by the ' - 'image cache', - help='Allows image information files to be stored in ' - 'non-standard locations'), cfg.IntOpt('remove_unused_resized_minimum_age_seconds', default=3600, help='Unused resized base images younger than this will not be ' 'removed'), - cfg.BoolOpt('checksum_base_images', - default=False, - deprecated_for_removal=True, - deprecated_since='14.0.0', - deprecated_reason='The image cache no longer periodically ' - 'calculates checksums of stored images. ' - 'Data integrity can be checked at the block ' - 'or filesystem level.', - help='Write a checksum for files in _base to disk'), - cfg.IntOpt('checksum_interval_seconds', - default=3600, - deprecated_for_removal=True, - deprecated_since='14.0.0', - deprecated_reason='The image cache no longer periodically ' - 'calculates checksums of stored images. ' - 'Data integrity can be checked at the block ' - 'or filesystem level.', - help='How frequently to checksum base images'), ] libvirt_lvm_opts = [ diff --git a/nova/tests/unit/virt/libvirt/test_imagecache.py b/nova/tests/unit/virt/libvirt/test_imagecache.py index 66297d11b622..e3a54e455ffe 100644 --- a/nova/tests/unit/virt/libvirt/test_imagecache.py +++ b/nova/tests/unit/virt/libvirt/test_imagecache.py @@ -96,7 +96,6 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): def test_list_base_images(self): listing = ['00000001', 'ephemeral_0_20_None', - '17d1b00b81642842e514494a78e804e9a511637c_5368709120.info', '00000004', 'swap_1000'] images = ['e97222e91fc4241f49a7f520d1dcf446751129b3_sm', @@ -302,14 +301,11 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): self.assertEqual([base_file1, base_file2, base_file3], res) @contextlib.contextmanager - def _make_base_file(self, lock=True, info=False): + def _make_base_file(self, lock=True): """Make a base file for testing.""" with utils.tempdir() as tmpdir: self.flags(instances_path=tmpdir) - self.flags(image_info_filename_pattern=('$instances_path/' - '%(image)s.info'), - group='libvirt') fname = os.path.join(tmpdir, 'aaa') base_file = open(fname, 'w') @@ -325,27 +321,13 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): lock_file.close() base_file = open(fname, 'r') - - # TODO(mdbooth): Info files are no longer created by Newton, - # but we must test that we continue to handle them correctly as - # they may still be around from before the upgrade, and they may - # be created by pre-Newton computes if we're on shared storage. - # Once we can be sure that all computes are running at least - # Newton (i.e. in Ocata), we can be sure that nothing is - # creating info files any more, and we can delete the tests for - # them. - if info: - # We're only checking for deletion, so contents are irrelevant - open(imagecache.get_info_filename(fname), 'w').close() - base_file.close() yield fname def test_remove_base_file(self): - with self._make_base_file(info=True) as fname: + with self._make_base_file() as fname: image_cache_manager = imagecache.ImageCacheManager() image_cache_manager._remove_base_file(fname) - info_fname = imagecache.get_info_filename(fname) lock_name = 'nova-' + os.path.split(fname)[-1] lock_dir = os.path.join(CONF.instances_path, 'locks') @@ -353,7 +335,6 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): # Files are initially too new to delete self.assertTrue(os.path.exists(fname)) - self.assertTrue(os.path.exists(info_fname)) self.assertTrue(os.path.exists(lock_file)) # Old files get cleaned up though @@ -363,27 +344,20 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): self.assertFalse(os.path.exists(fname)) self.assertFalse(os.path.exists(lock_file)) - # TODO(mdbooth): Remove test for deletion of info file in Ocata - # (see comment in _make_base_file) - self.assertFalse(os.path.exists(info_fname)) - def test_remove_base_file_original(self): - with self._make_base_file(info=True) as fname: + with self._make_base_file() as fname: image_cache_manager = imagecache.ImageCacheManager() image_cache_manager.originals = [fname] image_cache_manager._remove_base_file(fname) - info_fname = imagecache.get_info_filename(fname) # Files are initially too new to delete self.assertTrue(os.path.exists(fname)) - self.assertTrue(os.path.exists(info_fname)) # This file should stay longer than a resized image os.utime(fname, (-1, time.time() - 3601)) image_cache_manager._remove_base_file(fname) self.assertTrue(os.path.exists(fname)) - self.assertTrue(os.path.exists(info_fname)) # Originals don't stay forever though os.utime(fname, (-1, time.time() - 3600 * 25)) @@ -391,18 +365,11 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): self.assertFalse(os.path.exists(fname)) - # TODO(mdbooth): Remove test for deletion of info file in Ocata - # (see comment in _make_base_file) - self.assertFalse(os.path.exists(info_fname)) - def test_remove_base_file_dne(self): # This test is solely to execute the "does not exist" code path. We # don't expect the method being tested to do anything in this case. with utils.tempdir() as tmpdir: self.flags(instances_path=tmpdir) - self.flags(image_info_filename_pattern=('$instances_path/' - '%(image)s.info'), - group='libvirt') fname = os.path.join(tmpdir, 'aaa') image_cache_manager = imagecache.ImageCacheManager() @@ -412,9 +379,6 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): with intercept_log_messages() as stream: with utils.tempdir() as tmpdir: self.flags(instances_path=tmpdir) - self.flags(image_info_filename_pattern=('$instances_path/' - '%(image)s.info'), - group='libvirt') fname = os.path.join(tmpdir, 'aaa') @@ -481,15 +445,12 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): '/instance_path/_base', '/instance_path/instance-1/disk', '/instance_path/instance-2/disk', - '/instance_path/instance-3/disk', - '/instance_path/_base/%s.info' % hashed_42]: + '/instance_path/instance-3/disk']: return True for p in base_file_list: if path == fq_path(p): return True - if path == fq_path(p) + '.info': - return False if path in ['/instance_path/_base/%s_sm' % i for i in [hashed_1, hashed_21, @@ -617,23 +578,6 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): image_cache_manager = imagecache.ImageCacheManager() image_cache_manager.update(None, []) - def test_is_valid_info_file(self): - hashed = 'e97222e91fc4241f49a7f520d1dcf446751129b3' - - self.flags(instances_path='/tmp/no/such/dir/name/please') - self.flags(image_info_filename_pattern=('$instances_path/_base/' - '%(image)s.info'), - group='libvirt') - base_filename = os.path.join(CONF.instances_path, '_base', hashed) - - is_valid_info_file = imagecache.is_valid_info_file - self.assertFalse(is_valid_info_file('banana')) - self.assertFalse(is_valid_info_file( - os.path.join(CONF.instances_path, '_base', '00000001'))) - self.assertFalse(is_valid_info_file(base_filename)) - self.assertFalse(is_valid_info_file(base_filename + '.sha1')) - self.assertTrue(is_valid_info_file(base_filename + '.info')) - @mock.patch('nova.objects.InstanceList.get_by_filters') def test_run_image_cache_manager_pass(self, mock_instance_list): @@ -685,8 +629,6 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): def test_age_and_verify_swap_images(self, mock_utime, mock_remove, mock_getmtime, mock_exist, mock_lock): base_dir = '/tmp_age_test' - self.flags(image_info_filename_pattern=base_dir + '/%(image)s.info', - group='libvirt') image_cache_manager = imagecache.ImageCacheManager() expected_remove = set() diff --git a/nova/virt/libvirt/imagecache.py b/nova/virt/libvirt/imagecache.py index dfcc3f68fc29..afd4615c3e13 100644 --- a/nova/virt/libvirt/imagecache.py +++ b/nova/virt/libvirt/imagecache.py @@ -53,36 +53,6 @@ def get_cache_fname(image_id): return hashlib.sha1(image_id.encode('utf-8')).hexdigest() -def get_info_filename(base_path): - """Construct a filename for storing additional information about a base - image. - - Returns a filename. - """ - - base_file = os.path.basename(base_path) - return (CONF.libvirt.image_info_filename_pattern - % {'image': base_file}) - - -def is_valid_info_file(path): - """Test if a given path matches the pattern for info files.""" - - if six.PY2: - digest_size = hashlib.sha1().digestsize * 2 - else: - digest_size = hashlib.sha1().digest_size * 2 - regexp = (CONF.libvirt.image_info_filename_pattern - % {'image': ('([0-9a-f]{%(digest_size)d}|' - '[0-9a-f]{%(digest_size)d}_sm|' - '[0-9a-f]{%(digest_size)d}_[0-9]+)' - % {'digest_size': digest_size})}) - m = re.match(regexp, path) - if m: - return True - return False - - class ImageCacheManager(imagecache.ImageCacheManager): def __init__(self): super(ImageCacheManager, self).__init__() @@ -130,29 +100,7 @@ class ImageCacheManager(imagecache.ImageCacheManager): else: digest_size = hashlib.sha1().digest_size * 2 for ent in os.listdir(base_dir): - path = os.path.join(base_dir, ent) - if is_valid_info_file(path): - # TODO(mdbooth): In Newton we ignore these files, because if - # we're on shared storage they may be in use by a pre-Newton - # compute host. However, we have already removed all uses of - # these files in Newton, so once we can be sure that all - # compute hosts are running at least Newton (i.e. in Ocata), - # we can be sure that nothing is using info files any more. - # Therefore in Ocata, we should update this to simply delete - # these files here, i.e.: - # os.unlink(path) - # - # This will obsolete the code to cleanup these files in - # _remove_old_enough_file, so when updating this code to - # delete immediately, the cleanup code in - # _remove_old_enough_file can be removed. - # - # This cleanup code will delete all info files the first - # time it runs in Ocata, which means we can delete this - # block entirely in P. - pass - - elif len(ent) == digest_size: + if len(ent) == digest_size: self._store_image(base_dir, ent, original=True) elif len(ent) > digest_size + 2 and ent[digest_size] == '_': @@ -261,20 +209,6 @@ class ImageCacheManager(imagecache.ImageCacheManager): LOG.info('Removing base or swap file: %s', base_file) try: os.remove(base_file) - - # TODO(mdbooth): We have removed all uses of info files in - # Newton and we no longer create them, but they may still - # exist from before we upgraded, and they may still be - # created by older compute hosts if we're on shared storage. - # While there may still be pre-Newton computes writing here, - # the only safe place to delete info files is here, - # when deleting the cache entry. Once we can be sure that - # all computes are running at least Newton (i.e. in Ocata), - # we can delete these files unconditionally during the - # periodic task, which will make this code obsolete. - signature = get_info_filename(base_file) - if os.path.exists(signature): - os.remove(signature) except OSError as e: LOG.error('Failed to remove %(base_file)s, ' 'error was %(error)s', diff --git a/releasenotes/notes/remove-image-cache-checksumming-1c9f1bebfbd00673.yaml b/releasenotes/notes/remove-image-cache-checksumming-1c9f1bebfbd00673.yaml new file mode 100644 index 000000000000..c79447038be9 --- /dev/null +++ b/releasenotes/notes/remove-image-cache-checksumming-1c9f1bebfbd00673.yaml @@ -0,0 +1,5 @@ +--- +upgrade: + - The ``image_info_filename_pattern``, ``checksum_base_images``, and + ``checksum_interval_seconds`` options have been removed in the + ``[libvirt]`` config section. \ No newline at end of file