소스 검색

Merge pull request #1356 from dnephin/use_labels_instead_of_names

Use labels instead of names to identify containers
Aanand Prasad 10 년 전
부모
커밋
1e6d912fbc

+ 0 - 1
compose/__init__.py

@@ -1,4 +1,3 @@
 from __future__ import unicode_literals
-from .service import Service  # noqa:flake8
 
 __version__ = '1.3.0dev'

+ 19 - 14
compose/cli/main.py

@@ -11,6 +11,7 @@ from docker.errors import APIError
 import dockerpty
 
 from .. import __version__
+from .. import migration
 from ..project import NoSuchService, ConfigurationError
 from ..service import BuildError, CannotBeScaledError
 from ..config import parse_environment
@@ -81,20 +82,21 @@ class TopLevelCommand(Command):
       -v, --version             Print version and exit
 
     Commands:
-      build     Build or rebuild services
-      help      Get help on a command
-      kill      Kill containers
-      logs      View output from containers
-      port      Print the public port for a port binding
-      ps        List containers
-      pull      Pulls service images
-      restart   Restart services
-      rm        Remove stopped containers
-      run       Run a one-off command
-      scale     Set number of containers for a service
-      start     Start services
-      stop      Stop services
-      up        Create and start containers
+      build              Build or rebuild services
+      help               Get help on a command
+      kill               Kill containers
+      logs               View output from containers
+      port               Print the public port for a port binding
+      ps                 List containers
+      pull               Pulls service images
+      restart            Restart services
+      rm                 Remove stopped containers
+      run                Run a one-off command
+      scale              Set number of containers for a service
+      start              Start services
+      stop               Stop services
+      up                 Create and start containers
+      migrate_to_labels  Recreate containers to add labels
 
     """
     def docopt_options(self):
@@ -482,6 +484,9 @@ class TopLevelCommand(Command):
                 params = {} if timeout is None else {'timeout': int(timeout)}
                 project.stop(service_names=service_names, **params)
 
+    def migrate_to_labels(self, project, _options):
+        migration.migrate_project_to_labels(project)
+
 
 def list_containers(containers):
     return ", ".join(c.name for c in containers)

+ 6 - 0
compose/const.py

@@ -0,0 +1,6 @@
+
+LABEL_CONTAINER_NUMBER = 'com.docker.compose.container-number'
+LABEL_ONE_OFF = 'com.docker.compose.oneoff'
+LABEL_PROJECT = 'com.docker.compose.project'
+LABEL_SERVICE = 'com.docker.compose.service'
+LABEL_VERSION = 'com.docker.compose.version'

+ 9 - 5
compose/container.py

@@ -4,6 +4,8 @@ from __future__ import absolute_import
 import six
 from functools import reduce
 
+from .const import LABEL_CONTAINER_NUMBER, LABEL_SERVICE
+
 
 class Container(object):
     """
@@ -58,14 +60,15 @@ class Container(object):
 
     @property
     def name_without_project(self):
-        return '_'.join(self.dictionary['Name'].split('_')[1:])
+        return '{0}_{1}'.format(self.labels.get(LABEL_SERVICE), self.number)
 
     @property
     def number(self):
-        try:
-            return int(self.name.split('_')[-1])
-        except ValueError:
-            return None
+        number = self.labels.get(LABEL_CONTAINER_NUMBER)
+        if not number:
+            raise ValueError("Container {0} does not have a {1} label".format(
+                self.short_id, LABEL_CONTAINER_NUMBER))
+        return int(number)
 
     @property
     def ports(self):
@@ -159,6 +162,7 @@ class Container(object):
         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():

+ 35 - 0
compose/migration.py

@@ -0,0 +1,35 @@
+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)

+ 31 - 8
compose/project.py

@@ -1,12 +1,14 @@
 from __future__ import unicode_literals
 from __future__ import absolute_import
 import logging
-
 from functools import reduce
+
+from docker.errors import APIError
+
 from .config import get_service_name_from_net, ConfigurationError
-from .service import Service
+from .const import LABEL_PROJECT, LABEL_ONE_OFF
+from .service import Service, check_for_legacy_containers
 from .container import Container
-from docker.errors import APIError
 
 log = logging.getLogger(__name__)
 
@@ -60,6 +62,12 @@ class Project(object):
         self.services = services
         self.client = client
 
+    def labels(self, one_off=False):
+        return [
+            '{0}={1}'.format(LABEL_PROJECT, self.name),
+            '{0}={1}'.format(LABEL_ONE_OFF, "True" if one_off else "False"),
+        ]
+
     @classmethod
     def from_dicts(cls, name, service_dicts, client):
         """
@@ -75,6 +83,10 @@ class Project(object):
                                             volumes_from=volumes_from, **service_dict))
         return project
 
+    @property
+    def service_names(self):
+        return [service.name for service in self.services]
+
     def get_service(self, name):
         """
         Retrieve a service by name. Raises NoSuchService
