From 4e2de3c1ff4c30310e40f8fa4695cc0f518ce436 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Mon, 13 Aug 2018 19:29:29 -0700 Subject: [PATCH] Replace sequential container indexes with randomly generated IDs Signed-off-by: Joffrey F --- compose/cli/main.py | 14 +- compose/container.py | 9 +- compose/service.py | 38 ++--- compose/utils.py | 19 +++ tests/acceptance/cli_test.py | 138 ++++++++++-------- .../logs-tail-composefile/docker-compose.yml | 2 +- tests/integration/project_test.py | 11 +- tests/integration/service_test.py | 77 +++++----- tests/integration/state_test.py | 35 +++-- tests/unit/container_test.py | 6 +- tests/unit/service_test.py | 55 +++---- 11 files changed, 230 insertions(+), 174 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index e0acf0711..2cee9e033 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -474,15 +474,16 @@ class TopLevelCommand(object): -u, --user USER Run the command as this user. -T Disable pseudo-tty allocation. By default `docker-compose exec` allocates a TTY. - --index=index index of the container if there are multiple - instances of a service [default: 1] + --index=index "index" of the container if there are multiple + instances of a service. If missing, Compose will pick an + arbitrary container. -e, --env KEY=VAL Set environment variables (can be used multiple times, not supported in API < 1.25) -w, --workdir DIR Path to workdir directory for this command. """ environment = Environment.from_env_file(self.project_dir) use_cli = not environment.get_boolean('COMPOSE_INTERACTIVE_NO_CLI') - index = int(options.get('--index')) + index = options.get('--index') service = self.project.get_service(options['SERVICE']) detach = options.get('--detach') @@ -659,10 +660,11 @@ class TopLevelCommand(object): Options: --protocol=proto tcp or udp [default: tcp] - --index=index index of the container if there are multiple - instances of a service [default: 1] + --index=index "index" of the container if there are multiple + instances of a service. If missing, Compose will pick an + arbitrary container. """ - index = int(options.get('--index')) + index = options.get('--index') service = self.project.get_service(options['SERVICE']) try: container = service.get_container(number=index) diff --git a/compose/container.py b/compose/container.py index 8dac8cacd..9b5bbba04 100644 --- a/compose/container.py +++ b/compose/container.py @@ -10,6 +10,7 @@ from .const import LABEL_CONTAINER_NUMBER from .const import LABEL_PROJECT from .const import LABEL_SERVICE from .const import LABEL_VERSION +from .utils import truncate_id from .version import ComposeVersion @@ -80,7 +81,7 @@ class Container(object): @property def name_without_project(self): if self.name.startswith('{0}_{1}'.format(self.project, self.service)): - return '{0}_{1}'.format(self.service, self.number) + return '{0}_{1}'.format(self.service, self.short_number) else: return self.name @@ -90,7 +91,11 @@ class Container(object): if not number: raise ValueError("Container {0} does not have a {1} label".format( self.short_id, LABEL_CONTAINER_NUMBER)) - return int(number) + return number + + @property + def short_number(self): + return truncate_id(self.number) @property def ports(self): diff --git a/compose/service.py b/compose/service.py index 33bb3fe34..5989217d7 100644 --- a/compose/service.py +++ b/compose/service.py @@ -1,7 +1,6 @@ from __future__ import absolute_import from __future__ import unicode_literals -import itertools import logging import os import re @@ -49,9 +48,11 @@ from .errors import OperationFailedError from .parallel import parallel_execute from .progress_stream import stream_output from .progress_stream import StreamOutputError +from .utils import generate_random_id from .utils import json_hash from .utils import parse_bytes from .utils import parse_seconds_float +from .utils import truncate_id log = logging.getLogger(__name__) @@ -215,13 +216,17 @@ class Service(object): ) ) - def get_container(self, number=1): + def get_container(self, number=None): """Return a :class:`compose.container.Container` for this service. The container must be active, and match `number`. """ - - for container in self.containers(labels=['{0}={1}'.format(LABEL_CONTAINER_NUMBER, number)]): - return container + if number is not None and len(number) == 64: + for container in self.containers(labels=['{0}={1}'.format(LABEL_CONTAINER_NUMBER, number)]): + return container + else: + for container in self.containers(): + if number is None or container.number.startswith(number): + return container raise ValueError("No container found for %s_%s" % (self.name, number)) @@ -426,7 +431,6 @@ class Service(object): return has_diverged def _execute_convergence_create(self, scale, detached, start, project_services=None): - i = self._next_container_number() def create_and_start(service, n): container = service.create_container(number=n, quiet=True) @@ -437,7 +441,9 @@ class Service(object): return container containers, errors = parallel_execute( - [ServiceName(self.project, self.name, index) for index in range(i, i + scale)], + [ServiceName(self.project, self.name, number) for number in [ + self._next_container_number() for _ in range(scale) + ]], lambda service_name: create_and_start(self, service_name.number), lambda service_name: self.get_container_name(service_name.service, service_name.number), "Creating" @@ -568,7 +574,7 @@ class Service(object): container.rename_to_tmp_name() new_container = self.create_container( previous_container=container if not renew_anonymous_volumes else None, - number=container.labels.get(LABEL_CONTAINER_NUMBER), + number=container.number, quiet=True, ) if attach_logs: @@ -723,20 +729,8 @@ class Service(object): def get_volumes_from_names(self): return [s.source.name for s in self.volumes_from if isinstance(s.source, Service)] - # TODO: this would benefit from github.com/docker/docker/pull/14699 - # to remove the need to inspect every container def _next_container_number(self, one_off=False): - containers = itertools.chain( - self._fetch_containers( - all=True, - filters={'label': self.labels(one_off=one_off)} - ), self._fetch_containers( - all=True, - filters={'label': self.labels(one_off=one_off, legacy=True)} - ) - ) - numbers = [c.number for c in containers] - return 1 if not numbers else max(numbers) + 1 + return generate_random_id() def _fetch_containers(self, **fetch_options): # Account for containers that might have been removed since we fetched @@ -1377,7 +1371,7 @@ def build_container_name(project, service, number, one_off=False): bits = [project.lstrip('-_'), service] if one_off: bits.append('run') - return '_'.join(bits + [str(number)]) + return '_'.join(bits + [truncate_id(number)]) # Images diff --git a/compose/utils.py b/compose/utils.py index 956673b4b..8f0b3e549 100644 --- a/compose/utils.py +++ b/compose/utils.py @@ -7,6 +7,7 @@ import json import json.decoder import logging import ntpath +import random import six from docker.errors import DockerException @@ -151,3 +152,21 @@ def unquote_path(s): if s[0] == '"' and s[-1] == '"': return s[1:-1] return s + + +def generate_random_id(): + while True: + val = hex(random.getrandbits(32 * 8))[2:-1] + try: + int(truncate_id(val)) + continue + except ValueError: + return val + + +def truncate_id(value): + if ':' in value: + value = value[value.index(':') + 1:] + if len(value) > 12: + return value[:12] + return value diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 66e7d4c38..a41250d3d 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -99,7 +99,14 @@ class ContainerStateCondition(object): def __call__(self): try: - container = self.client.inspect_container(self.name) + if self.name.endswith('*'): + ctnrs = self.client.containers(all=True, filters={'name': self.name[:-1]}) + if len(ctnrs) > 0: + container = self.client.inspect_container(ctnrs[0]['Id']) + else: + return False + else: + container = self.client.inspect_container(self.name) return container['State']['Status'] == self.status except errors.APIError: return False @@ -540,16 +547,16 @@ class CLITestCase(DockerClientTestCase): def test_ps(self): self.project.get_service('simple').create_container() result = self.dispatch(['ps']) - assert 'simple-composefile_simple_1' in result.stdout + assert 'simple-composefile_simple_' in result.stdout def test_ps_default_composefile(self): self.base_dir = 'tests/fixtures/multiple-composefiles' self.dispatch(['up', '-d']) result = self.dispatch(['ps']) - assert 'multiple-composefiles_simple_1' in result.stdout - assert 'multiple-composefiles_another_1' in result.stdout - assert 'multiple-composefiles_yetanother_1' not in result.stdout + assert 'multiple-composefiles_simple_' in result.stdout + assert 'multiple-composefiles_another_' in result.stdout + assert 'multiple-composefiles_yetanother_' not in result.stdout def test_ps_alternate_composefile(self): config_path = os.path.abspath( @@ -560,9 +567,9 @@ class CLITestCase(DockerClientTestCase): self.dispatch(['-f', 'compose2.yml', 'up', '-d']) result = self.dispatch(['-f', 'compose2.yml', 'ps']) - assert 'multiple-composefiles_simple_1' not in result.stdout - assert 'multiple-composefiles_another_1' not in result.stdout - assert 'multiple-composefiles_yetanother_1' in result.stdout + assert 'multiple-composefiles_simple_' not in result.stdout + assert 'multiple-composefiles_another_' not in result.stdout + assert 'multiple-composefiles_yetanother_' in result.stdout def test_ps_services_filter_option(self): self.base_dir = 'tests/fixtures/ps-services-filter' @@ -956,13 +963,13 @@ class CLITestCase(DockerClientTestCase): assert len(self.project.containers(one_off=OneOffFilter.only, stopped=True)) == 2 result = self.dispatch(['down', '--rmi=local', '--volumes']) - assert 'Stopping v2-full_web_1' in result.stderr - assert 'Stopping v2-full_other_1' in result.stderr - assert 'Stopping v2-full_web_run_2' in result.stderr - assert 'Removing v2-full_web_1' in result.stderr - assert 'Removing v2-full_other_1' in result.stderr - assert 'Removing v2-full_web_run_1' in result.stderr - assert 'Removing v2-full_web_run_2' in result.stderr + assert 'Stopping v2-full_web_' in result.stderr + assert 'Stopping v2-full_other_' in result.stderr + assert 'Stopping v2-full_web_run_' in result.stderr + assert 'Removing v2-full_web_' in result.stderr + assert 'Removing v2-full_other_' in result.stderr + assert 'Removing v2-full_web_run_' in result.stderr + assert 'Removing v2-full_web_run_' in result.stderr assert 'Removing volume v2-full_data' in result.stderr assert 'Removing image v2-full_web' in result.stderr assert 'Removing image busybox' not in result.stderr @@ -1019,11 +1026,13 @@ class CLITestCase(DockerClientTestCase): def test_up_attached(self): self.base_dir = 'tests/fixtures/echo-services' result = self.dispatch(['up', '--no-color']) + simple_num = self.project.get_service('simple').containers(stopped=True)[0].short_number + another_num = self.project.get_service('another').containers(stopped=True)[0].short_number - assert 'simple_1 | simple' in result.stdout - assert 'another_1 | another' in result.stdout - assert 'simple_1 exited with code 0' in result.stdout - assert 'another_1 exited with code 0' in result.stdout + assert 'simple_{} | simple'.format(simple_num) in result.stdout + assert 'another_{} | another'.format(another_num) in result.stdout + assert 'simple_{} exited with code 0'.format(simple_num) in result.stdout + assert 'another_{} exited with code 0'.format(another_num) in result.stdout @v2_only() def test_up(self): @@ -1727,11 +1736,12 @@ class CLITestCase(DockerClientTestCase): def test_run_rm(self): self.base_dir = 'tests/fixtures/volume' proc = start_process(self.base_dir, ['run', '--rm', 'test']) + service = self.project.get_service('test') wait_on_condition(ContainerStateCondition( self.project.client, - 'volume_test_run_1', - 'running')) - service = self.project.get_service('test') + 'volume_test_run_*', + 'running') + ) containers = service.containers(one_off=OneOffFilter.only) assert len(containers) == 1 mounts = containers[0].get('Mounts') @@ -2054,39 +2064,39 @@ class CLITestCase(DockerClientTestCase): proc = start_process(self.base_dir, ['run', '-T', 'simple', 'top']) wait_on_condition(ContainerStateCondition( self.project.client, - 'simple-composefile_simple_run_1', + 'simple-composefile_simple_run_*', 'running')) os.kill(proc.pid, signal.SIGINT) wait_on_condition(ContainerStateCondition( self.project.client, - 'simple-composefile_simple_run_1', + 'simple-composefile_simple_run_*', 'exited')) def test_run_handles_sigterm(self): proc = start_process(self.base_dir, ['run', '-T', 'simple', 'top']) wait_on_condition(ContainerStateCondition( self.project.client, - 'simple-composefile_simple_run_1', + 'simple-composefile_simple_run_*', 'running')) os.kill(proc.pid, signal.SIGTERM) wait_on_condition(ContainerStateCondition( self.project.client, - 'simple-composefile_simple_run_1', + 'simple-composefile_simple_run_*', 'exited')) def test_run_handles_sighup(self): proc = start_process(self.base_dir, ['run', '-T', 'simple', 'top']) wait_on_condition(ContainerStateCondition( self.project.client, - 'simple-composefile_simple_run_1', + 'simple-composefile_simple_run_*', 'running')) os.kill(proc.pid, signal.SIGHUP) wait_on_condition(ContainerStateCondition( self.project.client, - 'simple-composefile_simple_run_1', + 'simple-composefile_simple_run_*', 'exited')) @mock.patch.dict(os.environ) @@ -2286,34 +2296,44 @@ class CLITestCase(DockerClientTestCase): proc = start_process(self.base_dir, ['logs', '-f']) self.dispatch(['up', '-d', 'another']) - wait_on_condition(ContainerStateCondition( - self.project.client, - 'logs-composefile_another_1', - 'exited')) + another_num = self.project.get_service('another').get_container().short_number + wait_on_condition( + ContainerStateCondition( + self.project.client, + 'logs-composefile_another_{}'.format(another_num), + 'exited' + ) + ) + simple_num = self.project.get_service('simple').get_container().short_number self.dispatch(['kill', 'simple']) result = wait_on_process(proc) assert 'hello' in result.stdout assert 'test' in result.stdout - assert 'logs-composefile_another_1 exited with code 0' in result.stdout - assert 'logs-composefile_simple_1 exited with code 137' in result.stdout + assert 'logs-composefile_another_{} exited with code 0'.format(another_num) in result.stdout + assert 'logs-composefile_simple_{} exited with code 137'.format(simple_num) in result.stdout def test_logs_follow_logs_from_restarted_containers(self): self.base_dir = 'tests/fixtures/logs-restart-composefile' proc = start_process(self.base_dir, ['up']) - wait_on_condition(ContainerStateCondition( - self.project.client, - 'logs-restart-composefile_another_1', - 'exited')) - + wait_on_condition( + ContainerStateCondition( + self.project.client, + 'logs-restart-composefile_another_*', + 'exited' + ) + ) self.dispatch(['kill', 'simple']) result = wait_on_process(proc) - assert result.stdout.count('logs-restart-composefile_another_1 exited with code 1') == 3 + assert len(re.findall( + r'logs-restart-composefile_another_[a-f0-9]{12} exited with code 1', + result.stdout + )) == 3 assert result.stdout.count('world') == 3 def test_logs_default(self): @@ -2346,10 +2366,10 @@ class CLITestCase(DockerClientTestCase): self.dispatch(['up']) result = self.dispatch(['logs', '--tail', '2']) - assert 'c\n' in result.stdout - assert 'd\n' in result.stdout - assert 'a\n' not in result.stdout - assert 'b\n' not in result.stdout + assert 'y\n' in result.stdout + assert 'z\n' in result.stdout + assert 'w\n' not in result.stdout + assert 'x\n' not in result.stdout def test_kill(self): self.dispatch(['up', '-d'], None) @@ -2523,9 +2543,9 @@ class CLITestCase(DockerClientTestCase): result = self.dispatch(['port', '--index=' + str(index), 'simple', str(number)]) return result.stdout.rstrip() - assert get_port(3000) == containers[0].get_local_port(3000) - assert get_port(3000, index=1) == containers[0].get_local_port(3000) - assert get_port(3000, index=2) == containers[1].get_local_port(3000) + assert get_port(3000) in (containers[0].get_local_port(3000), containers[1].get_local_port(3000)) + assert get_port(3000, index=containers[0].number) == containers[0].get_local_port(3000) + assert get_port(3000, index=containers[1].number) == containers[1].get_local_port(3000) assert get_port(3002) == "" def test_events_json(self): @@ -2561,7 +2581,7 @@ class CLITestCase(DockerClientTestCase): container, = self.project.containers() expected_template = ' container {} {}' - expected_meta_info = ['image=busybox:latest', 'name=simple-composefile_simple_1'] + expected_meta_info = ['image=busybox:latest', 'name=simple-composefile_simple_'] assert expected_template.format('create', container.id) in lines[0] assert expected_template.format('start', container.id) in lines[1] @@ -2643,8 +2663,11 @@ class CLITestCase(DockerClientTestCase): assert len(containers) == 2 web = containers[1] + db_num = containers[0].short_number - assert set(get_links(web)) == set(['db', 'mydb_1', 'extends_mydb_1']) + assert set(get_links(web)) == set( + ['db', 'mydb_{}'.format(db_num), 'extends_mydb_{}'.format(db_num)] + ) expected_env = set([ "FOO=1", @@ -2677,11 +2700,11 @@ class CLITestCase(DockerClientTestCase): self.base_dir = 'tests/fixtures/exit-code-from' proc = start_process( self.base_dir, - ['up', '--abort-on-container-exit', '--exit-code-from', 'another']) + ['up', '--abort-on-container-exit', '--exit-code-from', 'another'] + ) result = wait_on_process(proc, returncode=1) - - assert 'exit-code-from_another_1 exited with code 1' in result.stdout + assert re.findall(r'exit-code-from_another_[a-f0-9]{12} exited with code 1', result.stdout) def test_exit_code_from_signal_stop(self): self.base_dir = 'tests/fixtures/exit-code-from' @@ -2690,13 +2713,14 @@ class CLITestCase(DockerClientTestCase): ['up', '--abort-on-container-exit', '--exit-code-from', 'simple'] ) result = wait_on_process(proc, returncode=137) # SIGKILL - assert 'exit-code-from_another_1 exited with code 1' in result.stdout + num = self.project.get_service('another').containers(stopped=True)[0].short_number + assert 'exit-code-from_another_{} exited with code 1'.format(num) in result.stdout def test_images(self): self.project.get_service('simple').create_container() result = self.dispatch(['images']) assert 'busybox' in result.stdout - assert 'simple-composefile_simple_1' in result.stdout + assert 'simple-composefile_simple_' in result.stdout def test_images_default_composefile(self): self.base_dir = 'tests/fixtures/multiple-composefiles' @@ -2704,8 +2728,8 @@ class CLITestCase(DockerClientTestCase): result = self.dispatch(['images']) assert 'busybox' in result.stdout - assert 'multiple-composefiles_another_1' in result.stdout - assert 'multiple-composefiles_simple_1' in result.stdout + assert 'multiple-composefiles_another_' in result.stdout + assert 'multiple-composefiles_simple_' in result.stdout @mock.patch.dict(os.environ) def test_images_tagless_image(self): @@ -2725,7 +2749,7 @@ class CLITestCase(DockerClientTestCase): self.project.get_service('foo').create_container() result = self.dispatch(['images']) assert '' in result.stdout - assert 'tagless-image_foo_1' in result.stdout + assert 'tagless-image_foo_' in result.stdout def test_up_with_override_yaml(self): self.base_dir = 'tests/fixtures/override-yaml-files' diff --git a/tests/fixtures/logs-tail-composefile/docker-compose.yml b/tests/fixtures/logs-tail-composefile/docker-compose.yml index 80d8feaec..b70d0cc63 100644 --- a/tests/fixtures/logs-tail-composefile/docker-compose.yml +++ b/tests/fixtures/logs-tail-composefile/docker-compose.yml @@ -1,3 +1,3 @@ simple: image: busybox:latest - command: sh -c "echo a && echo b && echo c && echo d" + command: sh -c "echo w && echo x && echo y && echo z" diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 8813e84ce..858a8dfd7 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -90,7 +90,8 @@ class ProjectTest(DockerClientTestCase): project.up() containers = project.containers(['web']) - assert [c.name for c in containers] == ['composetest_web_1'] + assert len(containers) == 1 + assert containers[0].name.startswith('composetest_web_') def test_containers_with_extra_service(self): web = self.create_service('web') @@ -464,14 +465,14 @@ class ProjectTest(DockerClientTestCase): project.up(['db']) assert len(project.containers()) == 1 - old_db_id = project.containers()[0].id container, = project.containers() + old_db_id = container.id db_volume_path = container.get_mount('/var/db')['Source'] project.up(strategy=ConvergenceStrategy.never) assert len(project.containers()) == 2 - db_container = [c for c in project.containers() if 'db' in c.name][0] + db_container = [c for c in project.containers() if c.name == container.name][0] assert db_container.id == old_db_id assert db_container.get_mount('/var/db')['Source'] == db_volume_path @@ -1944,7 +1945,7 @@ class ProjectTest(DockerClientTestCase): containers = project.containers(stopped=True) assert len(containers) == 1 - assert containers[0].name == 'underscoretest_svc1_1' + assert containers[0].name.startswith('underscoretest_svc1_') assert containers[0].project == '_underscoretest' full_vol_name = 'underscoretest_foo' @@ -1965,7 +1966,7 @@ class ProjectTest(DockerClientTestCase): containers = project2.containers(stopped=True) assert len(containers) == 1 - assert containers[0].name == 'dashtest_svc1_1' + assert containers[0].name.startswith('dashtest_svc1_') assert containers[0].project == '-dashtest' full_vol_name = 'dashtest_foo' diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 88123152c..d7422e2f6 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -67,7 +67,7 @@ class ServiceTest(DockerClientTestCase): create_and_start_container(foo) assert len(foo.containers()) == 1 - assert foo.containers()[0].name == 'composetest_foo_1' + assert foo.containers()[0].name.startswith('composetest_foo_') assert len(bar.containers()) == 0 create_and_start_container(bar) @@ -77,8 +77,8 @@ class ServiceTest(DockerClientTestCase): assert len(bar.containers()) == 2 names = [c.name for c in bar.containers()] - assert 'composetest_bar_1' in names - assert 'composetest_bar_2' in names + assert len(names) == 2 + assert all(name.startswith('composetest_bar_') for name in names) def test_containers_one_off(self): db = self.create_service('db') @@ -89,18 +89,18 @@ class ServiceTest(DockerClientTestCase): def test_project_is_added_to_container_name(self): service = self.create_service('web') create_and_start_container(service) - assert service.containers()[0].name == 'composetest_web_1' + assert service.containers()[0].name.startswith('composetest_web_') def test_create_container_with_one_off(self): db = self.create_service('db') container = db.create_container(one_off=True) - assert container.name == 'composetest_db_run_1' + assert container.name.startswith('composetest_db_run_') def test_create_container_with_one_off_when_existing_container_is_running(self): db = self.create_service('db') db.start() container = db.create_container(one_off=True) - assert container.name == 'composetest_db_run_1' + assert container.name.startswith('composetest_db_run_') def test_create_container_with_unspecified_volume(self): service = self.create_service('db', volumes=[VolumeSpec.parse('/var/db')]) @@ -489,7 +489,7 @@ class ServiceTest(DockerClientTestCase): assert old_container.get('Config.Entrypoint') == ['top'] assert old_container.get('Config.Cmd') == ['-d', '1'] assert 'FOO=1' in old_container.get('Config.Env') - assert old_container.name == 'composetest_db_1' + assert old_container.name.startswith('composetest_db_') service.start_container(old_container) old_container.inspect() # reload volume data volume_path = old_container.get_mount('/etc')['Source'] @@ -503,7 +503,7 @@ class ServiceTest(DockerClientTestCase): assert new_container.get('Config.Entrypoint') == ['top'] assert new_container.get('Config.Cmd') == ['-d', '1'] assert 'FOO=2' in new_container.get('Config.Env') - assert new_container.name == 'composetest_db_1' + assert new_container.name.startswith('composetest_db_') assert new_container.get_mount('/etc')['Source'] == volume_path if not is_cluster(self.client): assert ( @@ -836,13 +836,13 @@ class ServiceTest(DockerClientTestCase): db = self.create_service('db') web = self.create_service('web', links=[(db, None)]) - create_and_start_container(db) - create_and_start_container(db) + db1 = create_and_start_container(db) + db2 = create_and_start_container(db) create_and_start_container(web) assert set(get_links(web.containers()[0])) == set([ - 'composetest_db_1', 'db_1', - 'composetest_db_2', 'db_2', + db1.name, db1.name_without_project, + db2.name, db2.name_without_project, 'db' ]) @@ -851,30 +851,33 @@ class ServiceTest(DockerClientTestCase): db = self.create_service('db') web = self.create_service('web', links=[(db, 'custom_link_name')]) - create_and_start_container(db) - create_and_start_container(db) + db1 = create_and_start_container(db) + db2 = create_and_start_container(db) create_and_start_container(web) assert set(get_links(web.containers()[0])) == set([ - 'composetest_db_1', 'db_1', - 'composetest_db_2', 'db_2', + db1.name, db1.name_without_project, + db2.name, db2.name_without_project, 'custom_link_name' ]) @no_cluster('No legacy links support in Swarm') def test_start_container_with_external_links(self): db = self.create_service('db') - web = self.create_service('web', external_links=['composetest_db_1', - 'composetest_db_2', - 'composetest_db_3:db_3']) + db_ctnrs = [create_and_start_container(db) for _ in range(3)] + web = self.create_service( + 'web', external_links=[ + 'composetest_db_{}'.format(db_ctnrs[0].short_number), + 'composetest_db_{}'.format(db_ctnrs[1].short_number), + 'composetest_db_{}:db_3'.format(db_ctnrs[2].short_number) + ] + ) - for _ in range(3): - create_and_start_container(db) create_and_start_container(web) assert set(get_links(web.containers()[0])) == set([ - 'composetest_db_1', - 'composetest_db_2', + 'composetest_db_{}'.format(db_ctnrs[0].short_number), + 'composetest_db_{}'.format(db_ctnrs[1].short_number), 'db_3' ]) @@ -892,14 +895,14 @@ class ServiceTest(DockerClientTestCase): def test_start_one_off_container_creates_links_to_its_own_service(self): db = self.create_service('db') - create_and_start_container(db) - create_and_start_container(db) + db1 = create_and_start_container(db) + db2 = create_and_start_container(db) c = create_and_start_container(db, one_off=OneOffFilter.only) assert set(get_links(c)) == set([ - 'composetest_db_1', 'db_1', - 'composetest_db_2', 'db_2', + db1.name, db1.name_without_project, + db2.name, db2.name_without_project, 'db' ]) @@ -1249,10 +1252,9 @@ class ServiceTest(DockerClientTestCase): test that those containers are restarted and not removed/recreated. """ service = self.create_service('web') - next_number = service._next_container_number() - valid_numbers = [next_number, next_number + 1] - service.create_container(number=next_number) - service.create_container(number=next_number + 1) + valid_numbers = [service._next_container_number(), service._next_container_number()] + service.create_container(number=valid_numbers[0]) + service.create_container(number=valid_numbers[1]) ParallelStreamWriter.instance = None with mock.patch('sys.stderr', new_callable=StringIO) as mock_stderr: @@ -1310,10 +1312,8 @@ class ServiceTest(DockerClientTestCase): assert len(service.containers()) == 1 assert service.containers()[0].is_running - assert ( - "ERROR: for composetest_web_2 Cannot create container for service" - " web: Boom" in mock_stderr.getvalue() - ) + assert "ERROR: for composetest_web_" in mock_stderr.getvalue() + assert "Cannot create container for service web: Boom" in mock_stderr.getvalue() def test_scale_with_unexpected_exception(self): """Test that when scaling if the API returns an error, that is not of type @@ -1580,7 +1580,6 @@ class ServiceTest(DockerClientTestCase): } compose_labels = { - LABEL_CONTAINER_NUMBER: '1', LABEL_ONE_OFF: 'False', LABEL_PROJECT: 'composetest', LABEL_SERVICE: 'web', @@ -1589,9 +1588,11 @@ class ServiceTest(DockerClientTestCase): expected = dict(labels_dict, **compose_labels) service = self.create_service('web', labels=labels_dict) - labels = create_and_start_container(service).labels.items() + ctnr = create_and_start_container(service) + labels = ctnr.labels.items() for pair in expected.items(): assert pair in labels + assert ctnr.labels[LABEL_CONTAINER_NUMBER] == ctnr.number def test_empty_labels(self): labels_dict = {'foo': '', 'bar': ''} @@ -1655,7 +1656,7 @@ class ServiceTest(DockerClientTestCase): def test_duplicate_containers(self): service = self.create_service('web') - options = service._get_container_create_options({}, 1) + options = service._get_container_create_options({}, service._next_container_number()) original = Container.create(service.client, **options) assert set(service.containers(stopped=True)) == set([original]) diff --git a/tests/integration/state_test.py b/tests/integration/state_test.py index 5992a02a4..7652c06c8 100644 --- a/tests/integration/state_test.py +++ b/tests/integration/state_test.py @@ -55,8 +55,8 @@ class BasicProjectTest(ProjectTestCase): def test_partial_change(self): old_containers = self.run_up(self.cfg) - old_db = [c for c in old_containers if c.name_without_project == 'db_1'][0] - old_web = [c for c in old_containers if c.name_without_project == 'web_1'][0] + old_db = [c for c in old_containers if c.name_without_project.startswith('db_')][0] + old_web = [c for c in old_containers if c.name_without_project.startswith('web_')][0] self.cfg['web']['command'] = '/bin/true' @@ -71,7 +71,7 @@ class BasicProjectTest(ProjectTestCase): created = list(new_containers - old_containers) assert len(created) == 1 - assert created[0].name_without_project == 'web_1' + assert created[0].name_without_project == old_web.name_without_project assert created[0].get('Config.Cmd') == ['/bin/true'] def test_all_change(self): @@ -114,7 +114,7 @@ class ProjectWithDependenciesTest(ProjectTestCase): def test_up(self): containers = self.run_up(self.cfg) - assert set(c.name_without_project for c in containers) == set(['db_1', 'web_1', 'nginx_1']) + assert set(c.service for c in containers) == set(['db', 'web', 'nginx']) def test_change_leaf(self): old_containers = self.run_up(self.cfg) @@ -122,7 +122,7 @@ class ProjectWithDependenciesTest(ProjectTestCase): self.cfg['nginx']['environment'] = {'NEW_VAR': '1'} new_containers = self.run_up(self.cfg) - assert set(c.name_without_project for c in new_containers - old_containers) == set(['nginx_1']) + assert set(c.service for c in new_containers - old_containers) == set(['nginx']) def test_change_middle(self): old_containers = self.run_up(self.cfg) @@ -130,7 +130,7 @@ class ProjectWithDependenciesTest(ProjectTestCase): self.cfg['web']['environment'] = {'NEW_VAR': '1'} new_containers = self.run_up(self.cfg) - assert set(c.name_without_project for c in new_containers - old_containers) == set(['web_1']) + assert set(c.service for c in new_containers - old_containers) == set(['web']) def test_change_middle_always_recreate_deps(self): old_containers = self.run_up(self.cfg, always_recreate_deps=True) @@ -138,8 +138,7 @@ class ProjectWithDependenciesTest(ProjectTestCase): self.cfg['web']['environment'] = {'NEW_VAR': '1'} new_containers = self.run_up(self.cfg, always_recreate_deps=True) - assert set(c.name_without_project - for c in new_containers - old_containers) == {'web_1', 'nginx_1'} + assert set(c.service for c in new_containers - old_containers) == {'web', 'nginx'} def test_change_root(self): old_containers = self.run_up(self.cfg) @@ -147,7 +146,7 @@ class ProjectWithDependenciesTest(ProjectTestCase): self.cfg['db']['environment'] = {'NEW_VAR': '1'} new_containers = self.run_up(self.cfg) - assert set(c.name_without_project for c in new_containers - old_containers) == set(['db_1']) + assert set(c.service for c in new_containers - old_containers) == set(['db']) def test_change_root_always_recreate_deps(self): old_containers = self.run_up(self.cfg, always_recreate_deps=True) @@ -155,8 +154,9 @@ class ProjectWithDependenciesTest(ProjectTestCase): self.cfg['db']['environment'] = {'NEW_VAR': '1'} new_containers = self.run_up(self.cfg, always_recreate_deps=True) - assert set(c.name_without_project - for c in new_containers - old_containers) == {'db_1', 'web_1', 'nginx_1'} + assert set(c.service for c in new_containers - old_containers) == { + 'db', 'web', 'nginx' + } def test_change_root_no_recreate(self): old_containers = self.run_up(self.cfg) @@ -195,9 +195,18 @@ class ProjectWithDependenciesTest(ProjectTestCase): web, = [c for c in containers if c.service == 'web'] nginx, = [c for c in containers if c.service == 'nginx'] + db, = [c for c in containers if c.service == 'db'] - assert set(get_links(web)) == {'composetest_db_1', 'db', 'db_1'} - assert set(get_links(nginx)) == {'composetest_web_1', 'web', 'web_1'} + assert set(get_links(web)) == { + 'composetest_db_{}'.format(db.short_number), + 'db', + 'db_{}'.format(db.short_number) + } + assert set(get_links(nginx)) == { + 'composetest_web_{}'.format(web.short_number), + 'web', + 'web_{}'.format(web.short_number) + } class ServiceStateTest(DockerClientTestCase): diff --git a/tests/unit/container_test.py b/tests/unit/container_test.py index d64263c1f..4f2f08302 100644 --- a/tests/unit/container_test.py +++ b/tests/unit/container_test.py @@ -30,7 +30,7 @@ class ContainerTest(unittest.TestCase): "Labels": { "com.docker.compose.project": "composetest", "com.docker.compose.service": "web", - "com.docker.compose.container-number": 7, + "com.docker.compose.container-number": "092cd63296fdc446ad432d3905dd1fcbe12a2ba6b52", }, } } @@ -77,7 +77,7 @@ class ContainerTest(unittest.TestCase): def test_number(self): container = Container(None, self.container_dict, has_been_inspected=True) - assert container.number == 7 + assert container.number == "092cd63296fdc446ad432d3905dd1fcbe12a2ba6b52" def test_name(self): container = Container.from_ps(None, @@ -88,7 +88,7 @@ class ContainerTest(unittest.TestCase): def test_name_without_project(self): self.container_dict['Name'] = "/composetest_web_7" container = Container(None, self.container_dict, has_been_inspected=True) - assert container.name_without_project == "web_7" + assert container.name_without_project == "web_092cd63296fd" def test_name_without_project_custom_container_name(self): self.container_dict['Name'] = "/custom_name_of_container" diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 791019a4e..ac234e624 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -41,6 +41,7 @@ from compose.service import parse_repository_tag from compose.service import Service from compose.service import ServiceNetworkMode from compose.service import warn_on_masked_volume +from compose.utils import generate_random_id as generate_id class ServiceTest(unittest.TestCase): @@ -81,8 +82,7 @@ class ServiceTest(unittest.TestCase): service = Service('db', self.mock_client, 'myproject', image='foo') assert [c.id for c in service.containers()] == ['1'] - assert service._next_container_number() == 2 - assert service.get_container(1).id == '1' + assert service.get_container().id == '1' def test_get_volumes_from_container(self): container_id = 'aabbccddee' @@ -164,7 +164,7 @@ class ServiceTest(unittest.TestCase): client=self.mock_client, mem_limit=1000000000, memswap_limit=2000000000) - service._get_container_create_options({'some': 'overrides'}, 1) + service._get_container_create_options({'some': 'overrides'}, generate_id()) assert self.mock_client.create_host_config.called assert self.mock_client.create_host_config.call_args[1]['mem_limit'] == 1000000000 @@ -173,10 +173,10 @@ class ServiceTest(unittest.TestCase): def test_self_reference_external_link(self): service = Service( name='foo', - external_links=['default_foo_1'] + external_links=['default_foo_bdfa3ed91e2c'] ) with pytest.raises(DependencyError): - service.get_container_name('foo', 1) + service.get_container_name('foo', 'bdfa3ed91e2c') def test_mem_reservation(self): self.mock_client.create_host_config.return_value = {} @@ -188,7 +188,7 @@ class ServiceTest(unittest.TestCase): client=self.mock_client, mem_reservation='512m' ) - service._get_container_create_options({'some': 'overrides'}, 1) + service._get_container_create_options({'some': 'overrides'}, generate_id()) assert self.mock_client.create_host_config.called is True assert self.mock_client.create_host_config.call_args[1]['mem_reservation'] == '512m' @@ -201,7 +201,7 @@ class ServiceTest(unittest.TestCase): hostname='name', client=self.mock_client, cgroup_parent='test') - service._get_container_create_options({'some': 'overrides'}, 1) + service._get_container_create_options({'some': 'overrides'}, generate_id()) assert self.mock_client.create_host_config.called assert self.mock_client.create_host_config.call_args[1]['cgroup_parent'] == 'test' @@ -218,7 +218,7 @@ class ServiceTest(unittest.TestCase): client=self.mock_client, log_driver='syslog', logging=logging) - service._get_container_create_options({'some': 'overrides'}, 1) + service._get_container_create_options({'some': 'overrides'}, generate_id()) assert self.mock_client.create_host_config.called assert self.mock_client.create_host_config.call_args[1]['log_config'] == { @@ -233,7 +233,7 @@ class ServiceTest(unittest.TestCase): image='foo', client=self.mock_client, stop_grace_period="1m35s") - opts = service._get_container_create_options({'image': 'foo'}, 1) + opts = service._get_container_create_options({'image': 'foo'}, generate_id()) assert opts['stop_timeout'] == 95 def test_split_domainname_none(self): @@ -242,7 +242,7 @@ class ServiceTest(unittest.TestCase): image='foo', hostname='name.domain.tld', client=self.mock_client) - opts = service._get_container_create_options({'image': 'foo'}, 1) + opts = service._get_container_create_options({'image': 'foo'}, generate_id()) assert opts['hostname'] == 'name.domain.tld', 'hostname' assert not ('domainname' in opts), 'domainname' @@ -253,7 +253,7 @@ class ServiceTest(unittest.TestCase): hostname='name.domain.tld', image='foo', client=self.mock_client) - opts = service._get_container_create_options({'image': 'foo'}, 1) + opts = service._get_container_create_options({'image': 'foo'}, generate_id()) assert opts['hostname'] == 'name', 'hostname' assert opts['domainname'] == 'domain.tld', 'domainname' @@ -265,7 +265,7 @@ class ServiceTest(unittest.TestCase): image='foo', domainname='domain.tld', client=self.mock_client) - opts = service._get_container_create_options({'image': 'foo'}, 1) + opts = service._get_container_create_options({'image': 'foo'}, generate_id()) assert opts['hostname'] == 'name', 'hostname' assert opts['domainname'] == 'domain.tld', 'domainname' @@ -277,7 +277,7 @@ class ServiceTest(unittest.TestCase): domainname='domain.tld', image='foo', client=self.mock_client) - opts = service._get_container_create_options({'image': 'foo'}, 1) + opts = service._get_container_create_options({'image': 'foo'}, generate_id()) assert opts['hostname'] == 'name.sub', 'hostname' assert opts['domainname'] == 'domain.tld', 'domainname' @@ -288,7 +288,7 @@ class ServiceTest(unittest.TestCase): use_networking=False, client=self.mock_client, ) - opts = service._get_container_create_options({'image': 'foo'}, 1) + opts = service._get_container_create_options({'image': 'foo'}, generate_id()) assert opts.get('hostname') is None def test_get_container_create_options_with_name_option(self): @@ -321,9 +321,8 @@ class ServiceTest(unittest.TestCase): prev_container.get.return_value = None opts = service._get_container_create_options( - {}, - 1, - previous_container=prev_container) + {}, generate_id(), previous_container=prev_container + ) assert service.options['labels'] == labels assert service.options['environment'] == environment @@ -358,7 +357,7 @@ class ServiceTest(unittest.TestCase): opts = service._get_container_create_options( {}, - 1, + generate_id(), previous_container=prev_container) assert opts['environment'] == ['affinity:container==ababab'] @@ -373,7 +372,7 @@ class ServiceTest(unittest.TestCase): opts = service._get_container_create_options( {}, - 1, + generate_id(), previous_container=prev_container) assert opts['environment'] == [] @@ -386,11 +385,11 @@ class ServiceTest(unittest.TestCase): @mock.patch('compose.service.Container', autospec=True) def test_get_container(self, mock_container_class): - container_dict = dict(Name='default_foo_2') + container_dict = dict(Name='default_foo_bdfa3ed91e2c') self.mock_client.containers.return_value = [container_dict] service = Service('foo', image='foo', client=self.mock_client) - container = service.get_container(number=2) + container = service.get_container(number="bdfa3ed91e2c") assert container == mock_container_class.from_ps.return_value mock_container_class.from_ps.assert_called_once_with( self.mock_client, container_dict) @@ -463,6 +462,7 @@ class ServiceTest(unittest.TestCase): @mock.patch('compose.service.Container', autospec=True) def test_recreate_container(self, _): mock_container = mock.create_autospec(Container) + mock_container.number = generate_id() service = Service('foo', client=self.mock_client, image='someimage') service.image = lambda: {'Id': 'abc123'} new_container = service.recreate_container(mock_container) @@ -476,6 +476,7 @@ class ServiceTest(unittest.TestCase): @mock.patch('compose.service.Container', autospec=True) def test_recreate_container_with_timeout(self, _): mock_container = mock.create_autospec(Container) + mock_container.number = generate_id() self.mock_client.inspect_image.return_value = {'Id': 'abc123'} service = Service('foo', client=self.mock_client, image='someimage') service.recreate_container(mock_container, timeout=1) @@ -711,9 +712,9 @@ class ServiceTest(unittest.TestCase): for api_version in set(API_VERSIONS.values()): self.mock_client.api_version = api_version - assert service._get_container_create_options({}, 1)['labels'][LABEL_CONFIG_HASH] == ( - config_hash - ) + assert service._get_container_create_options( + {}, generate_id() + )['labels'][LABEL_CONFIG_HASH] == config_hash def test_remove_image_none(self): web = Service('web', image='example', client=self.mock_client) @@ -971,7 +972,7 @@ class ServiceTest(unittest.TestCase): service = Service('foo', client=self.mock_client, environment=environment) - create_opts = service._get_container_create_options(override_options, 1) + create_opts = service._get_container_create_options(override_options, generate_id()) assert set(create_opts['environment']) == set(format_environment({ 'HTTP_PROXY': default_proxy_config['httpProxy'], 'http_proxy': default_proxy_config['httpProxy'], @@ -1296,7 +1297,7 @@ class ServiceVolumesTest(unittest.TestCase): service._get_container_create_options( override_options={}, - number=1, + number=generate_id(), ) assert set(self.mock_client.create_host_config.call_args[1]['binds']) == set([ @@ -1339,7 +1340,7 @@ class ServiceVolumesTest(unittest.TestCase): service._get_container_create_options( override_options={}, - number=1, + number=generate_id(), previous_container=Container(self.mock_client, {'Id': '123123123'}), )