From 0484e22a84cf430871b32d1136d94d3083214f61 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 2 Sep 2015 11:07:59 -0400 Subject: [PATCH] Add enum34 and use it to create a ConvergenceStrategy enum. Signed-off-by: Daniel Nephin --- compose/cli/main.py | 31 ++++++++++++++--------- compose/project.py | 38 ++++++++-------------------- compose/service.py | 30 +++++++++++++++------- docs/index.md | 2 +- requirements.txt | 1 + setup.py | 3 ++- tests/integration/cli_test.py | 6 ++--- tests/integration/project_test.py | 7 ++--- tests/integration/resilience_test.py | 7 ++--- tests/integration/state_test.py | 25 ++++++------------ tests/unit/cli/main_test.py | 32 +++++++++++++++++++++++ 11 files changed, 105 insertions(+), 77 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index 11d2d104c..cf971844b 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -19,6 +19,7 @@ from ..progress_stream import StreamOutputError from ..project import ConfigurationError from ..project import NoSuchService from ..service import BuildError +from ..service import ConvergenceStrategy from ..service import NeedsBuildError from .command import Command from .docopt_command import NoSuchCommand @@ -332,7 +333,7 @@ class TopLevelCommand(Command): project.up( service_names=deps, start_deps=True, - allow_recreate=False, + strategy=ConvergenceStrategy.never, ) tty = True @@ -515,29 +516,20 @@ class TopLevelCommand(Command): if options['--allow-insecure-ssl']: log.warn(INSECURE_SSL_WARNING) - detached = options['-d'] - monochrome = options['--no-color'] - start_deps = not options['--no-deps'] - allow_recreate = not options['--no-recreate'] - force_recreate = options['--force-recreate'] service_names = options['SERVICE'] timeout = int(options.get('--timeout') or DEFAULT_TIMEOUT) - if force_recreate and not allow_recreate: - raise UserError("--force-recreate and --no-recreate cannot be combined.") - to_attach = project.up( service_names=service_names, start_deps=start_deps, - allow_recreate=allow_recreate, - force_recreate=force_recreate, + strategy=convergence_strategy_from_opts(options), do_build=not options['--no-build'], timeout=timeout ) - if not detached: + if not options['-d']: log_printer = build_log_printer(to_attach, service_names, monochrome) attach_to_logs(project, log_printer, service_names, timeout) @@ -582,6 +574,21 @@ class TopLevelCommand(Command): print(get_version_info('full')) +def convergence_strategy_from_opts(options): + no_recreate = options['--no-recreate'] + force_recreate = options['--force-recreate'] + if force_recreate and no_recreate: + raise UserError("--force-recreate and --no-recreate cannot be combined.") + + if force_recreate: + return ConvergenceStrategy.always + + if no_recreate: + return ConvergenceStrategy.never + + return ConvergenceStrategy.changed + + def build_log_printer(containers, service_names, monochrome): if service_names: containers = [c for c in containers if c.service in service_names] diff --git a/compose/project.py b/compose/project.py index 8db20e766..9a6e98e01 100644 --- a/compose/project.py +++ b/compose/project.py @@ -15,6 +15,7 @@ from .const import LABEL_SERVICE from .container import Container from .legacy import check_for_legacy_containers from .service import ContainerNet +from .service import ConvergenceStrategy from .service import Net from .service import Service from .service import ServiceNet @@ -266,24 +267,16 @@ class Project(object): def up(self, service_names=None, start_deps=True, - allow_recreate=True, - force_recreate=False, + strategy=ConvergenceStrategy.changed, do_build=True, timeout=DEFAULT_TIMEOUT): - if force_recreate and not allow_recreate: - raise ValueError("force_recreate and allow_recreate are in conflict") - services = self.get_services(service_names, include_deps=start_deps) for service in services: service.remove_duplicate_containers() - plans = self._get_convergence_plans( - services, - allow_recreate=allow_recreate, - force_recreate=force_recreate, - ) + plans = self._get_convergence_plans(services, strategy) return [ container @@ -295,11 +288,7 @@ class Project(object): ) ] - def _get_convergence_plans(self, - services, - allow_recreate=True, - force_recreate=False): - + def _get_convergence_plans(self, services, strategy): plans = {} for service in services: @@ -310,20 +299,13 @@ class Project(object): and plans[name].action == 'recreate' ] - if updated_dependencies and allow_recreate: - log.debug( - '%s has upstream changes (%s)', - service.name, ", ".join(updated_dependencies), - ) - plan = service.convergence_plan( - allow_recreate=allow_recreate, - force_recreate=True, - ) + if updated_dependencies and strategy.allows_recreate: + log.debug('%s has upstream changes (%s)', + service.name, + ", ".join(updated_dependencies)) + plan = service.convergence_plan(ConvergenceStrategy.always) else: - plan = service.convergence_plan( - allow_recreate=allow_recreate, - force_recreate=force_recreate, - ) + plan = service.convergence_plan(strategy) plans[service.name] = plan diff --git a/compose/service.py b/compose/service.py index 8dc1efa1d..be74ca3a2 100644 --- a/compose/service.py +++ b/compose/service.py @@ -8,6 +8,7 @@ import sys from collections import namedtuple from operator import attrgetter +import enum import six from docker.errors import APIError from docker.utils import create_host_config @@ -86,6 +87,20 @@ ServiceName = namedtuple('ServiceName', 'project service number') ConvergencePlan = namedtuple('ConvergencePlan', 'action containers') +@enum.unique +class ConvergenceStrategy(enum.Enum): + """Enumeration for all possible convergence strategies. Values refer to + when containers should be recreated. + """ + changed = 1 + always = 2 + never = 3 + + @property + def allows_recreate(self): + return self is not type(self).never + + class Service(object): def __init__( self, @@ -326,22 +341,19 @@ class Service(object): else: return self.options['image'] - def convergence_plan(self, - allow_recreate=True, - force_recreate=False): - - if force_recreate and not allow_recreate: - raise ValueError("force_recreate and allow_recreate are in conflict") - + def convergence_plan(self, strategy=ConvergenceStrategy.changed): containers = self.containers(stopped=True) if not containers: return ConvergencePlan('create', []) - if not allow_recreate: + if strategy is ConvergenceStrategy.never: return ConvergencePlan('start', containers) - if force_recreate or self._containers_have_diverged(containers): + if ( + strategy is ConvergenceStrategy.always or + self._containers_have_diverged(containers) + ): return ConvergencePlan('recreate', containers) stopped = [c for c in containers if not c.is_running] diff --git a/docs/index.md b/docs/index.md index 59bf20094..4e4f58daf 100644 --- a/docs/index.md +++ b/docs/index.md @@ -206,7 +206,7 @@ At this point, you have seen the basics of how Compose works. ## Release Notes -To see a detailed list of changes for past and current releases of Docker +To see a detailed list of changes for past and current releases of Docker Compose, please refer to the [CHANGELOG](https://github.com/docker/compose/blob/master/CHANGELOG.md). ## Getting help diff --git a/requirements.txt b/requirements.txt index 587c04c5a..666efcd26 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,6 +2,7 @@ PyYAML==3.10 docker-py==1.3.1 dockerpty==0.3.4 docopt==0.6.1 +enum34==1.0.4 jsonschema==2.5.1 requests==2.7.0 six==1.7.3 diff --git a/setup.py b/setup.py index 737e074c8..29c5299e9 100644 --- a/setup.py +++ b/setup.py @@ -45,8 +45,9 @@ tests_require = [ ] -if sys.version_info[:1] < (3,): +if sys.version_info[:2] < (3, 4): tests_require.append('mock >= 1.0.1') + install_requires.append('enum34 >= 1.0.4, < 2') setup( diff --git a/tests/integration/cli_test.py b/tests/integration/cli_test.py index 9606ef41f..4a80d3369 100644 --- a/tests/integration/cli_test.py +++ b/tests/integration/cli_test.py @@ -223,7 +223,7 @@ class CLITestCase(DockerClientTestCase): self.assertTrue(config['AttachStdin']) @mock.patch('dockerpty.start') - def test_run_service_with_links(self, __): + def test_run_service_with_links(self, _): self.command.base_dir = 'tests/fixtures/links-composefile' self.command.dispatch(['run', 'web', '/bin/true'], None) db = self.project.get_service('db') @@ -232,14 +232,14 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(len(console.containers()), 0) @mock.patch('dockerpty.start') - def test_run_with_no_deps(self, __): + def test_run_with_no_deps(self, _): self.command.base_dir = 'tests/fixtures/links-composefile' self.command.dispatch(['run', '--no-deps', 'web', '/bin/true'], None) db = self.project.get_service('db') self.assertEqual(len(db.containers()), 0) @mock.patch('dockerpty.start') - def test_run_does_not_recreate_linked_containers(self, __): + def test_run_does_not_recreate_linked_containers(self, _): self.command.base_dir = 'tests/fixtures/links-composefile' self.command.dispatch(['up', '-d', 'db'], None) db = self.project.get_service('db') diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index fe63838fc..ad49ad10a 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -5,6 +5,7 @@ from compose import config from compose.const import LABEL_PROJECT from compose.container import Container from compose.project import Project +from compose.service import ConvergenceStrategy def build_service_dicts(service_config): @@ -224,7 +225,7 @@ class ProjectTest(DockerClientTestCase): old_db_id = project.containers()[0].id db_volume_path = project.containers()[0].get('Volumes./etc') - project.up(force_recreate=True) + project.up(strategy=ConvergenceStrategy.always) self.assertEqual(len(project.containers()), 2) db_container = [c for c in project.containers() if 'db' in c.name][0] @@ -243,7 +244,7 @@ class ProjectTest(DockerClientTestCase): old_db_id = project.containers()[0].id db_volume_path = project.containers()[0].inspect()['Volumes']['/var/db'] - project.up(allow_recreate=False) + project.up(strategy=ConvergenceStrategy.never) self.assertEqual(len(project.containers()), 2) db_container = [c for c in project.containers() if 'db' in c.name][0] @@ -267,7 +268,7 @@ class ProjectTest(DockerClientTestCase): old_db_id = old_containers[0].id db_volume_path = old_containers[0].inspect()['Volumes']['/var/db'] - project.up(allow_recreate=False) + project.up(strategy=ConvergenceStrategy.never) new_containers = project.containers(stopped=True) self.assertEqual(len(new_containers), 2) diff --git a/tests/integration/resilience_test.py b/tests/integration/resilience_test.py index 82a4680d8..befd72c7f 100644 --- a/tests/integration/resilience_test.py +++ b/tests/integration/resilience_test.py @@ -4,6 +4,7 @@ from __future__ import unicode_literals from .. import mock from .testcases import DockerClientTestCase from compose.project import Project +from compose.service import ConvergenceStrategy class ResilienceTest(DockerClientTestCase): @@ -16,14 +17,14 @@ class ResilienceTest(DockerClientTestCase): self.host_path = container.get('Volumes')['/var/db'] def test_successful_recreate(self): - self.project.up(force_recreate=True) + self.project.up(strategy=ConvergenceStrategy.always) container = self.db.containers()[0] self.assertEqual(container.get('Volumes')['/var/db'], self.host_path) def test_create_failure(self): with mock.patch('compose.service.Service.create_container', crash): with self.assertRaises(Crash): - self.project.up(force_recreate=True) + self.project.up(strategy=ConvergenceStrategy.always) self.project.up() container = self.db.containers()[0] @@ -32,7 +33,7 @@ class ResilienceTest(DockerClientTestCase): def test_start_failure(self): with mock.patch('compose.service.Service.start_container', crash): with self.assertRaises(Crash): - self.project.up(force_recreate=True) + self.project.up(strategy=ConvergenceStrategy.always) self.project.up() container = self.db.containers()[0] diff --git a/tests/integration/state_test.py b/tests/integration/state_test.py index d077f094d..93d0572a0 100644 --- a/tests/integration/state_test.py +++ b/tests/integration/state_test.py @@ -12,6 +12,7 @@ from .testcases import DockerClientTestCase from compose import config from compose.const import LABEL_CONFIG_HASH from compose.project import Project +from compose.service import ConvergenceStrategy class ProjectTestCase(DockerClientTestCase): @@ -151,7 +152,9 @@ class ProjectWithDependenciesTest(ProjectTestCase): old_containers = self.run_up(self.cfg) self.cfg['db']['environment'] = {'NEW_VAR': '1'} - new_containers = self.run_up(self.cfg, allow_recreate=False) + new_containers = self.run_up( + self.cfg, + strategy=ConvergenceStrategy.never) self.assertEqual(new_containers - old_containers, set()) @@ -175,23 +178,11 @@ class ProjectWithDependenciesTest(ProjectTestCase): def converge(service, - allow_recreate=True, - force_recreate=False, + strategy=ConvergenceStrategy.changed, 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, - force_recreate=force_recreate, - ) - - return service.execute_convergence_plan( - plan, - do_build=do_build, - timeout=1, - ) + """Create a converge plan from a strategy and execute the plan.""" + plan = service.convergence_plan(strategy) + return service.execute_convergence_plan(plan, do_build=do_build, timeout=1) class ServiceStateTest(DockerClientTestCase): diff --git a/tests/unit/cli/main_test.py b/tests/unit/cli/main_test.py index e3a4629e5..a5b369808 100644 --- a/tests/unit/cli/main_test.py +++ b/tests/unit/cli/main_test.py @@ -1,10 +1,13 @@ from __future__ import absolute_import from compose import container +from compose.cli.errors import UserError from compose.cli.log_printer import LogPrinter from compose.cli.main import attach_to_logs from compose.cli.main import build_log_printer +from compose.cli.main import convergence_strategy_from_opts from compose.project import Project +from compose.service import ConvergenceStrategy from tests import mock from tests import unittest @@ -55,3 +58,32 @@ class CLIMainTestCase(unittest.TestCase): project.stop.assert_called_once_with( service_names=service_names, timeout=timeout) + + +class ConvergeStrategyFromOptsTestCase(unittest.TestCase): + + def test_invalid_opts(self): + options = {'--force-recreate': True, '--no-recreate': True} + with self.assertRaises(UserError): + convergence_strategy_from_opts(options) + + def test_always(self): + options = {'--force-recreate': True, '--no-recreate': False} + self.assertEqual( + convergence_strategy_from_opts(options), + ConvergenceStrategy.always + ) + + def test_never(self): + options = {'--force-recreate': False, '--no-recreate': True} + self.assertEqual( + convergence_strategy_from_opts(options), + ConvergenceStrategy.never + ) + + def test_changed(self): + options = {'--force-recreate': False, '--no-recreate': False} + self.assertEqual( + convergence_strategy_from_opts(options), + ConvergenceStrategy.changed + )