Merge pull request #2314 from dnephin/fix_up_require_build_error

Fix `up` "requires build" error

compose/service.py

@@ -414,6 +414,7 @@ class Service(object):
             return [
                 self.recreate_container(
                     container,
+                    do_build=do_build,
                     timeout=timeout,
                     attach_logs=should_attach_logs
                 )
@@ -435,10 +436,12 @@ class Service(object):
         else:
             raise Exception("Invalid action: {}".format(action))
 
-    def recreate_container(self,
+    def recreate_container(
-                           container,
+            self,
-                           timeout=DEFAULT_TIMEOUT,
+            container,
-                           attach_logs=False):
+            do_build=False,
+            timeout=DEFAULT_TIMEOUT,
+            attach_logs=False):
         """Recreate a container.
 
         The original container is renamed to a temporary name so that data
@@ -450,7 +453,7 @@ class Service(object):
         container.stop(timeout=timeout)
         container.rename_to_tmp_name()
         new_container = self.create_container(
-            do_build=False,
+            do_build=do_build,
             previous_container=container,
             number=container.labels.get(LABEL_CONTAINER_NUMBER),
             quiet=True,

tests/integration/service_test.py

@@ -14,6 +14,7 @@ from .. import mock
 from .testcases import DockerClientTestCase
 from .testcases import pull_busybox
 from compose import __version__
+from compose.const import LABEL_CONFIG_HASH
 from compose.const import LABEL_CONTAINER_NUMBER
 from compose.const import LABEL_ONE_OFF
 from compose.const import LABEL_PROJECT
@@ -23,6 +24,7 @@ from compose.container import Container
 from compose.service import build_extra_hosts
 from compose.service import ConfigError
 from compose.service import ConvergencePlan
+from compose.service import ConvergenceStrategy
 from compose.service import Net
 from compose.service import Service
 from compose.service import VolumeFromSpec
@@ -930,3 +932,38 @@ class ServiceTest(DockerClientTestCase):
 
         self.assertEqual(set(service.containers(stopped=True)), set([original, duplicate]))
         self.assertEqual(set(service.duplicate_containers()), set([duplicate]))
+
+
+def converge(service,
+             strategy=ConvergenceStrategy.changed,
+             do_build=True):
+    """Create a converge plan from a strategy and execute the plan."""
+    plan = service.convergence_plan(strategy)
+    return service.execute_convergence_plan(plan, do_build=do_build, timeout=1)
+
+
+class ConfigHashTest(DockerClientTestCase):
+    def test_no_config_hash_when_one_off(self):
+        web = self.create_service('web')
+        container = web.create_container(one_off=True)
+        self.assertNotIn(LABEL_CONFIG_HASH, container.labels)
+
+    def test_no_config_hash_when_overriding_options(self):
+        web = self.create_service('web')
+        container = web.create_container(environment={'FOO': '1'})
+        self.assertNotIn(LABEL_CONFIG_HASH, container.labels)
+
+    def test_config_hash_with_custom_labels(self):
+        web = self.create_service('web', labels={'foo': '1'})
+        container = converge(web)[0]
+        self.assertIn(LABEL_CONFIG_HASH, container.labels)
+        self.assertIn('foo', container.labels)
+
+    def test_config_hash_sticks_around(self):
+        web = self.create_service('web', command=["top"])
+        container = converge(web)[0]
+        self.assertIn(LABEL_CONFIG_HASH, container.labels)
+
+        web = self.create_service('web', command=["top", "-d", "1"])
+        container = converge(web)[0]
+        self.assertIn(LABEL_CONFIG_HASH, container.labels)

tests/integration/state_test.py

@@ -4,13 +4,10 @@ by `docker-compose up`.
 """
 from __future__ import unicode_literals
 
-import os
+import py
-import shutil
-import tempfile
 
 from .testcases import DockerClientTestCase
 from compose.config import config
-from compose.const import LABEL_CONFIG_HASH
 from compose.project import Project
 from compose.service import ConvergenceStrategy
 
@@ -180,14 +177,6 @@ class ProjectWithDependenciesTest(ProjectTestCase):
         self.assertEqual(len(containers), 2)
 
 
-def converge(service,
-             strategy=ConvergenceStrategy.changed,
-             do_build=True):
-    """Create a converge plan from a strategy and execute the plan."""
-    plan = service.convergence_plan(strategy)
-    return service.execute_convergence_plan(plan, do_build=do_build, timeout=1)
-
-
 class ServiceStateTest(DockerClientTestCase):
     """Test cases for Service.convergence_plan."""
 
@@ -241,67 +230,49 @@ class ServiceStateTest(DockerClientTestCase):
 
         image_id = self.client.images(name='busybox')[0]['Id']
         self.client.tag(image_id, repository=repo, tag=tag)
+        self.addCleanup(self.client.remove_image, image)
 
-        try:
+        web = self.create_service('web', image=image)
-            web = self.create_service('web', image=image)
+        container = web.create_container()
-            container = web.create_container()
 
-            # update the image
+        # update the image
-            c = self.client.create_container(image, ['touch', '/hello.txt'])
+        c = self.client.create_container(image, ['touch', '/hello.txt'])
-            self.client.commit(c, repository=repo, tag=tag)
+        self.client.commit(c, repository=repo, tag=tag)
-            self.client.remove_container(c)
+        self.client.remove_container(c)
 
-            web = self.create_service('web', image=image)
+        web = self.create_service('web', image=image)
-            self.assertEqual(('recreate', [container]), web.convergence_plan())
+        self.assertEqual(('recreate', [container]), web.convergence_plan())
-
-        finally:
-            self.client.remove_image(image)
 
     def test_trigger_recreate_with_build(self):
-        context = tempfile.mkdtemp()
+        context = py.test.ensuretemp('test_trigger_recreate_with_build')
+        self.addCleanup(context.remove)
+
         base_image = "FROM busybox\nLABEL com.docker.compose.test_image=true\n"
+        dockerfile = context.join('Dockerfile')
+        dockerfile.write(base_image)
 
-        try:
+        web = self.create_service('web', build=str(context))
-            dockerfile = os.path.join(context, 'Dockerfile')
+        container = web.create_container()
 
-            with open(dockerfile, 'w') as f:
+        dockerfile.write(base_image + 'CMD echo hello world\n')
-                f.write(base_image)
+        web.build()
 
-            web = self.create_service('web', build=context)
+        web = self.create_service('web', build=str(context))
-            container = web.create_container()
+        self.assertEqual(('recreate', [container]), web.convergence_plan())
 
-            with open(dockerfile, 'w') as f:
+    def test_image_changed_to_build(self):
-                f.write(base_image + 'CMD echo hello world\n')
+        context = py.test.ensuretemp('test_image_changed_to_build')
-            web.build()
+        self.addCleanup(context.remove)
+        context.join('Dockerfile').write("""
+            FROM busybox
+            LABEL com.docker.compose.test_image=true
+        """)
 
-            web = self.create_service('web', build=context)
+        web = self.create_service('web', image='busybox')
-            self.assertEqual(('recreate', [container]), web.convergence_plan())
+        container = web.create_container()
-        finally:
-            shutil.rmtree(context)
 
-
+        web = self.create_service('web', build=str(context))
-class ConfigHashTest(DockerClientTestCase):
+        plan = web.convergence_plan()
-    def test_no_config_hash_when_one_off(self):
+        self.assertEqual(('recreate', [container]), plan)
-        web = self.create_service('web')
+        containers = web.execute_convergence_plan(plan)
-        container = web.create_container(one_off=True)
+        self.assertEqual(len(containers), 1)
-        self.assertNotIn(LABEL_CONFIG_HASH, container.labels)
-
-    def test_no_config_hash_when_overriding_options(self):
-        web = self.create_service('web')
-        container = web.create_container(environment={'FOO': '1'})
-        self.assertNotIn(LABEL_CONFIG_HASH, container.labels)
-
-    def test_config_hash_with_custom_labels(self):
-        web = self.create_service('web', labels={'foo': '1'})
-        container = converge(web)[0]
-        self.assertIn(LABEL_CONFIG_HASH, container.labels)
-        self.assertIn('foo', container.labels)
-
-    def test_config_hash_sticks_around(self):
-        web = self.create_service('web', command=["top"])
-        container = converge(web)[0]
-        self.assertIn(LABEL_CONFIG_HASH, container.labels)
-
-        web = self.create_service('web', command=["top", "-d", "1"])
-        container = converge(web)[0]
-        self.assertIn(LABEL_CONFIG_HASH, container.labels)

tests/unit/config/config_test.py

@@ -177,6 +177,7 @@ class ConfigTest(unittest.TestCase):
         details = config.ConfigDetails('.', [base_file, override_file])
 
         tmpdir = py.test.ensuretemp('config_test')
+        self.addCleanup(tmpdir.remove)
         tmpdir.join('common.yml').write("""
             base:
               labels: ['label=one']
@@ -412,6 +413,7 @@ class ConfigTest(unittest.TestCase):
 
     def test_load_yaml_with_yaml_error(self):
         tmpdir = py.test.ensuretemp('invalid_yaml_test')
+        self.addCleanup(tmpdir.remove)
         invalid_yaml_file = tmpdir.join('docker-compose.yml')
         invalid_yaml_file.write("""
             web: