diff --git a/compose/const.py b/compose/const.py new file mode 100644 index 0000000000..1ef1d2134a --- /dev/null +++ b/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' diff --git a/compose/container.py b/compose/container.py index fc3370d9e4..c853ee8c3a 100644 --- a/compose/container.py +++ b/compose/container.py @@ -4,6 +4,8 @@ import six from functools import reduce +from .const import CONTAINER_NUMBER_LABEL, SERVICE_LABEL + class Container(object): """ @@ -54,14 +56,11 @@ def name(self): @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): @@ -155,6 +154,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..0d150aacb2 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 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): """ @@ -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 a1c0f9258f..176e54c8c5 100644 --- a/compose/service.py +++ b/compose/service.py @@ -11,7 +11,13 @@ from docker.utils import create_host_config, LogConfig 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__) @@ -78,27 +84,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)) @@ -137,7 +133,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 = [] @@ -177,6 +172,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 +181,7 @@ def create_container(self, override_options['volumes_from'] = self._get_volumes_from(previous_container) container_options = self._get_container_create_options( override_options, + number or self._next_container_number(one_off=one_off), one_off=one_off, ) @@ -208,7 +205,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, @@ -249,6 +245,7 @@ def recreate_container(self, container, **override_options): new_container = self.create_container( do_build=False, previous_container=container, + number=container.labels.get(CONTAINER_NUMBER_LABEL), **override_options) self.start_container(new_container) container.remove() @@ -273,7 +270,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, @@ -295,14 +291,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): @@ -362,15 +363,17 @@ def _get_net(self): return net - def _get_container_create_options(self, override_options, one_off=False): + def _get_container_create_options( + self, + override_options, + number, + one_off=False): 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 @@ -403,6 +406,11 @@ def _get_container_create_options(self, override_options, one_off=False): 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) @@ -506,6 +514,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): @@ -527,23 +542,18 @@ def pull(self, insecure_registry=False): stream_output(output, sys.stdout) -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): diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index 31281a1d71..6ec06f6f89 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 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 583f72ef0b..123e468d8d 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -9,12 +9,12 @@ from compose import Service from compose.container import Container +from compose.const import SERVICE_LABEL, PROJECT_LABEL, ONE_OFF_LABEL from compose.service import ( APIError, ConfigError, build_port_bindings, build_volume_binding, - get_container_name, parse_repository_tag, parse_volume_spec, split_port, @@ -46,36 +46,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(PROJECT_LABEL), + '{0}=db'.format(SERVICE_LABEL), + '{0}=False'.format(ONE_OFF_LABEL), ] - 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' @@ -161,7 +152,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') @@ -172,7 +163,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') @@ -184,7 +175,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') @@ -196,7 +187,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') @@ -260,7 +251,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)