[ARVADOS] updated: b15e6c65b0b26e1210802f92374a5cf5bc15cbc4
git at public.curoverse.com
git at public.curoverse.com
Thu May 7 18:11:56 EDT 2015
Summary of changes:
services/dockercleaner/arvados_docker/cleaner.py | 46 +++++++----------
services/dockercleaner/tests/test_cleaner.py | 65 ++++--------------------
2 files changed, 29 insertions(+), 82 deletions(-)
discards 3180bb6fc5334e99d14fe6f3c123326c9891ccba (commit)
discards 80e1bb8d933e4d0ae18951840d57b27d23b40f7b (commit)
via b15e6c65b0b26e1210802f92374a5cf5bc15cbc4 (commit)
via a6495af88784d902148b1cc451e5c90a57f5731b (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (3180bb6fc5334e99d14fe6f3c123326c9891ccba)
\
N -- N -- N (b15e6c65b0b26e1210802f92374a5cf5bc15cbc4)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit b15e6c65b0b26e1210802f92374a5cf5bc15cbc4
Author: Brett Smith <brett at curoverse.com>
Date: Thu May 7 18:11:50 2015 -0400
3793: Log uncleaned images.
diff --git a/services/dockercleaner/arvados_docker/cleaner.py b/services/dockercleaner/arvados_docker/cleaner.py
index 9aec0e1..038eed7 100644
--- a/services/dockercleaner/arvados_docker/cleaner.py
+++ b/services/dockercleaner/arvados_docker/cleaner.py
@@ -177,6 +177,10 @@ class DockerImageUseRecorder(DockerEventListener):
class DockerImageCleaner(DockerImageUseRecorder):
event_handlers = DockerImageUseRecorder.event_handlers.copy()
+ def __init__(self, images, docker_client, events):
+ super().__init__(images, docker_client, events)
+ self.logged_unknown = set()
+
def new_container(self, event, container_hash):
container_image_id = container_hash['Image']
if not self.images.has_image(container_image_id):
@@ -195,6 +199,15 @@ class DockerImageCleaner(DockerImageUseRecorder):
logger.info("Removed image %s", image_id)
self.images.del_image(image_id)
+ @event_handlers.on('destroy')
+ def log_unknown_images(self, event):
+ unknown_ids = {image['Id'] for image in self.docker_client.images()
+ if not self.images.has_image(image['Id'])}
+ for image_id in (unknown_ids - self.logged_unknown):
+ logger.info("Image %s is loaded but unused, so it won't be cleaned",
+ image_id)
+ self.logged_unknown = unknown_ids
+
def human_size(size_str):
size_str = size_str.lower().rstrip('b')
commit a6495af88784d902148b1cc451e5c90a57f5731b
Author: Brett Smith <brett at curoverse.com>
Date: Thu May 7 17:18:15 2015 -0400
3793: Always keep one Docker image.
diff --git a/services/dockercleaner/arvados_docker/cleaner.py b/services/dockercleaner/arvados_docker/cleaner.py
index aa46c1a..9aec0e1 100644
--- a/services/dockercleaner/arvados_docker/cleaner.py
+++ b/services/dockercleaner/arvados_docker/cleaner.py
@@ -88,20 +88,23 @@ class DockerImages:
logger.debug("Unregistered container %s", cid)
def should_delete(self):
- # Build a set of images in use, and figure out how much space we have
- # left for Docker images after keeping them.
- used_images = set(self.container_image_map.values())
- space_left = (self.target_size - sum(self.images[image_id].size
- for image_id in used_images))
- # Build a list of images not in use, ordered by use time.
- lru_images = [image for image in self.images.values()
- if image.docker_id not in used_images]
+ if not self.images:
+ return
+ # Build a list of images, ordered by use time.
+ lru_images = list(self.images.values())
lru_images.sort(key=lambda image: image.last_used)
+ # Make sure we don't delete any images in use, or if there are
+ # none, the most recently used image.
+ if self.container_image_map:
+ keep_ids = set(self.container_image_map.values())
+ else:
+ keep_ids = {lru_images[-1].docker_id}
+ space_left = (self.target_size - sum(self.images[image_id].size
+ for image_id in keep_ids))
# Go through the list most recently used first, and note which
- # images can be saved with the space available.
- keep_ids = set()
+ # images can be saved with the space allotted.
for image in reversed(lru_images):
- if image.size <= space_left:
+ if (image.docker_id not in keep_ids) and (image.size <= space_left):
keep_ids.add(image.docker_id)
space_left -= image.size
# Yield the Docker IDs of any image we don't want to save, least
diff --git a/services/dockercleaner/tests/test_cleaner.py b/services/dockercleaner/tests/test_cleaner.py
index 1f03681..4a16d23 100644
--- a/services/dockercleaner/tests/test_cleaner.py
+++ b/services/dockercleaner/tests/test_cleaner.py
@@ -101,15 +101,32 @@ class DockerImagesTestCase(unittest.TestCase):
images.del_image(MockDockerId())
self.assertTrue(images.has_image(self.mock_images[0]['Id']))
- def test_images_under_target_not_deletable(self):
- images = self.setup_images(200, 100, 300, target_size=150)
- self.assertEqual({self.mock_images[ii]['Id'] for ii in (0, 2)},
- set(images.should_delete()))
-
- def test_all_images_deletable(self):
+ def test_one_image_always_kept(self):
+ # When crunch-job starts a job, it makes sure each compute node
+ # has the Docker image loaded, then it runs all the tasks with
+ # the assumption the image is on each node. As long as that's
+ # true, the cleaner should avoid removing every installed image:
+ # crunch-job might be counting on the most recent one to be
+ # available, even if it's not currently in use.
images = self.setup_images(None, None, target_size=1)
- self.assertEqual({image['Id'] for image in self.mock_images},
- set(images.should_delete()))
+ for use_time, image in enumerate(self.mock_images, 1):
+ user = MockContainer(image)
+ images.add_user(user, use_time)
+ images.end_user(user['Id'])
+ self.assertEqual([self.mock_images[0]['Id']],
+ list(images.should_delete()))
+
+ def test_images_under_target_not_deletable(self):
+ # The images are used in this order. target_size is set so it
+ # could hold the largest image, but not after the most recently
+ # used image is kept; then we have to fall back to the previous one.
+ images = self.setup_images(20, 30, 40, 10, target_size=45)
+ for use_time, image in enumerate(self.mock_images, 1):
+ user = MockContainer(image)
+ images.add_user(user, use_time)
+ images.end_user(user['Id'])
+ self.assertEqual([self.mock_images[ii]['Id'] for ii in [0, 2]],
+ list(images.should_delete()))
def test_images_in_use_not_deletable(self):
images = self.setup_images(None, None, target_size=1)
@@ -121,11 +138,12 @@ class DockerImagesTestCase(unittest.TestCase):
list(images.should_delete()))
def test_image_deletable_after_unused(self):
- images = self.setup_images(None, target_size=1)
- user = MockContainer(self.mock_images[-1])
- images.add_user(user, 1)
- images.end_user(user['Id'])
- self.assertEqual([self.mock_images[-1]['Id']],
+ images = self.setup_images(None, None, target_size=1)
+ users = [MockContainer(image) for image in self.mock_images]
+ images.add_user(users[0], 1)
+ images.add_user(users[1], 2)
+ images.end_user(users[0]['Id'])
+ self.assertEqual([self.mock_images[0]['Id']],
list(images.should_delete()))
def test_image_not_deletable_if_user_restarts(self):
@@ -145,12 +163,13 @@ class DockerImagesTestCase(unittest.TestCase):
self.assertEqual([], list(images.should_delete()))
def test_image_deletable_after_all_users_end(self):
- images = self.setup_images(None, target_size=1)
- users = [MockContainer(self.mock_images[0]) for ii in range(2)]
+ images = self.setup_images(None, None, target_size=1)
+ users = [MockContainer(self.mock_images[ii]) for ii in [0, 1, 1]]
images.add_user(users[0], 1)
images.add_user(users[1], 2)
- images.end_user(users[0]['Id'])
+ images.add_user(users[2], 3)
images.end_user(users[1]['Id'])
+ images.end_user(users[2]['Id'])
self.assertEqual([self.mock_images[-1]['Id']],
list(images.should_delete()))
@@ -162,7 +181,7 @@ class DockerImagesTestCase(unittest.TestCase):
images.add_user(users[2], 2)
for user in users:
images.end_user(user['Id'])
- self.assertEqual([self.mock_images[ii]['Id'] for ii in (1, 2, 0)],
+ self.assertEqual([self.mock_images[ii]['Id'] for ii in [1, 2]],
list(images.should_delete()))
def test_adding_user_without_image_does_not_implicitly_add_image(self):
@@ -195,7 +214,9 @@ class DockerImagesTestCase(unittest.TestCase):
self.assertTrue(images.has_image(image['Id']))
def test_target_size_set_from_daemon(self):
- images = self.setup_from_daemon(20, 10, target_size=15)
+ images = self.setup_from_daemon(20, 10, 5, target_size=15)
+ user = MockContainer(self.mock_images[-1])
+ images.add_user(user, 1)
self.assertEqual([self.mock_images[0]['Id']],
list(images.should_delete()))
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list