Resolves #1066, use labels to identify containers

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

compose/cli/docker_client.py

@@ -32,4 +32,4 @@ def docker_client():
         )
 
     timeout = int(os.environ.get('DOCKER_CLIENT_TIMEOUT', 60))
-    return Client(base_url=base_url, tls=tls_config, version='1.15', timeout=timeout)
+    return Client(base_url=base_url, tls=tls_config, version='1.18', timeout=timeout)

compose/const.py

@@ -0,0 +1,5 @@
+
+CONTAINER_NUMBER_LABEL = 'com.docker.compose.container_number'
+ONE_OFF_LABEL = 'com.docker.compose.oneoff'
+PROJECT_LABEL = 'com.docker.compose.project'
+SERVICE_LABEL = 'com.docker.compose.service'

compose/container.py

@@ -4,6 +4,8 @@
 import six
 from functools import reduce
 
+from .const import CONTAINER_NUMBER_LABEL, SERVICE_LABEL
+
 
 class Container(object):
     """
@@ -52,16 +54,17 @@ def short_id(self):
     def name(self):
         return self.dictionary['Name'][1:]
 
+    @property
+    def labels(self):
+        return self.get('Config.Labels') or {}
+
     @property
     def name_without_project(self):
-        return '_'.join(self.dictionary['Name'].split('_')[1:])
+        return '{0}_{1}'.format(self.labels.get(SERVICE_LABEL), self.number)
 
     @property
     def number(self):
-        try:
-            return int(self.name.split('_')[-1])
-        except ValueError:
-            return None
+        return int(self.labels.get(CONTAINER_NUMBER_LABEL) or 0)
 
     @property
     def ports(self):
@@ -147,6 +150,7 @@ def inspect(self):
         self.has_been_inspected = True
         return self.dictionary
 
+    # TODO: only used by tests, move to test module
     def links(self):
         links = []
         for container in self.client.containers():

compose/project.py

@@ -4,6 +4,7 @@
 
 from functools import reduce
 from .config import get_service_name_from_net, ConfigurationError
+from .const import PROJECT_LABEL, ONE_OFF_LABEL
 from .service import Service
 from .container import Container
 from docker.errors import APIError
@@ -60,6 +61,12 @@ def __init__(self, name, services, client):
         self.services = services
         self.client = client
 
+    def labels(self, one_off=False):
+        return [
+            '{0}={1}'.format(PROJECT_LABEL, self.name),
+            '{0}={1}'.format(ONE_OFF_LABEL, "True" if one_off else "False"),
+        ]
+
     @classmethod
     def from_dicts(cls, name, service_dicts, client):
         """
@@ -226,9 +233,9 @@ def remove_stopped(self, service_names=None, **options):
 
     def containers(self, service_names=None, stopped=False, one_off=False):
         return [Container.from_ps(self.client, container)
-                for container in self.client.containers(all=stopped)
-                for service in self.get_services(service_names)
-                if service.has_container(container, one_off=one_off)]
+                for container in self.client.containers(
+                    all=stopped,
+                    filters={'label': self.labels(one_off=one_off)})]
 
     def _inject_deps(self, acc, service):
         net_name = service.get_net_name()

compose/service.py

@@ -11,7 +11,13 @@
 from docker.utils import create_host_config
 
 from .config import DOCKER_CONFIG_KEYS
-from .container import Container, get_container_name
+from .const import (
+    CONTAINER_NUMBER_LABEL,
+    ONE_OFF_LABEL,
+    PROJECT_LABEL,
+    SERVICE_LABEL,
+)
+from .container import Container
 from .progress_stream import stream_output, StreamOutputError
 
 log = logging.getLogger(__name__)
@@ -74,27 +80,17 @@ def __init__(self, name, client=None, project='default', links=None, external_li
 
     def containers(self, stopped=False, one_off=False):
         return [Container.from_ps(self.client, container)
-                for container in self.client.containers(all=stopped)
-                if self.has_container(container, one_off=one_off)]
-
-    def has_container(self, container, one_off=False):
-        """Return True if `container` was created to fulfill this service."""
-        name = get_container_name(container)
-        if not name or not is_valid_name(name, one_off):
-            return False
-        project, name, _number = parse_name(name)
-        return project == self.project and name == self.name
+                for container in self.client.containers(
+                    all=stopped,
+                    filters={'label': self.labels(one_off=one_off)})]
 
     def get_container(self, number=1):
         """Return a :class:`compose.container.Container` for this service. The
         container must be active, and match `number`.
         """
