Merge pull request #1642 from aanand/fix-1573

Fix bug where duplicate container is leftover after 'up' fails

compose/project.py

@@ -226,6 +226,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,

compose/service.py

@@ -378,6 +378,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())
 

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):

tests/integration/service_test.py

@@ -729,3 +729,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]))