openstack
/
ironic
Code Issues Proposed changes

PXE to pass hints to ImageCache on how much space to reclaim 

After previous patch PXE triggers cache clean up, if there's not enough
disk space for deployment. However, standard clean up may reclaim too
little space, due to configuration. This patch implements hints to
ImageCache, so that it tries to reclaim exactly required amount
of disk space.

Note that this amount is calculated as total size of images multiplied
by two as an attempt to account for images converting to raw format.

Change-Id: I8f95b7e6334a37e9e6e98278a5b61a9ace7221b5
Closes-Bug: #1316168
This commit is contained in:
Dmitry Tantsur 2014-05-20 15:47:37 +02:00
parent 2a9c6acd57
commit 581ffa98d2
4 changed files with 123 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -146,12 +146,16 @@ class ImageCache(object):
utils.rmtree_without_raise(tmp_dir)
@lockutils.synchronized('master_image', 'ironic-')
def clean_up(self):
def clean_up(self, amount=None):
"""Clean up directory with images, keeping cache of the latest images.
Files with link count >1 are never deleted.
Protected by global lock, so that no one messes with master images
after we get listing and before we actually delete files.
:param amount: if present, amount of space to reclaim in bytes,
cleaning will stop, if this goal was reached,
even if it is possible to clean up more files
"""
if self.master_dir is None:
return
@ -159,31 +163,63 @@ class ImageCache(object):
LOG.debug("Starting clean up for master image cache %(dir)s" %
{'dir': self.master_dir})
amount_copy = amount
listing = _find_candidates_for_deletion(self.master_dir)
survived = self._clean_up_too_old(listing)
self._clean_up_ensure_cache_size(survived)
survived, amount = self._clean_up_too_old(listing, amount)
if amount is not None and amount <= 0:
return
amount = self._clean_up_ensure_cache_size(survived, amount)
if amount is not None and amount > 0:
LOG.warn(_("Cache clean up was unable to reclaim %(required)d MiB "
"of disk space, still %(left)d MiB required"),
{'required': amount_copy / 1024 / 1024,
'left': amount / 1024 / 1024})
def _clean_up_too_old(self, listing):
def _clean_up_too_old(self, listing, amount):
"""Clean up stage 1: drop images that are older than TTL.
This method removes files all files older than TTL seconds
unless 'amount' is non-None. If 'amount' is non-None,
it starts removing files older than TTL seconds,
oldest first, until the required 'amount' of space is reclaimed.
:param listing: list of tuples (file name, last used time)
:returns: list of files left after clean up
:param amount: if not None, amount of space to reclaim in bytes,
cleaning will stop, if this goal was reached,
even if it is possible to clean up more files
:returns: tuple (list of files left after clean up,
amount still to reclaim)
"""
threshold = time.time() - self._cache_ttl
survived = []
for file_name, last_used, stat in listing:
if last_used < threshold:
utils.unlink_without_raise(file_name)
try:
os.unlink(file_name)
except EnvironmentError as exc:
LOG.warn(_("Unable to delete file %(name)s from "
"master image cache: %(exc)s") %
{'name': file_name, 'exc': exc})
else:
if amount is not None:
amount -= stat.st_size
if amount <= 0:
amount = 0
break
else:
survived.append((file_name, last_used, stat))
return survived
return survived, amount
def _clean_up_ensure_cache_size(self, listing):
def _clean_up_ensure_cache_size(self, listing, amount):
"""Clean up stage 2: try to ensure cache size < threshold.
Try to delete the oldest files until conditions is satisfied
or no more files are eligable for delition.
:param listing: list of tuples (file name, last used time)
:param amount: amount of space to reclaim, if possible.
if amount is not None, it has higher priority than
cache size in settings
:returns: amount of space still required after clean up
"""
# NOTE(dtantsur): Sort listing to delete the oldest files first
listing = sorted(listing,
@ -193,7 +229,8 @@ class ImageCache(object):
for f in os.listdir(self.master_dir))
total_size = sum(os.path.getsize(f)
for f in total_listing)
while total_size > self._cache_size and listing:
while listing and (total_size > self._cache_size or
(amount is not None and amount > 0)):
file_name, last_used, stat = listing.pop()
try:
os.unlink(file_name)
@ -203,6 +240,8 @@ class ImageCache(object):
{'name': file_name, 'exc': exc})
else:
total_size -= stat.st_size
if amount is not None:
amount -= stat.st_size
if total_size > self._cache_size:
LOG.info(_("After cleaning up cache dir %(dir)s "
@ -210,6 +249,7 @@ class ImageCache(object):
"threshold %(expected)d") %
{'dir': self.master_dir, 'actual': total_size,
'expected': self._cache_size})
return max(amount, 0)
def _find_candidates_for_deletion(master_dir):