@@ -102,7 +114,7 @@ class Project(object):
         """
         if service_names is None or len(service_names) == 0:
             return self.get_services(
-                service_names=[s.name for s in self.services],
+                service_names=self.service_names,
                 include_deps=include_deps
             )
         else:
@@ -223,10 +235,21 @@ class Project(object):
             service.remove_stopped(**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)]
+        containers = [
+            Container.from_ps(self.client, container)
+            for container in self.client.containers(
+                all=stopped,
+                filters={'label': self.labels(one_off=one_off)})]
+
+        if not containers:
+            check_for_legacy_containers(
+                self.client,
+                self.name,
+                self.service_names,
+                stopped=stopped,
+                one_off=one_off)
+
+        return containers
 
     def _inject_deps(self, acc, service):
         net_name = service.get_net_name()

+ 91 - 45
compose/service.py

@@ -10,7 +10,15 @@ import six
 from docker.errors import APIError
 from docker.utils import create_host_config, LogConfig
 
+from . import __version__
 from .config import DOCKER_CONFIG_KEYS, merge_environment
+from .const import (
+    LABEL_CONTAINER_NUMBER,
+    LABEL_ONE_OFF,
+    LABEL_PROJECT,
+    LABEL_SERVICE,
+    LABEL_VERSION,
+)
 from .container import Container, get_container_name
 from .progress_stream import stream_output, StreamOutputError
 
@@ -78,28 +86,29 @@ class Service(object):
         self.options = options
 
     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)]
+        containers = [
+            Container.from_ps(self.client, container)
+            for container in self.client.containers(
+                all=stopped,
+                filters={'label': self.labels(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
+        if not containers:
+            check_for_legacy_containers(
+                self.client,
+                self.project,
+                [self.name],
+                stopped=stopped,
+                one_off=one_off)
+
+        return containers
 
     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(LABEL_CONTAINER_NUMBER, 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))
 
@@ -138,7 +147,6 @@ class Service(object):
         # 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,6 +186,7 @@ class Service(object):
                          insecure_registry=False,
                          do_build=True,
                          previous_container=None,
+                         number=None,
                          **override_options):
         """
         Create a container for this service. If the image doesn't exist, attempt to pull
@@ -185,6 +194,7 @@ class Service(object):
         """
         container_options = self._get_container_create_options(
             override_options,
+            number or self._next_container_number(one_off=one_off),
             one_off=one_off,
             previous_container=previous_container,
         )
@@ -209,7 +219,6 @@ class Service(object):
         """
         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,
@@ -256,6 +265,7 @@ class Service(object):
         new_container = self.create_container(
             do_build=False,
             previous_container=container,
+            number=container.labels.get(LABEL_CONTAINER_NUMBER),
             **override_options)
         self.start_container(new_container)
         container.remove()
@@ -280,7 +290,6 @@ class Service(object):
         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,
@@ -302,14 +311,19 @@ class Service(object):
         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):
