From c31e25af722fe2ee9bee1644515cd2d0d47c4864 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Fri, 3 Jul 2015 16:25:53 +0100 Subject: [PATCH] Merge pull request #1642 from aanand/fix-1573 Fix bug where duplicate container is leftover after 'up' fails (cherry picked from commit f42fd6a3ad17cf9688e709bfed1639196777a342) Signed-off-by: Aanand Prasad --- compose/project.py | 3 +++ compose/service.py | 20 +++++++++++++++ tests/integration/resilience_test.py | 37 ++++++++++++++++++---------- tests/integration/service_test.py | 15 +++++++++++ 4 files changed, 62 insertions(+), 13 deletions(-) diff --git a/compose/project.py b/compose/project.py index 7c78401e4..a9df73f19 100644 --- a/compose/project.py +++ b/compose/project.py @@ -225,6 +225,9 @@ class Project(object): services = self.get_services(service_names, include_deps=start_deps) + for service in services: + service.remove_duplicate_containers() + plans = self._get_convergence_plans( services, allow_recreate=allow_recreate, diff --git a/compose/service.py b/compose/service.py index 6c2cc4da5..0db63e64f 100644 --- a/compose/service.py +++ b/compose/service.py @@ -394,6 +394,26 @@ class Service(object): container.start() return container + def remove_duplicate_containers(self, timeout=DEFAULT_TIMEOUT): + for c in self.duplicate_containers(): + log.info('Removing %s...' % c.name) + c.stop(timeout=timeout) + c.remove() + + def duplicate_containers(self): + containers = sorted( + self.containers(stopped=True), + key=lambda c: c.get('Created'), + ) + + numbers = set() + + for c in containers: + if c.number in numbers: + yield c + else: + numbers.add(c.number) + def config_hash(self): return json_hash(self.config_dict()) diff --git a/tests/integration/resilience_test.py b/tests/integration/resilience_test.py index 8229e9d3c..392490902 100644 --- a/tests/integration/resilience_test.py +++ b/tests/integration/resilience_test.py @@ -8,25 +8,36 @@ from .testcases import DockerClientTestCase class ResilienceTest(DockerClientTestCase): - def test_recreate_fails(self): - db = self.create_service('db', volumes=['/var/db'], command='top') - project = Project('composetest', [db], self.client) + def setUp(self): + self.db = self.create_service('db', volumes=['/var/db'], command='top') + self.project = Project('composetest', [self.db], self.client) - container = db.create_container() - db.start_container(container) - host_path = container.get('Volumes')['/var/db'] + container = self.db.create_container() + self.db.start_container(container) + self.host_path = container.get('Volumes')['/var/db'] - project.up() - container = db.containers()[0] - self.assertEqual(container.get('Volumes')['/var/db'], host_path) + def test_successful_recreate(self): + self.project.up() + container = self.db.containers()[0] + self.assertEqual(container.get('Volumes')['/var/db'], self.host_path) + def test_create_failure(self): with mock.patch('compose.service.Service.create_container', crash): with self.assertRaises(Crash): - project.up() + self.project.up() - project.up() - container = db.containers()[0] - self.assertEqual(container.get('Volumes')['/var/db'], host_path) + self.project.up() + container = self.db.containers()[0] + self.assertEqual(container.get('Volumes')['/var/db'], self.host_path) + + def test_start_failure(self): + with mock.patch('compose.service.Service.start_container', crash): + with self.assertRaises(Crash): + self.project.up() + + self.project.up() + container = self.db.containers()[0] + self.assertEqual(container.get('Volumes')['/var/db'], self.host_path) class Crash(Exception): diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 32de5fa47..98fa4213c 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -705,3 +705,18 @@ class ServiceTest(DockerClientTestCase): self.assertEqual(1, len(device_config)) self.assertDictEqual(device_dict, device_config[0]) + + def test_duplicate_containers(self): + service = self.create_service('web') + + options = service._get_container_create_options({}, 1) + original = Container.create(service.client, **options) + + self.assertEqual(set(service.containers(stopped=True)), set([original])) + self.assertEqual(set(service.duplicate_containers()), set()) + + options['name'] = 'temporary_container_name' + duplicate = Container.create(service.client, **options) + + self.assertEqual(set(service.containers(stopped=True)), set([original, duplicate])) + self.assertEqual(set(service.duplicate_containers()), set([duplicate]))