From ea4230e7a2f53a116c22dce20632cb5355cf4c07 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 10 Nov 2015 18:52:21 -0500 Subject: [PATCH 1/2] Handle both SIGINT and SIGTERM for docker-compose up. Signed-off-by: Daniel Nephin --- compose/cli/main.py | 21 +++++++---- tests/acceptance/cli_test.py | 70 +++++++++++++++++++++++++++++------- tests/unit/cli/main_test.py | 8 ++--- 3 files changed, 76 insertions(+), 23 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index 806926d8..7b1e0aa3 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -658,17 +658,24 @@ def build_log_printer(containers, service_names, monochrome): def attach_to_logs(project, log_printer, service_names, timeout): print("Attaching to", list_containers(log_printer.containers)) - try: - log_printer.run() - finally: - def handler(signal, frame): - project.kill(service_names=service_names) - sys.exit(0) - signal.signal(signal.SIGINT, handler) + def force_shutdown(signal, frame): + project.kill(service_names=service_names) + sys.exit(2) + + def shutdown(signal, frame): + set_signal_handler(force_shutdown) print("Gracefully stopping... (press Ctrl+C again to force)") project.stop(service_names=service_names, timeout=timeout) + set_signal_handler(shutdown) + log_printer.run() + + +def set_signal_handler(handler): + signal.signal(signal.SIGINT, handler) + signal.signal(signal.SIGTERM, handler) + def list_containers(containers): return ", ".join(c.name for c in containers) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 88a43d7f..57f2039e 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -2,7 +2,9 @@ from __future__ import absolute_import import os import shlex +import signal import subprocess +import time from collections import namedtuple from operator import attrgetter @@ -20,6 +22,45 @@ BUILD_CACHE_TEXT = 'Using cache' BUILD_PULL_TEXT = 'Status: Image is up to date for busybox:latest' +def start_process(base_dir, options): + proc = subprocess.Popen( + ['docker-compose'] + options, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=base_dir) + print("Running process: %s" % proc.pid) + return proc + + +def wait_on_process(proc, returncode=0): + stdout, stderr = proc.communicate() + if proc.returncode != returncode: + print(stderr) + assert proc.returncode == returncode + return ProcessResult(stdout.decode('utf-8'), stderr.decode('utf-8')) + + +def wait_on_condition(condition, delay=0.1, timeout=5): + start_time = time.time() + while not condition(): + if time.time() - start_time > timeout: + raise AssertionError("Timeout: %s" % condition) + time.sleep(delay) + + +class ContainerCountCondition(object): + + def __init__(self, project, expected): + self.project = project + self.expected = expected + + def __call__(self): + return len(self.project.containers()) == self.expected + + def __str__(self): + return "waiting for counter count == %s" % self.expected + + class CLITestCase(DockerClientTestCase): def setUp(self): @@ -42,17 +83,8 @@ class CLITestCase(DockerClientTestCase): def dispatch(self, options, project_options=None, returncode=0): project_options = project_options or [] - proc = subprocess.Popen( - ['docker-compose'] + project_options + options, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - cwd=self.base_dir) - print("Running process: %s" % proc.pid) - stdout, stderr = proc.communicate() - if proc.returncode != returncode: - print(stderr) - assert proc.returncode == returncode - return ProcessResult(stdout.decode('utf-8'), stderr.decode('utf-8')) + proc = start_process(self.base_dir, project_options + options) + return wait_on_process(proc, returncode=returncode) def test_help(self): old_base_dir = self.base_dir @@ -291,7 +323,7 @@ class CLITestCase(DockerClientTestCase): returncode=1) def test_up_with_timeout(self): - self.dispatch(['up', '-d', '-t', '1'], None) + self.dispatch(['up', '-d', '-t', '1']) service = self.project.get_service('simple') another = self.project.get_service('another') self.assertEqual(len(service.containers()), 1) @@ -303,6 +335,20 @@ class CLITestCase(DockerClientTestCase): self.assertFalse(config['AttachStdout']) self.assertFalse(config['AttachStdin']) + def test_up_handles_sigint(self): + proc = start_process(self.base_dir, ['up', '-t', '2']) + wait_on_condition(ContainerCountCondition(self.project, 2)) + + os.kill(proc.pid, signal.SIGINT) + wait_on_condition(ContainerCountCondition(self.project, 0)) + + def test_up_handles_sigterm(self): + proc = start_process(self.base_dir, ['up', '-t', '2']) + wait_on_condition(ContainerCountCondition(self.project, 2)) + + os.kill(proc.pid, signal.SIGTERM) + wait_on_condition(ContainerCountCondition(self.project, 0)) + def test_run_service_without_links(self): self.base_dir = 'tests/fixtures/links-composefile' self.dispatch(['run', 'console', '/bin/true']) diff --git a/tests/unit/cli/main_test.py b/tests/unit/cli/main_test.py index ee837fcd..db37ac1a 100644 --- a/tests/unit/cli/main_test.py +++ b/tests/unit/cli/main_test.py @@ -57,11 +57,11 @@ class CLIMainTestCase(unittest.TestCase): with mock.patch('compose.cli.main.signal', autospec=True) as mock_signal: attach_to_logs(project, log_printer, service_names, timeout) - mock_signal.signal.assert_called_once_with(mock_signal.SIGINT, mock.ANY) + assert mock_signal.signal.mock_calls == [ + mock.call(mock_signal.SIGINT, mock.ANY), + mock.call(mock_signal.SIGTERM, mock.ANY), + ] log_printer.run.assert_called_once_with() - project.stop.assert_called_once_with( - service_names=service_names, - timeout=timeout) class SetupConsoleHandlerTestCase(unittest.TestCase): From 6236bb0019de51fe482e2ba6be8a99de471a6861 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 10 Nov 2015 19:43:05 -0500 Subject: [PATCH 2/2] Handle both SIGINT and SIGTERM for docker-compose run. Signed-off-by: Daniel Nephin --- compose/cli/main.py | 99 ++++++++++++++++++++---------------- tests/acceptance/cli_test.py | 47 +++++++++++++++++ 2 files changed, 102 insertions(+), 44 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index 7b1e0aa3..9fef8d04 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -368,7 +368,6 @@ class TopLevelCommand(DocoptCommand): allocates a TTY. """ service = project.get_service(options['SERVICE']) - detach = options['-d'] if IS_WINDOWS_PLATFORM and not detach: @@ -380,22 +379,6 @@ class TopLevelCommand(DocoptCommand): if options['--allow-insecure-ssl']: log.warn(INSECURE_SSL_WARNING) - if not options['--no-deps']: - deps = service.get_linked_service_names() - - if len(deps) > 0: - project.up( - service_names=deps, - start_deps=True, - strategy=ConvergenceStrategy.never, - ) - elif project.use_networking: - project.ensure_network_exists() - - tty = True - if detach or options['-T'] or not sys.stdin.isatty(): - tty = False - if options['COMMAND']: command = [options['COMMAND']] + options['ARGS'] else: @@ -403,7 +386,7 @@ class TopLevelCommand(DocoptCommand): container_options = { 'command': command, - 'tty': tty, + 'tty': not (detach or options['-T'] or not sys.stdin.isatty()), 'stdin_open': not detach, 'detach': detach, } @@ -435,31 +418,7 @@ class TopLevelCommand(DocoptCommand): if options['--name']: container_options['name'] = options['--name'] - try: - container = service.create_container( - quiet=True, - one_off=True, - **container_options - ) - except APIError as e: - legacy.check_for_legacy_containers( - project.client, - project.name, - [service.name], - allow_one_off=False, - ) - - raise e - - if detach: - container.start() - print(container.name) - else: - dockerpty.start(project.client, container.id, interactive=not options['-T']) - exit_code = container.wait() - if options['--rm']: - project.client.remove_container(container.id) - sys.exit(exit_code) + run_one_off_container(container_options, project, service, options) def scale(self, project, options): """ @@ -647,6 +606,58 @@ def convergence_strategy_from_opts(options): return ConvergenceStrategy.changed +def run_one_off_container(container_options, project, service, options): + if not options['--no-deps']: + deps = service.get_linked_service_names() + if deps: + project.up( + service_names=deps, + start_deps=True, + strategy=ConvergenceStrategy.never) + + if project.use_networking: + project.ensure_network_exists() + + try: + container = service.create_container( + quiet=True, + one_off=True, + **container_options) + except APIError: + legacy.check_for_legacy_containers( + project.client, + project.name, + [service.name], + allow_one_off=False) + raise + + if options['-d']: + container.start() + print(container.name) + return + + def remove_container(force=False): + if options['--rm']: + project.client.remove_container(container.id, force=True) + + def force_shutdown(signal, frame): + project.client.kill(container.id) + remove_container(force=True) + sys.exit(2) + + def shutdown(signal, frame): + set_signal_handler(force_shutdown) + project.client.stop(container.id) + remove_container() + sys.exit(1) + + set_signal_handler(shutdown) + dockerpty.start(project.client, container.id, interactive=not options['-T']) + exit_code = container.wait() + remove_container() + sys.exit(exit_code) + + def build_log_printer(containers, service_names, monochrome): if service_names: containers = [ @@ -657,7 +668,6 @@ def build_log_printer(containers, service_names, monochrome): def attach_to_logs(project, log_printer, service_names, timeout): - print("Attaching to", list_containers(log_printer.containers)) def force_shutdown(signal, frame): project.kill(service_names=service_names) @@ -668,6 +678,7 @@ def attach_to_logs(project, log_printer, service_names, timeout): print("Gracefully stopping... (press Ctrl+C again to force)") project.stop(service_names=service_names, timeout=timeout) + print("Attaching to", list_containers(log_printer.containers)) set_signal_handler(shutdown) log_printer.run() diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 57f2039e..b88ed280 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -8,6 +8,8 @@ import time from collections import namedtuple from operator import attrgetter +from docker import errors + from .. import mock from compose.cli.command import get_project from compose.cli.docker_client import docker_client @@ -61,6 +63,25 @@ class ContainerCountCondition(object): return "waiting for counter count == %s" % self.expected +class ContainerStateCondition(object): + + def __init__(self, client, name, running): + self.client = client + self.name = name + self.running = running + + # State.Running == true + def __call__(self): + try: + container = self.client.inspect_container(self.name) + return container['State']['Running'] == self.running + except errors.APIError: + return False + + def __str__(self): + return "waiting for container to have state %s" % self.expected + + class CLITestCase(DockerClientTestCase): def setUp(self): @@ -554,6 +575,32 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(len(networks), 1) self.assertEqual(container.human_readable_command, u'true') + def test_run_handles_sigint(self): + proc = start_process(self.base_dir, ['run', '-T', 'simple', 'top']) + wait_on_condition(ContainerStateCondition( + self.project.client, + 'simplecomposefile_simple_run_1', + running=True)) + + os.kill(proc.pid, signal.SIGINT) + wait_on_condition(ContainerStateCondition( + self.project.client, + 'simplecomposefile_simple_run_1', + running=False)) + + def test_run_handles_sigterm(self): + proc = start_process(self.base_dir, ['run', '-T', 'simple', 'top']) + wait_on_condition(ContainerStateCondition( + self.project.client, + 'simplecomposefile_simple_run_1', + running=True)) + + os.kill(proc.pid, signal.SIGTERM) + wait_on_condition(ContainerStateCondition( + self.project.client, + 'simplecomposefile_simple_run_1', + running=False)) + def test_rm(self): service = self.project.get_service('simple') service.create_container()