Add enum34 and use it to create a ConvergenceStrategy enum.

Signed-off-by: Daniel Nephin <[email protected]>

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]

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
 

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')
 
 
[email protected]
+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]

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

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

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(

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')

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)

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]

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):

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
+        )