Merge pull request #7457 from EricHripko/run-healthy

[Compose Spec] Make 'run' behave in the same way as 'up'

compose/cli/main.py

@@ -1298,31 +1298,28 @@ def build_one_off_container_options(options, detach, command):
 
 def run_one_off_container(container_options, project, service, options, toplevel_options,
                           toplevel_environment):
-    if not options['--no-deps']:
-        deps = service.get_dependency_names()
-        if deps:
-            project.up(
-                service_names=deps,
-                start_deps=True,
-                strategy=ConvergenceStrategy.never,
-                rescale=False
-            )
-
-    project.initialize()
-
-    container = service.create_container(
-        quiet=True,
+    detach = options.get('--detach')
+    use_network_aliases = options.get('--use-aliases')
+    containers = project.up(
+        service_names=[service.name],
+        start_deps=not options['--no-deps'],
+        strategy=ConvergenceStrategy.never,
+        detached=detach,
+        rescale=False,
         one_off=True,
-        **container_options)
-
-    use_network_aliases = options['--use-aliases']
+        override_options=container_options,
+    )
+    try:
+        container = next(c for c in containers if c.service == service.name)
+    except StopIteration:
+        raise OperationFailedError('Could not bring up the requested service')
 
-    if options.get('--detach'):
+    if detach:
         service.start_container(container, use_network_aliases)
         print(container.name)
         return
 
-    def remove_container(force=False):
+    def remove_container():
         if options['--rm']:
             project.client.remove_container(container.id, force=True, v=True)
 
@@ -1355,7 +1352,7 @@ def run_one_off_container(container_options, project, service, options, toplevel
             exit_code = 1
     except (signals.ShutdownException, signals.HangUpException):
         project.client.kill(container.id)
-        remove_container(force=True)
+        remove_container()
         sys.exit(2)
 
     remove_container()

compose/project.py

@@ -565,6 +565,8 @@ class Project(object):
            renew_anonymous_volumes=False,
            silent=False,
            cli=False,
+           one_off=False,
+           override_options=None,
            ):
 
         if cli:
@@ -584,7 +586,11 @@ class Project(object):
         for svc in services:
             svc.ensure_image_exists(do_build=do_build, silent=silent, cli=cli)
         plans = self._get_convergence_plans(
-            services, strategy, always_recreate_deps=always_recreate_deps)
+            services,
+            strategy,
+            always_recreate_deps=always_recreate_deps,
+            one_off=service_names if one_off else [],
+        )
 
         def do(service):
 
@@ -597,6 +603,7 @@ class Project(object):
                 start=start,
                 reset_container_image=reset_container_image,
                 renew_anonymous_volumes=renew_anonymous_volumes,
+                override_options=override_options,
             )
 
         def get_deps(service):
@@ -628,7 +635,7 @@ class Project(object):
         self.networks.initialize()
         self.volumes.initialize()
 
-    def _get_convergence_plans(self, services, strategy, always_recreate_deps=False):
+    def _get_convergence_plans(self, services, strategy, always_recreate_deps=False, one_off=None):
         plans = {}
 
         for service in services:
@@ -638,6 +645,7 @@ class Project(object):
                 if name in plans and
                 plans[name].action in ('recreate', 'create')
             ]
+            is_one_off = one_off and service.name in one_off
 
             if updated_dependencies and strategy.allows_recreate:
                 log.debug('%s has upstream changes (%s)',
@@ -649,11 +657,11 @@ class Project(object):
                 container_has_links = any(c.get('HostConfig.Links') for c in service.containers())
                 should_recreate_for_links = service_has_links ^ container_has_links
                 if always_recreate_deps or containers_stopped or should_recreate_for_links:
-                    plan = service.convergence_plan(ConvergenceStrategy.always)
+                    plan = service.convergence_plan(ConvergenceStrategy.always, is_one_off)
                 else:
-                    plan = service.convergence_plan(strategy)
+                    plan = service.convergence_plan(strategy, is_one_off)
             else:
-                plan = service.convergence_plan(strategy)
+                plan = service.convergence_plan(strategy, is_one_off)
 
             plans[service.name] = plan
 

compose/service.py

@@ -388,9 +388,12 @@ class Service(object):
             platform = self.default_platform
         return platform
 
-    def convergence_plan(self, strategy=ConvergenceStrategy.changed):
+    def convergence_plan(self, strategy=ConvergenceStrategy.changed, one_off=False):
         containers = self.containers(stopped=True)
 
+        if one_off:
+            return ConvergencePlan('one_off', [])
+
         if not containers:
             return ConvergencePlan('create', [])
 
@@ -439,25 +442,37 @@ class Service(object):
 
         return has_diverged
 
-    def _execute_convergence_create(self, scale, detached, start):
+    def _execute_convergence_create(self, scale, detached, start, one_off=False, override_options=None):
 
         i = self._next_container_number()
 
         def create_and_start(service, n):
-            container = service.create_container(number=n, quiet=True)
+            if one_off:
+                container = service.create_container(one_off=True, quiet=True, **override_options)
+            else:
+                container = service.create_container(number=n, quiet=True)
             if not detached:
                 container.attach_log_stream()
-            if start:
+            if start and not one_off:
                 self.start_container(container)
             return container
 
+        def get_name(service_name):
+            if one_off:
+                return "_".join([
+                    service_name.project,
+                    service_name.service,
+                    "run",
+                ])
+            return self.get_container_name(service_name.service, service_name.number)
+
         containers, errors = parallel_execute(
             [
                 ServiceName(self.project, self.name, index)
                 for index in range(i, i + scale)
             ],
             lambda service_name: create_and_start(self, service_name.number),
-            lambda service_name: self.get_container_name(service_name.service, service_name.number),
+            get_name,
             "Creating"
         )
         for error in errors.values():
@@ -528,16 +543,20 @@ class Service(object):
     def execute_convergence_plan(self, plan, timeout=None, detached=False,
                                  start=True, scale_override=None,
                                  rescale=True, reset_container_image=False,
-                                 renew_anonymous_volumes=False):
+                                 renew_anonymous_volumes=False, override_options=None):
         (action, containers) = plan
         scale = scale_override if scale_override is not None else self.scale_num
         containers = sorted(containers, key=attrgetter('number'))
 
         self.show_scale_warnings(scale)
 
