Merge pull request #1440 from aanand/legacy-fixes

Legacy fixes

compose/cli/main.py

@@ -11,7 +11,7 @@ from docker.errors import APIError
 import dockerpty
 
 from .. import __version__
-from .. import migration
+from .. import legacy
 from ..project import NoSuchService, ConfigurationError
 from ..service import BuildError, CannotBeScaledError, NeedsBuildError
 from ..config import parse_environment
@@ -495,7 +495,7 @@ class TopLevelCommand(Command):
 
         Usage: migrate-to-labels
         """
-        migration.migrate_project_to_labels(project)
+        legacy.migrate_project_to_labels(project)
 
 
 def list_containers(containers):

compose/legacy.py

@@ -0,0 +1,93 @@
+import logging
+import re
+
+from .container import get_container_name, Container
+
+
+log = logging.getLogger(__name__)
+
+
+# TODO: remove this section when migrate_project_to_labels is removed
+NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')
+
+
+def check_for_legacy_containers(
+        client,
+        project,
+        services,
+        stopped=False,
+        one_off=False):
+    """Check if there are containers named using the old naming convention
+    and warn the user that those containers may need to be migrated to
+    using labels, so that compose can find them.
+    """
+    names = get_legacy_container_names(
+        client,
+        project,
+        services,
+        stopped=stopped,
+        one_off=one_off)
+
+    for name in names:
+        log.warn(
+            "Compose found a found a container named %s without any "
+            "labels. As of compose 1.3.0 containers are identified with "
+            "labels instead of naming convention. If you'd like compose "
+            "to use this container, please run "
+            "`docker-compose migrate-to-labels`" % (name,))
+
+
+def add_labels(project, container, name):
+    project_name, service_name, one_off, number = NAME_RE.match(name).groups()
+    if project_name != project.name or service_name not in project.service_names:
+        return
+    service = project.get_service(service_name)
+    service.recreate_container(container)
+
+
+def migrate_project_to_labels(project):
+    log.info("Running migration to labels for project %s", project.name)
+
+    client = project.client
+    for container in client.containers(all=True):
+        name = get_container_name(container)
+        if not is_valid_name(name):
+            continue
+        add_labels(project, Container.from_ps(client, container), name)
+
+
+def get_legacy_container_names(
+        client,
+        project,
+        services,
+        stopped=False,
+        one_off=False):
+
+    for container in client.containers(all=stopped):
+        name = get_container_name(container)
+        for service in services:
+            if has_container(project, service, name, one_off=one_off):
+                yield name
+
+
+def has_container(project, service, name, one_off=False):
+    if not name or not is_valid_name(name, one_off):
+        return False
+    container_project, container_service, _container_number = parse_name(name)
+    return container_project == project and container_service == service
+
+
+def is_valid_name(name, one_off=False):
+    match = NAME_RE.match(name)
+    if match is None:
+        return False
+    if one_off:
+        return match.group(3) == 'run_'
+    else:
+        return match.group(3) is None
+
+
+def parse_name(name):
+    match = NAME_RE.match(name)
+    (project, service_name, _, suffix) = match.groups()
+    return (project, service_name, int(suffix))

compose/migration.py

@@ -1,35 +0,0 @@
-import logging
-import re
-
-from .container import get_container_name, Container
-
-
-log = logging.getLogger(__name__)
-
-
-# TODO: remove this section when migrate_project_to_labels is removed
-NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')
-
-
-def is_valid_name(name):
-    match = NAME_RE.match(name)
-    return match is not None
-
-
-def add_labels(project, container, name):
-    project_name, service_name, one_off, number = NAME_RE.match(name).groups()
-    if project_name != project.name or service_name not in project.service_names:
-        return
-    service = project.get_service(service_name)
-    service.recreate_container(container)
-
-
-def migrate_project_to_labels(project):
-    log.info("Running migration to labels for project %s", project.name)
-
-    client = project.client
-    for container in client.containers(all=True):
-        name = get_container_name(container)
-        if not is_valid_name(name):
-            continue
-        add_labels(project, Container.from_ps(client, container), name)

compose/project.py

@@ -7,8 +7,9 @@ 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 .service import Service, check_for_legacy_containers
+from .service import Service
 from .container import Container
+from .legacy import check_for_legacy_containers
 
 log = logging.getLogger(__name__)
 

compose/service.py

@@ -20,7 +20,8 @@ from .const import (
     LABEL_VERSION,
     LABEL_CONFIG_HASH,
 )
-from .container import Container, get_container_name
+from .container import Container
+from .legacy import check_for_legacy_containers
 from .progress_stream import stream_output, StreamOutputError
 from .utils import json_hash
 
@@ -770,31 +771,6 @@ def build_container_labels(label_options, service_labels, number, one_off=False)
     return labels
 
 
-def check_for_legacy_containers(
-        client,
-        project,
-        services,
-        stopped=False,
-        one_off=False):
-    """Check if there are containers named using the old naming convention
-    and warn the user that those containers may need to be migrated to
-    using labels, so that compose can find them.
-    """
-    for container in client.containers(all=stopped):
-        name = get_container_name(container)
-        for service in services:
-            prefix = '%s_%s_%s' % (project, service, 'run_' if one_off else '')
-            if not name.startswith(prefix):
-                continue
-
-            log.warn(
-                "Compose found a found a container named %s without any "
-                "labels. As of compose 1.3.0 containers are identified with "
-                "labels instead of naming convention. If you'd like compose "
-                "to use this container, please run "
-                "`docker-compose migrate-to-labels`" % (name,))
-
-
 def parse_restart_spec(restart_config):
     if not restart_config:
         return None

tests/integration/legacy_test.py

@@ -0,0 +1,57 @@
+import mock
+
+from compose import legacy
+from compose.project import Project
+from .testcases import DockerClientTestCase
+
+
+class ProjectTest(DockerClientTestCase):
+
+    def setUp(self):
+        super(ProjectTest, self).setUp()
+
+        self.services = [
+            self.create_service('web'),
+            self.create_service('db'),
+        ]
+
+        self.project = Project('composetest', self.services, self.client)
+
+        # Create a legacy container for each service
+        for service in self.services:
+            service.ensure_image_exists()
+            self.client.create_container(
+                name='{}_{}_1'.format(self.project.name, service.name),
+                **service.options
+            )
+
+        # Create a single one-off legacy container
+        self.client.create_container(
+            name='{}_{}_run_1'.format(self.project.name, self.services[0].name),
+            **self.services[0].options
+        )
+
+    def get_names(self, **kwargs):
+        if 'stopped' not in kwargs:
+            kwargs['stopped'] = True
+
+        return list(legacy.get_legacy_container_names(
+            self.client,
+            self.project.name,
+            [s.name for s in self.services],
+            **kwargs
+        ))
+
+    def test_get_legacy_container_names(self):
+        self.assertEqual(len(self.get_names()), len(self.services))
+
+    def test_get_legacy_container_names_one_off(self):
+        self.assertEqual(len(self.get_names(one_off=True)), 1)
+
+    def test_migration_to_labels(self):
+        with mock.patch.object(legacy, 'log', autospec=True) as mock_log:
+            self.assertEqual(self.project.containers(stopped=True), [])
+            self.assertEqual(mock_log.warn.call_count, len(self.services))
+
+        legacy.migrate_project_to_labels(self.project)
+        self.assertEqual(len(self.project.containers(stopped=True)), len(self.services))

tests/integration/migration_test.py

@@ -1,23 +0,0 @@
-import mock
-
-from compose import service, migration
-from compose.project import Project
-from .testcases import DockerClientTestCase
-
-
-class ProjectTest(DockerClientTestCase):
-
-    def test_migration_to_labels(self):
-        web = self.create_service('web')
-        db = self.create_service('db')
-        project = Project('composetest', [web, db], self.client)
-
-        self.client.create_container(name='composetest_web_1', **web.options)
-        self.client.create_container(name='composetest_db_1', **db.options)
-
-        with mock.patch.object(service, 'log', autospec=True) as mock_log:
-            self.assertEqual(project.containers(stopped=True), [])
-            self.assertEqual(mock_log.warn.call_count, 2)
-
-        migration.migrate_project_to_labels(project)
-        self.assertEqual(len(project.containers(stopped=True)), 2)