From b573b87a92d12bad82a41377e8cbaa891ac0dfb9 Mon Sep 17 00:00:00 2001
From: Ben Firshman <ben@firshman.co.uk>
Date: Fri, 11 Jul 2014 14:52:47 -0700
Subject: [PATCH] Fix race condition in recreate containers

Container might have stopped between checking `is_running` and
calling `stop()`, which then threw an exception.

Signed-off-by: Ben Firshman <ben@firshman.co.uk>
---
 fig/service.py                    |  9 ++++++++-
 tests/integration/service_test.py | 25 +++++++++++++++++++------
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/fig/service.py b/fig/service.py
index e3cbb2969..dfc4c1bed 100644
--- a/fig/service.py
+++ b/fig/service.py
@@ -177,8 +177,15 @@ class Service(object):
             return tuples
 
     def recreate_container(self, container, **override_options):
-        if container.is_running:
+        try:
             container.stop(timeout=1)
+        except APIError as e:
+            if (e.response.status_code == 500
+                    and e.explanation
+                    and 'no such process' in str(e.explanation)):
+                pass
+            else:
+                raise
 
         intermediate_container = Container.create(
             self.client,
diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py
index 4700f4101..ec936f881 100644
--- a/tests/integration/service_test.py
+++ b/tests/integration/service_test.py
@@ -113,12 +113,12 @@ class ServiceTest(DockerClientTestCase):
             'db',
             environment={'FOO': '1'},
             volumes=['/var/db'],
-            entrypoint=['ps'],
-            command=['ax']
+            entrypoint=['sleep'],
+            command=['300']
         )
         old_container = service.create_container()
-        self.assertEqual(old_container.dictionary['Config']['Entrypoint'], ['ps'])
-        self.assertEqual(old_container.dictionary['Config']['Cmd'], ['ax'])
+        self.assertEqual(old_container.dictionary['Config']['Entrypoint'], ['sleep'])
+        self.assertEqual(old_container.dictionary['Config']['Cmd'], ['300'])
         self.assertIn('FOO=1', old_container.dictionary['Config']['Env'])
         self.assertEqual(old_container.name, 'figtest_db_1')
         service.start_container(old_container)
@@ -134,8 +134,8 @@ class ServiceTest(DockerClientTestCase):
         new_container = tuples[0][1]
         self.assertEqual(intermediate_container.dictionary['Config']['Entrypoint'], ['echo'])
 
-        self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['ps'])
-        self.assertEqual(new_container.dictionary['Config']['Cmd'], ['ax'])
+        self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep'])
+        self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300'])
         self.assertIn('FOO=2', new_container.dictionary['Config']['Env'])
         self.assertEqual(new_container.name, 'figtest_db_1')
         self.assertEqual(new_container.inspect()['Volumes']['/var/db'], volume_path)
@@ -145,6 +145,19 @@ class ServiceTest(DockerClientTestCase):
         self.assertNotEqual(old_container.id, new_container.id)
         self.assertRaises(APIError, lambda: self.client.inspect_container(intermediate_container.id))
 
+    def test_recreate_containers_when_containers_are_stopped(self):
+        service = self.create_service(
+            'db',
+            environment={'FOO': '1'},
+            volumes=['/var/db'],
+            entrypoint=['sleep'],
+            command=['300']
+        )
+        old_container = service.create_container()
+        self.assertEqual(len(service.containers(stopped=True)), 1)
+        service.recreate_containers()
+        self.assertEqual(len(service.containers(stopped=True)), 1)
+
     def test_start_container_passes_through_options(self):
         db = self.create_service('db')
         db.start_container(environment={'FOO': 'BAR'})