Refactor migration logic

- Rename `migration` module to `legacy` to make its legacy-ness explicit

- Move `check_for_legacy_containers` into `legacy` module

- Fix migration test so it can be run in isolation

Signed-off-by: Aanand Prasad <[email protected]>

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/migration.py → compose/legacy.py

@@ -16,6 +16,31 @@ def is_valid_name(name):
     return match is not None
 
 
+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 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:

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
 
@@ -767,31 +768,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,30 @@
+import mock
+
+from compose import legacy
+from compose.project import Project
+from .testcases import DockerClientTestCase
+
+
+class ProjectTest(DockerClientTestCase):
+
+    def test_migration_to_labels(self):
+        services = [
+            self.create_service('web'),
+            self.create_service('db'),
+        ]
+
+        project = Project('composetest', services, self.client)
+
+        for service in services:
+            service.ensure_image_exists()
+            self.client.create_container(
+                name='{}_{}_1'.format(project.name, service.name),
+                **service.options
+            )
+
+        with mock.patch.object(legacy, 'log', autospec=True) as mock_log:
+            self.assertEqual(project.containers(stopped=True), [])
+            self.assertEqual(mock_log.warn.call_count, 2)
+
+        legacy.migrate_project_to_labels(project)
+        self.assertEqual(len(project.containers(stopped=True)), 2)

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)