@@ -369,6 +383,7 @@ class Service(object):
     def _get_container_create_options(
             self,
             override_options,
+            number,
             one_off=False,
             previous_container=None):
         container_options = dict(
@@ -376,9 +391,7 @@ class Service(object):
             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
@@ -419,6 +432,11 @@ class Service(object):
         if self.can_be_built():
             container_options['image'] = self.full_name
 
+        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)
@@ -520,6 +538,13 @@ class Service(object):
         """
         return '%s_%s' % (self.project, self.name)
 
+    def labels(self, one_off=False):
+        return [
+            '{0}={1}'.format(LABEL_PROJECT, self.project),
+            '{0}={1}'.format(LABEL_SERVICE, self.name),
+            '{0}={1}'.format(LABEL_ONE_OFF, "True" if one_off else "False")
+        ]
+
     def can_be_scaled(self):
         for port in self.options.get('ports', []):
             if ':' in str(port):
@@ -585,23 +610,44 @@ def merge_volume_bindings(volumes_option, previous_container):
     return volume_bindings
 
 
-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 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[LABEL_CONTAINER_NUMBER] = str(number)
+    labels[LABEL_VERSION] = __version__
+    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
 
-def parse_name(name):
-    match = NAME_RE.match(name)
-    (project, service_name, _, suffix) = match.groups()
-    return ServiceName(project, service_name, int(suffix))
+            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):

+ 6 - 1
tests/integration/cli_test.py

@@ -21,6 +21,8 @@ class CLITestCase(DockerClientTestCase):
         sys.exit = self.old_sys_exit
         self.project.kill()
         self.project.remove_stopped()
+        for container in self.project.containers(stopped=True, one_off=True):
+            container.remove(force=True)
 
     @property
     def project(self):
@@ -62,6 +64,10 @@ class CLITestCase(DockerClientTestCase):
 
     @patch('sys.stdout', new_callable=StringIO)
     def test_ps_alternate_composefile(self, mock_stdout):
+        config_path = os.path.abspath(
+            'tests/fixtures/multiple-composefiles/compose2.yml')
+        self._project = self.command.get_project(config_path)
+
         self.command.base_dir = 'tests/fixtures/multiple-composefiles'
         self.command.dispatch(['-f', 'compose2.yml', 'up', '-d'], None)
         self.command.dispatch(['-f', 'compose2.yml', 'ps'], None)
@@ -416,7 +422,6 @@ class CLITestCase(DockerClientTestCase):
         self.assertEqual(len(project.get_service('another').containers()), 0)
 
     def test_port(self):
-
         self.command.base_dir = 'tests/fixtures/ports-composefile'
         self.command.dispatch(['up', '-d'], None)
         container = self.project.get_service('simple').get_container()

+ 23 - 0
tests/integration/migration_test.py

@@ -0,0 +1,23 @@
+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)

+ 21 - 12
tests/integration/service_test.py

@@ -8,11 +8,19 @@ import tempfile
 import shutil
 import six
 
-from compose import Service
+from compose import __version__
+from compose.const import (
+    LABEL_CONTAINER_NUMBER,
+    LABEL_ONE_OFF,
+    LABEL_PROJECT,
+    LABEL_SERVICE,
+    LABEL_VERSION,
+)
 from compose.service import (
     CannotBeScaledError,
-    build_extra_hosts,
     ConfigError,
+    Service,
+    build_extra_hosts,
 )
 from compose.container import Container
 from docker.errors import APIError
@@ -633,17 +641,18 @@ class ServiceTest(DockerClientTestCase):
             'com.example.label-with-empty-value': "",
         }
 
-        service = self.create_service('web', labels=labels_dict)
-        labels = create_and_start_container(service).labels.items()
-        for pair in labels_dict.items():
-            self.assertIn(pair, labels)
-
-        labels_list = ["%s=%s" % pair for pair in labels_dict.items()]
+        compose_labels = {
+            LABEL_CONTAINER_NUMBER: '1',
+            LABEL_ONE_OFF: 'False',
+            LABEL_PROJECT: 'composetest',
+            LABEL_SERVICE: 'web',
+            LABEL_VERSION: __version__,
+        }
+        expected = dict(labels_dict, **compose_labels)
 
-        service = self.create_service('web', labels=labels_list)
-        labels = create_and_start_container(service).labels.items()
-        for pair in labels_dict.items():
-            self.assertIn(pair, labels)
+        service = self.create_service('web', labels=labels_dict)
+        labels = create_and_start_container(service).labels
+        self.assertEqual(labels, expected)
 
     def test_empty_labels(self):
         labels_list = ['foo', 'bar']

+ 1 - 0
tests/integration/testcases.py

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

+ 21 - 8
tests/unit/container_test.py

@@ -5,6 +5,7 @@ import mock
 import docker
 
 from compose.container import Container
+from compose.container import get_container_name
 
 
 class ContainerTest(unittest.TestCase):
@@ -23,6 +24,13 @@ class ContainerTest(unittest.TestCase):
             "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 @@ class ContainerTest(unittest.TestCase):
         })
 
     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 @@ class ContainerTest(unittest.TestCase):
         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 @@ class ContainerTest(unittest.TestCase):
         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')

+ 20 - 29
tests/unit/service_test.py

@@ -7,15 +7,15 @@ import mock
 import docker
 from requests import Response
 
-from compose import Service
+from compose.service import Service
 from compose.container import Container
+from compose.const import LABEL_SERVICE, LABEL_PROJECT, LABEL_ONE_OFF
 from compose.service import (
     APIError,
     ConfigError,
     build_port_bindings,
     build_volume_binding,
     get_container_data_volumes,
-    get_container_name,
     merge_volume_bindings,
     parse_repository_tag,
     parse_volume_spec,
@@ -48,36 +48,27 @@ class ServiceTest(unittest.TestCase):
         self.assertRaises(ConfigError, lambda: Service(name='foo', project='_', image='foo'))
         Service(name='foo', project='bar', image='foo')
 
-    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')
-
     def test_containers(self):
-        service = Service('db', client=self.mock_client, image='foo', project='myproject')
-
+        service = Service('db', self.mock_client, 'myproject', image='foo')
         self.mock_client.containers.return_value = []
         self.assertEqual(service.containers(), [])
 
+    def test_containers_with_containers(self):
         self.mock_client.containers.return_value = [
-            {'Image': 'busybox', 'Id': 'OUT_1', 'Names': ['/myproject', '/foo/bar']},
-            {'Image': 'busybox', 'Id': 'OUT_2', 'Names': ['/myproject_db']},
-            {'Image': 'busybox', 'Id': 'OUT_3', 'Names': ['/db_1']},
-            {'Image': 'busybox', 'Id': 'IN_1', 'Names': ['/myproject_db_1', '/myproject_web_1/db']},
+            dict(Name=str(i), Image='foo', Id=i) for i in range(3)
         ]
-        self.assertEqual([c.id for c in service.containers()], ['IN_1'])
-
-    def test_containers_prefixed(self):
-        service = Service('db', client=self.mock_client, image='foo', project='myproject')
+        service = Service('db', self.mock_client, 'myproject', image='foo')
+        self.assertEqual([c.id for c in service.containers()], range(3))
 
-        self.mock_client.containers.return_value = [
-            {'Image': 'busybox', 'Id': 'OUT_1', 'Names': ['/swarm-host-1/myproject', '/swarm-host-1/foo/bar']},
-            {'Image': 'busybox', 'Id': 'OUT_2', 'Names': ['/swarm-host-1/myproject_db']},
-            {'Image': 'busybox', 'Id': 'OUT_3', 'Names': ['/swarm-host-1/db_1']},
-            {'Image': 'busybox', 'Id': 'IN_1', 'Names': ['/swarm-host-1/myproject_db_1', '/swarm-host-1/myproject_web_1/db']},
+        expected_labels = [
+            '{0}=myproject'.format(LABEL_PROJECT),
+            '{0}=db'.format(LABEL_SERVICE),
+            '{0}=False'.format(LABEL_ONE_OFF),
         ]
-        self.assertEqual([c.id for c in service.containers()], ['IN_1'])
+
+        self.mock_client.containers.assert_called_once_with(
+            all=False,
+            filters={'label': expected_labels})
 
     def test_get_volumes_from_container(self):
         container_id = 'aabbccddee'
@@ -156,7 +147,7 @@ class ServiceTest(unittest.TestCase):
     def test_split_domainname_none(self):
         service = Service('foo', image='foo', hostname='name', client=self.mock_client)
         self.mock_client.containers.return_value = []
-        opts = service._get_container_create_options({'image': 'foo'})
+        opts = service._get_container_create_options({'image': 'foo'}, 1)
         self.assertEqual(opts['hostname'], 'name', 'hostname')
         self.assertFalse('domainname' in opts, 'domainname')
 
@@ -167,7 +158,7 @@ class ServiceTest(unittest.TestCase):
             image='foo',
             client=self.mock_client)
         self.mock_client.containers.return_value = []
-        opts = service._get_container_create_options({'image': 'foo'})
+        opts = service._get_container_create_options({'image': 'foo'}, 1)
         self.assertEqual(opts['hostname'], 'name', 'hostname')
         self.assertEqual(opts['domainname'], 'domain.tld', 'domainname')
 
@@ -179,7 +170,7 @@ class ServiceTest(unittest.TestCase):
             domainname='domain.tld',
             client=self.mock_client)
         self.mock_client.containers.return_value = []
-        opts = service._get_container_create_options({'image': 'foo'})
+        opts = service._get_container_create_options({'image': 'foo'}, 1)
         self.assertEqual(opts['hostname'], 'name', 'hostname')
         self.assertEqual(opts['domainname'], 'domain.tld', 'domainname')
 
@@ -191,7 +182,7 @@ class ServiceTest(unittest.TestCase):
             image='foo',
             client=self.mock_client)
         self.mock_client.containers.return_value = []
-        opts = service._get_container_create_options({'image': 'foo'})
+        opts = service._get_container_create_options({'image': 'foo'}, 1)
         self.assertEqual(opts['hostname'], 'name.sub', 'hostname')
         self.assertEqual(opts['domainname'], 'domain.tld', 'domainname')
 
@@ -255,7 +246,7 @@ class ServiceTest(unittest.TestCase):
             tag='sometag',
             insecure_registry=True,
             stream=True)
-        mock_log.info.assert_called_once_with(
+        mock_log.info.assert_called_with(
             'Pulling foo (someimage:sometag)...')
 
     @mock.patch('compose.service.Container', autospec=True)