[ARVADOS] updated: 80e1bb8d933e4d0ae18951840d57b27d23b40f7b
git at public.curoverse.com
git at public.curoverse.com
Thu May 7 16:21:48 EDT 2015
Summary of changes:
services/dockercleaner/arvados_docker/cleaner.py | 37 ++++++++++---
services/dockercleaner/tests/test_cleaner.py | 67 ++++++++++++++++++++----
2 files changed, 86 insertions(+), 18 deletions(-)
via 80e1bb8d933e4d0ae18951840d57b27d23b40f7b (commit)
from df5a73a8e8ebff9ab6c49d6ea41e62740f0f20dd (commit)
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 80e1bb8d933e4d0ae18951840d57b27d23b40f7b
Author: Brett Smith <brett at curoverse.com>
Date: Thu May 7 16:21:45 2015 -0400
3793: Add unused image polling, per code review.
diff --git a/services/dockercleaner/arvados_docker/cleaner.py b/services/dockercleaner/arvados_docker/cleaner.py
index aa46c1a..54f23f6 100644
--- a/services/dockercleaner/arvados_docker/cleaner.py
+++ b/services/dockercleaner/arvados_docker/cleaner.py
@@ -51,12 +51,15 @@ class DockerImages:
self.images = {}
self.container_image_map = {}
- @classmethod
- def from_daemon(cls, target_size, docker_client):
- images = cls(target_size)
+ def load_from_daemon(self, docker_client):
+ images_gone = set(self.images.keys())
for image in docker_client.images():
- images.add_image(image)
- return images
+ try:
+ images_gone.remove(image['Id'])
+ except KeyError:
+ self.add_image(image)
+ for image_id in images_gone:
+ self.del_image(image_id)
def add_image(self, image_hash):
image = DockerImage(image_hash)
@@ -174,6 +177,11 @@ class DockerImageUseRecorder(DockerEventListener):
class DockerImageCleaner(DockerImageUseRecorder):
event_handlers = DockerImageUseRecorder.event_handlers.copy()
+ def __init__(self, images, docker_client, events, poll_wait=3600):
+ super().__init__(images, docker_client, events)
+ self.poll_wait = poll_wait
+ self.next_poll = int(time.time()) + self.poll_wait
+
def new_container(self, event, container_hash):
container_image_id = container_hash['Image']
if not self.images.has_image(container_image_id):
@@ -182,6 +190,16 @@ class DockerImageCleaner(DockerImageUseRecorder):
return super().new_container(event, container_hash)
@event_handlers.on('destroy')
+ def poll_for_images(self, event):
+ # Our primary source of information about images is querying them
+ # when they're used by a new container. Periodically check for
+ # images that are loaded but never used.
+ if event['time'] < self.next_poll:
+ return
+ self.next_poll = event['time'] + self.poll_wait
+ self.images.load_from_daemon(self.docker_client)
+
+ @event_handlers.on('destroy')
def clean_images(self, event=None):
for image_id in self.images.should_delete():
try:
@@ -210,6 +228,9 @@ def parse_arguments(arguments):
'--keep', action='store', type=human_size, required=True,
help="size of Docker images to keep, suffixed with K/M/G/T")
parser.add_argument(
+ '--poll-time', action='store', type=lambda s: int(s) * 60, default=3600,
+ help="minutes between polls for unused images")
+ parser.add_argument(
'--verbose', '-v', action='count', default=0,
help="log more information")
return parser.parse_args(arguments)
@@ -223,14 +244,16 @@ def setup_logging(args):
logger.setLevel(logging.ERROR - (10 * args.verbose))
def run(args, docker_client):
+ images = DockerImages(args.keep)
start_time = int(time.time())
logger.debug("Loading Docker activity through present")
- images = DockerImages.from_daemon(args.keep, docker_client)
+ images.load_from_daemon(docker_client)
use_recorder = DockerImageUseRecorder(
images, docker_client, docker_client.events(since=1, until=start_time))
use_recorder.run()
cleaner = DockerImageCleaner(
- images, docker_client, docker_client.events(since=start_time))
+ images, docker_client, docker_client.events(since=start_time),
+ args.poll_time)
logger.info("Starting cleanup loop")
cleaner.clean_images()
cleaner.run()
diff --git a/services/dockercleaner/tests/test_cleaner.py b/services/dockercleaner/tests/test_cleaner.py
index 1f03681..c8dccbb 100644
--- a/services/dockercleaner/tests/test_cleaner.py
+++ b/services/dockercleaner/tests/test_cleaner.py
@@ -75,12 +75,17 @@ class DockerImageTestCase(unittest.TestCase):
class DockerImagesTestCase(unittest.TestCase):
def setUp(self):
self.mock_images = []
+ self.docker_client = mock.MagicMock(name='docker_client')
+ self.docker_client.images.side_effect = self.iter_mock_images
+
+ def iter_mock_images(self):
+ return iter(self.mock_images)
- def setup_mock_images(self, *vsizes):
+ def add_mock_images(self, *vsizes):
self.mock_images.extend(MockImage(vsize=vsize) for vsize in vsizes)
def setup_images(self, *vsizes, target_size=1000000):
- self.setup_mock_images(*vsizes)
+ self.add_mock_images(*vsizes)
images = cleaner.DockerImages(target_size)
for image in self.mock_images:
images.add_image(image)
@@ -183,19 +188,44 @@ class DockerImagesTestCase(unittest.TestCase):
images.end_user(user['Id'])
self.assertEqual([], list(images.should_delete()))
- def setup_from_daemon(self, *vsizes, target_size=1500000):
- self.setup_mock_images(*vsizes)
- docker_client = mock.MagicMock(name='docker_client')
- docker_client.images.return_value = iter(self.mock_images)
- return cleaner.DockerImages.from_daemon(target_size, docker_client)
-
def test_images_loaded_from_daemon(self):
- images = self.setup_from_daemon(None, None)
+ self.add_mock_images(None, None)
+ images = cleaner.DockerImages(100)
+ images.load_from_daemon(self.docker_client)
+ for image in self.mock_images:
+ self.assertTrue(images.has_image(image['Id']))
+
+ def test_new_images_loaded_from_daemon(self):
+ images = cleaner.DockerImages(100)
+ for count in range(2):
+ self.add_mock_images(None)
+ images.load_from_daemon(self.docker_client)
for image in self.mock_images:
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)
+ def test_images_removed_when_gone_from_daemon_listing(self):
+ images = cleaner.DockerImages(100)
+ self.add_mock_images(None, None)
+ images.load_from_daemon(self.docker_client)
+ gone_image = self.mock_images.pop()
+ images.load_from_daemon(self.docker_client)
+ self.assertTrue(images.has_image(self.mock_images[0]['Id']))
+ self.assertFalse(images.has_image(gone_image['Id']))
+
+ def test_reloading_image_from_daemon_does_not_lose_users(self):
+ images = self.setup_images(10, 20, target_size=15)
+ user = MockContainer(self.mock_images[-1])
+ images.add_user(user, 1)
+ images.load_from_daemon(self.docker_client)
+ self.assertEqual([self.mock_images[0]['Id']],
+ list(images.should_delete()))
+
+ def test_reloading_image_from_daemon_does_not_lose_use_times(self):
+ images = self.setup_images(10, 20, target_size=20)
+ user = MockContainer(self.mock_images[-1])
+ images.add_user(user, 1)
+ images.end_user(user['Id'])
+ images.load_from_daemon(self.docker_client)
self.assertEqual([self.mock_images[0]['Id']],
list(images.should_delete()))
@@ -288,6 +318,20 @@ class DockerImageCleanerTestCase(DockerImageUseRecorderTestCase):
self.docker_client.remove_image.assert_called_with(delete_id)
self.assertFalse(self.images.del_image.called)
+ def test_images_polled_from_daemon_periodically(self):
+ self.recorder.next_poll = 1
+ self.events.append(MockEvent('destroy', time=2))
+ self.events.append(MockEvent('destroy', time=2))
+ self.recorder.run()
+ self.assertEqual(1, self.images.load_from_daemon.call_count)
+ self.images.load_from_daemon.assert_called_with(self.docker_client)
+
+ def test_images_not_polled_from_daemon_excessively(self):
+ self.recorder.next_poll = 2
+ self.events.append(MockEvent('destroy', time=1))
+ self.recorder.run()
+ self.assertEqual(0, self.images.load_from_daemon.call_count)
+
class HumanSizeTestCase(unittest.TestCase):
def check(self, human_str, count, exp):
@@ -325,6 +369,7 @@ class RunTestCase(unittest.TestCase):
test_start_time = int(time.time())
self.docker_client.events.return_value = []
cleaner.run(self.args, self.docker_client)
+ self.assertLessEqual(1, self.docker_client.images.call_count)
self.assertEqual(2, self.docker_client.events.call_count)
event_kwargs = [args[1] for args in
self.docker_client.events.call_args_list]
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list