openstack/nova
Code Issues Proposed changes

Include removal of ephemeral backing files in the image cache manager

Browse Source 
If CONF.image_cache.remove_unused_base_images is True, the base and
swap files are removed during the image cache manager's periodic
task while the ephemeral backing files are never deleted. This is a
long standing bug and this patch proposes to remove the ephemeral
backing files in the same way as for the swap files.

Change-Id: Ibf722265a4450fb81d40dee635667e39bc5b3edc
Closes-bug: #1602193
This commit is contained in:
Surya Seetharaman
2019-10-18 12:41:02 +02:00
committed by Lee Yarwood
parent 2745e68537
commit f44700935f
4 changed files with 117 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -95,7 +95,7 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
def test_list_base_images(self):
listing = ['00000001',
'ephemeral_0_20_None',
'ephemeral_20_abcdefg',
'00000004',
'swap_1000']
images = ['e97222e91fc4241f49a7f520d1dcf446751129b3_sm',
@ -149,6 +149,9 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
self.assertEqual(1, len(image_cache_manager.back_swap_images))
self.assertIn('swap_1000', image_cache_manager.back_swap_images)
self.assertEqual(1, len(image_cache_manager.back_ephemeral_images))
self.assertIn(
'ephemeral_20_abcdefg', image_cache_manager.back_ephemeral_images)
def test_list_backing_images_small(self):
self.stub_out('os.listdir',
@ -621,6 +624,19 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
expect_set = set(['swap_123', 'swap_456'])
self.assertEqual(image_cache_manager.back_swap_images, expect_set)
def test_store_ephemeral_image(self):
image_cache_manager = imagecache.ImageCacheManager()
image_cache_manager._store_ephemeral_image('ephemeral_')
image_cache_manager._store_ephemeral_image('ephemeral_123')
image_cache_manager._store_ephemeral_image('ephemeral_456_40d1d2c')
image_cache_manager._store_ephemeral_image('ephemeral_abc_40d1f2e')
image_cache_manager._store_ephemeral_image('123_ephemeral_40e1d2d')
image_cache_manager._store_ephemeral_image('ephemeral_129_')
self.assertEqual(len(image_cache_manager.back_ephemeral_images), 1)
expect_set = set(['ephemeral_456_40d1d2c'])
self.assertEqual(image_cache_manager.back_ephemeral_images, expect_set)
@mock.patch.object(lockutils, 'external_lock')
@mock.patch('os.path.exists')
@mock.patch('os.path.getmtime')
@ -659,6 +675,36 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
self.assertEqual(set(['swap_128']), expected_exist)
self.assertEqual(set(['swap_256']), expected_remove)
@mock.patch.object(lockutils, 'external_lock')
@mock.patch('os.path.exists')
@mock.patch('os.path.getmtime')
@mock.patch('os.remove')
@mock.patch('nova.privsep.path.utime')
def test_age_and_verify_ephemeral_images(self, mock_utime, mock_remove,
mock_getmtime, mock_exist, mock_lock):
base_dir = '/tmp_age_test'
image_cache_manager = imagecache.ImageCacheManager()
expected_remove = set()
expected_exist = set(['ephemeral_456_40d1d2c', 'ephemeral_2_41e1f2c'])
image_cache_manager.back_ephemeral_images.add('ephemeral_456_40d1d2c')
image_cache_manager.back_ephemeral_images.add('ephemeral_2_41e1f2c')
image_cache_manager.used_ephemeral_images.add('ephemeral_2_41e1f2c')
mock_getmtime.side_effect = lambda path: time.time() - 1000000
mock_exist.side_effect = \
lambda path: os.path.dirname(path) == base_dir and \
os.path.basename(path) in expected_exist
def removefile(path):
self.assertEqual(base_dir, os.path.dirname(path),
'Attempt to remove unexpected path')
fn = os.path.basename(path)
expected_remove.add(fn)
expected_exist.remove(fn)
mock_remove.side_effect = removefile
image_cache_manager._age_and_verify_ephemeral_images(None, base_dir)
self.assertEqual(set(['ephemeral_2_41e1f2c']), expected_exist)
self.assertEqual(set(['ephemeral_456_40d1d2c']), expected_remove)
@mock.patch.object(utils, 'synchronized')
@mock.patch.object(imagecache.ImageCacheManager, '_get_age_of_file',
return_value=(True, 100))

17
nova/tests/unit/virt/test_imagecache.py
View File

@ -49,6 +49,18 @@ swap_bdm_256 = [block_device.BlockDeviceDict(
'volume_size': 256,
'boot_index': -1})]
ephemeral_bdm = [block_device.BlockDeviceDict(
{'id': 1, 'instance_uuid': uuids.instance,
'device_name': '/dev/sdc1',
'source_type': 'blank',
'destination_type': 'local',
'disk_bus': 'scsi',
'device_type': 'disk',
'volume_size': 4,
'guest_format': 'ext4',
'delete_on_termination': True,
'boot_index': -1})]
class ImageCacheManagerTests(test.NoDBTestCase):
@ -104,7 +116,7 @@ class ImageCacheManagerTests(test.NoDBTestCase):
ctxt = context.get_admin_context()
swap_bdm_256_list = block_device_obj.block_device_make_list_from_dicts(
ctxt, swap_bdm_256)
ctxt, swap_bdm_256 + ephemeral_bdm)
swap_bdm_128_list = block_device_obj.block_device_make_list_from_dicts(
ctxt, swap_bdm_128)
mock_bdms_by_uuid.return_value = {uuids.instance_1: swap_bdm_256_list,
@ -134,6 +146,9 @@ class ImageCacheManagerTests(test.NoDBTestCase):
self.assertIn('swap_128', running['used_swap_images'])
self.assertIn('swap_256', running['used_swap_images'])
self.assertEqual(len(running['used_ephemeral_images']), 1)
self.assertIn('ephemeral_4_0706d66', running['used_ephemeral_images'])
@mock.patch.object(objects.BlockDeviceMappingList,
'bdms_by_instance_uuid')
def test_list_resizing_instances(self, mock_bdms_by_uuid):

15
nova/virt/imagecache.py
View File

@ -47,10 +47,12 @@ class ImageCacheManager(object):
- used_images
- instance_names
- used_swap_images
- used_ephemeral_images
"""
used_images = {}
instance_names = set()
used_swap_images = set()
used_ephemeral_images = set()
instance_bdms = objects.BlockDeviceMappingList.bdms_by_instance_uuid(
context, [instance.uuid for instance in all_instances])
@ -84,10 +86,21 @@ class ImageCacheManager(object):
if swap:
swap_image = 'swap_' + str(swap[0]['swap_size'])
used_swap_images.add(swap_image)
ephemeral = driver_block_device.convert_ephemerals(bdms)
if ephemeral:
os_type = nova.privsep.fs.get_fs_type_for_os_type(
instance.os_type)
file_name = nova.privsep.fs.get_file_extension_for_os_type(
os_type, CONF.default_ephemeral_format)
ephemeral_gb = str(ephemeral[0]['size'])
ephemeral_image = "ephemeral_%s_%s" % (
ephemeral_gb, file_name)
used_ephemeral_images.add(ephemeral_image)
return {'used_images': used_images,
'instance_names': instance_names,
'used_swap_images': used_swap_images}
'used_swap_images': used_swap_images,
'used_ephemeral_images': used_ephemeral_images}
def _scan_base_images(self, base_dir):
"""Scan base images present in base_dir and populate internal

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

@ -67,6 +67,9 @@ class ImageCacheManager(imagecache.ImageCacheManager):
self.back_swap_images = set()
self.used_swap_images = set()
self.back_ephemeral_images = set()
self.used_ephemeral_images = set()
self.active_base_files = []
self.originals = []
self.removable_base_files = []
@ -88,6 +91,15 @@ class ImageCacheManager(imagecache.ImageCacheManager):
LOG.debug('Adding %s into backend swap images', ent)
self.back_swap_images.add(ent)
def _store_ephemeral_image(self, ent):
"""Store base ephemeral images for later examination."""
names = ent.split('_')
if len(names) == 3 and names[0] == 'ephemeral':
if len(names[1]) > 0 and names[1].isdigit():
if len(names[2]) == 7 and isinstance(names[2], str):
LOG.debug('Adding %s into backend ephemeral images', ent)
self.back_ephemeral_images.add(ent)
def _scan_base_images(self, base_dir):
"""Scan base images in base_dir and call _store_image or
_store_swap_image on each as appropriate. These methods populate
@ -104,6 +116,7 @@ class ImageCacheManager(imagecache.ImageCacheManager):
else:
self._store_swap_image(ent)
self._store_ephemeral_image(ent)
def _list_backing_images(self):
"""List the backing images currently in use."""
@ -185,7 +198,7 @@ class ImageCacheManager(imagecache.ImageCacheManager):
return (True, age)
def _remove_old_enough_file(self, base_file, maxage, remove_lock=True):
"""Remove a single swap or base file if it is old enough."""
"""Remove a single swap, base or ephemeral file if it is old enough."""
exists, age = self._get_age_of_file(base_file)
if not exists:
return
@ -202,7 +215,7 @@ class ImageCacheManager(imagecache.ImageCacheManager):
if not exists or age < maxage:
return
LOG.info('Removing base or swap file: %s', base_file)
LOG.info('Removing base, swap or ephemeral file: %s', base_file)
try:
os.remove(base_file)
except OSError as e:
@ -212,7 +225,8 @@ class ImageCacheManager(imagecache.ImageCacheManager):
'error': e})
if age < maxage:
LOG.info('Base or swap file too young to remove: %s', base_file)
LOG.info('Base, swap or ephemeral file too young to remove: %s',
base_file)
else:
_inner_remove_old_enough_file()
if remove_lock:
@ -230,6 +244,12 @@ class ImageCacheManager(imagecache.ImageCacheManager):
{'lock_file': lock_file,
'error': e})
def _remove_ephemeral_file(self, base_file):
"""Remove a single ephemeral base file if it is old enough."""
maxage = CONF.image_cache.remove_unused_original_minimum_age_seconds
self._remove_old_enough_file(base_file, maxage, remove_lock=False)
def _remove_swap_file(self, base_file):
"""Remove a single swap base file if it is old enough."""
maxage = CONF.image_cache.remove_unused_original_minimum_age_seconds
@ -260,6 +280,21 @@ class ImageCacheManager(imagecache.ImageCacheManager):
{'id': img_id, 'base_file': base_file})
nova.privsep.path.utime(base_file)
def _age_and_verify_ephemeral_images(self, context, base_dir):
LOG.debug('Verify ephemeral images')
for ent in self.back_ephemeral_images:
base_file = os.path.join(base_dir, ent)
if ent in self.used_ephemeral_images and os.path.exists(base_file):
nova.privsep.path.utime(base_file)
elif self.remove_unused_base_images:
self._remove_ephemeral_file(base_file)
error_images = self.used_ephemeral_images - self.back_ephemeral_images
for error_image in error_images:
LOG.warning('%s ephemeral image was used by instance'
' but no back files existing!', error_image)
def _age_and_verify_swap_images(self, context, base_dir):
LOG.debug('Verify swap images')
@ -347,9 +382,11 @@ class ImageCacheManager(imagecache.ImageCacheManager):
self.used_images = running['used_images']
self.instance_names = running['instance_names']
self.used_swap_images = running['used_swap_images']
self.used_ephemeral_images = running['used_ephemeral_images']
# perform the aging and image verification
self._age_and_verify_cached_images(context, all_instances, base_dir)
self._age_and_verify_swap_images(context, base_dir)
self._age_and_verify_ephemeral_images(context, base_dir)
def get_disk_usage(self):
try: