openstack
/
ironic
Code Issues Proposed changes

Control extra space for images conversion in image_cache 

Size of image converted to raw format can be growing up to virtual_size.
This patch adds new _fetch_to_raw function to image_cache which try to
free extra space for image conversion if needed.

Closes-Bug: #1356362
Change-Id: I69712b078113ef0640859389ef4ab9039135f612
This commit is contained in:
Yuriy Zveryanskyy 2014-08-21 10:35:49 +03:00
parent b56db42aa3
commit d7cdc75841
3 changed files with 89 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ironic/common/images.py
View File

@ -292,3 +292,19 @@ def download_size(context, image_href, image_service=None):
if not image_service:
image_service = service.Service(version=1, context=context)
return image_service.show(image_href)['size']
def converted_size(path):
"""Get size of converted raw image.
The size of image converted to raw format can be growing up to the virtual
size of the image.
:param path: path to the image file.
:returns: virtual size of the image or 0 if conversion not needed.
"""
data = qemu_img_info(path)
if data.file_format == "raw" or not CONF.force_raw_images:
return 0
return data.virtual_size

72
ironic/drivers/modules/image_cache.py
View File

@ -87,11 +87,9 @@ class ImageCache(object):
#NOTE(ghe): We don't share images between instances/hosts
if not CONF.parallel_image_downloads:
with lockutils.lock(img_download_lock_name, 'ironic-'):
images.fetch_to_raw(ctx, uuid, dest_path,
self._image_service)
_fetch_to_raw(ctx, uuid, dest_path, self._image_service)
else:
images.fetch_to_raw(ctx, uuid, dest_path,
self._image_service)
_fetch_to_raw(ctx, uuid, dest_path, self._image_service)
return
#TODO(ghe): have hard links and counts the same behaviour in all fs
@ -143,8 +141,7 @@ class ImageCache(object):
tmp_dir = tempfile.mkdtemp(dir=self.master_dir)
tmp_path = os.path.join(tmp_dir, uuid)
try:
images.fetch_to_raw(ctx, uuid, tmp_path,
self._image_service)
_fetch_to_raw(ctx, uuid, tmp_path, self._image_service)
# NOTE(dtantsur): no need for global lock here - master_path
# will have link count >1 at any moment, so won't be cleaned up
os.link(tmp_path, master_path)
@ -283,6 +280,47 @@ def _free_disk_space_for(path):
return stat.f_frsize * stat.f_bavail
def _fetch_to_raw(context, image_href, path, image_service=None):
"""Fetch image and convert to raw format if needed."""
path_tmp = "%s.part" % path
images.fetch(context, image_href, path_tmp, image_service)
required_space = images.converted_size(path_tmp)
directory = os.path.dirname(path_tmp)
_clean_up_caches(directory, required_space)
images.image_to_raw(image_href, path, path_tmp)
def _clean_up_caches(directory, amount):
"""Explicitly cleanup caches based on their priority (if required).
:param directory: the directory (of the cache) to be freed up.
:param amount: amount of space to reclaim.
:raises: InsufficientDiskSpace exception, if we cannot free up enough space
after trying all the caches.
"""
free = _free_disk_space_for(directory)
if amount < free:
return
# NOTE(dtantsur): filter caches, whose directory is on the same device
st_dev = os.stat(directory).st_dev
caches_to_clean = [x[1]() for x in _cache_cleanup_list]
caches = (c for c in caches_to_clean
if os.stat(c.master_dir).st_dev == st_dev)
for cache_to_clean in caches:
cache_to_clean.clean_up(amount=(amount - free))
free = _free_disk_space_for(directory)
if amount < free:
break
else:
raise exception.InsufficientDiskSpace(path=directory,
required=amount / 1024 / 1024,
actual=free / 1024 / 1024,
)
def clean_up_caches(ctx, directory, images_info):
"""Explicitly cleanup caches based on their priority (if required).
@ -300,27 +338,7 @@ def clean_up_caches(ctx, directory, images_info):
"""
total_size = sum(images.download_size(ctx, uuid)
for (uuid, path) in images_info)
free = _free_disk_space_for(directory)
if total_size >= free:
# NOTE(dtantsur): filter caches, whose directory is on the same device
st_dev = os.stat(directory).st_dev
caches_to_clean = [x[1]() for x in _cache_cleanup_list]
caches = (c for c in caches_to_clean
if os.stat(c.master_dir).st_dev == st_dev)
for cache_to_clean in caches:
# NOTE(dtantsur): multiplying by 2 is an attempt to account for
# images converting to raw format
cache_to_clean.clean_up(amount=(2 * total_size - free))
free = _free_disk_space_for(directory)
if total_size < free:
break
else:
raise exception.InsufficientDiskSpace(path=directory,
required=total_size / 1024 / 1024,
actual=free / 1024 / 1024,
)
_clean_up_caches(directory, total_size)
def cleanup(priority):

37
ironic/tests/drivers/test_image_cache.py
View File

@ -33,7 +33,7 @@ def touch(filename):
open(filename, 'w').close()
@mock.patch.object(images, 'fetch_to_raw')
@mock.patch.object(image_cache, '_fetch_to_raw')
class TestImageCacheFetch(base.TestCase):
def setUp(self):
@ -241,7 +241,7 @@ class TestImageCacheCleanUp(base.TestCase):
mock_clean_ttl.assert_called_once_with(mock.ANY, None)
@mock.patch.object(utils, 'rmtree_without_raise')
@mock.patch.object(images, 'fetch_to_raw')
@mock.patch.object(image_cache, '_fetch_to_raw')
def test_temp_images_not_cleaned(self, mock_fetch_to_raw, mock_rmtree):
def _fake_fetch_to_raw(ctx, uuid, tmp_path, *args):
with open(tmp_path, 'w') as fp:
@ -258,7 +258,7 @@ class TestImageCacheCleanUp(base.TestCase):
self.assertTrue(mock_rmtree.called)
@mock.patch.object(utils, 'rmtree_without_raise')
@mock.patch.object(images, 'fetch_to_raw')
@mock.patch.object(image_cache, '_fetch_to_raw')
def test_temp_dir_exception(self, mock_fetch_to_raw, mock_rmtree):
mock_fetch_to_raw.side_effect = exception.IronicException
self.assertRaises(exception.IronicException,
@ -355,7 +355,7 @@ class CleanupImageCacheTestCase(base.TestCase):
mock_statvfs.assert_called_with('master_dir')
self.assertEqual(2, mock_statvfs.call_count)
self.mock_first_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 1))
amount=(42 - 1))
self.assertFalse(self.mock_second_cache.return_value.clean_up.called)
# Since we are using generator expression in clean_up_caches, stat on
@ -389,7 +389,7 @@ class CleanupImageCacheTestCase(base.TestCase):
mock_statvfs.assert_called_with('master_dir')
self.assertEqual(2, mock_statvfs.call_count)
self.mock_second_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 1))
amount=(42 - 1))
self.assertFalse(self.mock_first_cache.return_value.clean_up.called)
# Since first cache exists on a different partition, it wouldn't be
@ -422,9 +422,9 @@ class CleanupImageCacheTestCase(base.TestCase):
mock_statvfs.assert_called_with('master_dir')
self.assertEqual(3, mock_statvfs.call_count)
self.mock_first_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 1))
amount=(42 - 1))
self.mock_second_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 2))
amount=(42 - 2))
mock_stat_calls_expected = [mock.call('master_dir'),
mock.call('first_cache_dir'),
@ -453,9 +453,9 @@ class CleanupImageCacheTestCase(base.TestCase):
mock_statvfs.assert_called_with('master_dir')
self.assertEqual(3, mock_statvfs.call_count)
self.mock_first_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 1))
amount=(42 - 1))
self.mock_second_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 1))
amount=(42 - 1))
mock_stat_calls_expected = [mock.call('master_dir'),
mock.call('first_cache_dir'),
@ -465,3 +465,22 @@ class CleanupImageCacheTestCase(base.TestCase):
mock.call('master_dir')]
self.assertEqual(mock_stat_calls_expected, mock_stat.mock_calls)
self.assertEqual(mock_statvfs_calls_expected, mock_statvfs.mock_calls)
class TestFetchCleanup(base.TestCase):
def setUp(self):
super(TestFetchCleanup, self).setUp()
@mock.patch.object(images, 'converted_size')
@mock.patch.object(images, 'fetch')
@mock.patch.object(images, 'image_to_raw')
@mock.patch.object(image_cache, '_clean_up_caches')
def test__fetch_to_raw(self, mock_clean, mock_raw, mock_fetch, mock_size):
mock_size.return_value = 100
image_cache._fetch_to_raw('fake', 'fake-uuid', '/foo/bar')
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
'/foo/bar.part', None)
mock_clean.assert_called_once_with('/foo', 100)
mock_raw.assert_called_once_with('fake-uuid', '/foo/bar',
'/foo/bar.part')