-        if action == 'create':
+        if action in ['create', 'one_off']:
             return self._execute_convergence_create(
-                scale, detached, start
+                scale,
+                detached,
+                start,
+                one_off=(action == 'one_off'),
+                override_options=override_options
             )
 
         # The create action needs always needs an initial scale, but otherwise,

tests/acceptance/cli_test.py

@@ -1780,6 +1780,14 @@ services:
         assert len(db.containers()) == 1
         assert len(console.containers()) == 0
 
+    def test_run_service_with_unhealthy_dependencies(self):
+        self.base_dir = 'tests/fixtures/v2-unhealthy-dependencies'
+        result = self.dispatch(['run', 'web', '/bin/true'], returncode=1)
+        assert re.search(
+            re.compile('for web .*is unhealthy.*', re.MULTILINE),
+            result.stderr
+        )
+
     def test_run_service_with_scaled_dependencies(self):
         self.base_dir = 'tests/fixtures/v2-dependencies'
         self.dispatch(['up', '-d', '--scale', 'db=2', '--scale', 'console=0'])

tests/fixtures/v2-unhealthy-dependencies/docker-compose.yml

@@ -0,0 +1,19 @@
+version: "2.1"
+services:
+    db:
+      image: busybox:1.31.0-uclibc
+      command: top
+      healthcheck:
+        test: exit 1
+        interval: 1s
+        timeout: 1s
+        retries: 1
+    web:
+      image: busybox:1.31.0-uclibc
+      command: top
+      depends_on:
+        db:
+          condition: service_healthy
+    console:
+      image: busybox:1.31.0-uclibc
+      command: top

tests/unit/cli_test.py

@@ -18,6 +18,8 @@ from compose.cli.docopt_command import NoSuchCommand
 from compose.cli.errors import UserError
 from compose.cli.main import TopLevelCommand
 from compose.const import IS_WINDOWS_PLATFORM
+from compose.const import LABEL_SERVICE
+from compose.container import Container
 from compose.project import Project
 
 
@@ -94,12 +96,26 @@ class CLITestCase(unittest.TestCase):
     @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason="requires dockerpty")
     @mock.patch('compose.cli.main.RunOperation', autospec=True)
     @mock.patch('compose.cli.main.PseudoTerminal', autospec=True)
+    @mock.patch('compose.service.Container.create')
     @mock.patch.dict(os.environ)
-    def test_run_interactive_passes_logs_false(self, mock_pseudo_terminal, mock_run_operation):
+    def test_run_interactive_passes_logs_false(
+            self,
+            mock_container_create,
+            mock_pseudo_terminal,
+            mock_run_operation,
+    ):
         os.environ['COMPOSE_INTERACTIVE_NO_CLI'] = 'true'
         mock_client = mock.create_autospec(docker.APIClient)
         mock_client.api_version = DEFAULT_DOCKER_API_VERSION
         mock_client._general_configs = {}
+        mock_container_create.return_value = Container(mock_client, {
+            'Id': '37b35e0ba80d91009d37e16f249b32b84f72bda269985578ed6c75a0a13fcaa8',
+            'Config': {
+                'Labels': {
+                    LABEL_SERVICE: 'service',
+                }
+            },
+        }, has_been_inspected=True)
         project = Project.from_config(
             name='composetest',
             client=mock_client,
@@ -132,10 +148,20 @@ class CLITestCase(unittest.TestCase):
         _, _, call_kwargs = mock_run_operation.mock_calls[0]
         assert call_kwargs['logs'] is False
 
-    def test_run_service_with_restart_always(self):
+    @mock.patch('compose.service.Container.create')
+    def test_run_service_with_restart_always(self, mock_container_create):
         mock_client = mock.create_autospec(docker.APIClient)
         mock_client.api_version = DEFAULT_DOCKER_API_VERSION
         mock_client._general_configs = {}
+        mock_container_create.return_value = Container(mock_client, {
+            'Id': '37b35e0ba80d91009d37e16f249b32b84f72bda269985578ed6c75a0a13fcaa8',
+            'Name': 'composetest_service_37b35',
+            'Config': {
+                'Labels': {
+                    LABEL_SERVICE: 'service',
+                }
+            },
+        }, has_been_inspected=True)
 
         project = Project.from_config(
             name='composetest',