-        for container in self.client.containers():
-            if not self.has_container(container):
-                continue
-            _, _, container_number = parse_name(get_container_name(container))
-            if container_number == number:
-                return Container.from_ps(self.client, container)
+        labels = self.labels() + ['{0}={1}'.format(CONTAINER_NUMBER_LABEL, number)]
+        for container in self.client.containers(filters={'label': labels}):
+            return Container.from_ps(self.client, container)
 
         raise ValueError("No container found for %s_%s" % (self.name, number))
 
@@ -133,7 +129,6 @@ def scale(self, desired_num):
         # Create enough containers
         containers = self.containers(stopped=True)
         while len(containers) < desired_num:
-            log.info("Creating %s..." % self._next_container_name(containers))
             containers.append(self.create_container(detach=True))
 
         running_containers = []
@@ -178,8 +173,11 @@ def create_container(self,
         Create a container for this service. If the image doesn't exist, attempt to pull
         it.
         """
+        number = self._next_container_number(one_off=one_off)
+        log.info("Creating %s..." % self.get_container_name(number, one_off))
         container_options = self._get_container_create_options(
             override_options,
+            number,
             one_off=one_off,
             intermediate_container=intermediate_container,
         )
@@ -210,7 +208,6 @@ def recreate_containers(self, insecure_registry=False, do_build=True, **override
         """
         containers = self.containers(stopped=True)
         if not containers:
-            log.info("Creating %s..." % self._next_container_name(containers))
             container = self.create_container(
                 insecure_registry=insecure_registry,
                 do_build=do_build,
@@ -284,7 +281,6 @@ def start_or_create_containers(
         containers = self.containers(stopped=True)
 
         if not containers:
-            log.info("Creating %s..." % self._next_container_name(containers))
             new_container = self.create_container(
                 insecure_registry=insecure_registry,
                 detach=detach,
@@ -306,14 +302,19 @@ def get_net_name(self):
         else:
             return
 
-    def _next_container_name(self, all_containers, one_off=False):
-        bits = [self.project, self.name]
-        if one_off:
-            bits.append('run')
-        return '_'.join(bits + [str(self._next_container_number(all_containers))])
-
-    def _next_container_number(self, all_containers):
-        numbers = [parse_name(c.name).number for c in all_containers]
+    def get_container_name(self, number, one_off=False):
+        # TODO: Implement issue #652 here
+        return build_container_name(self.project, self.name, number, one_off)
+
+    # TODO: this would benefit from github.com/docker/docker/pull/11943
+    # to remove the need to inspect every container
+    def _next_container_number(self, one_off=False):
+        numbers = [
+            Container.from_ps(self.client, container).number
+            for container in self.client.containers(
+                all=True,
+                filters={'label': self.labels(one_off=one_off)})
+        ]
         return 1 if not numbers else max(numbers) + 1
 
     def _get_links(self, link_to_self):
@@ -373,15 +374,18 @@ def _get_net(self):
 
         return net
 
-    def _get_container_create_options(self, override_options, one_off=False, intermediate_container=None):
+    def _get_container_create_options(
+            self,
+            override_options,
+            number,
+            one_off=False,
+            intermediate_container=None):
         container_options = dict(
             (k, self.options[k])
             for k in DOCKER_CONFIG_KEYS if k in self.options)
         container_options.update(override_options)
 
-        container_options['name'] = self._next_container_name(
-            self.containers(stopped=True, one_off=one_off),
-            one_off)
+        container_options['name'] = self.get_container_name(number, one_off)
 
         # If a qualified hostname was given, split it into an
         # unqualified hostname and a domainname unless domainname
@@ -416,6 +420,11 @@ def _get_container_create_options(self, override_options, one_off=False, interme
         else:
             container_options['image'] = self._get_image_name(container_options['image'])
 
+        container_options['labels'] = build_container_labels(
+            container_options.get('labels', {}),
+            self.labels(one_off=one_off),
+            number)
+
         # Delete options which are only used when starting
         for key in DOCKER_START_KEYS:
             container_options.pop(key, None)
@@ -508,6 +517,13 @@ def full_name(self):
         """
         return '%s_%s' % (self.project, self.name)
 
+    def labels(self, one_off=False):
+        return [
+            '{0}={1}'.format(PROJECT_LABEL, self.project),
+            '{0}={1}'.format(SERVICE_LABEL, self.name),
+            '{0}={1}'.format(ONE_OFF_LABEL, "True" if one_off else "False")
+        ]
+
     def can_be_scaled(self):
         for port in self.options.get('ports', []):
             if ':' in str(port):
@@ -524,23 +540,18 @@ def pull(self, insecure_registry=False):
             )
 
 
-NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')
-
-
-def is_valid_name(name, one_off=False):
-    match = NAME_RE.match(name)
-    if match is None:
-        return False
+def build_container_name(project, service, number, one_off=False):
+    bits = [project, service]
     if one_off:
-        return match.group(3) == 'run_'
-    else:
-        return match.group(3) is None
+        bits.append('run')
+    return '_'.join(bits + [str(number)])
 
 
-def parse_name(name):
-    match = NAME_RE.match(name)
-    (project, service_name, _, suffix) = match.groups()
-    return ServiceName(project, service_name, int(suffix))
+def build_container_labels(label_options, service_labels, number, one_off=False):
+    labels = label_options or {}
+    labels.update(label.split('=', 1) for label in service_labels)
+    labels[CONTAINER_NUMBER_LABEL] = str(number)
+    return labels
 
 
 def parse_restart_spec(restart_config):

requirements.txt

@@ -1,5 +1,5 @@
 PyYAML==3.10
-docker-py==1.0.0
+-e git+https://github.com/docker/docker-py.git@70ce156e26d283d181e6ec10bd1309ddc1da1bbd#egg=docker-py
 dockerpty==0.3.2
 docopt==0.6.1
 requests==2.2.1

setup.py

@@ -27,10 +27,10 @@ def find_version(*file_paths):
 install_requires = [
     'docopt >= 0.6.1, < 0.7',
     'PyYAML >= 3.10, < 4',
-    'requests >= 2.2.1, < 2.6',
+    'requests >= 2.6, < 2.7',
     'texttable >= 0.8.1, < 0.9',
     'websocket-client >= 0.11.0, < 1.0',
-    'docker-py >= 1.0.0, < 1.2',
+    'docker-py >= 1.1.0, < 1.2',
     'dockerpty >= 0.3.2, < 0.4',
     'six >= 1.3.0, < 2',
 ]

tests/integration/testcases.py

@@ -12,6 +12,7 @@ class DockerClientTestCase(unittest.TestCase):
     def setUpClass(cls):
         cls.client = docker_client()
 
+    # TODO: update to use labels
     def setUp(self):
         for c in self.client.containers(all=True):
             if c['Names'] and 'composetest' in c['Names'][0]:

tests/unit/container_test.py

@@ -5,6 +5,7 @@
 import docker
 
 from compose.container import Container
+from compose.container import get_container_name
 
 
 class ContainerTest(unittest.TestCase):
@@ -23,6 +24,13 @@ def setUp(self):
             "NetworkSettings": {
                 "Ports": {},
             },
+            "Config": {
+                "Labels": {
+                    "com.docker.compose.project": "composetest",
+                    "com.docker.compose.service": "web",
+                    "com.docker.compose.container_number": 7,
+                },
+            }
         }
 
     def test_from_ps(self):
@@ -65,10 +73,8 @@ def test_environment(self):
         })
 
     def test_number(self):
-        container = Container.from_ps(None,
-                                      self.container_dict,
-                                      has_been_inspected=True)
-        self.assertEqual(container.number, 1)
+        container = Container(None, self.container_dict, has_been_inspected=True)
+        self.assertEqual(container.number, 7)
 
     def test_name(self):
         container = Container.from_ps(None,
@@ -77,10 +83,8 @@ def test_name(self):
         self.assertEqual(container.name, "composetest_db_1")
 
     def test_name_without_project(self):
-        container = Container.from_ps(None,
-                                      self.container_dict,
-                                      has_been_inspected=True)
-        self.assertEqual(container.name_without_project, "db_1")
+        container = Container(None, self.container_dict, has_been_inspected=True)
+        self.assertEqual(container.name_without_project, "web_7")
 
     def test_inspect_if_not_inspected(self):
         mock_client = mock.create_autospec(docker.Client)
@@ -130,3 +134,12 @@ def test_get(self):
         self.assertEqual(container.get('Status'), "Up 8 seconds")
         self.assertEqual(container.get('HostConfig.VolumesFrom'), ["volume_id"])
         self.assertEqual(container.get('Foo.Bar.DoesNotExist'), None)
+
+
+class GetContainerNameTestCase(unittest.TestCase):
+
+    def test_get_container_name(self):
+        self.assertIsNone(get_container_name({}))
+        self.assertEqual(get_container_name({'Name': 'myproject_db_1'}), 'myproject_db_1')
+        self.assertEqual(get_container_name({'Names': ['/myproject_db_1', '/myproject_web_1/db']}), 'myproject_db_1')
+        self.assertEqual(get_container_name({'Names': ['/swarm-host-1/myproject_db_1', '/swarm-host-1/myproject_web_1/db']}), 'myproject_db_1')