From a73190e1cc2c57ba2ac0361b376b2adc5c4dabeb Mon Sep 17 00:00:00 2001 From: "Nathan J. Mehl" Date: Fri, 27 Jan 2017 08:56:02 -0800 Subject: [PATCH 1/2] Add support for returning the exit value of a specific container Current best practice for using docker-compose as a tool for continuous integration requires fragile shell pipelines to query the exit status of composed containers, e.g.: http://stackoverflow.com/questions/29568352/using-docker-compose-with-ci-how-to-deal-with-exit-codes-and-daemonized-linked http://blog.ministryofprogramming.com/docker-compose-and-exit-codes/ This PR adds a `--forward-exitval ` flag that allows `docker-compose up` to return the exit value of a specified container. The container may optionally have a number specified (foo_2) otherwise the first is defaulted to. Signed-off-by: Nathan J. Mehl --- compose/cli/main.py | 47 ++++++++++++++++++- contrib/completion/bash/docker-compose | 2 +- tests/acceptance/cli_test.py | 10 ++++ .../forward-exitval/docker-compose.yml | 6 +++ 4 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/forward-exitval/docker-compose.yml diff --git a/compose/cli/main.py b/compose/cli/main.py index 51ba36a09..bbd6952a0 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -854,6 +854,8 @@ class TopLevelCommand(object): running. (default: 10) --remove-orphans Remove containers for services not defined in the Compose file + --forward-exitval SERVICE Return the exit value of the selected service container. + Requires --abort-on-container-exit. """ start_deps = not options['--no-deps'] cascade_stop = options['--abort-on-container-exit'] @@ -861,10 +863,14 @@ class TopLevelCommand(object): timeout = timeout_from_opts(options) remove_orphans = options['--remove-orphans'] detached = options.get('-d') + forward_exitval = container_exitval_from_opts(options) if detached and cascade_stop: raise UserError("--abort-on-container-exit and -d cannot be combined.") + if forward_exitval and not cascade_stop: + raise UserError("--forward-exitval requires --abort-on-container-exit.") + with up_shutdown_context(self.project, service_names, timeout, detached): to_attach = self.project.up( service_names=service_names, @@ -878,9 +884,11 @@ class TopLevelCommand(object): if detached: return + all_containers = filter_containers_to_service_names(to_attach, service_names) + log_printer = log_printer_from_project( self.project, - filter_containers_to_service_names(to_attach, service_names), + all_containers, options['--no-color'], {'follow': True}, cascade_stop, @@ -891,6 +899,22 @@ class TopLevelCommand(object): if cascade_stop: print("Aborting on container exit...") self.project.stop(service_names=service_names, timeout=timeout) + if forward_exitval: + def is_us(container): + return container.name_without_project == forward_exitval + candidates = filter(is_us, all_containers) + if not candidates: + log.error('No containers matching the spec "%s" were run.', + forward_exitval) + sys.exit(2) + if len(candidates) > 1: + log.error('Multiple (%d) containers matching the spec "%s" ' + 'were found; cannot forward exit code because we ' + 'do not know which one to.', len(candidates), + forward_exitval) + sys.exit(2) + exit_code = candidates[0].inspect()['State']['ExitCode'] + sys.exit(exit_code) @classmethod def version(cls, options): @@ -923,6 +947,27 @@ def convergence_strategy_from_opts(options): return ConvergenceStrategy.changed +def container_exitval_from_opts(options): + """ Assemble a container name suitable for mapping into the + output of filter_containers_to_service_names. If the + container name ends in an underscore followed by a + positive integer, the user has deliberately specified + a container number and we believe her. Otherwise, append + `_1` to the name so as to return the exit value of the + first such named container. + """ + container_name = options.get('--forward-exitval') + if not container_name: + return None + segments = container_name.split('_') + if segments[-1].isdigit() and int(segments[-1]) > 0: + return '_'.join(segments) + else: + log.warn('"%s" does not specify a container number, ' + 'defaulting to "%s_1"', container_name, container_name) + return '_'.join(segments + ['1']) + + def timeout_from_opts(options): timeout = options.get('--timeout') return None if timeout is None else int(timeout) diff --git a/contrib/completion/bash/docker-compose b/contrib/completion/bash/docker-compose index 79f0fc313..979942f97 100644 --- a/contrib/completion/bash/docker-compose +++ b/contrib/completion/bash/docker-compose @@ -467,7 +467,7 @@ _docker_compose_up() { case "$cur" in -*) - COMPREPLY=( $( compgen -W "--abort-on-container-exit --build -d --force-recreate --help --no-build --no-color --no-deps --no-recreate --timeout -t --remove-orphans" -- "$cur" ) ) + COMPREPLY=( $( compgen -W "--forward-exitval --abort-on-container-exit --build -d --force-recreate --help --no-build --no-color --no-deps --no-recreate --timeout -t --remove-orphans" -- "$cur" ) ) ;; *) __docker_compose_services_all diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 8366ca75e..6e03c448c 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -1927,3 +1927,13 @@ class CLITestCase(DockerClientTestCase): self.dispatch(['up', '-d']) result = self.dispatch(['top']) assert result.stdout.count("top") == 4 + + def test_forward_exitval(self): + self.base_dir = 'tests/fixtures/forward-exitval' + proc = start_process( + self.base_dir, + ['up', '--abort-on-container-exit', '--forward-exitval', 'another']) + + result = wait_on_process(proc, returncode=1) + + assert 'forwardexitval_another_1 exited with code 1' in result.stdout diff --git a/tests/fixtures/forward-exitval/docker-compose.yml b/tests/fixtures/forward-exitval/docker-compose.yml new file mode 100644 index 000000000..687e78b97 --- /dev/null +++ b/tests/fixtures/forward-exitval/docker-compose.yml @@ -0,0 +1,6 @@ +simple: + image: busybox:latest + command: sh -c "echo hello && tail -f /dev/null" +another: + image: busybox:latest + command: /bin/false From cffb76d4d9a731495195dec7acaab0b1b438f47c Mon Sep 17 00:00:00 2001 From: "Nathan J. Mehl" Date: Mon, 27 Feb 2017 07:21:47 -0800 Subject: [PATCH 2/2] Address comments - set flag name to `--exit-code-from` (and rename some variable, function and test names to match) - force cascade_stop to true when exit-code-from flag is set - use lambda in filter statement - check that selected container name is in the project before running - remove fancy parsing of service name to container mappings: if there are multiple containers in a service, return the first nonzero exit value if any - flake8 changes Signed-off-by: Nathan J. Mehl --- compose/cli/main.py | 78 +++++++++---------- contrib/completion/bash/docker-compose | 2 +- tests/acceptance/cli_test.py | 6 +- .../docker-compose.yml | 0 4 files changed, 42 insertions(+), 44 deletions(-) rename tests/fixtures/{forward-exitval => exit-code-from}/docker-compose.yml (100%) diff --git a/compose/cli/main.py b/compose/cli/main.py index bbd6952a0..f1d343f3a 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -854,23 +854,20 @@ class TopLevelCommand(object): running. (default: 10) --remove-orphans Remove containers for services not defined in the Compose file - --forward-exitval SERVICE Return the exit value of the selected service container. + --exit-code-from SERVICE Return the exit code of the selected service container. Requires --abort-on-container-exit. """ start_deps = not options['--no-deps'] + exit_value_from = exitval_from_opts(options, self.project) cascade_stop = options['--abort-on-container-exit'] service_names = options['SERVICE'] timeout = timeout_from_opts(options) remove_orphans = options['--remove-orphans'] detached = options.get('-d') - forward_exitval = container_exitval_from_opts(options) if detached and cascade_stop: raise UserError("--abort-on-container-exit and -d cannot be combined.") - if forward_exitval and not cascade_stop: - raise UserError("--forward-exitval requires --abort-on-container-exit.") - with up_shutdown_context(self.project, service_names, timeout, detached): to_attach = self.project.up( service_names=service_names, @@ -884,11 +881,11 @@ class TopLevelCommand(object): if detached: return - all_containers = filter_containers_to_service_names(to_attach, service_names) + attached_containers = filter_containers_to_service_names(to_attach, service_names) log_printer = log_printer_from_project( self.project, - all_containers, + attached_containers, options['--no-color'], {'follow': True}, cascade_stop, @@ -899,22 +896,31 @@ class TopLevelCommand(object): if cascade_stop: print("Aborting on container exit...") self.project.stop(service_names=service_names, timeout=timeout) - if forward_exitval: - def is_us(container): - return container.name_without_project == forward_exitval - candidates = filter(is_us, all_containers) + if exit_value_from: + candidates = filter( + lambda c: c.service == exit_value_from, + attached_containers) if not candidates: log.error('No containers matching the spec "%s" were run.', - forward_exitval) + exit_value_from) sys.exit(2) if len(candidates) > 1: - log.error('Multiple (%d) containers matching the spec "%s" ' - 'were found; cannot forward exit code because we ' - 'do not know which one to.', len(candidates), - forward_exitval) - sys.exit(2) - exit_code = candidates[0].inspect()['State']['ExitCode'] - sys.exit(exit_code) + exit_values = [] + for candidate in candidates: + exit_val = candidate.inspect()['State']['ExitCode'] + if exit_val: + exit_values.append(exit_val) + if exit_values: + log.warn('Multiple (%d) containers matching the service name "%s" ' + 'were found and at least one exited nonzero; returning ' + 'the first non-zero exit code. See above for detailed ' + 'exit statuses.', len(candidates), exit_value_from) + sys.exit(exit_values[0]) + else: + exit_value = candidates[0].inspect()['State']['ExitCode'] + log.error('Returning exit value %d from container %s.', exit_value, + candidates[0].name_without_project) + sys.exit(exit_value) @classmethod def version(cls, options): @@ -947,32 +953,24 @@ def convergence_strategy_from_opts(options): return ConvergenceStrategy.changed -def container_exitval_from_opts(options): - """ Assemble a container name suitable for mapping into the - output of filter_containers_to_service_names. If the - container name ends in an underscore followed by a - positive integer, the user has deliberately specified - a container number and we believe her. Otherwise, append - `_1` to the name so as to return the exit value of the - first such named container. - """ - container_name = options.get('--forward-exitval') - if not container_name: - return None - segments = container_name.split('_') - if segments[-1].isdigit() and int(segments[-1]) > 0: - return '_'.join(segments) - else: - log.warn('"%s" does not specify a container number, ' - 'defaulting to "%s_1"', container_name, container_name) - return '_'.join(segments + ['1']) - - def timeout_from_opts(options): timeout = options.get('--timeout') return None if timeout is None else int(timeout) +def exitval_from_opts(options, project): + exit_value_from = options.get('--exit-code-from') + if exit_value_from: + if not options.get('--abort-on-container-exit'): + log.warn('using --exit-code-from implies --abort-on-container-exit') + options['--abort-on-container-exit'] = True + if exit_value_from not in [s.name for s in project.get_services()]: + log.error('No service named "%s" was found in your compose file.', + exit_value_from) + sys.exit(2) + return exit_value_from + + def image_type_from_opt(flag, value): if not value: return ImageType.none diff --git a/contrib/completion/bash/docker-compose b/contrib/completion/bash/docker-compose index 979942f97..3acbb5806 100644 --- a/contrib/completion/bash/docker-compose +++ b/contrib/completion/bash/docker-compose @@ -467,7 +467,7 @@ _docker_compose_up() { case "$cur" in -*) - COMPREPLY=( $( compgen -W "--forward-exitval --abort-on-container-exit --build -d --force-recreate --help --no-build --no-color --no-deps --no-recreate --timeout -t --remove-orphans" -- "$cur" ) ) + COMPREPLY=( $( compgen -W "--exit-code-from --abort-on-container-exit --build -d --force-recreate --help --no-build --no-color --no-deps --no-recreate --timeout -t --remove-orphans" -- "$cur" ) ) ;; *) __docker_compose_services_all diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 6e03c448c..42c4de384 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -1929,11 +1929,11 @@ class CLITestCase(DockerClientTestCase): assert result.stdout.count("top") == 4 def test_forward_exitval(self): - self.base_dir = 'tests/fixtures/forward-exitval' + self.base_dir = 'tests/fixtures/exit-code-from' proc = start_process( self.base_dir, - ['up', '--abort-on-container-exit', '--forward-exitval', 'another']) + ['up', '--abort-on-container-exit', '--exit-code-from', 'another']) result = wait_on_process(proc, returncode=1) - assert 'forwardexitval_another_1 exited with code 1' in result.stdout + assert 'exitcodefrom_another_1 exited with code 1' in result.stdout diff --git a/tests/fixtures/forward-exitval/docker-compose.yml b/tests/fixtures/exit-code-from/docker-compose.yml similarity index 100% rename from tests/fixtures/forward-exitval/docker-compose.yml rename to tests/fixtures/exit-code-from/docker-compose.yml