From a6b602d086dee4b2d17b3cf731163a77f2d617db Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Tue, 8 Oct 2019 17:14:23 +0100 Subject: [PATCH] Support attaching to dependencies on up When using the 'up' command, only services listed as arguments are attached to, which can be very different to the 'no argument' case if a service has many and deep dependencies: - It's not clear when dependencies have failed to start. Have to run 'compose ps' separately to find out. - It's not clear when dependencies are erroring. Have to run 'compose logs' separately to find out. With a simple setup, it's possible to work around theses issue by using the 'up' command without arguments. But when there are lots of 'top-level' services, with common dependencies, in a single config, using 'up' without arguments isn't practical due to resource limits and the sheer volume of output from other services. This introduces a new '--attach-dependencies' flag to optionally attach dependent containers as part of the 'up' command. This makes their logs visible in the output, alongside the listed services. It also means we benefit from the '--abort-on-container-exit' behaviour when dependencies fail to start, giving more visibility of the failure. Signed-off-by: Ben Thorner --- compose/cli/main.py | 18 ++++++++++----- contrib/completion/bash/docker-compose | 2 +- contrib/completion/zsh/_docker-compose | 3 ++- tests/acceptance/cli_test.py | 20 +++++++++++++++++ .../docker-compose.yml | 10 +++++++++ .../docker-compose.yml | 10 +++++++++ tests/unit/cli/main_test.py | 22 ++++++++++++++----- 7 files changed, 72 insertions(+), 13 deletions(-) create mode 100644 tests/fixtures/abort-on-container-exit-dependencies/docker-compose.yml create mode 100644 tests/fixtures/echo-services-dependencies/docker-compose.yml diff --git a/compose/cli/main.py b/compose/cli/main.py index 200d4eeac..b6ad404dc 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -1012,6 +1012,7 @@ class TopLevelCommand(object): --build Build images before starting containers. --abort-on-container-exit Stops all containers if any container was stopped. Incompatible with -d. + --attach-dependencies Attach to dependent containers -t, --timeout TIMEOUT Use this timeout in seconds for container shutdown when attached or when containers are already running. (default: 10) @@ -1033,16 +1034,18 @@ class TopLevelCommand(object): remove_orphans = options['--remove-orphans'] detached = options.get('--detach') no_start = options.get('--no-start') + attach_dependencies = options.get('--attach-dependencies') - if detached and (cascade_stop or exit_value_from): - raise UserError("--abort-on-container-exit and -d cannot be combined.") + if detached and (cascade_stop or exit_value_from or attach_dependencies): + raise UserError( + "-d cannot be combined with --abort-on-container-exit or --attach-dependencies.") ignore_orphans = self.toplevel_environment.get_boolean('COMPOSE_IGNORE_ORPHANS') if ignore_orphans and remove_orphans: raise UserError("COMPOSE_IGNORE_ORPHANS and --remove-orphans cannot be combined.") - opts = ['--detach', '--abort-on-container-exit', '--exit-code-from'] + opts = ['--detach', '--abort-on-container-exit', '--exit-code-from', '--attach-dependencies'] for excluded in [x for x in opts if options.get(x) and no_start]: raise UserError('--no-start and {} cannot be combined.'.format(excluded)) @@ -1087,7 +1090,10 @@ class TopLevelCommand(object): if detached or no_start: return - attached_containers = filter_containers_to_service_names(to_attach, service_names) + attached_containers = filter_attached_containers( + to_attach, + service_names, + attach_dependencies) log_printer = log_printer_from_project( self.project, @@ -1392,8 +1398,8 @@ def log_printer_from_project( log_args=log_args) -def filter_containers_to_service_names(containers, service_names): - if not service_names: +def filter_attached_containers(containers, service_names, attach_dependencies=False): + if attach_dependencies or not service_names: return containers return [ diff --git a/contrib/completion/bash/docker-compose b/contrib/completion/bash/docker-compose index 23c48b7f4..ad0ce44c1 100644 --- a/contrib/completion/bash/docker-compose +++ b/contrib/completion/bash/docker-compose @@ -545,7 +545,7 @@ _docker_compose_up() { case "$cur" in -*) - COMPREPLY=( $( compgen -W "--abort-on-container-exit --always-recreate-deps --build -d --detach --exit-code-from --force-recreate --help --no-build --no-color --no-deps --no-recreate --no-start --renew-anon-volumes -V --remove-orphans --scale --timeout -t" -- "$cur" ) ) + COMPREPLY=( $( compgen -W "--abort-on-container-exit --always-recreate-deps --attach-dependencies --build -d --detach --exit-code-from --force-recreate --help --no-build --no-color --no-deps --no-recreate --no-start --renew-anon-volumes -V --remove-orphans --scale --timeout -t" -- "$cur" ) ) ;; *) __docker_compose_complete_services diff --git a/contrib/completion/zsh/_docker-compose b/contrib/completion/zsh/_docker-compose index 277bf0d3c..de1414984 100755 --- a/contrib/completion/zsh/_docker-compose +++ b/contrib/completion/zsh/_docker-compose @@ -284,7 +284,7 @@ __docker-compose_subcommand() { (up) _arguments \ $opts_help \ - '(--abort-on-container-exit)-d[Detached mode: Run containers in the background, print new container names. Incompatible with --abort-on-container-exit.]' \ + '(--abort-on-container-exit)-d[Detached mode: Run containers in the background, print new container names. Incompatible with --abort-on-container-exit and --attach-dependencies.]' \ $opts_no_color \ $opts_no_deps \ $opts_force_recreate \ @@ -292,6 +292,7 @@ __docker-compose_subcommand() { $opts_no_build \ "(--no-build)--build[Build images before starting containers.]" \ "(-d)--abort-on-container-exit[Stops all containers if any container was stopped. Incompatible with -d.]" \ + "(-d)--attach-dependencies[Attach to dependent containers. Incompatible with -d.]" \ '(-t --timeout)'{-t,--timeout}"[Use this timeout in seconds for container shutdown when attached or when containers are already running. (default: 10)]:seconds: " \ '--scale[SERVICE=NUM Scale SERVICE to NUM instances. Overrides the `scale` setting in the Compose file if present.]:service scale SERVICE=NUM: ' \ '--exit-code-from=[Return the exit code of the selected service container. Implies --abort-on-container-exit]:service:__docker-compose_services' \ diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 3a207c83a..7fa7fc548 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -1571,6 +1571,26 @@ services: assert len(db.containers()) == 0 assert len(console.containers()) == 0 + def test_up_with_attach_dependencies(self): + self.base_dir = 'tests/fixtures/echo-services-dependencies' + result = self.dispatch(['up', '--attach-dependencies', '--no-color', 'simple'], None) + simple_name = self.project.get_service('simple').containers(stopped=True)[0].name_without_project + another_name = self.project.get_service('another').containers( + stopped=True + )[0].name_without_project + + assert '{} | simple'.format(simple_name) in result.stdout + assert '{} | another'.format(another_name) in result.stdout + + def test_up_handles_aborted_dependencies(self): + self.base_dir = 'tests/fixtures/abort-on-container-exit-dependencies' + proc = start_process( + self.base_dir, + ['up', 'simple', '--attach-dependencies', '--abort-on-container-exit']) + wait_on_condition(ContainerCountCondition(self.project, 0)) + proc.wait() + assert proc.returncode == 1 + def test_up_with_force_recreate(self): self.dispatch(['up', '-d'], None) service = self.project.get_service('simple') diff --git a/tests/fixtures/abort-on-container-exit-dependencies/docker-compose.yml b/tests/fixtures/abort-on-container-exit-dependencies/docker-compose.yml new file mode 100644 index 000000000..cd10c851c --- /dev/null +++ b/tests/fixtures/abort-on-container-exit-dependencies/docker-compose.yml @@ -0,0 +1,10 @@ +version: "2.0" +services: + simple: + image: busybox:1.31.0-uclibc + command: top + depends_on: + - another + another: + image: busybox:1.31.0-uclibc + command: ls /thecakeisalie diff --git a/tests/fixtures/echo-services-dependencies/docker-compose.yml b/tests/fixtures/echo-services-dependencies/docker-compose.yml new file mode 100644 index 000000000..5329e0033 --- /dev/null +++ b/tests/fixtures/echo-services-dependencies/docker-compose.yml @@ -0,0 +1,10 @@ +version: "2.0" +services: + simple: + image: busybox:1.31.0-uclibc + command: echo simple + depends_on: + - another + another: + image: busybox:1.31.0-uclibc + command: echo another diff --git a/tests/unit/cli/main_test.py b/tests/unit/cli/main_test.py index aadb9d459..067c74f0b 100644 --- a/tests/unit/cli/main_test.py +++ b/tests/unit/cli/main_test.py @@ -12,7 +12,7 @@ from compose.cli.formatter import ConsoleWarningFormatter from compose.cli.main import build_one_off_container_options from compose.cli.main import call_docker from compose.cli.main import convergence_strategy_from_opts -from compose.cli.main import filter_containers_to_service_names +from compose.cli.main import filter_attached_containers from compose.cli.main import get_docker_start_call from compose.cli.main import setup_console_handler from compose.cli.main import warn_for_swarm_mode @@ -37,7 +37,7 @@ def logging_handler(): class TestCLIMainTestCase(object): - def test_filter_containers_to_service_names(self): + def test_filter_attached_containers(self): containers = [ mock_container('web', 1), mock_container('web', 2), @@ -46,17 +46,29 @@ class TestCLIMainTestCase(object): mock_container('another', 1), ] service_names = ['web', 'db'] - actual = filter_containers_to_service_names(containers, service_names) + actual = filter_attached_containers(containers, service_names) assert actual == containers[:3] - def test_filter_containers_to_service_names_all(self): + def test_filter_attached_containers_with_dependencies(self): + containers = [ + mock_container('web', 1), + mock_container('web', 2), + mock_container('db', 1), + mock_container('other', 1), + mock_container('another', 1), + ] + service_names = ['web', 'db'] + actual = filter_attached_containers(containers, service_names, attach_dependencies=True) + assert actual == containers + + def test_filter_attached_containers_all(self): containers = [ mock_container('web', 1), mock_container('db', 1), mock_container('other', 1), ] service_names = [] - actual = filter_containers_to_service_names(containers, service_names) + actual = filter_attached_containers(containers, service_names) assert actual == containers def test_warning_in_swarm_mode(self):