Remove extra ensure_image_exists() which causes duplicate builds.

Signed-off-by: Daniel Nephin <[email protected]>

compose/project.py

@@ -309,12 +309,13 @@ class Project(object):
     ):
         services = self.get_services_without_duplicate(service_names, include_deps=True)
 
+        for svc in services:
+            svc.ensure_image_exists(do_build=do_build)
         plans = self._get_convergence_plans(services, strategy)
 
         for service in services:
             service.execute_convergence_plan(
                 plans[service.name],
-                do_build,
                 detached=True,
                 start=False)
 
@@ -366,21 +367,19 @@ class Project(object):
            remove_orphans=False):
 
         self.initialize()
+        self.find_orphan_containers(remove_orphans)
+
         services = self.get_services_without_duplicate(
             service_names,
             include_deps=start_deps)
 
-        plans = self._get_convergence_plans(services, strategy)
-
         for svc in services:
             svc.ensure_image_exists(do_build=do_build)
-
-        self.find_orphan_containers(remove_orphans)
+        plans = self._get_convergence_plans(services, strategy)
 
         def do(service):
             return service.execute_convergence_plan(
                 plans[service.name],
-                do_build=do_build,
                 timeout=timeout,
                 detached=detached
             )

compose/service.py

@@ -254,7 +254,6 @@ class Service(object):
 
     def create_container(self,
                          one_off=False,
-                         do_build=BuildAction.none,
                          previous_container=None,
                          number=None,
                          quiet=False,
@@ -263,7 +262,9 @@ class Service(object):
         Create a container for this service. If the image doesn't exist, attempt to pull
         it.
         """
-        self.ensure_image_exists(do_build=do_build)
+        # This is only necessary for `scale` and `volumes_from`
+        # auto-creating containers to satisfy the dependency.
+        self.ensure_image_exists()
 
         container_options = self._get_container_create_options(
             override_options,
@@ -363,7 +364,6 @@ class Service(object):
 
     def execute_convergence_plan(self,
                                  plan,
-                                 do_build=BuildAction.none,
                                  timeout=DEFAULT_TIMEOUT,
                                  detached=False,
                                  start=True):
@@ -371,7 +371,7 @@ class Service(object):
         should_attach_logs = not detached
 
         if action == 'create':
-            container = self.create_container(do_build=do_build)
+            container = self.create_container()
 
             if should_attach_logs:
                 container.attach_log_stream()
@@ -385,7 +385,6 @@ class Service(object):
             return [
                 self.recreate_container(
                     container,
-                    do_build=do_build,
                     timeout=timeout,
                     attach_logs=should_attach_logs,
                     start_new_container=start
@@ -412,7 +411,6 @@ class Service(object):
     def recreate_container(
             self,
             container,
-            do_build=BuildAction.none,
             timeout=DEFAULT_TIMEOUT,
             attach_logs=False,
             start_new_container=True):
@@ -427,7 +425,6 @@ class Service(object):
         container.stop(timeout=timeout)
         container.rename_to_tmp_name()
         new_container = self.create_container(
-            do_build=do_build,
             previous_container=container,
             number=container.labels.get(LABEL_CONTAINER_NUMBER),
             quiet=True,

tests/integration/service_test.py

@@ -1037,12 +1037,10 @@ class ServiceTest(DockerClientTestCase):
         self.assertEqual(set(service.duplicate_containers()), set([duplicate]))
 
 
-def converge(service,
-             strategy=ConvergenceStrategy.changed,
-             do_build=True):
+def converge(service, strategy=ConvergenceStrategy.changed):
     """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)
+    return service.execute_convergence_plan(plan, timeout=1)
 
 
 class ConfigHashTest(DockerClientTestCase):

tests/unit/service_test.py

@@ -420,7 +420,7 @@ class ServiceTest(unittest.TestCase):
             parse_repository_tag("url:5000/repo@sha256:digest"),
             ("url:5000/repo", "sha256:digest", "@"))
 
-    def test_create_container_with_build(self):
+    def test_create_container(self):
         service = Service('foo', client=self.mock_client, build={'context': '.'})
         self.mock_client.inspect_image.side_effect = [
             NoSuchImageError,
@@ -431,7 +431,7 @@ class ServiceTest(unittest.TestCase):
         ]
 
         with mock.patch('compose.service.log', autospec=True) as mock_log:
-            service.create_container(do_build=BuildAction.none)
+            service.create_container()
             assert mock_log.warn.called
             _, args, _ = mock_log.warn.mock_calls[0]
             assert 'was built because it did not already exist' in args[0]
@@ -448,20 +448,20 @@ class ServiceTest(unittest.TestCase):
             buildargs=None,
         )
 
-    def test_create_container_no_build(self):
+    def test_ensure_image_exists_no_build(self):
         service = Service('foo', client=self.mock_client, build={'context': '.'})
         self.mock_client.inspect_image.return_value = {'Id': 'abc123'}
 
-        service.create_container(do_build=BuildAction.skip)
-        self.assertFalse(self.mock_client.build.called)
+        service.ensure_image_exists(do_build=BuildAction.skip)
+        assert not self.mock_client.build.called
 
-    def test_create_container_no_build_but_needs_build(self):
+    def test_ensure_image_exists_no_build_but_needs_build(self):
         service = Service('foo', client=self.mock_client, build={'context': '.'})
         self.mock_client.inspect_image.side_effect = NoSuchImageError
         with pytest.raises(NeedsBuildError):
-            service.create_container(do_build=BuildAction.skip)
+            service.ensure_image_exists(do_build=BuildAction.skip)
 
-    def test_create_container_force_build(self):
+    def test_ensure_image_exists_force_build(self):
         service = Service('foo', client=self.mock_client, build={'context': '.'})
         self.mock_client.inspect_image.return_value = {'Id': 'abc123'}
         self.mock_client.build.return_value = [
@@ -469,7 +469,7 @@ class ServiceTest(unittest.TestCase):
         ]
 
         with mock.patch('compose.service.log', autospec=True) as mock_log:
-            service.create_container(do_build=BuildAction.force)
+            service.ensure_image_exists(do_build=BuildAction.force)
 
         assert not mock_log.warn.called
         self.mock_client.build.assert_called_once_with(