openstack/nova
Code Issues Proposed changes

config: remove deprecated checksum options

Browse Source 
This patch removes the legacy code for image checksumming
as well as configuration values that are not longer being
used.

Change-Id: I9c552e33456bb862688beaabe69f2b72bb8ebcce
This commit is contained in:
Mohammed Naser 2019-08-14 22:13:10 -04:00
parent 872a823d9a
commit e141bcdb25
4 changed files with 10 additions and 156 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
nova/conf/libvirt.py
View File

@ -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 = [

66
nova/tests/unit/virt/libvirt/test_imagecache.py
View File

@ -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()

68
nova/virt/libvirt/imagecache.py
View File

@ -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',

5
releasenotes/notes/remove-image-cache-checksumming-1c9f1bebfbd00673.yaml Normal file
View File

@ -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.