Migrate containers in dependency order

This fixes a bug where migration would fail with an error if a
downstream container was migrated before its upstream dependencies, due
to `check_for_legacy_containers()` being implicitly called when we fetch
`links`, `volumes_from` or `net` dependencies.

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

compose/legacy.py

@@ -35,15 +35,15 @@ def check_for_legacy_containers(
     and warn the user that those containers may need to be migrated to
     using labels, so that compose can find them.
     """
-    names = list(get_legacy_container_names(
+    containers = list(get_legacy_containers(
         client,
         project,
         services,
         stopped=stopped,
         one_off=one_off))
 
-    if names:
-        raise LegacyContainersError(names)
+    if containers:
+        raise LegacyContainersError([c.name for c in containers])
 
 
 class LegacyContainersError(Exception):
@@ -61,8 +61,8 @@ class LegacyContainersError(Exception):
     __str__ = __unicode__
 
 
-def add_labels(project, container, name):
-    project_name, service_name, one_off, number = NAME_RE.match(name).groups()
+def add_labels(project, container):
+    project_name, service_name, one_off, number = NAME_RE.match(container.name).groups()
     if project_name != project.name or service_name not in project.service_names:
         return
     service = project.get_service(service_name)
@@ -72,26 +72,31 @@ def add_labels(project, container, name):
 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)
+    containers = get_legacy_containers(
+        project.client,
+        project.name,
+        project.service_names,
+        stopped=True,
+        one_off=False)
 
+    for container in containers:
+        add_labels(project, container)
 
-def get_legacy_container_names(
+
+def get_legacy_containers(
         client,
         project,
         services,
         stopped=False,
         one_off=False):
 
-    for container in client.containers(all=stopped):
-        name = get_container_name(container)
-        for service in services:
+    containers = client.containers(all=stopped)
+
+    for service in services:
+        for container in containers:
+            name = get_container_name(container)
             if has_container(project, service, name, one_off=one_off):
-                yield name
+                yield Container.from_ps(client, container)
 
 
 def has_container(project, service, name, one_off=False):

tests/integration/legacy_test.py

@@ -8,20 +8,21 @@ class ProjectTest(DockerClientTestCase):
     def setUp(self):
         super(ProjectTest, self).setUp()
 
-        self.services = [
-            self.create_service('web'),
-            self.create_service('db'),
-        ]
+        db = self.create_service('db')
+        web = self.create_service('web', links=[(db, 'db')])
+        nginx = self.create_service('nginx', links=[(web, 'web')])
 
+        self.services = [db, web, nginx]
         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(
+            container = self.client.create_container(
                 name='{}_{}_1'.format(self.project.name, service.name),
                 **service.options
             )
+            self.client.start(container)
 
         # Create a single one-off legacy container
         self.client.create_container(
@@ -29,11 +30,8 @@ class ProjectTest(DockerClientTestCase):
             **self.services[0].options
         )
 
-    def get_names(self, **kwargs):
-        if 'stopped' not in kwargs:
-            kwargs['stopped'] = True
-
-        return list(legacy.get_legacy_container_names(
+    def get_legacy_containers(self, **kwargs):
+        return list(legacy.get_legacy_containers(
             self.client,
             self.project.name,
             [s.name for s in self.services],
@@ -41,10 +39,10 @@ class ProjectTest(DockerClientTestCase):
         ))
 
     def test_get_legacy_container_names(self):
-        self.assertEqual(len(self.get_names()), len(self.services))
+        self.assertEqual(len(self.get_legacy_containers()), len(self.services))
 
     def test_get_legacy_container_names_one_off(self):
-        self.assertEqual(len(self.get_names(one_off=True)), 1)
+        self.assertEqual(len(self.get_legacy_containers(stopped=True, one_off=True)), 1)
 
     def test_migration_to_labels(self):
         with self.assertRaises(legacy.LegacyContainersError) as cm:
@@ -52,7 +50,7 @@ class ProjectTest(DockerClientTestCase):
 
         self.assertEqual(
             set(cm.exception.names),
-            set(['composetest_web_1', 'composetest_db_1']),
+            set(['composetest_db_1', 'composetest_web_1', 'composetest_nginx_1']),
         )
 
         legacy.migrate_project_to_labels(self.project)