From 06db577105d54394842b4634038a54e55e83252f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sun, 14 Jun 2015 17:11:29 -0400 Subject: [PATCH] Move converge() to a test module, and use a short timeout for tests. Signed-off-by: Daniel Nephin --- compose/cli/main.py | 19 ++++++++--------- compose/const.py | 1 + compose/project.py | 4 ++-- compose/service.py | 28 ++++--------------------- tests/integration/service_test.py | 33 +++++++++++++++++++++--------- tests/integration/state_test.py | 34 ++++++++++++++++++++++++++----- tests/unit/service_test.py | 2 +- 7 files changed, 69 insertions(+), 52 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index a9d04472f..8d21beafe 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -10,7 +10,9 @@ import sys from docker.errors import APIError import dockerpty -from .. import __version__, legacy +from .. import __version__ +from .. import legacy +from ..const import DEFAULT_TIMEOUT from ..project import NoSuchService, ConfigurationError from ..service import BuildError, NeedsBuildError from ..config import parse_environment @@ -394,9 +396,8 @@ class TopLevelCommand(Command): -t, --timeout TIMEOUT Specify a shutdown timeout in seconds. (default: 10) """ - timeout = options.get('--timeout') - params = {} if timeout is None else {'timeout': int(timeout)} - project.stop(service_names=options['SERVICE'], **params) + timeout = float(options.get('--timeout') or DEFAULT_TIMEOUT) + project.stop(service_names=options['SERVICE'], timeout=timeout) def restart(self, project, options): """ @@ -408,9 +409,8 @@ class TopLevelCommand(Command): -t, --timeout TIMEOUT Specify a shutdown timeout in seconds. (default: 10) """ - timeout = options.get('--timeout') - params = {} if timeout is None else {'timeout': int(timeout)} - project.restart(service_names=options['SERVICE'], **params) + timeout = float(options.get('--timeout') or DEFAULT_TIMEOUT) + project.restart(service_names=options['SERVICE'], timeout=timeout) def up(self, project, options): """ @@ -452,7 +452,7 @@ class TopLevelCommand(Command): allow_recreate = not options['--no-recreate'] smart_recreate = options['--x-smart-recreate'] service_names = options['SERVICE'] - timeout = int(options['--timeout']) if options['--timeout'] is not None else None + timeout = float(options.get('--timeout') or DEFAULT_TIMEOUT) project.up( service_names=service_names, @@ -479,8 +479,7 @@ class TopLevelCommand(Command): signal.signal(signal.SIGINT, handler) print("Gracefully stopping... (press Ctrl+C again to force)") - params = {} if timeout is None else {'timeout': timeout} - project.stop(service_names=service_names, **params) + project.stop(service_names=service_names, timeout=timeout) def migrate_to_labels(self, project, _options): """ diff --git a/compose/const.py b/compose/const.py index f76fb572c..709c3a10d 100644 --- a/compose/const.py +++ b/compose/const.py @@ -1,4 +1,5 @@ +DEFAULT_TIMEOUT = 10 LABEL_CONTAINER_NUMBER = 'com.docker.compose.container-number' LABEL_ONE_OFF = 'com.docker.compose.oneoff' LABEL_PROJECT = 'com.docker.compose.project' diff --git a/compose/project.py b/compose/project.py index ddf681d5d..b4ed12ea4 100644 --- a/compose/project.py +++ b/compose/project.py @@ -6,7 +6,7 @@ from functools import reduce from docker.errors import APIError from .config import get_service_name_from_net, ConfigurationError -from .const import LABEL_PROJECT, LABEL_SERVICE, LABEL_ONE_OFF +from .const import LABEL_PROJECT, LABEL_SERVICE, LABEL_ONE_OFF, DEFAULT_TIMEOUT from .service import Service from .container import Container from .legacy import check_for_legacy_containers @@ -212,7 +212,7 @@ class Project(object): smart_recreate=False, insecure_registry=False, do_build=True, - timeout=None): + timeout=DEFAULT_TIMEOUT): services = self.get_services(service_names, include_deps=start_deps) diff --git a/compose/service.py b/compose/service.py index 7a0264c2d..53073ffbd 100644 --- a/compose/service.py +++ b/compose/service.py @@ -13,6 +13,7 @@ from docker.utils import create_host_config, LogConfig from . import __version__ from .config import DOCKER_CONFIG_KEYS, merge_environment from .const import ( + DEFAULT_TIMEOUT, LABEL_CONTAINER_NUMBER, LABEL_ONE_OFF, LABEL_PROJECT, @@ -251,26 +252,6 @@ class Service(object): else: return self.options['image'] - def converge(self, - allow_recreate=True, - smart_recreate=False, - insecure_registry=False, - do_build=True): - """ - If a container for this service doesn't exist, create and start one. If there are - any, stop them, create+start new ones, and remove the old containers. - """ - plan = self.convergence_plan( - allow_recreate=allow_recreate, - smart_recreate=smart_recreate, - ) - - return self.execute_convergence_plan( - plan, - insecure_registry=insecure_registry, - do_build=do_build, - ) - def convergence_plan(self, allow_recreate=True, smart_recreate=False): @@ -312,7 +293,7 @@ class Service(object): plan, insecure_registry=False, do_build=True, - timeout=None): + timeout=DEFAULT_TIMEOUT): (action, containers) = plan if action == 'create': @@ -352,7 +333,7 @@ class Service(object): def recreate_container(self, container, insecure_registry=False, - timeout=None): + timeout=DEFAULT_TIMEOUT): """Recreate a container. The original container is renamed to a temporary name so that data @@ -361,8 +342,7 @@ class Service(object): """ log.info("Recreating %s..." % container.name) try: - stop_params = {} if timeout is None else {'timeout': timeout} - container.stop(**stop_params) + container.stop(timeout=timeout) except APIError as e: if (e.response.status_code == 500 and e.explanation diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 5a725f07c..3b3ac22f9 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -2,8 +2,9 @@ from __future__ import unicode_literals from __future__ import absolute_import import os from os import path -import mock +from docker.errors import APIError +import mock import tempfile import shutil import six @@ -18,11 +19,11 @@ from compose.const import ( ) from compose.service import ( ConfigError, + ConvergencePlan, Service, build_extra_hosts, ) from compose.container import Container -from docker.errors import APIError from .testcases import DockerClientTestCase @@ -249,7 +250,7 @@ class ServiceTest(DockerClientTestCase): self.assertIn(volume_container_2.id, host_container.get('HostConfig.VolumesFrom')) - def test_converge(self): + def test_execute_convergence_plan_recreate(self): service = self.create_service( 'db', environment={'FOO': '1'}, @@ -269,7 +270,8 @@ class ServiceTest(DockerClientTestCase): num_containers_before = len(self.client.containers(all=True)) service.options['environment']['FOO'] = '2' - new_container = service.converge()[0] + new_container, = service.execute_convergence_plan( + ConvergencePlan('recreate', [old_container])) self.assertEqual(new_container.get('Config.Entrypoint'), ['top']) self.assertEqual(new_container.get('Config.Cmd'), ['-d', '1']) @@ -286,7 +288,7 @@ class ServiceTest(DockerClientTestCase): self.client.inspect_container, old_container.id) - def test_converge_when_containers_are_stopped(self): + def test_execute_convergence_plan_when_containers_are_stopped(self): service = self.create_service( 'db', environment={'FOO': '1'}, @@ -295,11 +297,21 @@ class ServiceTest(DockerClientTestCase): command=['-d', '1'] ) service.create_container() - self.assertEqual(len(service.containers(stopped=True)), 1) - service.converge() - self.assertEqual(len(service.containers(stopped=True)), 1) - def test_converge_with_image_declared_volume(self): + containers = service.containers(stopped=True) + self.assertEqual(len(containers), 1) + container, = containers + self.assertFalse(container.is_running) + + service.execute_convergence_plan(ConvergencePlan('start', [container])) + + containers = service.containers() + self.assertEqual(len(containers), 1) + container.inspect() + self.assertEqual(container, containers[0]) + self.assertTrue(container.is_running) + + def test_execute_convergence_plan_with_image_declared_volume(self): service = Service( project='composetest', name='db', @@ -311,7 +323,8 @@ class ServiceTest(DockerClientTestCase): self.assertEqual(old_container.get('Volumes').keys(), ['/data']) volume_path = old_container.get('Volumes')['/data'] - new_container = service.converge()[0] + new_container, = service.execute_convergence_plan( + ConvergencePlan('recreate', [old_container])) self.assertEqual(new_container.get('Volumes').keys(), ['/data']) self.assertEqual(new_container.get('Volumes')['/data'], volume_path) diff --git a/tests/integration/state_test.py b/tests/integration/state_test.py index b99a299a0..cd59d13c9 100644 --- a/tests/integration/state_test.py +++ b/tests/integration/state_test.py @@ -12,8 +12,8 @@ from .testcases import DockerClientTestCase class ProjectTestCase(DockerClientTestCase): def run_up(self, cfg, **kwargs): - if 'smart_recreate' not in kwargs: - kwargs['smart_recreate'] = True + kwargs.setdefault('smart_recreate', True) + kwargs.setdefault('timeout', 0.1) project = self.make_project(cfg) project.up(**kwargs) @@ -153,7 +153,31 @@ class ProjectWithDependenciesTest(ProjectTestCase): self.assertEqual(new_containers - old_containers, set()) +def converge(service, + allow_recreate=True, + smart_recreate=False, + insecure_registry=False, + do_build=True): + """ + If a container for this service doesn't exist, create and start one. If there are + any, stop them, create+start new ones, and remove the old containers. + """ + plan = service.convergence_plan( + allow_recreate=allow_recreate, + smart_recreate=smart_recreate, + ) + + return service.execute_convergence_plan( + plan, + insecure_registry=insecure_registry, + do_build=do_build, + timeout=0.1, + ) + + class ServiceStateTest(DockerClientTestCase): + """Test cases for Service.convergence_plan.""" + def test_trigger_create(self): web = self.create_service('web') self.assertEqual(('create', []), web.convergence_plan(smart_recreate=True)) @@ -250,15 +274,15 @@ class ConfigHashTest(DockerClientTestCase): def test_config_hash_with_custom_labels(self): web = self.create_service('web', labels={'foo': '1'}) - container = web.converge()[0] + container = converge(web)[0] self.assertIn(LABEL_CONFIG_HASH, container.labels) self.assertIn('foo', container.labels) def test_config_hash_sticks_around(self): web = self.create_service('web', command=["top"]) - container = web.converge()[0] + container = converge(web)[0] self.assertIn(LABEL_CONFIG_HASH, container.labels) web = self.create_service('web', command=["top", "-d", "1"]) - container = web.converge()[0] + container = converge(web)[0] self.assertIn(LABEL_CONFIG_HASH, container.labels) diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 595b9d373..82ea04101 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -246,7 +246,7 @@ class ServiceTest(unittest.TestCase): service.image = lambda: {'Id': 'abc123'} new_container = service.recreate_container(mock_container) - mock_container.stop.assert_called_once_with() + mock_container.stop.assert_called_once_with(timeout=10) self.mock_client.rename.assert_called_once_with( mock_container.id, '%s_%s' % (mock_container.short_id, mock_container.name))