[ARVADOS] updated: 7bde93de3260e08503c7bf9c06fd3448972d378a
git at public.curoverse.com
git at public.curoverse.com
Mon Nov 2 15:46:40 EST 2015
Summary of changes:
.../install-compute-node.html.textile.liquid | 2 +-
services/dockercleaner/arvados_docker/cleaner.py | 37 ++++++++---
services/dockercleaner/tests/test_cleaner.py | 72 +++++++++++++++++++---
3 files changed, 90 insertions(+), 21 deletions(-)
via 7bde93de3260e08503c7bf9c06fd3448972d378a (commit)
via 1057d0912fef22aeaa707fd428521a76806a21d0 (commit)
from e10ccaba824b4f60ddc516903304351496b5fdca (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 7bde93de3260e08503c7bf9c06fd3448972d378a
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Nov 2 15:46:22 2015 -0500
7444: Clean stopped containers at startup.
diff --git a/doc/install/install-compute-node.html.textile.liquid b/doc/install/install-compute-node.html.textile.liquid
index 25d0473..aa4f37d 100644
--- a/doc/install/install-compute-node.html.textile.liquid
+++ b/doc/install/install-compute-node.html.textile.liquid
@@ -79,7 +79,7 @@ h2. Configure the Docker cleaner
The arvados-docker-cleaner program removes least recently used docker images as needed to keep disk usage below a configured limit.
{% include 'notebox_begin' %}
-This also removes all containers as soon as they exit, as if they were run with `docker run --rm`. If you need to debug or inspect containers after they stop, temporarily stop arvados-docker-cleaner or run it with the `--no-remove-stopped-containers` flag.
+This also removes all containers as soon as they exit, as if they were run with `docker run --rm`. If you need to debug or inspect containers after they stop, temporarily stop arvados-docker-cleaner or run it with `--remove-stopped-containers never`.
{% include 'notebox_end' %}
On Debian-based systems, install runit:
diff --git a/services/dockercleaner/arvados_docker/cleaner.py b/services/dockercleaner/arvados_docker/cleaner.py
index 89d6521..f9d727f 100755
--- a/services/dockercleaner/arvados_docker/cleaner.py
+++ b/services/dockercleaner/arvados_docker/cleaner.py
@@ -189,11 +189,7 @@ class DockerImageCleaner(DockerImageUseRecorder):
self.images.add_image(image_hash)
return super().new_container(event, container_hash)
- @event_handlers.on('die')
- def clean_container(self, event=None):
- if not self.remove_stopped_containers:
- return
- cid = event['id']
+ def _remove_container(self, cid):
try:
self.docker_client.remove_container(cid)
except docker.errors.APIError as error:
@@ -201,6 +197,22 @@ class DockerImageCleaner(DockerImageUseRecorder):
else:
logger.info("Removed container %s", cid)
+ @event_handlers.on('die')
+ def clean_container(self, event=None):
+ if not self.remove_stopped_containers:
+ return
+ self._remove_container(event['id'])
+
+ def check_stopped_containers(self, remove=False):
+ logger.info("Checking for stopped containers")
+ for c in self.docker_client.containers(filters={'status': 'exited'}):
+ logger.info("Container %s %s", c['Id'], c['Status'])
+ if c['Status'][:6] != 'Exited':
+ logger.error("Unexpected status %s for container %s",
+ c['Status'], c['Id'])
+ elif remove:
+ self._remove_container(c['Id'])
+
@event_handlers.on('destroy')
def clean_images(self, event=None):
for image_id in self.images.should_delete():
@@ -239,9 +251,11 @@ def parse_arguments(arguments):
'--quota', action='store', type=human_size, required=True,
help="space allowance for Docker images, suffixed with K/M/G/T")
parser.add_argument(
- '--no-remove-stopped-containers', action='store_false', default=True,
- dest='remove_stopped_containers',
- help="do not remove containers (default: remove on exit)")
+ '--remove-stopped-containers', type=str, default='always',
+ choices=['never', 'onexit', 'always'],
+ help="""when to remove stopped containers (default: always, i.e., remove
+ stopped containers found at startup, and remove containers as
+ soon as they exit)""")
parser.add_argument(
'--verbose', '-v', action='count', default=0,
help="log more information")
@@ -264,9 +278,12 @@ def run(args, docker_client):
use_recorder.run()
cleaner = DockerImageCleaner(
images, docker_client, docker_client.events(since=start_time),
- remove_stopped_containers=args.remove_stopped_containers)
- logger.info("Starting cleanup loop")
+ remove_stopped_containers=args.remove_stopped_containers != 'never')
+ cleaner.check_stopped_containers(
+ remove=args.remove_stopped_containers == 'always')
+ logger.info("Checking image quota at startup")
cleaner.clean_images()
+ logger.info("Listening for docker events")
cleaner.run()
def main(arguments):
diff --git a/services/dockercleaner/tests/test_cleaner.py b/services/dockercleaner/tests/test_cleaner.py
index 6793923..a9ecc92 100644
--- a/services/dockercleaner/tests/test_cleaner.py
+++ b/services/dockercleaner/tests/test_cleaner.py
@@ -375,20 +375,54 @@ class RunTestCase(unittest.TestCase):
class ContainerRemovalTestCase(unittest.TestCase):
+ LIFECYCLE = ['create', 'attach', 'start', 'resize', 'die', 'destroy']
+
def setUp(self):
self.args = mock.MagicMock(name='args')
self.docker_client = mock.MagicMock(name='docker_client')
-
- def test_remove_on_die(self):
- mockID = MockDockerId()
+ self.existingCID = MockDockerId()
+ self.docker_client.containers.return_value = [{
+ 'Id': self.existingCID,
+ 'Status': 'Exited (0) 6 weeks ago',
+ }, {
+ # If docker_client.containers() returns non-exited
+ # containers for some reason, do not remove them.
+ 'Id': MockDockerId(),
+ 'Status': 'Running',
+ }]
+ self.newCID = MockDockerId()
self.docker_client.events.return_value = [
- MockEvent(x, docker_id=mockID).encoded()
- for x in ['create', 'attach', 'start', 'resize', 'die', 'destroy']]
+ MockEvent(e, docker_id=self.newCID).encoded()
+ for e in self.LIFECYCLE]
+
+ def test_remove_onexit(self):
+ self.args.remove_stopped_containers = 'onexit'
+ cleaner.run(self.args, self.docker_client)
+ self.docker_client.remove_container.assert_called_once_with(self.newCID)
+
+ def test_remove_always(self):
+ self.args.remove_stopped_containers = 'always'
cleaner.run(self.args, self.docker_client)
- self.docker_client.remove_container.assert_called_once_with(mockID)
+ self.docker_client.remove_container.assert_any_call(self.existingCID)
+ self.docker_client.remove_container.assert_any_call(self.newCID)
+ self.assertEqual(2, self.docker_client.remove_container.call_count)
- def test_disabled_flag(self):
- self.args.remove_stopped_containers = False
- self.docker_client.events.return_value = [MockEvent('die').encoded()]
+ def test_remove_never(self):
+ self.args.remove_stopped_containers = 'never'
cleaner.run(self.args, self.docker_client)
self.assertEqual(0, self.docker_client.remove_container.call_count)
+
+ def test_container_exited_between_subscribe_events_and_check_existing(self):
+ self.args.remove_stopped_containers = 'always'
+ self.docker_client.events.return_value = [
+ MockEvent(e, docker_id=self.existingCID).encoded()
+ for e in ['die', 'destroy']]
+ cleaner.run(self.args, self.docker_client)
+ # Subscribed to events before getting the list of existing
+ # exited containers?
+ self.docker_client.assert_has_calls([
+ mock.call.events(since=mock.ANY),
+ mock.call.containers(filters={'status':'exited'})])
+ # Asked to delete the container twice?
+ self.docker_client.remove_container.assert_has_calls([mock.call(self.existingCID)] * 2)
+ self.assertEqual(2, self.docker_client.remove_container.call_count)
commit 1057d0912fef22aeaa707fd428521a76806a21d0
Author: Tom Clegg <tom at curoverse.com>
Date: Mon Nov 2 14:03:52 2015 -0500
7444: Test deletion error handling.
diff --git a/services/dockercleaner/tests/test_cleaner.py b/services/dockercleaner/tests/test_cleaner.py
index 52efabc..6793923 100644
--- a/services/dockercleaner/tests/test_cleaner.py
+++ b/services/dockercleaner/tests/test_cleaner.py
@@ -223,13 +223,14 @@ class DockerImagesTestCase(unittest.TestCase):
class DockerImageUseRecorderTestCase(unittest.TestCase):
TEST_CLASS = cleaner.DockerImageUseRecorder
+ TEST_CLASS_INIT_KWARGS = {}
def setUp(self):
self.images = mock.MagicMock(name='images')
self.docker_client = mock.MagicMock(name='docker_client')
self.events = []
self.recorder = self.TEST_CLASS(self.images, self.docker_client,
- self.encoded_events)
+ self.encoded_events, **self.TEST_CLASS_INIT_KWARGS)
@property
def encoded_events(self):
@@ -310,6 +311,23 @@ class DockerImageCleanerTestCase(DockerImageUseRecorderTestCase):
self.assertFalse(self.images.del_image.called)
+class DockerContainerCleanerTestCase(DockerImageUseRecorderTestCase):
+ TEST_CLASS = cleaner.DockerImageCleaner
+ TEST_CLASS_INIT_KWARGS = {'remove_stopped_containers': True}
+
+ @mock.patch('arvados_docker.cleaner.logger')
+ def test_failed_container_deletion_handling(self, mockLogger):
+ cid = MockDockerId()
+ self.docker_client.remove_container.side_effect = MockException(500)
+ self.events.append(MockEvent('die', docker_id=cid))
+ self.recorder.run()
+ self.docker_client.remove_container.assert_called_with(cid)
+ self.assertEqual("Failed to remove container %s: %s",
+ mockLogger.warning.call_args[0][0])
+ self.assertEqual(cid,
+ mockLogger.warning.call_args[0][1])
+
+
class HumanSizeTestCase(unittest.TestCase):
def check(self, human_str, count, exp):
self.assertEqual(count * (1024 ** exp),
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list