4
ironic/drivers/modules/pxe.py
View File

@ -277,7 +277,9 @@ def _cleanup_caches_if_required(ctx, cache, images_info):
caches = [c for c in (InstanceImageCache(), TFTPImageCache())
if os.stat(c.master_dir).st_dev == st_dev]
for cache_to_clean in caches:
cache_to_clean.clean_up()
# 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(cache.master_dir)
if total_size < free:
break

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

@ -117,6 +117,7 @@ class TestImageCacheCleanUp(base.TestCase):
@mock.patch.object(image_cache.ImageCache, '_clean_up_ensure_cache_size')
def test_clean_up_old_deleted(self, mock_clean_size):
mock_clean_size.return_value = None
files = [os.path.join(self.master_dir, str(i))
for i in range(2)]
for filename in files:
@ -127,6 +128,7 @@ class TestImageCacheCleanUp(base.TestCase):
with mock.patch.object(time, 'time', lambda: new_current_time):
self.cache.clean_up()
mock_clean_size.assert_called_once_with(mock.ANY, None)
survived = mock_clean_size.call_args[0][0]
self.assertEqual(1, len(survived))
self.assertEqual(files[0], survived[0][0])
@ -135,8 +137,24 @@ class TestImageCacheCleanUp(base.TestCase):
self.assertEqual(int(new_current_time - 100),
int(survived[0][2].st_mtime))
@mock.patch.object(image_cache.ImageCache, '_clean_up_ensure_cache_size')
def test_clean_up_old_with_amount(self, mock_clean_size):
files = [os.path.join(self.master_dir, str(i))
for i in range(2)]
for filename in files:
open(filename, 'wb').write('X')
new_current_time = time.time() + 900
with mock.patch.object(time, 'time', lambda: new_current_time):
self.cache.clean_up(amount=1)
self.assertFalse(mock_clean_size.called)
# Exactly one file is expected to be deleted
self.assertTrue(any(os.path.exists(f) for f in files))
self.assertFalse(all(os.path.exists(f) for f in files))
@mock.patch.object(image_cache.ImageCache, '_clean_up_ensure_cache_size')
def test_clean_up_files_with_links_untouched(self, mock_clean_size):
mock_clean_size.return_value = None
files = [os.path.join(self.master_dir, str(i))
for i in range(2)]
for filename in files:
@ -149,11 +167,11 @@ class TestImageCacheCleanUp(base.TestCase):
for filename in files:
self.assertTrue(os.path.exists(filename))
mock_clean_size.assert_called_once_with([])
mock_clean_size.assert_called_once_with([], None)
@mock.patch.object(image_cache.ImageCache, '_clean_up_too_old')
def test_clean_up_ensure_cache_size(self, mock_clean_ttl):
mock_clean_ttl.side_effect = lambda listing: listing
mock_clean_ttl.side_effect = lambda *xx: xx
# NOTE(dtantsur): Cache size in test is 10 bytes, we create 6 files
# with 3 bytes each and expect 3 to be deleted
files = [os.path.join(self.master_dir, str(i))
@ -175,10 +193,36 @@ class TestImageCacheCleanUp(base.TestCase):
for filename in files[3:]:
self.assertFalse(os.path.exists(filename))
mock_clean_ttl.assert_called_once_with(mock.ANY, None)
@mock.patch.object(image_cache.ImageCache, '_clean_up_too_old')
def test_clean_up_ensure_cache_size_with_amount(self, mock_clean_ttl):
mock_clean_ttl.side_effect = lambda *xx: xx
# NOTE(dtantsur): Cache size in test is 10 bytes, we create 6 files
# with 3 bytes each and set amount to be 15, 5 files are to be deleted
files = [os.path.join(self.master_dir, str(i))
for i in range(6)]
for filename in files:
with open(filename, 'w') as fp:
fp.write('123')
# NOTE(dtantsur): Make 1 file 'newer' to check that
# old ones are deleted first
new_current_time = time.time() + 100
os.utime(files[0], (new_current_time, new_current_time))
with mock.patch.object(time, 'time', lambda: new_current_time):
self.cache.clean_up(amount=15)
self.assertTrue(os.path.exists(files[0]))
for filename in files[5:]:
self.assertFalse(os.path.exists(filename))
mock_clean_ttl.assert_called_once_with(mock.ANY, 15)
@mock.patch.object(image_cache.LOG, 'info')
@mock.patch.object(image_cache.ImageCache, '_clean_up_too_old')
def test_clean_up_cache_still_large(self, mock_clean_ttl, mock_log):
mock_clean_ttl.side_effect = lambda listing: listing
mock_clean_ttl.side_effect = lambda *xx: xx
# NOTE(dtantsur): Cache size in test is 10 bytes, we create 2 files
# than cannot be deleted and expected this to be logged
files = [os.path.join(self.master_dir, str(i))
@ -193,6 +237,7 @@ class TestImageCacheCleanUp(base.TestCase):
for filename in files:
self.assertTrue(os.path.exists(filename))
self.assertTrue(mock_log.called)
mock_clean_ttl.assert_called_once_with(mock.ANY, None)
@mock.patch.object(utils, 'rmtree_without_raise')
@mock.patch.object(images, 'fetch_to_raw')
@ -219,3 +264,13 @@ class TestImageCacheCleanUp(base.TestCase):
self.cache._download_image,
'uuid', 'fake', 'fake')
self.assertTrue(mock_rmtree.called)
@mock.patch.object(image_cache.LOG, 'warn')
@mock.patch.object(image_cache.ImageCache, '_clean_up_too_old')
@mock.patch.object(image_cache.ImageCache, '_clean_up_ensure_cache_size')
def test_clean_up_amount_not_satisfied(self, mock_clean_size,
mock_clean_ttl, mock_log):
mock_clean_ttl.side_effect = lambda *xx: xx
mock_clean_size.side_effect = lambda listing, amount: amount
self.cache.clean_up(amount=15)
self.assertTrue(mock_log.called)

20
ironic/tests/drivers/test_pxe.py
View File

@ -410,7 +410,8 @@ class PXEPrivateFetchImagesTestCase(db_base.DbTestCase):
mock_statvfs.assert_called_with('master_dir')
self.assertEqual(2, mock_statvfs.call_count)
cache.fetch_image.assert_called_once_with('uuid', 'path', ctx=None)
mock_instance_cache.return_value.clean_up.assert_called_once_with()
mock_instance_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 1))
self.assertFalse(mock_tftp_cache.return_value.clean_up.called)
self.assertEqual(3, mock_stat.call_count)
@ -436,7 +437,8 @@ class PXEPrivateFetchImagesTestCase(db_base.DbTestCase):
mock_statvfs.assert_called_with('master_dir')
self.assertEqual(2, mock_statvfs.call_count)
cache.fetch_image.assert_called_once_with('uuid', 'path', ctx=None)
mock_tftp_cache.return_value.clean_up.assert_called_once_with()
mock_tftp_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 1))
self.assertFalse(mock_instance_cache.return_value.clean_up.called)
self.assertEqual(3, mock_stat.call_count)
@ -449,7 +451,7 @@ class PXEPrivateFetchImagesTestCase(db_base.DbTestCase):
mock_show.return_value = dict(size=42)
mock_statvfs.side_effect = [
mock.Mock(f_frsize=1, f_bavail=1),
mock.Mock(f_frsize=1, f_bavail=1),
mock.Mock(f_frsize=1, f_bavail=2),
mock.Mock(f_frsize=1, f_bavail=1024)
]
@ -460,8 +462,10 @@ class PXEPrivateFetchImagesTestCase(db_base.DbTestCase):
mock_statvfs.assert_called_with('master_dir')
self.assertEqual(3, mock_statvfs.call_count)
cache.fetch_image.assert_called_once_with('uuid', 'path', ctx=None)
mock_instance_cache.return_value.clean_up.assert_called_once_with()
mock_tftp_cache.return_value.clean_up.assert_called_once_with()
mock_instance_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 1))
mock_tftp_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 2))
self.assertEqual(3, mock_stat.call_count)
@mock.patch.object(os, 'stat')
@ -481,8 +485,10 @@ class PXEPrivateFetchImagesTestCase(db_base.DbTestCase):
mock_statvfs.assert_called_with('master_dir')
self.assertEqual(3, mock_statvfs.call_count)
self.assertFalse(cache.return_value.fetch_image.called)
mock_instance_cache.return_value.clean_up.assert_called_once_with()
mock_tftp_cache.return_value.clean_up.assert_called_once_with()
mock_instance_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 1))
mock_tftp_cache.return_value.clean_up.assert_called_once_with(
amount=(42 * 2 - 1))
self.assertEqual(3, mock_stat.call_count)