From 28d2aff8b8f65666d4659b9a25671228396dc7dc Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sun, 26 Apr 2015 21:21:55 -0400 Subject: [PATCH 1/3] Fix teardown for integration tests. Signed-off-by: Daniel Nephin --- tests/integration/cli_test.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/integration/cli_test.py b/tests/integration/cli_test.py index c7e2ea3438..92789363e4 100644 --- a/tests/integration/cli_test.py +++ b/tests/integration/cli_test.py @@ -21,6 +21,8 @@ def tearDown(self): 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 @@ def test_ps_default_composefile(self, mock_stdout): @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 @@ def test_scale(self): 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() From ed50a0a3a02821f5a6e50fc46db864e386258921 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sun, 26 Apr 2015 17:09:20 -0400 Subject: [PATCH 2/3] Resolves #1066, use labels to identify containers Signed-off-by: Daniel Nephin --- compose/__init__.py | 1 - compose/const.py | 6 ++ compose/container.py | 10 +-- compose/project.py | 13 +++- compose/service.py | 100 ++++++++++++++++-------------- tests/integration/service_test.py | 33 ++++++---- tests/integration/testcases.py | 1 + tests/unit/container_test.py | 29 ++++++--- tests/unit/service_test.py | 49 ++++++--------- 9 files changed, 139 insertions(+), 103 deletions(-) create mode 100644 compose/const.py diff --git a/compose/__init__.py b/compose/__init__.py index 2de2a7f8b8..045e791449 100644 --- a/compose/__init__.py +++ b/compose/__init__.py @@ -1,4 +1,3 @@ from __future__ import unicode_literals -from .service import Service # noqa:flake8 __version__ = '1.3.0dev' diff --git a/compose/const.py b/compose/const.py new file mode 100644 index 0000000000..0714a6dbf6 --- /dev/null +++ b/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' diff --git a/compose/container.py b/compose/container.py index 6388ca80c4..183d5fde89 100644 --- a/compose/container.py +++ b/compose/container.py @@ -4,6 +4,8 @@ import six from functools import reduce +from .const import LABEL_CONTAINER_NUMBER, LABEL_SERVICE + class Container(object): """ @@ -58,14 +60,11 @@ def name(self): @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 + return int(self.labels.get(LABEL_CONTAINER_NUMBER) or 0) @property def ports(self): @@ -159,6 +158,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(): diff --git a/compose/project.py b/compose/project.py index 2f3675ffba..d22bdf4dc8 100644 --- a/compose/project.py +++ b/compose/project.py @@ -4,6 +4,7 @@ from functools import reduce from .config import get_service_name_from_net, ConfigurationError +from .const import LABEL_PROJECT, LABEL_ONE_OFF 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(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): """ @@ -224,9 +231,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() diff --git a/compose/service.py b/compose/service.py index 08d203274f..3c62dbebb8 100644 --- a/compose/service.py +++ b/compose/service.py @@ -10,8 +10,16 @@ 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 .container import Container, get_container_name +from .const import ( + LABEL_CONTAINER_NUMBER, + LABEL_ONE_OFF, + LABEL_PROJECT, + LABEL_SERVICE, + LABEL_VERSION, +) +from .container import Container from .progress_stream import stream_output, StreamOutputError log = logging.getLogger(__name__) @@ -79,27 +87,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(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 +136,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,6 +175,7 @@ def create_container(self, 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 +183,7 @@ def create_container(self, """ 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 +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, @@ -256,6 +254,7 @@ def recreate_container(self, container, **override_options): 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 +279,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, @@ -302,14 +300,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): @@ -369,6 +372,7 @@ def _get_net(self): def _get_container_create_options( self, override_options, + number, one_off=False, previous_container=None): container_options = dict( @@ -376,9 +380,7 @@ def _get_container_create_options( 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 +421,11 @@ def _get_container_create_options( 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 +527,13 @@ def full_name(self): """ 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 +599,19 @@ 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 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[LABEL_CONTAINER_NUMBER] = str(number) + labels[LABEL_VERSION] = __version__ + return labels def parse_restart_spec(restart_config): diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index bc21ab0186..47c826ec5b 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -8,11 +8,19 @@ 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 @@ def test_labels(self): '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'] diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index 31281a1d71..4a0f7248a7 100644 --- a/tests/integration/testcases.py +++ b/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]: diff --git a/tests/unit/container_test.py b/tests/unit/container_test.py index 7637adf58b..b04df6592e 100644 --- a/tests/unit/container_test.py +++ b/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') diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 2ea94edbd3..fa252062c5 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -7,15 +7,15 @@ 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 @@ def test_project_validation(self): 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 @@ def test_build_port_bindings_with_nonmatching_internal_ports(self): 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 @@ def test_split_domainname_fqdn(self): 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 @@ def test_split_domainname_both(self): 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 @@ def test_split_domainname_weird(self): 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 @@ def test_create_container_from_insecure_registry( 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) From 62059d55e60e01d395fb8a5e99cda364ce517b49 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sat, 9 May 2015 12:53:59 -0400 Subject: [PATCH 3/3] Add migration warning and option to migrate to labels. Signed-off-by: Daniel Nephin --- compose/cli/main.py | 33 ++++++++++++--------- compose/container.py | 6 +++- compose/migration.py | 35 ++++++++++++++++++++++ compose/project.py | 32 +++++++++++++++----- compose/service.py | 46 +++++++++++++++++++++++++---- tests/integration/migration_test.py | 23 +++++++++++++++ tests/unit/container_test.py | 2 +- 7 files changed, 148 insertions(+), 29 deletions(-) create mode 100644 compose/migration.py create mode 100644 tests/integration/migration_test.py diff --git a/compose/cli/main.py b/compose/cli/main.py index a2375516e9..0c5d5a7f64 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -11,6 +11,7 @@ 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): @@ -483,6 +485,9 @@ def handler(signal, frame): 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) diff --git a/compose/container.py b/compose/container.py index 183d5fde89..3e462088fe 100644 --- a/compose/container.py +++ b/compose/container.py @@ -64,7 +64,11 @@ def name_without_project(self): @property def number(self): - return int(self.labels.get(LABEL_CONTAINER_NUMBER) or 0) + 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): diff --git a/compose/migration.py b/compose/migration.py new file mode 100644 index 0000000000..16b5dd1678 --- /dev/null +++ b/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) diff --git a/compose/project.py b/compose/project.py index d22bdf4dc8..8ca144813c 100644 --- a/compose/project.py +++ b/compose/project.py @@ -1,13 +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 .const import LABEL_PROJECT, LABEL_ONE_OFF -from .service import Service +from .service import Service, check_for_legacy_containers from .container import Container -from docker.errors import APIError log = logging.getLogger(__name__) @@ -82,6 +83,10 @@ def from_dicts(cls, name, service_dicts, client): 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 @@ -109,7 +114,7 @@ def get_services(self, service_names=None, include_deps=False): """ 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: @@ -230,10 +235,21 @@ def remove_stopped(self, service_names=None, **options): 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, - filters={'label': self.labels(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() diff --git a/compose/service.py b/compose/service.py index 3c62dbebb8..dc34a9bc25 100644 --- a/compose/service.py +++ b/compose/service.py @@ -19,7 +19,7 @@ LABEL_SERVICE, LABEL_VERSION, ) -from .container import Container +from .container import Container, get_container_name from .progress_stream import stream_output, StreamOutputError log = logging.getLogger(__name__) @@ -86,10 +86,21 @@ def __init__(self, name, client=None, project='default', links=None, external_li 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, - filters={'label': self.labels(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.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 @@ -614,6 +625,31 @@ 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 diff --git a/tests/integration/migration_test.py b/tests/integration/migration_test.py new file mode 100644 index 0000000000..133d231481 --- /dev/null +++ b/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) diff --git a/tests/unit/container_test.py b/tests/unit/container_test.py index b04df6592e..2313d4b8ef 100644 --- a/tests/unit/container_test.py +++ b/tests/unit/container_test.py @@ -28,7 +28,7 @@ def setUp(self): "Labels": { "com.docker.compose.project": "composetest", "com.docker.compose.service": "web", - "com.docker.compose.container_number": 7, + "com.docker.compose.container-number": 7, }, } }