Fix bugs with one-off legacy containers

- One-off containers were included in the warning log messages, which can
  make for unreadable output when there are lots (as there often are).

- Compose was attempting to recreate one-off containers as normal
  containers when migrating.

Fixed by implementing the exact naming logic from before we used labels.

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

compose/legacy.py

@@ -11,11 +11,6 @@ log = logging.getLogger(__name__)
 NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')
 
 
-def is_valid_name(name):
-    match = NAME_RE.match(name)
-    return match is not None
-
-
 def check_for_legacy_containers(
         client,
         project,
@@ -42,20 +37,6 @@ def check_for_legacy_containers(
             "`docker-compose migrate-to-labels`" % (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:
-            prefix = '%s_%s_%s' % (project, service, 'run_' if one_off else '')
-            if name.startswith(prefix):
-                yield 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:
@@ -73,3 +54,40 @@ def migrate_project_to_labels(project):
         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))

tests/integration/legacy_test.py

@@ -17,6 +17,7 @@ class ProjectTest(DockerClientTestCase):
 
         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(
@@ -24,6 +25,12 @@ class ProjectTest(DockerClientTestCase):
                 **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
@@ -38,6 +45,9 @@ class ProjectTest(DockerClientTestCase):
     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), [])