From 8d816fc2f39ae0c2e95c758c78e1108e72c09446 Mon Sep 17 00:00:00 2001 From: Mazz Mosley Date: Wed, 28 Oct 2015 16:27:14 +0000 Subject: [PATCH 01/35] Add cross references for env/cli Signed-off-by: Mazz Mosley --- docs/reference/docker-compose.md | 15 +++++++++------ docs/reference/overview.md | 9 +++++++-- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/docs/reference/docker-compose.md b/docs/reference/docker-compose.md index 32fcbe706..8712072e5 100644 --- a/docs/reference/docker-compose.md +++ b/docs/reference/docker-compose.md @@ -87,15 +87,18 @@ relative to the current working directory. The `-f` flag is optional. If you don't provide this flag on the command line, Compose traverses the working directory and its subdirectories looking for a -`docker-compose.yml` and a `docker-compose.override.yml` file. You must supply -at least the `docker-compose.yml` file. If both files are present, Compose -combines the two files into a single configuration. The configuration in the -`docker-compose.override.yml` file is applied over and in addition to the values -in the `docker-compose.yml` file. +`docker-compose.yml` and a `docker-compose.override.yml` file. You must +supply at least the `docker-compose.yml` file. If both files are present, +Compose combines the two files into a single configuration. The configuration +in the `docker-compose.override.yml` file is applied over and in addition to +the values in the `docker-compose.yml` file. + +See also the `COMPOSE_FILE` [environment variable](overview.md#compose-file). Each configuration has a project name. If you supply a `-p` flag, you can specify a project name. If you don't specify the flag, Compose uses the current -directory name. +directory name. See also the `COMPOSE_PROJECT_NAME` [environment variable]( +overview.md#compose-project-name) ## Where to go next diff --git a/docs/reference/overview.md b/docs/reference/overview.md index 3f589a9de..8e3967b22 100644 --- a/docs/reference/overview.md +++ b/docs/reference/overview.md @@ -32,11 +32,16 @@ Docker command-line client. If you're using `docker-machine`, then the `eval "$( Sets the project name. This value is prepended along with the service name to the container container on start up. For example, if you project name is `myapp` and it includes two services `db` and `web` then compose starts containers named `myapp_db_1` and `myapp_web_1` respectively. -Setting this is optional. If you do not set this, the `COMPOSE_PROJECT_NAME` defaults to the `basename` of the current working directory. +Setting this is optional. If you do not set this, the `COMPOSE_PROJECT_NAME` +defaults to the `basename` of the project directory. See also the `-p` +[command-line option](docker-compose.md). ### COMPOSE\_FILE -Specify the file containing the compose configuration. If not provided, Compose looks for a file named `docker-compose.yml` in the current directory and then each parent directory in succession until a file by that name is found. +Specify the file containing the compose configuration. If not provided, +Compose looks for a file named `docker-compose.yml` in the current directory +and then each parent directory in succession until a file by that name is +found. See also the `-f` [command-line option](docker-compose.md). ### COMPOSE\_API\_VERSION From d28b2027b8f584c3492bdc6554b73e926a583356 Mon Sep 17 00:00:00 2001 From: Mazz Mosley Date: Wed, 28 Oct 2015 16:48:35 +0000 Subject: [PATCH 02/35] Clarify `dockerfile` requires `build` key Credit to @funkyfuture for the first PR addressing the clarification. https://github.com/docker/compose/pull/1767 Signed-off-by: Mazz Mosley --- docs/compose-file.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/compose-file.md b/docs/compose-file.md index d5aeaa3f2..51d1f5e1a 100644 --- a/docs/compose-file.md +++ b/docs/compose-file.md @@ -105,8 +105,10 @@ Custom DNS search domains. Can be a single value or a list. Alternate Dockerfile. -Compose will use an alternate file to build with. +Compose will use an alternate file to build with. A build path must also be +specified using the `build` key. + build: /path/to/build/dir dockerfile: Dockerfile-alternate Using `dockerfile` together with `image` is not allowed. Attempting to do so results in an error. From c42918ec7c736b16394deeb30a7724ed5d403a52 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Sun, 18 Oct 2015 00:40:51 +0530 Subject: [PATCH 03/35] Fix specifies_host_port() to handle port binding with host IP but no host port Signed-off-by: Viranch Mehta --- compose/service.py | 24 +++++++++++++-- tests/unit/service_test.py | 62 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/compose/service.py b/compose/service.py index cc4915992..3b05264bb 100644 --- a/compose/service.py +++ b/compose/service.py @@ -767,10 +767,28 @@ class Service(object): return self.options.get('container_name') def specifies_host_port(self): - for port in self.options.get('ports', []): - if ':' in str(port): + def has_host_port(binding): + _, external_bindings = split_port(binding) + + # there are no external bindings + if external_bindings is None: + return False + + # we only need to check the first binding from the range + external_binding = external_bindings[0] + + # non-tuple binding means there is a host port specified + if not isinstance(external_binding, tuple): return True - return False + + # extract actual host port from tuple of (host_ip, host_port) + _, host_port = external_binding + if host_port is not None: + return True + + return False + + return any(has_host_port(binding) for binding in self.options.get('ports', [])) def pull(self, ignore_pull_failures=False): if 'image' not in self.options: diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 311d5c95e..0cff98990 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -427,6 +427,68 @@ class ServiceTest(unittest.TestCase): } self.assertEqual(config_dict, expected) + def test_specifies_host_port_with_no_ports(self): + service = Service( + 'foo', + image='foo') + self.assertEqual(service.specifies_host_port(), False) + + def test_specifies_host_port_with_container_port(self): + service = Service( + 'foo', + image='foo', + ports=["2000"]) + self.assertEqual(service.specifies_host_port(), False) + + def test_specifies_host_port_with_host_port(self): + service = Service( + 'foo', + image='foo', + ports=["1000:2000"]) + self.assertEqual(service.specifies_host_port(), True) + + def test_specifies_host_port_with_host_ip_no_port(self): + service = Service( + 'foo', + image='foo', + ports=["127.0.0.1::2000"]) + self.assertEqual(service.specifies_host_port(), False) + + def test_specifies_host_port_with_host_ip_and_port(self): + service = Service( + 'foo', + image='foo', + ports=["127.0.0.1:1000:2000"]) + self.assertEqual(service.specifies_host_port(), True) + + def test_specifies_host_port_with_container_port_range(self): + service = Service( + 'foo', + image='foo', + ports=["2000-3000"]) + self.assertEqual(service.specifies_host_port(), False) + + def test_specifies_host_port_with_host_port_range(self): + service = Service( + 'foo', + image='foo', + ports=["1000-2000:2000-3000"]) + self.assertEqual(service.specifies_host_port(), True) + + def test_specifies_host_port_with_host_ip_no_port_range(self): + service = Service( + 'foo', + image='foo', + ports=["127.0.0.1::2000-3000"]) + self.assertEqual(service.specifies_host_port(), False) + + def test_specifies_host_port_with_host_ip_and_port_range(self): + service = Service( + 'foo', + image='foo', + ports=["127.0.0.1:1000-2000:2000-3000"]) + self.assertEqual(service.specifies_host_port(), True) + def test_get_links_with_networking(self): service = Service( 'foo', From 16a74f3797a9afb06a6f522f452e0426d873221f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 16 Nov 2015 12:55:35 -0500 Subject: [PATCH 04/35] Fix texttable dep. 0.8.2 was removed from pypi. Signed-off-by: Daniel Nephin --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 60327d728..659cb57f4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,5 +6,5 @@ enum34==1.0.4 jsonschema==2.5.1 requests==2.7.0 six==1.7.3 -texttable==0.8.2 +texttable==0.8.4 websocket-client==0.32.0 From 8f70c8cdeb28f82d9ccf4f060ccd9b44d002c502 Mon Sep 17 00:00:00 2001 From: Simon van der Veldt Date: Wed, 18 Nov 2015 21:38:58 +0100 Subject: [PATCH 05/35] run.sh script: Also pass DOCKER_TLS_VERIFY and DOCKER_CERT_PATH env vars to compose container Signed-off-by: Simon van der Veldt --- script/run.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/run.sh b/script/run.sh index a9b954774..9563b2e9c 100755 --- a/script/run.sh +++ b/script/run.sh @@ -26,7 +26,7 @@ fi if [ -S "$DOCKER_HOST" ]; then DOCKER_ADDR="-v $DOCKER_HOST:$DOCKER_HOST -e DOCKER_HOST" else - DOCKER_ADDR="-e DOCKER_HOST" + DOCKER_ADDR="-e DOCKER_HOST -e DOCKER_TLS_VERIFY -e DOCKER_CERT_PATH" fi From 0117148a360ccc1bc783e16435314bd6c69eaf11 Mon Sep 17 00:00:00 2001 From: Stefan Scherer Date: Fri, 13 Nov 2015 08:33:51 +0100 Subject: [PATCH 06/35] Use uname to build target name for different platforms Signed-off-by: Stefan Scherer --- script/build-linux-inner | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/build-linux-inner b/script/build-linux-inner index 01137ff24..47d5eb2e7 100755 --- a/script/build-linux-inner +++ b/script/build-linux-inner @@ -2,7 +2,7 @@ set -ex -TARGET=dist/docker-compose-Linux-x86_64 +TARGET=dist/docker-compose-$(uname -s)-$(uname -m) VENV=/code/.tox/py27 mkdir -p `pwd`/dist From 09f6a876cfc2c7cdbaba9383e0828d4435537bea Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 16 Nov 2015 12:35:26 -0500 Subject: [PATCH 07/35] Fixes #2398 - the build progress stream can contain empty json objects. Previously these empty objects would hit a bug in splitting objects causing it crash. With this fix the empty objects are returned properly. Signed-off-by: Daniel Nephin --- compose/utils.py | 12 ++++++------ tests/unit/utils_test.py | 28 ++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/compose/utils.py b/compose/utils.py index 2c6c4584d..a013035e9 100644 --- a/compose/utils.py +++ b/compose/utils.py @@ -102,7 +102,7 @@ def stream_as_text(stream): def line_splitter(buffer, separator=u'\n'): index = buffer.find(six.text_type(separator)) if index == -1: - return None, None + return None return buffer[:index + 1], buffer[index + 1:] @@ -120,11 +120,11 @@ def split_buffer(stream, splitter=None, decoder=lambda a: a): for data in stream_as_text(stream): buffered += data while True: - item, rest = splitter(buffered) - if not item: + buffer_split = splitter(buffered) + if buffer_split is None: break - buffered = rest + item, buffered = buffer_split yield item if buffered: @@ -140,7 +140,7 @@ def json_splitter(buffer): rest = buffer[json.decoder.WHITESPACE.match(buffer, index).end():] return obj, rest except ValueError: - return None, None + return None def json_stream(stream): @@ -148,7 +148,7 @@ def json_stream(stream): This handles streams which are inconsistently buffered (some entries may be newline delimited, and others are not). """ - return split_buffer(stream_as_text(stream), json_splitter, json_decoder.decode) + return split_buffer(stream, json_splitter, json_decoder.decode) def write_out_msg(stream, lines, msg_index, msg, status="done"): diff --git a/tests/unit/utils_test.py b/tests/unit/utils_test.py index e3d0bc00b..15999dde9 100644 --- a/tests/unit/utils_test.py +++ b/tests/unit/utils_test.py @@ -1,25 +1,21 @@ # encoding: utf-8 from __future__ import unicode_literals -from .. import unittest from compose import utils -class JsonSplitterTestCase(unittest.TestCase): +class TestJsonSplitter(object): def test_json_splitter_no_object(self): data = '{"foo": "bar' - self.assertEqual(utils.json_splitter(data), (None, None)) + assert utils.json_splitter(data) is None def test_json_splitter_with_object(self): data = '{"foo": "bar"}\n \n{"next": "obj"}' - self.assertEqual( - utils.json_splitter(data), - ({'foo': 'bar'}, '{"next": "obj"}') - ) + assert utils.json_splitter(data) == ({'foo': 'bar'}, '{"next": "obj"}') -class StreamAsTextTestCase(unittest.TestCase): +class TestStreamAsText(object): def test_stream_with_non_utf_unicode_character(self): stream = [b'\xed\xf3\xf3'] @@ -30,3 +26,19 @@ class StreamAsTextTestCase(unittest.TestCase): stream = ['ěĝ'.encode('utf-8')] output, = utils.stream_as_text(stream) assert output == 'ěĝ' + + +class TestJsonStream(object): + + def test_with_falsy_entries(self): + stream = [ + '{"one": "two"}\n{}\n', + "[1, 2, 3]\n[]\n", + ] + output = list(utils.json_stream(stream)) + assert output == [ + {'one': 'two'}, + {}, + [1, 2, 3], + [], + ] From 3a395892fc18bd097f767d6a420dc887f9e76e8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Seguin?= Date: Sat, 14 Nov 2015 12:19:57 +0100 Subject: [PATCH 08/35] Fix restart with stopped containers. Fixes #1814 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Seguin --- compose/service.py | 2 +- tests/acceptance/cli_test.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/compose/service.py b/compose/service.py index 3b05264bb..4f449d150 100644 --- a/compose/service.py +++ b/compose/service.py @@ -185,7 +185,7 @@ class Service(object): c.kill(**options) def restart(self, **options): - for c in self.containers(): + for c in self.containers(stopped=True): log.info("Restarting %s" % c.name) c.restart(**options) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 88a43d7f0..34a2d166e 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -597,6 +597,15 @@ class CLITestCase(DockerClientTestCase): started_at, ) + def test_restart_stopped_container(self): + service = self.project.get_service('simple') + container = service.create_container() + container.start() + container.kill() + self.assertEqual(len(service.containers(stopped=True)), 1) + self.dispatch(['restart', '-t', '1'], None) + self.assertEqual(len(service.containers(stopped=False)), 1) + def test_scale(self): project = self.project From e5a02d30524ac83fa0bc27e2697e1d2f69330658 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 13 Nov 2015 10:49:17 -0500 Subject: [PATCH 09/35] Fix extra warnings on masked volumes. Signed-off-by: Daniel Nephin --- compose/service.py | 5 ++++- tests/unit/service_test.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/compose/service.py b/compose/service.py index 4f449d150..b79fd9001 100644 --- a/compose/service.py +++ b/compose/service.py @@ -963,7 +963,10 @@ def warn_on_masked_volume(volumes_option, container_volumes, service): for volume in container_volumes) for volume in volumes_option: - if container_volumes.get(volume.internal) != volume.external: + if ( + volume.internal in container_volumes and + container_volumes.get(volume.internal) != volume.external + ): log.warn(( "Service \"{service}\" is using volume \"{volume}\" from the " "previous container. Host mapping \"{host_path}\" has no effect. " diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 0cff98990..808c391cd 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -26,6 +26,8 @@ from compose.service import parse_volume_spec from compose.service import Service from compose.service import ServiceNet from compose.service import VolumeFromSpec +from compose.service import VolumeSpec +from compose.service import warn_on_masked_volume class ServiceTest(unittest.TestCase): @@ -750,6 +752,39 @@ class ServiceVolumesTest(unittest.TestCase): ['/mnt/sda1/host/path:/data:rw'], ) + def test_warn_on_masked_volume_no_warning_when_no_container_volumes(self): + volumes_option = [VolumeSpec('/home/user', '/path', 'rw')] + container_volumes = [] + service = 'service_name' + + with mock.patch('compose.service.log') as mock_log: + warn_on_masked_volume(volumes_option, container_volumes, service) + + assert not mock_log.warn.called + + def test_warn_on_masked_volume_when_masked(self): + volumes_option = [VolumeSpec('/home/user', '/path', 'rw')] + container_volumes = [ + VolumeSpec('/var/lib/docker/path', '/path', 'rw'), + VolumeSpec('/var/lib/docker/path', '/other', 'rw'), + ] + service = 'service_name' + + with mock.patch('compose.service.log') as mock_log: + warn_on_masked_volume(volumes_option, container_volumes, service) + + mock_log.warn.called_once_with(mock.ANY) + + def test_warn_on_masked_no_warning_with_same_path(self): + volumes_option = [VolumeSpec('/home/user', '/path', 'rw')] + container_volumes = [VolumeSpec('/home/user', '/path', 'rw')] + service = 'service_name' + + with mock.patch('compose.service.log') as mock_log: + warn_on_masked_volume(volumes_option, container_volumes, service) + + assert not mock_log.warn.called + def test_create_with_special_volume_mode(self): self.mock_client.inspect_image.return_value = {'Id': 'imageid'} From be5b7b6f0e3b8dd330b93523b7a98e47e8d9a833 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 10 Nov 2015 18:52:21 -0500 Subject: [PATCH 10/35] Handle both SIGINT and SIGTERM for docker-compose up. Signed-off-by: Daniel Nephin --- compose/cli/main.py | 21 +++++++---- tests/acceptance/cli_test.py | 70 +++++++++++++++++++++++++++++------- tests/unit/cli/main_test.py | 8 ++--- 3 files changed, 76 insertions(+), 23 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index 806926d84..7b1e0aa35 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -658,17 +658,24 @@ def build_log_printer(containers, service_names, monochrome): def attach_to_logs(project, log_printer, service_names, timeout): print("Attaching to", list_containers(log_printer.containers)) - try: - log_printer.run() - finally: - def handler(signal, frame): - project.kill(service_names=service_names) - sys.exit(0) - signal.signal(signal.SIGINT, handler) + def force_shutdown(signal, frame): + project.kill(service_names=service_names) + sys.exit(2) + + def shutdown(signal, frame): + set_signal_handler(force_shutdown) print("Gracefully stopping... (press Ctrl+C again to force)") project.stop(service_names=service_names, timeout=timeout) + set_signal_handler(shutdown) + log_printer.run() + + +def set_signal_handler(handler): + signal.signal(signal.SIGINT, handler) + signal.signal(signal.SIGTERM, handler) + def list_containers(containers): return ", ".join(c.name for c in containers) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 34a2d166e..3fda83291 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -2,7 +2,9 @@ from __future__ import absolute_import import os import shlex +import signal import subprocess +import time from collections import namedtuple from operator import attrgetter @@ -20,6 +22,45 @@ BUILD_CACHE_TEXT = 'Using cache' BUILD_PULL_TEXT = 'Status: Image is up to date for busybox:latest' +def start_process(base_dir, options): + proc = subprocess.Popen( + ['docker-compose'] + options, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=base_dir) + print("Running process: %s" % proc.pid) + return proc + + +def wait_on_process(proc, returncode=0): + stdout, stderr = proc.communicate() + if proc.returncode != returncode: + print(stderr) + assert proc.returncode == returncode + return ProcessResult(stdout.decode('utf-8'), stderr.decode('utf-8')) + + +def wait_on_condition(condition, delay=0.1, timeout=5): + start_time = time.time() + while not condition(): + if time.time() - start_time > timeout: + raise AssertionError("Timeout: %s" % condition) + time.sleep(delay) + + +class ContainerCountCondition(object): + + def __init__(self, project, expected): + self.project = project + self.expected = expected + + def __call__(self): + return len(self.project.containers()) == self.expected + + def __str__(self): + return "waiting for counter count == %s" % self.expected + + class CLITestCase(DockerClientTestCase): def setUp(self): @@ -42,17 +83,8 @@ class CLITestCase(DockerClientTestCase): def dispatch(self, options, project_options=None, returncode=0): project_options = project_options or [] - proc = subprocess.Popen( - ['docker-compose'] + project_options + options, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - cwd=self.base_dir) - print("Running process: %s" % proc.pid) - stdout, stderr = proc.communicate() - if proc.returncode != returncode: - print(stderr) - assert proc.returncode == returncode - return ProcessResult(stdout.decode('utf-8'), stderr.decode('utf-8')) + proc = start_process(self.base_dir, project_options + options) + return wait_on_process(proc, returncode=returncode) def test_help(self): old_base_dir = self.base_dir @@ -291,7 +323,7 @@ class CLITestCase(DockerClientTestCase): returncode=1) def test_up_with_timeout(self): - self.dispatch(['up', '-d', '-t', '1'], None) + self.dispatch(['up', '-d', '-t', '1']) service = self.project.get_service('simple') another = self.project.get_service('another') self.assertEqual(len(service.containers()), 1) @@ -303,6 +335,20 @@ class CLITestCase(DockerClientTestCase): self.assertFalse(config['AttachStdout']) self.assertFalse(config['AttachStdin']) + def test_up_handles_sigint(self): + proc = start_process(self.base_dir, ['up', '-t', '2']) + wait_on_condition(ContainerCountCondition(self.project, 2)) + + os.kill(proc.pid, signal.SIGINT) + wait_on_condition(ContainerCountCondition(self.project, 0)) + + def test_up_handles_sigterm(self): + proc = start_process(self.base_dir, ['up', '-t', '2']) + wait_on_condition(ContainerCountCondition(self.project, 2)) + + os.kill(proc.pid, signal.SIGTERM) + wait_on_condition(ContainerCountCondition(self.project, 0)) + def test_run_service_without_links(self): self.base_dir = 'tests/fixtures/links-composefile' self.dispatch(['run', 'console', '/bin/true']) diff --git a/tests/unit/cli/main_test.py b/tests/unit/cli/main_test.py index ee837fcd4..db37ac1af 100644 --- a/tests/unit/cli/main_test.py +++ b/tests/unit/cli/main_test.py @@ -57,11 +57,11 @@ class CLIMainTestCase(unittest.TestCase): with mock.patch('compose.cli.main.signal', autospec=True) as mock_signal: attach_to_logs(project, log_printer, service_names, timeout) - mock_signal.signal.assert_called_once_with(mock_signal.SIGINT, mock.ANY) + assert mock_signal.signal.mock_calls == [ + mock.call(mock_signal.SIGINT, mock.ANY), + mock.call(mock_signal.SIGTERM, mock.ANY), + ] log_printer.run.assert_called_once_with() - project.stop.assert_called_once_with( - service_names=service_names, - timeout=timeout) class SetupConsoleHandlerTestCase(unittest.TestCase): From 83760d0e9ee04ace2fa4da2efa13a3a933b62cd2 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 10 Nov 2015 19:43:05 -0500 Subject: [PATCH 11/35] Handle both SIGINT and SIGTERM for docker-compose run. Signed-off-by: Daniel Nephin --- compose/cli/main.py | 99 ++++++++++++++++++++---------------- tests/acceptance/cli_test.py | 47 +++++++++++++++++ 2 files changed, 102 insertions(+), 44 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index 7b1e0aa35..9fef8d041 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -368,7 +368,6 @@ class TopLevelCommand(DocoptCommand): allocates a TTY. """ service = project.get_service(options['SERVICE']) - detach = options['-d'] if IS_WINDOWS_PLATFORM and not detach: @@ -380,22 +379,6 @@ class TopLevelCommand(DocoptCommand): if options['--allow-insecure-ssl']: log.warn(INSECURE_SSL_WARNING) - if not options['--no-deps']: - deps = service.get_linked_service_names() - - if len(deps) > 0: - project.up( - service_names=deps, - start_deps=True, - strategy=ConvergenceStrategy.never, - ) - elif project.use_networking: - project.ensure_network_exists() - - tty = True - if detach or options['-T'] or not sys.stdin.isatty(): - tty = False - if options['COMMAND']: command = [options['COMMAND']] + options['ARGS'] else: @@ -403,7 +386,7 @@ class TopLevelCommand(DocoptCommand): container_options = { 'command': command, - 'tty': tty, + 'tty': not (detach or options['-T'] or not sys.stdin.isatty()), 'stdin_open': not detach, 'detach': detach, } @@ -435,31 +418,7 @@ class TopLevelCommand(DocoptCommand): if options['--name']: container_options['name'] = options['--name'] - try: - container = service.create_container( - quiet=True, - one_off=True, - **container_options - ) - except APIError as e: - legacy.check_for_legacy_containers( - project.client, - project.name, - [service.name], - allow_one_off=False, - ) - - raise e - - if detach: - container.start() - print(container.name) - else: - dockerpty.start(project.client, container.id, interactive=not options['-T']) - exit_code = container.wait() - if options['--rm']: - project.client.remove_container(container.id) - sys.exit(exit_code) + run_one_off_container(container_options, project, service, options) def scale(self, project, options): """ @@ -647,6 +606,58 @@ def convergence_strategy_from_opts(options): return ConvergenceStrategy.changed +def run_one_off_container(container_options, project, service, options): + if not options['--no-deps']: + deps = service.get_linked_service_names() + if deps: + project.up( + service_names=deps, + start_deps=True, + strategy=ConvergenceStrategy.never) + + if project.use_networking: + project.ensure_network_exists() + + try: + container = service.create_container( + quiet=True, + one_off=True, + **container_options) + except APIError: + legacy.check_for_legacy_containers( + project.client, + project.name, + [service.name], + allow_one_off=False) + raise + + if options['-d']: + container.start() + print(container.name) + return + + def remove_container(force=False): + if options['--rm']: + project.client.remove_container(container.id, force=True) + + def force_shutdown(signal, frame): + project.client.kill(container.id) + remove_container(force=True) + sys.exit(2) + + def shutdown(signal, frame): + set_signal_handler(force_shutdown) + project.client.stop(container.id) + remove_container() + sys.exit(1) + + set_signal_handler(shutdown) + dockerpty.start(project.client, container.id, interactive=not options['-T']) + exit_code = container.wait() + remove_container() + sys.exit(exit_code) + + def build_log_printer(containers, service_names, monochrome): if service_names: containers = [ @@ -657,7 +668,6 @@ def build_log_printer(containers, service_names, monochrome): def attach_to_logs(project, log_printer, service_names, timeout): - print("Attaching to", list_containers(log_printer.containers)) def force_shutdown(signal, frame): project.kill(service_names=service_names) @@ -668,6 +678,7 @@ def attach_to_logs(project, log_printer, service_names, timeout): print("Gracefully stopping... (press Ctrl+C again to force)") project.stop(service_names=service_names, timeout=timeout) + print("Attaching to", list_containers(log_printer.containers)) set_signal_handler(shutdown) log_printer.run() diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 3fda83291..7ca6e8194 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -8,6 +8,8 @@ import time from collections import namedtuple from operator import attrgetter +from docker import errors + from .. import mock from compose.cli.command import get_project from compose.cli.docker_client import docker_client @@ -61,6 +63,25 @@ class ContainerCountCondition(object): return "waiting for counter count == %s" % self.expected +class ContainerStateCondition(object): + + def __init__(self, client, name, running): + self.client = client + self.name = name + self.running = running + + # State.Running == true + def __call__(self): + try: + container = self.client.inspect_container(self.name) + return container['State']['Running'] == self.running + except errors.APIError: + return False + + def __str__(self): + return "waiting for container to have state %s" % self.expected + + class CLITestCase(DockerClientTestCase): def setUp(self): @@ -554,6 +575,32 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(len(networks), 1) self.assertEqual(container.human_readable_command, u'true') + def test_run_handles_sigint(self): + proc = start_process(self.base_dir, ['run', '-T', 'simple', 'top']) + wait_on_condition(ContainerStateCondition( + self.project.client, + 'simplecomposefile_simple_run_1', + running=True)) + + os.kill(proc.pid, signal.SIGINT) + wait_on_condition(ContainerStateCondition( + self.project.client, + 'simplecomposefile_simple_run_1', + running=False)) + + def test_run_handles_sigterm(self): + proc = start_process(self.base_dir, ['run', '-T', 'simple', 'top']) + wait_on_condition(ContainerStateCondition( + self.project.client, + 'simplecomposefile_simple_run_1', + running=True)) + + os.kill(proc.pid, signal.SIGTERM) + wait_on_condition(ContainerStateCondition( + self.project.client, + 'simplecomposefile_simple_run_1', + running=False)) + def test_rm(self): service = self.project.get_service('simple') service.create_container() From 8fb6fb7b19467e20189c710ebd7b65ec071d7adc Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 18 Nov 2015 14:51:01 -0500 Subject: [PATCH 12/35] Fix env_file and environment when used with extends. Signed-off-by: Daniel Nephin --- compose/config/config.py | 22 +++++++-------- tests/integration/testcases.py | 3 +- tests/unit/config/config_test.py | 48 ++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 201266208..fa214767b 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -257,16 +257,11 @@ class ServiceExtendsResolver(object): def run(self): self.detect_cycle() - service_dict = dict(self.service_config.config) - env = resolve_environment(self.working_dir, self.service_config.config) - if env: - service_dict['environment'] = env - service_dict.pop('env_file', None) - - if 'extends' in service_dict: + if 'extends' in self.service_config.config: service_dict = self.resolve_extends(*self.validate_and_construct_extends()) + return self.service_config._replace(config=service_dict) - return self.service_config._replace(config=service_dict) + return self.service_config def validate_and_construct_extends(self): extends = self.service_config.config['extends'] @@ -316,16 +311,15 @@ class ServiceExtendsResolver(object): return filename -def resolve_environment(working_dir, service_dict): +def resolve_environment(service_config): """Unpack any environment variables from an env_file, if set. Interpolate environment values if set. """ - if 'environment' not in service_dict and 'env_file' not in service_dict: - return {} + service_dict = service_config.config env = {} if 'env_file' in service_dict: - for env_file in get_env_files(working_dir, service_dict): + for env_file in get_env_files(service_config.working_dir, service_dict): env.update(env_vars_from_file(env_file)) env.update(parse_environment(service_dict.get('environment'))) @@ -362,6 +356,10 @@ def process_service(service_config): working_dir = service_config.working_dir service_dict = dict(service_config.config) + if 'environment' in service_dict or 'env_file' in service_dict: + service_dict['environment'] = resolve_environment(service_config) + service_dict.pop('env_file', None) + if 'volumes' in service_dict and service_dict.get('volume_driver') is None: service_dict['volumes'] = resolve_volume_paths(working_dir, service_dict) diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index d63f05916..de2d1a701 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -46,7 +46,8 @@ class DockerClientTestCase(unittest.TestCase): service_config = ServiceConfig('.', None, name, kwargs) options = process_service(service_config) - options['environment'] = resolve_environment('.', kwargs) + options['environment'] = resolve_environment( + service_config._replace(config=options)) labels = options.setdefault('labels', {}) labels['com.docker.compose.test-name'] = self.id() diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 3038af80d..c69e34306 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -1317,6 +1317,54 @@ class ExtendsTest(unittest.TestCase): }, ])) + def test_extends_with_environment_and_env_files(self): + tmpdir = py.test.ensuretemp('test_extends_with_environment') + self.addCleanup(tmpdir.remove) + commondir = tmpdir.mkdir('common') + commondir.join('base.yml').write(""" + app: + image: 'example/app' + env_file: + - 'envs' + environment: + - SECRET + """) + tmpdir.join('docker-compose.yml').write(""" + ext: + extends: + file: common/base.yml + service: app + env_file: + - 'envs' + environment: + - THING + """) + commondir.join('envs').write(""" + COMMON_ENV_FILE=1 + """) + tmpdir.join('envs').write(""" + FROM_ENV_FILE=1 + """) + + expected = [ + { + 'name': 'ext', + 'image': 'example/app', + 'environment': { + 'SECRET': 'secret', + 'FROM_ENV_FILE': '1', + 'COMMON_ENV_FILE': '1', + 'THING': 'thing', + }, + }, + ] + with mock.patch.dict(os.environ): + os.environ['SECRET'] = 'secret' + os.environ['THING'] = 'thing' + config = load_from_filename(str(tmpdir.join('docker-compose.yml'))) + + assert config == expected + @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason='paths use slash') class ExpandPathTest(unittest.TestCase): From 9ce402495160c126ea0ec43c24b2c2f03ca56f2f Mon Sep 17 00:00:00 2001 From: Brandon Burton Date: Fri, 20 Nov 2015 16:02:37 -0800 Subject: [PATCH 13/35] Fixing matrix include so `os: linux` goes to trusty Signed-off-by: Brandon Burton --- .travis.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3310e2ad9..3bb365a14 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,16 +2,14 @@ sudo: required language: python -services: - - docker - matrix: include: - os: linux + services: + - docker - os: osx language: generic - install: ./script/travis/install script: From 210a14cf28fa5e537cf81ffd23d5a29f86a1e298 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 19 Nov 2015 12:55:03 -0500 Subject: [PATCH 14/35] Add note about required pip version. Signed-off-by: Daniel Nephin --- docs/install.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/install.md b/docs/install.md index f8c9db638..5bbd6e595 100644 --- a/docs/install.md +++ b/docs/install.md @@ -70,6 +70,7 @@ to get started. $ pip install docker-compose +> **Note:** pip version 6.0 or greater is required ### Install as a container From 844e2c3d268e2af8cdce556b1b53f8658c6e4881 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 19 Nov 2015 14:52:50 -0500 Subject: [PATCH 15/35] Fix use case link in readme. Signed-off-by: Daniel Nephin --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4c967aebc..b8a3f4659 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ see [the list of features](docs/index.md#features). Compose is great for development, testing, and staging environments, as well as CI workflows. You can learn more about each case in -[Common Use Cases](#common-use-cases). +[Common Use Cases](docs/index.md#common-use-cases). Using Compose is basically a three-step process. From a264470cc05bd76fee567294e8801dfe7dadcdcf Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 20 Nov 2015 17:51:36 -0500 Subject: [PATCH 16/35] Make sure we always have the latest busybox image, so that build --pull tests don't flake. Signed-off-by: Daniel Nephin --- tests/acceptance/cli_test.py | 5 +++++ tests/integration/testcases.py | 6 +----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 7ca6e8194..88ec45738 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -15,6 +15,7 @@ from compose.cli.command import get_project from compose.cli.docker_client import docker_client from compose.container import Container from tests.integration.testcases import DockerClientTestCase +from tests.integration.testcases import pull_busybox ProcessResult = namedtuple('ProcessResult', 'stdout stderr') @@ -184,6 +185,8 @@ class CLITestCase(DockerClientTestCase): assert BUILD_PULL_TEXT not in result.stdout def test_build_pull(self): + # Make sure we have the latest busybox already + pull_busybox(self.client) self.base_dir = 'tests/fixtures/simple-dockerfile' self.dispatch(['build', 'simple'], None) @@ -192,6 +195,8 @@ class CLITestCase(DockerClientTestCase): assert BUILD_PULL_TEXT in result.stdout def test_build_no_cache_pull(self): + # Make sure we have the latest busybox already + pull_busybox(self.client) self.base_dir = 'tests/fixtures/simple-dockerfile' self.dispatch(['build', 'simple']) diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index de2d1a701..2c5ca9fdd 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -1,7 +1,6 @@ from __future__ import absolute_import from __future__ import unicode_literals -from docker import errors from docker.utils import version_lt from pytest import skip @@ -16,10 +15,7 @@ from compose.service import Service def pull_busybox(client): - try: - client.inspect_image('busybox:latest') - except errors.APIError: - client.pull('busybox:latest', stream=False) + client.pull('busybox:latest', stream=False) class DockerClientTestCase(unittest.TestCase): From 3b6cc7a7bbf6ce79fff98ca43b10778913a059dd Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 24 Nov 2015 10:40:36 -0500 Subject: [PATCH 17/35] Add missing assert and autospec. Signed-off-by: Daniel Nephin --- tests/unit/service_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 808c391cd..85d1479d5 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -757,7 +757,7 @@ class ServiceVolumesTest(unittest.TestCase): container_volumes = [] service = 'service_name' - with mock.patch('compose.service.log') as mock_log: + with mock.patch('compose.service.log', autospec=True) as mock_log: warn_on_masked_volume(volumes_option, container_volumes, service) assert not mock_log.warn.called @@ -770,17 +770,17 @@ class ServiceVolumesTest(unittest.TestCase): ] service = 'service_name' - with mock.patch('compose.service.log') as mock_log: + with mock.patch('compose.service.log', autospec=True) as mock_log: warn_on_masked_volume(volumes_option, container_volumes, service) - mock_log.warn.called_once_with(mock.ANY) + mock_log.warn.assert_called_once_with(mock.ANY) def test_warn_on_masked_no_warning_with_same_path(self): volumes_option = [VolumeSpec('/home/user', '/path', 'rw')] container_volumes = [VolumeSpec('/home/user', '/path', 'rw')] service = 'service_name' - with mock.patch('compose.service.log') as mock_log: + with mock.patch('compose.service.log', autospec=True) as mock_log: warn_on_masked_volume(volumes_option, container_volumes, service) assert not mock_log.warn.called From bea2072b95a2fe679c2d33a2d6c6672dacc4a52f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 12 Nov 2015 17:29:58 -0500 Subject: [PATCH 18/35] Add the git sha to version output Signed-off-by: Daniel Nephin --- .gitignore | 1 + Dockerfile.run | 2 +- MANIFEST.in | 1 + compose/cli/command.py | 4 ++-- compose/cli/utils.py | 39 +++++++++++++++++++++++++++---------- docker-compose.spec | 24 ++++++++++++++++++----- script/build-image | 1 + script/build-linux | 1 + script/build-linux-inner | 1 + script/build-osx | 1 + script/build-windows.ps1 | 2 ++ script/release/push-release | 1 + script/write-git-sha | 7 +++++++ 13 files changed, 67 insertions(+), 18 deletions(-) create mode 100755 script/write-git-sha diff --git a/.gitignore b/.gitignore index 83a08a0e6..da7282797 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ /docs/_site /venv README.rst +compose/GITSHA diff --git a/Dockerfile.run b/Dockerfile.run index 9f3745fef..792077ad7 100644 --- a/Dockerfile.run +++ b/Dockerfile.run @@ -8,6 +8,6 @@ COPY requirements.txt /code/requirements.txt RUN pip install -r /code/requirements.txt ADD dist/docker-compose-release.tar.gz /code/docker-compose -RUN pip install /code/docker-compose/docker-compose-* +RUN pip install --no-deps /code/docker-compose/docker-compose-* ENTRYPOINT ["/usr/bin/docker-compose"] diff --git a/MANIFEST.in b/MANIFEST.in index 0342e35be..8c6f932ba 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -7,6 +7,7 @@ include *.md exclude README.md include README.rst include compose/config/*.json +include compose/GITSHA recursive-include contrib/completion * recursive-include tests * global-exclude *.pyc diff --git a/compose/cli/command.py b/compose/cli/command.py index 525217ee7..6094b5305 100644 --- a/compose/cli/command.py +++ b/compose/cli/command.py @@ -12,12 +12,12 @@ from requests.exceptions import SSLError from . import errors from . import verbose_proxy -from .. import __version__ from .. import config from ..project import Project from ..service import ConfigError from .docker_client import docker_client from .utils import call_silently +from .utils import get_version_info from .utils import is_mac from .utils import is_ubuntu @@ -71,7 +71,7 @@ def get_client(verbose=False, version=None): client = docker_client(version=version) if verbose: version_info = six.iteritems(client.version()) - log.info("Compose version %s", __version__) + log.info(get_version_info('full')) log.info("Docker base_url: %s", client.base_url) log.info("Docker version: %s", ", ".join("%s=%s" % item for item in version_info)) diff --git a/compose/cli/utils.py b/compose/cli/utils.py index 07510e2f3..dd859edc4 100644 --- a/compose/cli/utils.py +++ b/compose/cli/utils.py @@ -7,10 +7,10 @@ import platform import ssl import subprocess -from docker import version as docker_py_version +import docker from six.moves import input -from .. import __version__ +import compose def yesno(prompt, default=None): @@ -57,13 +57,32 @@ def is_ubuntu(): def get_version_info(scope): - versioninfo = 'docker-compose version: %s' % __version__ + versioninfo = 'docker-compose version {}, build {}'.format( + compose.__version__, + get_build_version()) + if scope == 'compose': return versioninfo - elif scope == 'full': - return versioninfo + '\n' \ - + "docker-py version: %s\n" % docker_py_version \ - + "%s version: %s\n" % (platform.python_implementation(), platform.python_version()) \ - + "OpenSSL version: %s" % ssl.OPENSSL_VERSION - else: - raise RuntimeError('passed unallowed value to `cli.utils.get_version_info`') + if scope == 'full': + return ( + "{}\n" + "docker-py version: {}\n" + "{} version: {}\n" + "OpenSSL version: {}" + ).format( + versioninfo, + docker.version, + platform.python_implementation(), + platform.python_version(), + ssl.OPENSSL_VERSION) + + raise ValueError("{} is not a valid version scope".format(scope)) + + +def get_build_version(): + filename = os.path.join(os.path.dirname(compose.__file__), 'GITSHA') + if not os.path.exists(filename): + return 'unknown' + + with open(filename) as fh: + return fh.read().strip() diff --git a/docker-compose.spec b/docker-compose.spec index 678fc1323..24d03e05b 100644 --- a/docker-compose.spec +++ b/docker-compose.spec @@ -9,18 +9,32 @@ a = Analysis(['bin/docker-compose'], runtime_hooks=None, cipher=block_cipher) -pyz = PYZ(a.pure, - cipher=block_cipher) +pyz = PYZ(a.pure, cipher=block_cipher) exe = EXE(pyz, a.scripts, a.binaries, a.zipfiles, a.datas, - [('compose/config/fields_schema.json', 'compose/config/fields_schema.json', 'DATA')], - [('compose/config/service_schema.json', 'compose/config/service_schema.json', 'DATA')], + [ + ( + 'compose/config/fields_schema.json', + 'compose/config/fields_schema.json', + 'DATA' + ), + ( + 'compose/config/service_schema.json', + 'compose/config/service_schema.json', + 'DATA' + ), + ( + 'compose/GITSHA', + 'compose/GITSHA', + 'DATA' + ) + ], name='docker-compose', debug=False, strip=None, upx=True, - console=True ) + console=True) diff --git a/script/build-image b/script/build-image index 3ac9729b4..897335054 100755 --- a/script/build-image +++ b/script/build-image @@ -10,6 +10,7 @@ fi TAG=$1 VERSION="$(python setup.py --version)" +./script/write-git-sha python setup.py sdist cp dist/docker-compose-$VERSION.tar.gz dist/docker-compose-release.tar.gz docker build -t docker/compose:$TAG -f Dockerfile.run . diff --git a/script/build-linux b/script/build-linux index ade18bc53..47fb45e17 100755 --- a/script/build-linux +++ b/script/build-linux @@ -9,4 +9,5 @@ docker build -t "$TAG" . | tail -n 200 docker run \ --rm --entrypoint="script/build-linux-inner" \ -v $(pwd)/dist:/code/dist \ + -v $(pwd)/.git:/code/.git \ "$TAG" diff --git a/script/build-linux-inner b/script/build-linux-inner index 47d5eb2e7..9bf7c95d9 100755 --- a/script/build-linux-inner +++ b/script/build-linux-inner @@ -9,6 +9,7 @@ mkdir -p `pwd`/dist chmod 777 `pwd`/dist $VENV/bin/pip install -q -r requirements-build.txt +./script/write-git-sha su -c "$VENV/bin/pyinstaller docker-compose.spec" user mv dist/docker-compose $TARGET $TARGET version diff --git a/script/build-osx b/script/build-osx index 042964e4b..168fd4309 100755 --- a/script/build-osx +++ b/script/build-osx @@ -9,6 +9,7 @@ virtualenv -p /usr/local/bin/python venv venv/bin/pip install -r requirements.txt venv/bin/pip install -r requirements-build.txt venv/bin/pip install --no-deps . +./script/write-git-sha venv/bin/pyinstaller docker-compose.spec mv dist/docker-compose dist/docker-compose-Darwin-x86_64 dist/docker-compose-Darwin-x86_64 version diff --git a/script/build-windows.ps1 b/script/build-windows.ps1 index 42a4a501c..28011b1db 100644 --- a/script/build-windows.ps1 +++ b/script/build-windows.ps1 @@ -47,6 +47,8 @@ virtualenv .\venv .\venv\Scripts\pip install --no-deps . .\venv\Scripts\pip install --allow-external pyinstaller -r requirements-build.txt +git rev-parse --short HEAD | out-file -encoding ASCII compose\GITSHA + # Build binary # pyinstaller has lots of warnings, so we need to run with ErrorAction = Continue $ErrorActionPreference = "Continue" diff --git a/script/release/push-release b/script/release/push-release index ccdf24960..b754d40f0 100755 --- a/script/release/push-release +++ b/script/release/push-release @@ -57,6 +57,7 @@ docker push docker/compose:$VERSION echo "Uploading sdist to pypi" pandoc -f markdown -t rst README.md -o README.rst sed -i -e 's/logo.png?raw=true/https:\/\/github.com\/docker\/compose\/raw\/master\/logo.png?raw=true/' README.rst +./script/write-git-sha python setup.py sdist if [ "$(command -v twine 2> /dev/null)" ]; then twine upload ./dist/docker-compose-${VERSION}.tar.gz diff --git a/script/write-git-sha b/script/write-git-sha new file mode 100755 index 000000000..d16743c6f --- /dev/null +++ b/script/write-git-sha @@ -0,0 +1,7 @@ +#!/bin/bash +# +# Write the current commit sha to the file GITSHA. This file is included in +# packaging so that `docker-compose version` can include the git sha. +# +set -e +git rev-parse --short HEAD > compose/GITSHA From 7e21b05f057911c11c447a463e5ab3e392a16640 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 13 Nov 2015 17:39:02 -0500 Subject: [PATCH 19/35] Remove project name validation project name is already normalized to a valid name before creating a service. Signed-off-by: Daniel Nephin --- compose/service.py | 4 ---- tests/unit/service_test.py | 5 ----- 2 files changed, 9 deletions(-) diff --git a/compose/service.py b/compose/service.py index b79fd9001..6fe0058fb 100644 --- a/compose/service.py +++ b/compose/service.py @@ -18,7 +18,6 @@ from docker.utils.ports import split_port from . import __version__ from .config import DOCKER_CONFIG_KEYS from .config import merge_environment -from .config.validation import VALID_NAME_CHARS from .const import DEFAULT_TIMEOUT from .const import IS_WINDOWS_PLATFORM from .const import LABEL_CONFIG_HASH @@ -119,9 +118,6 @@ class Service(object): net=None, **options ): - if not re.match('^%s+$' % VALID_NAME_CHARS, project): - raise ConfigError('Invalid project name "%s" - only %s are allowed' % (project, VALID_NAME_CHARS)) - self.name = name self.client = client self.project = project diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 85d1479d5..98da5f186 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -35,11 +35,6 @@ class ServiceTest(unittest.TestCase): def setUp(self): self.mock_client = mock.create_autospec(docker.Client) - def test_project_validation(self): - self.assertRaises(ConfigError, lambda: Service(name='foo', project='>', image='foo')) - - Service(name='foo', project='bar.bar__', image='foo') - def test_containers(self): service = Service('db', self.mock_client, 'myproject', image='foo') self.mock_client.containers.return_value = [] From e549875e896ac49f203463f5a2997b2b0297e00e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 13 Nov 2015 18:20:09 -0500 Subject: [PATCH 20/35] Move parsing of volumes_from to the last step of config parsing. Includes creating a new compose.config.types module for all the domain objects. Signed-off-by: Daniel Nephin --- compose/config/config.py | 19 +++++++++++++++++++ compose/config/types.py | 28 ++++++++++++++++++++++++++++ compose/project.py | 18 ++++++------------ compose/service.py | 19 +------------------ tests/integration/project_test.py | 2 +- tests/integration/service_test.py | 2 +- tests/unit/project_test.py | 23 +++++++++++++---------- tests/unit/service_test.py | 1 + tests/unit/sort_service_test.py | 7 ++++--- 9 files changed, 74 insertions(+), 45 deletions(-) create mode 100644 compose/config/types.py diff --git a/compose/config/config.py b/compose/config/config.py index fa214767b..b21e639ff 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -1,3 +1,5 @@ +from __future__ import absolute_import + import codecs import logging import os @@ -11,6 +13,7 @@ from .errors import CircularReference from .errors import ComposeFileNotFound from .errors import ConfigurationError from .interpolation import interpolate_environment_variables +from .types import VolumeFromSpec from .validation import validate_against_fields_schema from .validation import validate_against_service_schema from .validation import validate_extends_file_path @@ -197,8 +200,12 @@ def load(config_details): service_dict) resolver = ServiceExtendsResolver(service_config) service_dict = process_service(resolver.run()) + + # TODO: move to validate_service() validate_against_service_schema(service_dict, service_config.name) validate_paths(service_dict) + + service_dict = finalize_service(service_config._replace(config=service_dict)) service_dict['name'] = service_config.name return service_dict @@ -352,6 +359,7 @@ def validate_ulimits(ulimit_config): "than 'hard' value".format(ulimit_config)) +# TODO: rename to normalize_service def process_service(service_config): working_dir = service_config.working_dir service_dict = dict(service_config.config) @@ -369,12 +377,23 @@ def process_service(service_config): if 'labels' in service_dict: service_dict['labels'] = parse_labels(service_dict['labels']) + # TODO: move to a validate_service() if 'ulimits' in service_dict: validate_ulimits(service_dict['ulimits']) return service_dict +def finalize_service(service_config): + service_dict = dict(service_config.config) + + if 'volumes_from' in service_dict: + service_dict['volumes_from'] = [ + VolumeFromSpec.parse(vf) for vf in service_dict['volumes_from']] + + return service_dict + + def merge_service_dicts_from_files(base, override): """When merging services from multiple files we need to merge the `extends` field. This is not handled by `merge_service_dicts()` which is used to diff --git a/compose/config/types.py b/compose/config/types.py new file mode 100644 index 000000000..73bfd4184 --- /dev/null +++ b/compose/config/types.py @@ -0,0 +1,28 @@ +""" +Types for objects parsed from the configuration. +""" +from __future__ import absolute_import +from __future__ import unicode_literals + +from collections import namedtuple + +from compose.config.errors import ConfigurationError + + +class VolumeFromSpec(namedtuple('_VolumeFromSpec', 'source mode')): + + @classmethod + def parse(cls, volume_from_config): + parts = volume_from_config.split(':') + if len(parts) > 2: + raise ConfigurationError( + "volume_from {} has incorrect format, should be " + "service[:mode]".format(volume_from_config)) + + if len(parts) == 1: + source = parts[0] + mode = 'rw' + else: + source, mode = parts + + return cls(source, mode) diff --git a/compose/project.py b/compose/project.py index 41af86261..69f084752 100644 --- a/compose/project.py +++ b/compose/project.py @@ -18,10 +18,8 @@ from .legacy import check_for_legacy_containers from .service import ContainerNet from .service import ConvergenceStrategy from .service import Net -from .service import parse_volume_from_spec from .service import Service from .service import ServiceNet -from .service import VolumeFromSpec from .utils import parallel_execute @@ -38,10 +36,7 @@ def sort_service_dicts(services): return [link.split(':')[0] for link in links] def get_service_names_from_volumes_from(volumes_from): - return [ - parse_volume_from_spec(volume_from).source - for volume_from in volumes_from - ] + return [volume_from.source for volume_from in volumes_from] def get_service_dependents(service_dict, services): name = service_dict['name'] @@ -192,16 +187,15 @@ class Project(object): def get_volumes_from(self, service_dict): volumes_from = [] if 'volumes_from' in service_dict: - for volume_from_config in service_dict.get('volumes_from', []): - volume_from_spec = parse_volume_from_spec(volume_from_config) + for volume_from_spec in service_dict.get('volumes_from', []): # Get service try: - service_name = self.get_service(volume_from_spec.source) - volume_from_spec = VolumeFromSpec(service_name, volume_from_spec.mode) + service = self.get_service(volume_from_spec.source) + volume_from_spec = volume_from_spec._replace(source=service) except NoSuchService: try: - container_name = Container.from_id(self.client, volume_from_spec.source) - volume_from_spec = VolumeFromSpec(container_name, volume_from_spec.mode) + container = Container.from_id(self.client, volume_from_spec.source) + volume_from_spec = volume_from_spec._replace(source=container) except APIError: raise ConfigurationError( 'Service "%s" mounts volumes from "%s", which is ' diff --git a/compose/service.py b/compose/service.py index 6fe0058fb..3d4103777 100644 --- a/compose/service.py +++ b/compose/service.py @@ -67,6 +67,7 @@ class BuildError(Exception): self.reason = reason +# TODO: remove class ConfigError(ValueError): pass @@ -83,9 +84,6 @@ class NoSuchImageError(Exception): VolumeSpec = namedtuple('VolumeSpec', 'external internal mode') -VolumeFromSpec = namedtuple('VolumeFromSpec', 'source mode') - - ServiceName = namedtuple('ServiceName', 'project service number') @@ -1044,21 +1042,6 @@ def build_volume_from(volume_from_spec): return ["{}:{}".format(volume_from_spec.source.id, volume_from_spec.mode)] -def parse_volume_from_spec(volume_from_config): - parts = volume_from_config.split(':') - if len(parts) > 2: - raise ConfigError("Volume %s has incorrect format, should be " - "external:internal[:mode]" % volume_from_config) - - if len(parts) == 1: - source = parts[0] - mode = 'rw' - else: - source, mode = parts - - return VolumeFromSpec(source, mode) - - # Labels diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 2ce319005..d65d7ef0c 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -3,12 +3,12 @@ from __future__ import unicode_literals from .testcases import DockerClientTestCase from compose.cli.docker_client import docker_client from compose.config import config +from compose.config.types import VolumeFromSpec from compose.const import LABEL_PROJECT from compose.container import Container from compose.project import Project from compose.service import ConvergenceStrategy from compose.service import Net -from compose.service import VolumeFromSpec def build_service_dicts(service_config): diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index aaa4f01ec..7fbaae8c6 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -14,6 +14,7 @@ from .. import mock from .testcases import DockerClientTestCase from .testcases import pull_busybox from compose import __version__ +from compose.config.types import VolumeFromSpec from compose.const import LABEL_CONFIG_HASH from compose.const import LABEL_CONTAINER_NUMBER from compose.const import LABEL_ONE_OFF @@ -27,7 +28,6 @@ from compose.service import ConvergencePlan from compose.service import ConvergenceStrategy from compose.service import Net from compose.service import Service -from compose.service import VolumeFromSpec def create_and_start_container(service, **override_options): diff --git a/tests/unit/project_test.py b/tests/unit/project_test.py index b38f5c783..f8178ed8b 100644 --- a/tests/unit/project_test.py +++ b/tests/unit/project_test.py @@ -4,6 +4,7 @@ import docker from .. import mock from .. import unittest +from compose.config.types import VolumeFromSpec from compose.const import LABEL_SERVICE from compose.container import Container from compose.project import Project @@ -43,7 +44,7 @@ class ProjectTest(unittest.TestCase): { 'name': 'db', 'image': 'busybox:latest', - 'volumes_from': ['volume'] + 'volumes_from': [VolumeFromSpec('volume', 'ro')] }, { 'name': 'volume', @@ -167,7 +168,7 @@ class ProjectTest(unittest.TestCase): { 'name': 'test', 'image': 'busybox:latest', - 'volumes_from': ['aaa'] + 'volumes_from': [VolumeFromSpec('aaa', 'rw')] } ], self.mock_client) self.assertEqual(project.get_service('test')._get_volumes_from(), [container_id + ":rw"]) @@ -190,17 +191,13 @@ class ProjectTest(unittest.TestCase): { 'name': 'test', 'image': 'busybox:latest', - 'volumes_from': ['vol'] + 'volumes_from': [VolumeFromSpec('vol', 'rw')] } ], self.mock_client) self.assertEqual(project.get_service('test')._get_volumes_from(), [container_name + ":rw"]) - @mock.patch.object(Service, 'containers') - def test_use_volumes_from_service_container(self, mock_return): + def test_use_volumes_from_service_container(self): container_ids = ['aabbccddee', '12345'] - mock_return.return_value = [ - mock.Mock(id=container_id, spec=Container) - for container_id in container_ids] project = Project.from_dicts('test', [ { @@ -210,10 +207,16 @@ class ProjectTest(unittest.TestCase): { 'name': 'test', 'image': 'busybox:latest', - 'volumes_from': ['vol'] + 'volumes_from': [VolumeFromSpec('vol', 'rw')] } ], None) - self.assertEqual(project.get_service('test')._get_volumes_from(), [container_ids[0] + ':rw']) + with mock.patch.object(Service, 'containers') as mock_return: + mock_return.return_value = [ + mock.Mock(id=container_id, spec=Container) + for container_id in container_ids] + self.assertEqual( + project.get_service('test')._get_volumes_from(), + [container_ids[0] + ':rw']) def test_net_unset(self): project = Project.from_dicts('test', [ diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 98da5f186..83dd61589 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -6,6 +6,7 @@ import pytest from .. import mock from .. import unittest +from compose.config.types import VolumeFromSpec from compose.const import IS_WINDOWS_PLATFORM from compose.const import LABEL_CONFIG_HASH from compose.const import LABEL_ONE_OFF diff --git a/tests/unit/sort_service_test.py b/tests/unit/sort_service_test.py index a7e522a1d..ef0882877 100644 --- a/tests/unit/sort_service_test.py +++ b/tests/unit/sort_service_test.py @@ -1,4 +1,5 @@ from .. import unittest +from compose.config.types import VolumeFromSpec from compose.project import DependencyError from compose.project import sort_service_dicts @@ -73,7 +74,7 @@ class SortServiceTest(unittest.TestCase): }, { 'name': 'parent', - 'volumes_from': ['child'] + 'volumes_from': [VolumeFromSpec('child', 'rw')] }, { 'links': ['parent'], @@ -116,7 +117,7 @@ class SortServiceTest(unittest.TestCase): }, { 'name': 'parent', - 'volumes_from': ['child'] + 'volumes_from': [VolumeFromSpec('child', 'ro')] }, { 'name': 'child' @@ -141,7 +142,7 @@ class SortServiceTest(unittest.TestCase): }, { 'name': 'two', - 'volumes_from': ['one'] + 'volumes_from': [VolumeFromSpec('one', 'rw')] }, { 'name': 'one' From b19315b57ea4bd3b4594ca4b5c8abf9d18a3d8a0 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 13 Nov 2015 18:29:25 -0500 Subject: [PATCH 21/35] Move restart spec to the config.types module. Signed-off-by: Daniel Nephin --- compose/config/config.py | 4 ++++ compose/config/types.py | 17 +++++++++++++++++ compose/service.py | 22 +--------------------- tests/integration/service_test.py | 24 ++++++------------------ tests/unit/cli_test.py | 2 +- 5 files changed, 29 insertions(+), 40 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index b21e639ff..8b36c6806 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -13,6 +13,7 @@ from .errors import CircularReference from .errors import ComposeFileNotFound from .errors import ConfigurationError from .interpolation import interpolate_environment_variables +from .types import parse_restart_spec from .types import VolumeFromSpec from .validation import validate_against_fields_schema from .validation import validate_against_service_schema @@ -391,6 +392,9 @@ def finalize_service(service_config): service_dict['volumes_from'] = [ VolumeFromSpec.parse(vf) for vf in service_dict['volumes_from']] + if 'restart' in service_dict: + service_dict['restart'] = parse_restart_spec(service_dict['restart']) + return service_dict diff --git a/compose/config/types.py b/compose/config/types.py index 73bfd4184..0ab53c825 100644 --- a/compose/config/types.py +++ b/compose/config/types.py @@ -26,3 +26,20 @@ class VolumeFromSpec(namedtuple('_VolumeFromSpec', 'source mode')): source, mode = parts return cls(source, mode) + + +def parse_restart_spec(restart_config): + if not restart_config: + return None + parts = restart_config.split(':') + if len(parts) > 2: + raise ConfigurationError( + "Restart %s has incorrect format, should be " + "mode[:max_retry]" % restart_config) + if len(parts) == 2: + name, max_retry_count = parts + else: + name, = parts + max_retry_count = 0 + + return {'Name': name, 'MaximumRetryCount': int(max_retry_count)} diff --git a/compose/service.py b/compose/service.py index 3d4103777..85b6004bc 100644 --- a/compose/service.py +++ b/compose/service.py @@ -663,8 +663,6 @@ class Service(object): if isinstance(dns_search, six.string_types): dns_search = [dns_search] - restart = parse_restart_spec(options.get('restart', None)) - extra_hosts = build_extra_hosts(options.get('extra_hosts', None)) read_only = options.get('read_only', None) @@ -682,7 +680,7 @@ class Service(object): devices=devices, dns=dns, dns_search=dns_search, - restart_policy=restart, + restart_policy=options.get('restart'), cap_add=cap_add, cap_drop=cap_drop, mem_limit=options.get('mem_limit'), @@ -1058,24 +1056,6 @@ def build_container_labels(label_options, service_labels, number, config_hash): return labels -# Restart policy - - -def parse_restart_spec(restart_config): - if not restart_config: - return None - parts = restart_config.split(':') - if len(parts) > 2: - raise ConfigError("Restart %s has incorrect format, should be " - "mode[:max_retry]" % restart_config) - if len(parts) == 2: - name, max_retry_count = parts - else: - name, = parts - max_retry_count = 0 - - return {'Name': name, 'MaximumRetryCount': int(max_retry_count)} - # Ulimits diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 7fbaae8c6..87744ad56 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -779,23 +779,21 @@ class ServiceTest(DockerClientTestCase): container = create_and_start_container(service) self.assertIsNone(container.get('HostConfig.Dns')) - def test_dns_single_value(self): - service = self.create_service('web', dns='8.8.8.8') - container = create_and_start_container(service) - self.assertEqual(container.get('HostConfig.Dns'), ['8.8.8.8']) - def test_dns_list(self): service = self.create_service('web', dns=['8.8.8.8', '9.9.9.9']) container = create_and_start_container(service) self.assertEqual(container.get('HostConfig.Dns'), ['8.8.8.8', '9.9.9.9']) def test_restart_always_value(self): - service = self.create_service('web', restart='always') + service = self.create_service('web', restart={'Name': 'always'}) container = create_and_start_container(service) self.assertEqual(container.get('HostConfig.RestartPolicy.Name'), 'always') def test_restart_on_failure_value(self): - service = self.create_service('web', restart='on-failure:5') + service = self.create_service('web', restart={ + 'Name': 'on-failure', + 'MaximumRetryCount': 5 + }) container = create_and_start_container(service) self.assertEqual(container.get('HostConfig.RestartPolicy.Name'), 'on-failure') self.assertEqual(container.get('HostConfig.RestartPolicy.MaximumRetryCount'), 5) @@ -810,17 +808,7 @@ class ServiceTest(DockerClientTestCase): container = create_and_start_container(service) self.assertEqual(container.get('HostConfig.CapDrop'), ['SYS_ADMIN', 'NET_ADMIN']) - def test_dns_search_no_value(self): - service = self.create_service('web') - container = create_and_start_container(service) - self.assertIsNone(container.get('HostConfig.DnsSearch')) - - def test_dns_search_single_value(self): - service = self.create_service('web', dns_search='example.com') - container = create_and_start_container(service) - self.assertEqual(container.get('HostConfig.DnsSearch'), ['example.com']) - - def test_dns_search_list(self): + def test_dns_search(self): service = self.create_service('web', dns_search=['dc1.example.com', 'dc2.example.com']) container = create_and_start_container(service) self.assertEqual(container.get('HostConfig.DnsSearch'), ['dc1.example.com', 'dc2.example.com']) diff --git a/tests/unit/cli_test.py b/tests/unit/cli_test.py index 5b63d2e84..23dc42629 100644 --- a/tests/unit/cli_test.py +++ b/tests/unit/cli_test.py @@ -124,7 +124,7 @@ class CLITestCase(unittest.TestCase): mock_project.get_service.return_value = Service( 'service', client=mock_client, - restart='always', + restart={'Name': 'always', 'MaximumRetryCount': 0}, image='someimage') command.run(mock_project, { 'SERVICE': 'service', From 5d39813e1bbf3b25f60c1e230d2493d5b3b5be37 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 13 Nov 2015 18:58:24 -0500 Subject: [PATCH 22/35] Fixes #2008 - re-use list_or_dict schema for all the types At the same time, moves extra_hosts validation to the config module. Signed-off-by: Daniel Nephin --- compose/config/config.py | 4 ++++ compose/config/fields_schema.json | 31 +++++++++++--------------- compose/config/types.py | 16 ++++++++++++++ compose/config/validation.py | 4 ++-- compose/service.py | 36 +++---------------------------- tests/integration/service_test.py | 33 ---------------------------- tests/unit/config/config_test.py | 25 ++++++++++++++++++++- tests/unit/config/types_test.py | 29 +++++++++++++++++++++++++ 8 files changed, 90 insertions(+), 88 deletions(-) create mode 100644 tests/unit/config/types_test.py diff --git a/compose/config/config.py b/compose/config/config.py index 8b36c6806..893784f86 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -13,6 +13,7 @@ from .errors import CircularReference from .errors import ComposeFileNotFound from .errors import ConfigurationError from .interpolation import interpolate_environment_variables +from .types import parse_extra_hosts from .types import parse_restart_spec from .types import VolumeFromSpec from .validation import validate_against_fields_schema @@ -378,6 +379,9 @@ def process_service(service_config): if 'labels' in service_dict: service_dict['labels'] = parse_labels(service_dict['labels']) + if 'extra_hosts' in service_dict: + service_dict['extra_hosts'] = parse_extra_hosts(service_dict['extra_hosts']) + # TODO: move to a validate_service() if 'ulimits' in service_dict: validate_ulimits(service_dict['ulimits']) diff --git a/compose/config/fields_schema.json b/compose/config/fields_schema.json index ca3b3a502..9cbcfd1b2 100644 --- a/compose/config/fields_schema.json +++ b/compose/config/fields_schema.json @@ -37,22 +37,7 @@ "domainname": {"type": "string"}, "entrypoint": {"$ref": "#/definitions/string_or_list"}, "env_file": {"$ref": "#/definitions/string_or_list"}, - - "environment": { - "oneOf": [ - { - "type": "object", - "patternProperties": { - ".+": { - "type": ["string", "number", "boolean", "null"], - "format": "environment" - } - }, - "additionalProperties": false - }, - {"type": "array", "items": {"type": "string"}, "uniqueItems": true} - ] - }, + "environment": {"$ref": "#/definitions/list_or_dict"}, "expose": { "type": "array", @@ -165,10 +150,18 @@ "list_or_dict": { "oneOf": [ - {"type": "array", "items": {"type": "string"}, "uniqueItems": true}, - {"type": "object"} + { + "type": "object", + "patternProperties": { + ".+": { + "type": ["string", "number", "boolean", "null"], + "format": "bool-value-in-mapping" + } + }, + "additionalProperties": false + }, + {"type": "array", "items": {"type": "string"}, "uniqueItems": true} ] } - } } diff --git a/compose/config/types.py b/compose/config/types.py index 0ab53c825..b6add0894 100644 --- a/compose/config/types.py +++ b/compose/config/types.py @@ -43,3 +43,19 @@ def parse_restart_spec(restart_config): max_retry_count = 0 return {'Name': name, 'MaximumRetryCount': int(max_retry_count)} + + +def parse_extra_hosts(extra_hosts_config): + if not extra_hosts_config: + return {} + + if isinstance(extra_hosts_config, dict): + return dict(extra_hosts_config) + + if isinstance(extra_hosts_config, list): + extra_hosts_dict = {} + for extra_hosts_line in extra_hosts_config: + # TODO: validate string contains ':' ? + host, ip = extra_hosts_line.split(':') + extra_hosts_dict[host.strip()] = ip.strip() + return extra_hosts_dict diff --git a/compose/config/validation.py b/compose/config/validation.py index 38866b0f4..38020366d 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -49,7 +49,7 @@ def format_ports(instance): return True -@FormatChecker.cls_checks(format="environment") +@FormatChecker.cls_checks(format="bool-value-in-mapping") def format_boolean_in_environment(instance): """ Check if there is a boolean in the environment and display a warning. @@ -273,7 +273,7 @@ def validate_against_fields_schema(config, filename): _validate_against_schema( config, "fields_schema.json", - format_checker=["ports", "environment"], + format_checker=["ports", "bool-value-in-mapping"], filename=filename) diff --git a/compose/service.py b/compose/service.py index 85b6004bc..dd2433646 100644 --- a/compose/service.py +++ b/compose/service.py @@ -655,6 +655,7 @@ class Service(object): pid = options.get('pid', None) security_opt = options.get('security_opt', None) + # TODO: these options are already normalized by config dns = options.get('dns', None) if isinstance(dns, six.string_types): dns = [dns] @@ -663,9 +664,6 @@ class Service(object): if isinstance(dns_search, six.string_types): dns_search = [dns_search] - extra_hosts = build_extra_hosts(options.get('extra_hosts', None)) - read_only = options.get('read_only', None) - devices = options.get('devices', None) cgroup_parent = options.get('cgroup_parent', None) ulimits = build_ulimits(options.get('ulimits', None)) @@ -687,8 +685,8 @@ class Service(object): memswap_limit=options.get('memswap_limit'), ulimits=ulimits, log_config=log_config, - extra_hosts=extra_hosts, - read_only=read_only, + extra_hosts=options.get('extra_hosts'), + read_only=options.get('read_only'), pid_mode=pid, security_opt=security_opt, ipc_mode=options.get('ipc'), @@ -1072,31 +1070,3 @@ def build_ulimits(ulimit_config): ulimits.append(ulimit_dict) return ulimits - - -# Extra hosts - - -def build_extra_hosts(extra_hosts_config): - if not extra_hosts_config: - return {} - - if isinstance(extra_hosts_config, list): - extra_hosts_dict = {} - for extra_hosts_line in extra_hosts_config: - if not isinstance(extra_hosts_line, six.string_types): - raise ConfigError( - "extra_hosts_config \"%s\" must be either a list of strings or a string->string mapping," % - extra_hosts_config - ) - host, ip = extra_hosts_line.split(':') - extra_hosts_dict.update({host.strip(): ip.strip()}) - extra_hosts_config = extra_hosts_dict - - if isinstance(extra_hosts_config, dict): - return extra_hosts_config - - raise ConfigError( - "extra_hosts_config \"%s\" must be either a list of strings or a string->string mapping," % - extra_hosts_config - ) diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 87744ad56..3831e95a5 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -22,8 +22,6 @@ from compose.const import LABEL_PROJECT from compose.const import LABEL_SERVICE from compose.const import LABEL_VERSION from compose.container import Container -from compose.service import build_extra_hosts -from compose.service import ConfigError from compose.service import ConvergencePlan from compose.service import ConvergenceStrategy from compose.service import Net @@ -133,37 +131,6 @@ class ServiceTest(DockerClientTestCase): container.start() self.assertEqual(container.get('HostConfig.CpuShares'), 73) - def test_build_extra_hosts(self): - # string - self.assertRaises(ConfigError, lambda: build_extra_hosts("www.example.com: 192.168.0.17")) - - # list of strings - self.assertEqual(build_extra_hosts( - ["www.example.com:192.168.0.17"]), - {'www.example.com': '192.168.0.17'}) - self.assertEqual(build_extra_hosts( - ["www.example.com: 192.168.0.17"]), - {'www.example.com': '192.168.0.17'}) - self.assertEqual(build_extra_hosts( - ["www.example.com: 192.168.0.17", - "static.example.com:192.168.0.19", - "api.example.com: 192.168.0.18"]), - {'www.example.com': '192.168.0.17', - 'static.example.com': '192.168.0.19', - 'api.example.com': '192.168.0.18'}) - - # list of dictionaries - self.assertRaises(ConfigError, lambda: build_extra_hosts( - [{'www.example.com': '192.168.0.17'}, - {'api.example.com': '192.168.0.18'}])) - - # dictionaries - self.assertEqual(build_extra_hosts( - {'www.example.com': '192.168.0.17', - 'api.example.com': '192.168.0.18'}), - {'www.example.com': '192.168.0.17', - 'api.example.com': '192.168.0.18'}) - def test_create_container_with_extra_hosts_list(self): extra_hosts = ['somehost:162.242.195.82', 'otherhost:50.31.209.229'] service = self.create_service('db', extra_hosts=extra_hosts) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index c69e34306..f923fb370 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -32,7 +32,7 @@ def service_sort(services): return sorted(services, key=itemgetter('name')) -def build_config_details(contents, working_dir, filename): +def build_config_details(contents, working_dir='working_dir', filename='filename.yml'): return config.ConfigDetails( working_dir, [config.ConfigFile(filename, contents)]) @@ -512,6 +512,29 @@ class ConfigTest(unittest.TestCase): assert 'line 3, column 32' in exc.exconly() + def test_validate_extra_hosts_invalid(self): + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details({ + 'web': { + 'image': 'alpine', + 'extra_hosts': "www.example.com: 192.168.0.17", + } + })) + assert "'extra_hosts' contains an invalid type" in exc.exconly() + + def test_validate_extra_hosts_invalid_list(self): + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details({ + 'web': { + 'image': 'alpine', + 'extra_hosts': [ + {'www.example.com': '192.168.0.17'}, + {'api.example.com': '192.168.0.18'} + ], + } + })) + assert "which is an invalid type" in exc.exconly() + class InterpolationTest(unittest.TestCase): @mock.patch.dict(os.environ) diff --git a/tests/unit/config/types_test.py b/tests/unit/config/types_test.py new file mode 100644 index 000000000..25692ca37 --- /dev/null +++ b/tests/unit/config/types_test.py @@ -0,0 +1,29 @@ +from compose.config.types import parse_extra_hosts + + +def test_parse_extra_hosts_list(): + expected = {'www.example.com': '192.168.0.17'} + assert parse_extra_hosts(["www.example.com:192.168.0.17"]) == expected + + expected = {'www.example.com': '192.168.0.17'} + assert parse_extra_hosts(["www.example.com: 192.168.0.17"]) == expected + + assert parse_extra_hosts([ + "www.example.com: 192.168.0.17", + "static.example.com:192.168.0.19", + "api.example.com: 192.168.0.18" + ]) == { + 'www.example.com': '192.168.0.17', + 'static.example.com': '192.168.0.19', + 'api.example.com': '192.168.0.18' + } + + +def test_parse_extra_hosts_dict(): + assert parse_extra_hosts({ + 'www.example.com': '192.168.0.17', + 'api.example.com': '192.168.0.18' + }) == { + 'www.example.com': '192.168.0.17', + 'api.example.com': '192.168.0.18' + } From 8572d5090366811834e9247fac17ade4a3d8a813 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 13 Nov 2015 19:40:10 -0500 Subject: [PATCH 23/35] Move volume parsing to config.types module This removes the last of the old service.ConfigError Signed-off-by: Daniel Nephin --- compose/cli/command.py | 17 ++---- compose/config/config.py | 5 ++ compose/config/types.py | 59 +++++++++++++++++++ compose/service.py | 69 ++--------------------- tests/acceptance/cli_test.py | 2 +- tests/integration/project_test.py | 11 ++-- tests/integration/resilience_test.py | 6 +- tests/integration/service_test.py | 43 ++++++-------- tests/integration/testcases.py | 11 ++-- tests/unit/config/config_test.py | 38 +++++++------ tests/unit/config/types_test.py | 37 ++++++++++++ tests/unit/service_test.py | 84 +++++++--------------------- 12 files changed, 186 insertions(+), 196 deletions(-) diff --git a/compose/cli/command.py b/compose/cli/command.py index 6094b5305..157e00161 100644 --- a/compose/cli/command.py +++ b/compose/cli/command.py @@ -14,7 +14,6 @@ from . import errors from . import verbose_proxy from .. import config from ..project import Project -from ..service import ConfigError from .docker_client import docker_client from .utils import call_silently from .utils import get_version_info @@ -84,16 +83,12 @@ def get_project(base_dir, config_path=None, project_name=None, verbose=False, config_details = config.find(base_dir, config_path) api_version = '1.21' if use_networking else None - try: - return Project.from_dicts( - get_project_name(config_details.working_dir, project_name), - config.load(config_details), - get_client(verbose=verbose, version=api_version), - use_networking=use_networking, - network_driver=network_driver, - ) - except ConfigError as e: - raise errors.UserError(six.text_type(e)) + return Project.from_dicts( + get_project_name(config_details.working_dir, project_name), + config.load(config_details), + get_client(verbose=verbose, version=api_version), + use_networking=use_networking, + network_driver=network_driver) def get_project_name(working_dir, project_name=None): diff --git a/compose/config/config.py b/compose/config/config.py index 893784f86..8bedeffe8 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -16,6 +16,7 @@ from .interpolation import interpolate_environment_variables from .types import parse_extra_hosts from .types import parse_restart_spec from .types import VolumeFromSpec +from .types import VolumeSpec from .validation import validate_against_fields_schema from .validation import validate_against_service_schema from .validation import validate_extends_file_path @@ -396,6 +397,10 @@ def finalize_service(service_config): service_dict['volumes_from'] = [ VolumeFromSpec.parse(vf) for vf in service_dict['volumes_from']] + if 'volumes' in service_dict: + service_dict['volumes'] = [ + VolumeSpec.parse(v) for v in service_dict['volumes']] + if 'restart' in service_dict: service_dict['restart'] = parse_restart_spec(service_dict['restart']) diff --git a/compose/config/types.py b/compose/config/types.py index b6add0894..cec1f6cfd 100644 --- a/compose/config/types.py +++ b/compose/config/types.py @@ -4,9 +4,11 @@ Types for objects parsed from the configuration. from __future__ import absolute_import from __future__ import unicode_literals +import os from collections import namedtuple from compose.config.errors import ConfigurationError +from compose.const import IS_WINDOWS_PLATFORM class VolumeFromSpec(namedtuple('_VolumeFromSpec', 'source mode')): @@ -59,3 +61,60 @@ def parse_extra_hosts(extra_hosts_config): host, ip = extra_hosts_line.split(':') extra_hosts_dict[host.strip()] = ip.strip() return extra_hosts_dict + + +def normalize_paths_for_engine(external_path, internal_path): + """Windows paths, c:\my\path\shiny, need to be changed to be compatible with + the Engine. Volume paths are expected to be linux style /c/my/path/shiny/ + """ + if not IS_WINDOWS_PLATFORM: + return external_path, internal_path + + if external_path: + drive, tail = os.path.splitdrive(external_path) + + if drive: + external_path = '/' + drive.lower().rstrip(':') + tail + + external_path = external_path.replace('\\', '/') + + return external_path, internal_path.replace('\\', '/') + + +class VolumeSpec(namedtuple('_VolumeSpec', 'external internal mode')): + + @classmethod + def parse(cls, volume_config): + """Parse a volume_config path and split it into external:internal[:mode] + parts to be returned as a valid VolumeSpec. + """ + if IS_WINDOWS_PLATFORM: + # relative paths in windows expand to include the drive, eg C:\ + # so we join the first 2 parts back together to count as one + drive, tail = os.path.splitdrive(volume_config) + parts = tail.split(":") + + if drive: + parts[0] = drive + parts[0] + else: + parts = volume_config.split(':') + + if len(parts) > 3: + raise ConfigurationError( + "Volume %s has incorrect format, should be " + "external:internal[:mode]" % volume_config) + + if len(parts) == 1: + external, internal = normalize_paths_for_engine( + None, + os.path.normpath(parts[0])) + else: + external, internal = normalize_paths_for_engine( + os.path.normpath(parts[0]), + os.path.normpath(parts[1])) + + mode = 'rw' + if len(parts) == 3: + mode = parts[2] + + return cls(external, internal, mode) diff --git a/compose/service.py b/compose/service.py index dd2433646..eb411d8aa 100644 --- a/compose/service.py +++ b/compose/service.py @@ -2,7 +2,6 @@ from __future__ import absolute_import from __future__ import unicode_literals import logging -import os import re import sys from collections import namedtuple @@ -18,8 +17,8 @@ from docker.utils.ports import split_port from . import __version__ from .config import DOCKER_CONFIG_KEYS from .config import merge_environment +from .config.types import VolumeSpec from .const import DEFAULT_TIMEOUT -from .const import IS_WINDOWS_PLATFORM from .const import LABEL_CONFIG_HASH from .const import LABEL_CONTAINER_NUMBER from .const import LABEL_ONE_OFF @@ -67,11 +66,6 @@ class BuildError(Exception): self.reason = reason -# TODO: remove -class ConfigError(ValueError): - pass - - class NeedsBuildError(Exception): def __init__(self, service): self.service = service @@ -81,9 +75,6 @@ class NoSuchImageError(Exception): pass -VolumeSpec = namedtuple('VolumeSpec', 'external internal mode') - - ServiceName = namedtuple('ServiceName', 'project service number') @@ -613,8 +604,7 @@ class Service(object): if 'volumes' in container_options: container_options['volumes'] = dict( - (parse_volume_spec(v).internal, {}) - for v in container_options['volumes']) + (v.internal, {}) for v in container_options['volumes']) container_options['environment'] = merge_environment( self.options.get('environment'), @@ -899,11 +889,10 @@ def parse_repository_tag(repo_path): # Volumes -def merge_volume_bindings(volumes_option, previous_container): +def merge_volume_bindings(volumes, previous_container): """Return a list of volume bindings for a container. Container data volumes are replaced by those from the previous container. """ - volumes = [parse_volume_spec(volume) for volume in volumes_option or []] volume_bindings = dict( build_volume_binding(volume) for volume in volumes @@ -925,7 +914,7 @@ def get_container_data_volumes(container, volumes_option): volumes = [] container_volumes = container.get('Volumes') or {} image_volumes = [ - parse_volume_spec(volume) + VolumeSpec.parse(volume) for volume in container.image_config['ContainerConfig'].get('Volumes') or {} ] @@ -972,56 +961,6 @@ def build_volume_binding(volume_spec): return volume_spec.internal, "{}:{}:{}".format(*volume_spec) -def normalize_paths_for_engine(external_path, internal_path): - """Windows paths, c:\my\path\shiny, need to be changed to be compatible with - the Engine. Volume paths are expected to be linux style /c/my/path/shiny/ - """ - if not IS_WINDOWS_PLATFORM: - return external_path, internal_path - - if external_path: - drive, tail = os.path.splitdrive(external_path) - - if drive: - external_path = '/' + drive.lower().rstrip(':') + tail - - external_path = external_path.replace('\\', '/') - - return external_path, internal_path.replace('\\', '/') - - -def parse_volume_spec(volume_config): - """ - Parse a volume_config path and split it into external:internal[:mode] - parts to be returned as a valid VolumeSpec. - """ - if IS_WINDOWS_PLATFORM: - # relative paths in windows expand to include the drive, eg C:\ - # so we join the first 2 parts back together to count as one - drive, tail = os.path.splitdrive(volume_config) - parts = tail.split(":") - - if drive: - parts[0] = drive + parts[0] - else: - parts = volume_config.split(':') - - if len(parts) > 3: - raise ConfigError("Volume %s has incorrect format, should be " - "external:internal[:mode]" % volume_config) - - if len(parts) == 1: - external, internal = normalize_paths_for_engine(None, os.path.normpath(parts[0])) - else: - external, internal = normalize_paths_for_engine(os.path.normpath(parts[0]), os.path.normpath(parts[1])) - - mode = 'rw' - if len(parts) == 3: - mode = parts[2] - - return VolumeSpec(external, internal, mode) - - def build_volume_from(volume_from_spec): """ volume_from can be either a service or a container. We want to return the diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 88ec45738..282a52195 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -38,7 +38,7 @@ def start_process(base_dir, options): def wait_on_process(proc, returncode=0): stdout, stderr = proc.communicate() if proc.returncode != returncode: - print(stderr) + print(stderr.decode('utf-8')) assert proc.returncode == returncode return ProcessResult(stdout.decode('utf-8'), stderr.decode('utf-8')) diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index d65d7ef0c..443ff9783 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -4,6 +4,7 @@ from .testcases import DockerClientTestCase from compose.cli.docker_client import docker_client from compose.config import config from compose.config.types import VolumeFromSpec +from compose.config.types import VolumeSpec from compose.const import LABEL_PROJECT from compose.container import Container from compose.project import Project @@ -214,7 +215,7 @@ class ProjectTest(DockerClientTestCase): def test_project_up(self): web = self.create_service('web') - db = self.create_service('db', volumes=['/var/db']) + db = self.create_service('db', volumes=[VolumeSpec.parse('/var/db')]) project = Project('composetest', [web, db], self.client) project.start() self.assertEqual(len(project.containers()), 0) @@ -238,7 +239,7 @@ class ProjectTest(DockerClientTestCase): def test_recreate_preserves_volumes(self): web = self.create_service('web') - db = self.create_service('db', volumes=['/etc']) + db = self.create_service('db', volumes=[VolumeSpec.parse('/etc')]) project = Project('composetest', [web, db], self.client) project.start() self.assertEqual(len(project.containers()), 0) @@ -257,7 +258,7 @@ class ProjectTest(DockerClientTestCase): def test_project_up_with_no_recreate_running(self): web = self.create_service('web') - db = self.create_service('db', volumes=['/var/db']) + db = self.create_service('db', volumes=[VolumeSpec.parse('/var/db')]) project = Project('composetest', [web, db], self.client) project.start() self.assertEqual(len(project.containers()), 0) @@ -277,7 +278,7 @@ class ProjectTest(DockerClientTestCase): def test_project_up_with_no_recreate_stopped(self): web = self.create_service('web') - db = self.create_service('db', volumes=['/var/db']) + db = self.create_service('db', volumes=[VolumeSpec.parse('/var/db')]) project = Project('composetest', [web, db], self.client) project.start() self.assertEqual(len(project.containers()), 0) @@ -316,7 +317,7 @@ class ProjectTest(DockerClientTestCase): def test_project_up_starts_links(self): console = self.create_service('console') - db = self.create_service('db', volumes=['/var/db']) + db = self.create_service('db', volumes=[VolumeSpec.parse('/var/db')]) web = self.create_service('web', links=[(db, 'db')]) project = Project('composetest', [web, db, console], self.client) diff --git a/tests/integration/resilience_test.py b/tests/integration/resilience_test.py index 53aedfecf..7f75356d8 100644 --- a/tests/integration/resilience_test.py +++ b/tests/integration/resilience_test.py @@ -3,13 +3,17 @@ from __future__ import unicode_literals from .. import mock from .testcases import DockerClientTestCase +from compose.config.types import VolumeSpec from compose.project import Project from compose.service import ConvergenceStrategy class ResilienceTest(DockerClientTestCase): def setUp(self): - self.db = self.create_service('db', volumes=['/var/db'], command='top') + self.db = self.create_service( + 'db', + volumes=[VolumeSpec.parse('/var/db')], + command='top') self.project = Project('composetest', [self.db], self.client) container = self.db.create_container() diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 3831e95a5..6808280f0 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -15,6 +15,7 @@ from .testcases import DockerClientTestCase from .testcases import pull_busybox from compose import __version__ from compose.config.types import VolumeFromSpec +from compose.config.types import VolumeSpec from compose.const import LABEL_CONFIG_HASH from compose.const import LABEL_CONTAINER_NUMBER from compose.const import LABEL_ONE_OFF @@ -114,7 +115,7 @@ class ServiceTest(DockerClientTestCase): self.assertEqual(container.name, 'composetest_db_run_1') def test_create_container_with_unspecified_volume(self): - service = self.create_service('db', volumes=['/var/db']) + service = self.create_service('db', volumes=[VolumeSpec.parse('/var/db')]) container = service.create_container() container.start() self.assertIn('/var/db', container.get('Volumes')) @@ -176,7 +177,9 @@ class ServiceTest(DockerClientTestCase): host_path = '/tmp/host-path' container_path = '/container-path' - service = self.create_service('db', volumes=['%s:%s' % (host_path, container_path)]) + service = self.create_service( + 'db', + volumes=[VolumeSpec(host_path, container_path, 'rw')]) container = service.create_container() container.start() @@ -189,11 +192,10 @@ class ServiceTest(DockerClientTestCase): msg=("Last component differs: %s, %s" % (actual_host_path, host_path))) def test_recreate_preserves_volume_with_trailing_slash(self): - """ - When the Compose file specifies a trailing slash in the container path, make + """When the Compose file specifies a trailing slash in the container path, make sure we copy the volume over when recreating. """ - service = self.create_service('data', volumes=['/data/']) + service = self.create_service('data', volumes=[VolumeSpec.parse('/data/')]) old_container = create_and_start_container(service) volume_path = old_container.get('Volumes')['/data'] @@ -207,7 +209,7 @@ class ServiceTest(DockerClientTestCase): """ host_path = '/tmp/data' container_path = '/data' - volumes = ['{}:{}/'.format(host_path, container_path)] + volumes = [VolumeSpec.parse('{}:{}/'.format(host_path, container_path))] tmp_container = self.client.create_container( 'busybox', 'true', @@ -261,7 +263,7 @@ class ServiceTest(DockerClientTestCase): service = self.create_service( 'db', environment={'FOO': '1'}, - volumes=['/etc'], + volumes=[VolumeSpec.parse('/etc')], entrypoint=['top'], command=['-d', '1'] ) @@ -299,7 +301,7 @@ class ServiceTest(DockerClientTestCase): service = self.create_service( 'db', environment={'FOO': '1'}, - volumes=['/var/db'], + volumes=[VolumeSpec.parse('/var/db')], entrypoint=['top'], command=['-d', '1'] ) @@ -337,10 +339,8 @@ class ServiceTest(DockerClientTestCase): self.assertEqual(new_container.get('Volumes')['/data'], volume_path) def test_execute_convergence_plan_when_image_volume_masks_config(self): - service = Service( - project='composetest', - name='db', - client=self.client, + service = self.create_service( + 'db', build='tests/fixtures/dockerfile-with-volume', ) @@ -348,7 +348,7 @@ class ServiceTest(DockerClientTestCase): self.assertEqual(list(old_container.get('Volumes').keys()), ['/data']) volume_path = old_container.get('Volumes')['/data'] - service.options['volumes'] = ['/tmp:/data'] + service.options['volumes'] = [VolumeSpec.parse('/tmp:/data')] with mock.patch('compose.service.log') as mock_log: new_container, = service.execute_convergence_plan( @@ -857,22 +857,11 @@ class ServiceTest(DockerClientTestCase): for pair in expected.items(): self.assertIn(pair, labels) - service.kill() - service.remove_stopped() - - labels_list = ["%s=%s" % pair for pair in labels_dict.items()] - - service = self.create_service('web', labels=labels_list) - labels = create_and_start_container(service).labels.items() - for pair in expected.items(): - self.assertIn(pair, labels) - def test_empty_labels(self): - labels_list = ['foo', 'bar'] - - service = self.create_service('web', labels=labels_list) + labels_dict = {'foo': '', 'bar': ''} + service = self.create_service('web', labels=labels_dict) labels = create_and_start_container(service).labels.items() - for name in labels_list: + for name in labels_dict: self.assertIn((name, ''), labels) def test_custom_container_name(self): diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index 2c5ca9fdd..f0e5c6d85 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -6,9 +6,7 @@ from pytest import skip from .. import unittest from compose.cli.docker_client import docker_client -from compose.config.config import process_service from compose.config.config import resolve_environment -from compose.config.config import ServiceConfig from compose.const import LABEL_PROJECT from compose.progress_stream import stream_output from compose.service import Service @@ -41,13 +39,12 @@ class DockerClientTestCase(unittest.TestCase): kwargs['command'] = ["top"] service_config = ServiceConfig('.', None, name, kwargs) - options = process_service(service_config) - options['environment'] = resolve_environment( - service_config._replace(config=options)) - labels = options.setdefault('labels', {}) + kwargs['environment'] = resolve_environment(service_config) + + labels = dict(kwargs.setdefault('labels', {})) labels['com.docker.compose.test-name'] = self.id() - return Service(name, client=self.client, project='composetest', **options) + return Service(name, client=self.client, project='composetest', **kwargs) def check_build(self, *args, **kwargs): kwargs.setdefault('rm', True) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index f923fb370..b2a4cd68f 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -11,6 +11,7 @@ import pytest from compose.config import config from compose.config.errors import ConfigurationError +from compose.config.types import VolumeSpec from compose.const import IS_WINDOWS_PLATFORM from tests import mock from tests import unittest @@ -147,7 +148,7 @@ class ConfigTest(unittest.TestCase): 'name': 'web', 'build': '/', 'links': ['db'], - 'volumes': ['/home/user/project:/code'], + 'volumes': [VolumeSpec.parse('/home/user/project:/code')], }, { 'name': 'db', @@ -211,7 +212,7 @@ class ConfigTest(unittest.TestCase): { 'name': 'web', 'image': 'example/web', - 'volumes': ['/home/user/project:/code'], + 'volumes': [VolumeSpec.parse('/home/user/project:/code')], 'labels': {'label': 'one'}, }, ] @@ -626,14 +627,11 @@ class VolumeConfigTest(unittest.TestCase): @mock.patch.dict(os.environ) def test_volume_binding_with_environment_variable(self): os.environ['VOLUME_PATH'] = '/host/path' - d = config.load( - build_config_details( - {'foo': {'build': '.', 'volumes': ['${VOLUME_PATH}:/container/path']}}, - '.', - None, - ) - )[0] - self.assertEqual(d['volumes'], ['/host/path:/container/path']) + d = config.load(build_config_details( + {'foo': {'build': '.', 'volumes': ['${VOLUME_PATH}:/container/path']}}, + '.', + ))[0] + self.assertEqual(d['volumes'], [VolumeSpec.parse('/host/path:/container/path')]) @pytest.mark.skipif(IS_WINDOWS_PLATFORM, reason='posix paths') @mock.patch.dict(os.environ) @@ -1031,19 +1029,21 @@ class EnvTest(unittest.TestCase): build_config_details( {'foo': {'build': '.', 'volumes': ['$HOSTENV:$CONTAINERENV']}}, "tests/fixtures/env", - None, ) )[0] - self.assertEqual(set(service_dict['volumes']), set(['/tmp:/host/tmp'])) + self.assertEqual( + set(service_dict['volumes']), + set([VolumeSpec.parse('/tmp:/host/tmp')])) service_dict = config.load( build_config_details( {'foo': {'build': '.', 'volumes': ['/opt${HOSTENV}:/opt${CONTAINERENV}']}}, "tests/fixtures/env", - None, ) )[0] - self.assertEqual(set(service_dict['volumes']), set(['/opt/tmp:/opt/host/tmp'])) + self.assertEqual( + set(service_dict['volumes']), + set([VolumeSpec.parse('/opt/tmp:/opt/host/tmp')])) def load_from_filename(filename): @@ -1290,8 +1290,14 @@ class ExtendsTest(unittest.TestCase): dicts = load_from_filename('tests/fixtures/volume-path/docker-compose.yml') paths = [ - '%s:/foo' % os.path.abspath('tests/fixtures/volume-path/common/foo'), - '%s:/bar' % os.path.abspath('tests/fixtures/volume-path/bar'), + VolumeSpec( + os.path.abspath('tests/fixtures/volume-path/common/foo'), + '/foo', + 'rw'), + VolumeSpec( + os.path.abspath('tests/fixtures/volume-path/bar'), + '/bar', + 'rw') ] self.assertEqual(set(dicts[0]['volumes']), set(paths)) diff --git a/tests/unit/config/types_test.py b/tests/unit/config/types_test.py index 25692ca37..4df665485 100644 --- a/tests/unit/config/types_test.py +++ b/tests/unit/config/types_test.py @@ -1,4 +1,9 @@ +import pytest + +from compose.config.errors import ConfigurationError from compose.config.types import parse_extra_hosts +from compose.config.types import VolumeSpec +from compose.const import IS_WINDOWS_PLATFORM def test_parse_extra_hosts_list(): @@ -27,3 +32,35 @@ def test_parse_extra_hosts_dict(): 'www.example.com': '192.168.0.17', 'api.example.com': '192.168.0.18' } + + +class TestVolumeSpec(object): + + def test_parse_volume_spec_only_one_path(self): + spec = VolumeSpec.parse('/the/volume') + assert spec == (None, '/the/volume', 'rw') + + def test_parse_volume_spec_internal_and_external(self): + spec = VolumeSpec.parse('external:interval') + assert spec == ('external', 'interval', 'rw') + + def test_parse_volume_spec_with_mode(self): + spec = VolumeSpec.parse('external:interval:ro') + assert spec == ('external', 'interval', 'ro') + + spec = VolumeSpec.parse('external:interval:z') + assert spec == ('external', 'interval', 'z') + + def test_parse_volume_spec_too_many_parts(self): + with pytest.raises(ConfigurationError) as exc: + VolumeSpec.parse('one:two:three:four') + assert 'has incorrect format' in exc.exconly() + + @pytest.mark.xfail((not IS_WINDOWS_PLATFORM), reason='does not have a drive') + def test_parse_volume_windows_absolute_path(self): + windows_path = "c:\\Users\\me\\Documents\\shiny\\config:\\opt\\shiny\\config:ro" + assert VolumeSpec.parse(windows_path) == ( + "/c/Users/me/Documents/shiny/config", + "/opt/shiny/config", + "ro" + ) diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 83dd61589..c87d31b1d 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -2,12 +2,11 @@ from __future__ import absolute_import from __future__ import unicode_literals import docker -import pytest from .. import mock from .. import unittest from compose.config.types import VolumeFromSpec -from compose.const import IS_WINDOWS_PLATFORM +from compose.config.types import VolumeSpec from compose.const import LABEL_CONFIG_HASH from compose.const import LABEL_ONE_OFF from compose.const import LABEL_PROJECT @@ -15,7 +14,6 @@ from compose.const import LABEL_SERVICE from compose.container import Container from compose.service import build_ulimits from compose.service import build_volume_binding -from compose.service import ConfigError from compose.service import ContainerNet from compose.service import get_container_data_volumes from compose.service import merge_volume_bindings @@ -23,7 +21,6 @@ from compose.service import NeedsBuildError from compose.service import Net from compose.service import NoSuchImageError from compose.service import parse_repository_tag -from compose.service import parse_volume_spec from compose.service import Service from compose.service import ServiceNet from compose.service import VolumeFromSpec @@ -585,46 +582,12 @@ class ServiceVolumesTest(unittest.TestCase): def setUp(self): self.mock_client = mock.create_autospec(docker.Client) - def test_parse_volume_spec_only_one_path(self): - spec = parse_volume_spec('/the/volume') - self.assertEqual(spec, (None, '/the/volume', 'rw')) - - def test_parse_volume_spec_internal_and_external(self): - spec = parse_volume_spec('external:interval') - self.assertEqual(spec, ('external', 'interval', 'rw')) - - def test_parse_volume_spec_with_mode(self): - spec = parse_volume_spec('external:interval:ro') - self.assertEqual(spec, ('external', 'interval', 'ro')) - - spec = parse_volume_spec('external:interval:z') - self.assertEqual(spec, ('external', 'interval', 'z')) - - def test_parse_volume_spec_too_many_parts(self): - with self.assertRaises(ConfigError): - parse_volume_spec('one:two:three:four') - - @pytest.mark.xfail((not IS_WINDOWS_PLATFORM), reason='does not have a drive') - def test_parse_volume_windows_absolute_path(self): - windows_absolute_path = "c:\\Users\\me\\Documents\\shiny\\config:\\opt\\shiny\\config:ro" - - spec = parse_volume_spec(windows_absolute_path) - - self.assertEqual( - spec, - ( - "/c/Users/me/Documents/shiny/config", - "/opt/shiny/config", - "ro" - ) - ) - def test_build_volume_binding(self): - binding = build_volume_binding(parse_volume_spec('/outside:/inside')) - self.assertEqual(binding, ('/inside', '/outside:/inside:rw')) + binding = build_volume_binding(VolumeSpec.parse('/outside:/inside')) + assert binding == ('/inside', '/outside:/inside:rw') def test_get_container_data_volumes(self): - options = [parse_volume_spec(v) for v in [ + options = [VolumeSpec.parse(v) for v in [ '/host/volume:/host/volume:ro', '/new/volume', '/existing/volume', @@ -648,19 +611,19 @@ class ServiceVolumesTest(unittest.TestCase): }, has_been_inspected=True) expected = [ - parse_volume_spec('/var/lib/docker/aaaaaaaa:/existing/volume:rw'), - parse_volume_spec('/var/lib/docker/cccccccc:/mnt/image/data:rw'), + VolumeSpec.parse('/var/lib/docker/aaaaaaaa:/existing/volume:rw'), + VolumeSpec.parse('/var/lib/docker/cccccccc:/mnt/image/data:rw'), ] volumes = get_container_data_volumes(container, options) - self.assertEqual(sorted(volumes), sorted(expected)) + assert sorted(volumes) == sorted(expected) def test_merge_volume_bindings(self): options = [ - '/host/volume:/host/volume:ro', - '/host/rw/volume:/host/rw/volume', - '/new/volume', - '/existing/volume', + VolumeSpec.parse('/host/volume:/host/volume:ro'), + VolumeSpec.parse('/host/rw/volume:/host/rw/volume'), + VolumeSpec.parse('/new/volume'), + VolumeSpec.parse('/existing/volume'), ] self.mock_client.inspect_image.return_value = { @@ -686,8 +649,8 @@ class ServiceVolumesTest(unittest.TestCase): 'web', image='busybox', volumes=[ - '/host/path:/data1', - '/host/path:/data2', + VolumeSpec.parse('/host/path:/data1'), + VolumeSpec.parse('/host/path:/data2'), ], client=self.mock_client, ) @@ -716,7 +679,7 @@ class ServiceVolumesTest(unittest.TestCase): service = Service( 'web', image='busybox', - volumes=['/host/path:/data'], + volumes=[VolumeSpec.parse('/host/path:/data')], client=self.mock_client, ) @@ -784,22 +747,17 @@ class ServiceVolumesTest(unittest.TestCase): def test_create_with_special_volume_mode(self): self.mock_client.inspect_image.return_value = {'Id': 'imageid'} - create_calls = [] - - def create_container(*args, **kwargs): - create_calls.append((args, kwargs)) - return {'Id': 'containerid'} - - self.mock_client.create_container = create_container - - volumes = ['/tmp:/foo:z'] + self.mock_client.create_container.return_value = {'Id': 'containerid'} + volume = '/tmp:/foo:z' Service( 'web', client=self.mock_client, image='busybox', - volumes=volumes, + volumes=[VolumeSpec.parse(volume)], ).create_container() - self.assertEqual(len(create_calls), 1) - self.assertEqual(self.mock_client.create_host_config.call_args[1]['binds'], volumes) + assert self.mock_client.create_container.call_count == 1 + self.assertEqual( + self.mock_client.create_host_config.call_args[1]['binds'], + [volume]) From da27f8e7e244883eaf9dec24c00c7179b63a94f3 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 13 Nov 2015 19:49:14 -0500 Subject: [PATCH 24/35] Remove unnecessary intermediate variables in get_container_host_config. Signed-off-by: Daniel Nephin --- compose/service.py | 44 ++++++++++++-------------------------------- 1 file changed, 12 insertions(+), 32 deletions(-) diff --git a/compose/service.py b/compose/service.py index eb411d8aa..34eb4c952 100644 --- a/compose/service.py +++ b/compose/service.py @@ -511,7 +511,7 @@ class Service(object): # 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 + # 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 = filter(None, [ @@ -633,54 +633,34 @@ class Service(object): def _get_container_host_config(self, override_options, one_off=False): options = dict(self.options, **override_options) - port_bindings = build_port_bindings(options.get('ports') or []) - privileged = options.get('privileged', False) - cap_add = options.get('cap_add', None) - cap_drop = options.get('cap_drop', None) log_config = LogConfig( type=options.get('log_driver', ""), config=options.get('log_opt', None) ) - pid = options.get('pid', None) - security_opt = options.get('security_opt', None) - - # TODO: these options are already normalized by config - dns = options.get('dns', None) - if isinstance(dns, six.string_types): - dns = [dns] - - dns_search = options.get('dns_search', None) - if isinstance(dns_search, six.string_types): - dns_search = [dns_search] - - devices = options.get('devices', None) - cgroup_parent = options.get('cgroup_parent', None) - ulimits = build_ulimits(options.get('ulimits', None)) - return self.client.create_host_config( links=self._get_links(link_to_self=one_off), - port_bindings=port_bindings, + port_bindings=build_port_bindings(options.get('ports') or []), binds=options.get('binds'), volumes_from=self._get_volumes_from(), - privileged=privileged, + privileged=options.get('privileged', False), network_mode=self.net.mode, - devices=devices, - dns=dns, - dns_search=dns_search, + devices=options.get('devices'), + dns=options.get('dns'), + dns_search=options.get('dns_search'), restart_policy=options.get('restart'), - cap_add=cap_add, - cap_drop=cap_drop, + cap_add=options.get('cap_add'), + cap_drop=options.get('cap_drop'), mem_limit=options.get('mem_limit'), memswap_limit=options.get('memswap_limit'), - ulimits=ulimits, + ulimits=build_ulimits(options.get('ulimits')), log_config=log_config, extra_hosts=options.get('extra_hosts'), read_only=options.get('read_only'), - pid_mode=pid, - security_opt=security_opt, + pid_mode=options.get('pid'), + security_opt=options.get('security_opt'), ipc_mode=options.get('ipc'), - cgroup_parent=cgroup_parent + cgroup_parent=options.get('cgroup_parent'), ) def build(self, no_cache=False, pull=False, force_rm=False): From 81f0e72bd2fa33327c7fe1ceb9303d0614ffd3bd Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 17 Nov 2015 13:35:28 -0500 Subject: [PATCH 25/35] Move service sorting to config package. Signed-off-by: Daniel Nephin --- compose/config/__init__.py | 1 - compose/config/config.py | 17 ++---- compose/config/errors.py | 4 ++ compose/config/sort_services.py | 55 +++++++++++++++++++ compose/project.py | 51 +---------------- tests/integration/testcases.py | 1 + tests/unit/config/config_test.py | 23 +++++++- .../sort_services_test.py} | 6 +- tests/unit/project_test.py | 23 -------- tests/unit/service_test.py | 2 - 10 files changed, 91 insertions(+), 92 deletions(-) create mode 100644 compose/config/sort_services.py rename tests/unit/{sort_service_test.py => config/sort_services_test.py} (98%) diff --git a/compose/config/__init__.py b/compose/config/__init__.py index ec607e087..6fe9ff9fb 100644 --- a/compose/config/__init__.py +++ b/compose/config/__init__.py @@ -2,7 +2,6 @@ from .config import ConfigurationError from .config import DOCKER_CONFIG_KEYS from .config import find -from .config import get_service_name_from_net from .config import load from .config import merge_environment from .config import parse_environment diff --git a/compose/config/config.py b/compose/config/config.py index 8bedeffe8..0ca6817e7 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -13,6 +13,8 @@ from .errors import CircularReference from .errors import ComposeFileNotFound from .errors import ConfigurationError from .interpolation import interpolate_environment_variables +from .sort_services import get_service_name_from_net +from .sort_services import sort_service_dicts from .types import parse_extra_hosts from .types import parse_restart_spec from .types import VolumeFromSpec @@ -213,10 +215,10 @@ def load(config_details): return service_dict def build_services(config_file): - return [ + return sort_service_dicts([ build_service(config_file.filename, name, service_dict) for name, service_dict in config_file.config.items() - ] + ]) def merge_services(base, override): all_service_names = set(base) | set(override) @@ -643,17 +645,6 @@ def to_list(value): return value -def get_service_name_from_net(net_config): - if not net_config: - return - - if not net_config.startswith('container:'): - return - - _, net_name = net_config.split(':', 1) - return net_name - - def load_yaml(filename): try: with open(filename, 'r') as fh: diff --git a/compose/config/errors.py b/compose/config/errors.py index 037b7ec84..6d6a69df9 100644 --- a/compose/config/errors.py +++ b/compose/config/errors.py @@ -6,6 +6,10 @@ class ConfigurationError(Exception): return self.msg +class DependencyError(ConfigurationError): + pass + + class CircularReference(ConfigurationError): def __init__(self, trail): self.trail = trail diff --git a/compose/config/sort_services.py b/compose/config/sort_services.py new file mode 100644 index 000000000..5d9adab11 --- /dev/null +++ b/compose/config/sort_services.py @@ -0,0 +1,55 @@ +from compose.config.errors import DependencyError + + +def get_service_name_from_net(net_config): + if not net_config: + return + + if not net_config.startswith('container:'): + return + + _, net_name = net_config.split(':', 1) + return net_name + + +def sort_service_dicts(services): + # Topological sort (Cormen/Tarjan algorithm). + unmarked = services[:] + temporary_marked = set() + sorted_services = [] + + def get_service_names(links): + return [link.split(':')[0] for link in links] + + def get_service_names_from_volumes_from(volumes_from): + return [volume_from.source for volume_from in volumes_from] + + def get_service_dependents(service_dict, services): + name = service_dict['name'] + return [ + service for service in services + if (name in get_service_names(service.get('links', [])) or + name in get_service_names_from_volumes_from(service.get('volumes_from', [])) or + name == get_service_name_from_net(service.get('net'))) + ] + + def visit(n): + if n['name'] in temporary_marked: + if n['name'] in get_service_names(n.get('links', [])): + raise DependencyError('A service can not link to itself: %s' % n['name']) + if n['name'] in n.get('volumes_from', []): + raise DependencyError('A service can not mount itself as volume: %s' % n['name']) + else: + raise DependencyError('Circular import between %s' % ' and '.join(temporary_marked)) + if n in unmarked: + temporary_marked.add(n['name']) + for m in get_service_dependents(n, services): + visit(m) + temporary_marked.remove(n['name']) + unmarked.remove(n) + sorted_services.insert(0, n) + + while unmarked: + visit(unmarked[-1]) + + return sorted_services diff --git a/compose/project.py b/compose/project.py index 69f084752..53e53cb1a 100644 --- a/compose/project.py +++ b/compose/project.py @@ -8,7 +8,7 @@ from docker.errors import APIError from docker.errors import NotFound from .config import ConfigurationError -from .config import get_service_name_from_net +from .config.sort_services import get_service_name_from_net from .const import DEFAULT_TIMEOUT from .const import LABEL_ONE_OFF from .const import LABEL_PROJECT @@ -26,49 +26,6 @@ from .utils import parallel_execute log = logging.getLogger(__name__) -def sort_service_dicts(services): - # Topological sort (Cormen/Tarjan algorithm). - unmarked = services[:] - temporary_marked = set() - sorted_services = [] - - def get_service_names(links): - return [link.split(':')[0] for link in links] - - def get_service_names_from_volumes_from(volumes_from): - return [volume_from.source for volume_from in volumes_from] - - def get_service_dependents(service_dict, services): - name = service_dict['name'] - return [ - service for service in services - if (name in get_service_names(service.get('links', [])) or - name in get_service_names_from_volumes_from(service.get('volumes_from', [])) or - name == get_service_name_from_net(service.get('net'))) - ] - - def visit(n): - if n['name'] in temporary_marked: - if n['name'] in get_service_names(n.get('links', [])): - raise DependencyError('A service can not link to itself: %s' % n['name']) - if n['name'] in n.get('volumes_from', []): - raise DependencyError('A service can not mount itself as volume: %s' % n['name']) - else: - raise DependencyError('Circular import between %s' % ' and '.join(temporary_marked)) - if n in unmarked: - temporary_marked.add(n['name']) - for m in get_service_dependents(n, services): - visit(m) - temporary_marked.remove(n['name']) - unmarked.remove(n) - sorted_services.insert(0, n) - - while unmarked: - visit(unmarked[-1]) - - return sorted_services - - class Project(object): """ A collection of services. @@ -96,7 +53,7 @@ class Project(object): if use_networking: remove_links(service_dicts) - for service_dict in sort_service_dicts(service_dicts): + for service_dict in service_dicts: links = project.get_links(service_dict) volumes_from = project.get_volumes_from(service_dict) net = project.get_net(service_dict) @@ -424,7 +381,3 @@ class NoSuchService(Exception): def __str__(self): return self.msg - - -class DependencyError(ConfigurationError): - pass diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index f0e5c6d85..334693f70 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -7,6 +7,7 @@ from pytest import skip from .. import unittest from compose.cli.docker_client import docker_client from compose.config.config import resolve_environment +from compose.config.config import ServiceConfig from compose.const import LABEL_PROJECT from compose.progress_stream import stream_output from compose.service import Service diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index b2a4cd68f..a5eeb64f9 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -77,7 +77,7 @@ class ConfigTest(unittest.TestCase): ) ) - def test_config_invalid_service_names(self): + def test_load_config_invalid_service_names(self): for invalid_name in ['?not?allowed', ' ', '', '!', '/', '\xe2']: with pytest.raises(ConfigurationError) as exc: config.load(build_config_details( @@ -232,6 +232,27 @@ class ConfigTest(unittest.TestCase): assert "service 'bogus' doesn't have any configuration" in exc.exconly() assert "In file 'override.yaml'" in exc.exconly() + def test_load_sorts_in_dependency_order(self): + config_details = build_config_details({ + 'web': { + 'image': 'busybox:latest', + 'links': ['db'], + }, + 'db': { + 'image': 'busybox:latest', + 'volumes_from': ['volume:ro'] + }, + 'volume': { + 'image': 'busybox:latest', + 'volumes': ['/tmp'], + } + }) + services = config.load(config_details) + + assert services[0]['name'] == 'volume' + assert services[1]['name'] == 'db' + assert services[2]['name'] == 'web' + def test_config_valid_service_names(self): for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']: services = config.load( diff --git a/tests/unit/sort_service_test.py b/tests/unit/config/sort_services_test.py similarity index 98% rename from tests/unit/sort_service_test.py rename to tests/unit/config/sort_services_test.py index ef0882877..8d0c3ae40 100644 --- a/tests/unit/sort_service_test.py +++ b/tests/unit/config/sort_services_test.py @@ -1,7 +1,7 @@ -from .. import unittest +from compose.config.errors import DependencyError +from compose.config.sort_services import sort_service_dicts from compose.config.types import VolumeFromSpec -from compose.project import DependencyError -from compose.project import sort_service_dicts +from tests import unittest class SortServiceTest(unittest.TestCase): diff --git a/tests/unit/project_test.py b/tests/unit/project_test.py index f8178ed8b..f4c6f8ca1 100644 --- a/tests/unit/project_test.py +++ b/tests/unit/project_test.py @@ -34,29 +34,6 @@ class ProjectTest(unittest.TestCase): self.assertEqual(project.get_service('db').name, 'db') self.assertEqual(project.get_service('db').options['image'], 'busybox:latest') - def test_from_dict_sorts_in_dependency_order(self): - project = Project.from_dicts('composetest', [ - { - 'name': 'web', - 'image': 'busybox:latest', - 'links': ['db'], - }, - { - 'name': 'db', - 'image': 'busybox:latest', - 'volumes_from': [VolumeFromSpec('volume', 'ro')] - }, - { - 'name': 'volume', - 'image': 'busybox:latest', - 'volumes': ['/tmp'], - } - ], None) - - self.assertEqual(project.services[0].name, 'volume') - self.assertEqual(project.services[1].name, 'db') - self.assertEqual(project.services[2].name, 'web') - def test_from_config(self): dicts = [ { diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index c87d31b1d..1c8b441f3 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -23,8 +23,6 @@ from compose.service import NoSuchImageError from compose.service import parse_repository_tag from compose.service import Service from compose.service import ServiceNet -from compose.service import VolumeFromSpec -from compose.service import VolumeSpec from compose.service import warn_on_masked_volume From fa975d7fbefd9578b02287ad5a2763efa3a446c7 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 23 Nov 2015 13:24:57 -0500 Subject: [PATCH 26/35] Properly resolve environment from all sources. Split env resolving into two phases. The first phase is to expand the paths of env_files, which is done before merging extends. Once all files are merged together, the final phase is to read the env_files and use them as the base for environment variables. Signed-off-by: Daniel Nephin --- compose/config/config.py | 34 +++++------- tests/integration/testcases.py | 5 +- tests/unit/config/config_test.py | 94 +++++++++++++++++--------------- 3 files changed, 64 insertions(+), 69 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 0ca6817e7..cbebeca83 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -323,16 +323,13 @@ class ServiceExtendsResolver(object): return filename -def resolve_environment(service_config): +def resolve_environment(service_dict): """Unpack any environment variables from an env_file, if set. Interpolate environment values if set. """ - service_dict = service_config.config - env = {} - if 'env_file' in service_dict: - for env_file in get_env_files(service_config.working_dir, service_dict): - env.update(env_vars_from_file(env_file)) + for env_file in service_dict.get('env_file', []): + env.update(env_vars_from_file(env_file)) env.update(parse_environment(service_dict.get('environment'))) return dict(resolve_env_var(k, v) for k, v in six.iteritems(env)) @@ -369,9 +366,11 @@ def process_service(service_config): working_dir = service_config.working_dir service_dict = dict(service_config.config) - if 'environment' in service_dict or 'env_file' in service_dict: - service_dict['environment'] = resolve_environment(service_config) - service_dict.pop('env_file', None) + if 'env_file' in service_dict: + service_dict['env_file'] = [ + expand_path(working_dir, path) + for path in to_list(service_dict['env_file']) + ] if 'volumes' in service_dict and service_dict.get('volume_driver') is None: service_dict['volumes'] = resolve_volume_paths(working_dir, service_dict) @@ -395,6 +394,10 @@ def process_service(service_config): def finalize_service(service_config): service_dict = dict(service_config.config) + if 'environment' in service_dict or 'env_file' in service_dict: + service_dict['environment'] = resolve_environment(service_dict) + service_dict.pop('env_file', None) + if 'volumes_from' in service_dict: service_dict['volumes_from'] = [ VolumeFromSpec.parse(vf) for vf in service_dict['volumes_from']] @@ -456,7 +459,7 @@ def merge_service_dicts(base, override): if key in base or key in override: d[key] = base.get(key, []) + override.get(key, []) - list_or_string_keys = ['dns', 'dns_search'] + list_or_string_keys = ['dns', 'dns_search', 'env_file'] for key in list_or_string_keys: if key in base or key in override: @@ -477,17 +480,6 @@ def merge_environment(base, override): return env -def get_env_files(working_dir, options): - if 'env_file' not in options: - return {} - - env_files = options.get('env_file', []) - if not isinstance(env_files, list): - env_files = [env_files] - - return [expand_path(working_dir, path) for path in env_files] - - def parse_environment(environment): if not environment: return {} diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index 334693f70..9ea68e39c 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -7,7 +7,6 @@ from pytest import skip from .. import unittest from compose.cli.docker_client import docker_client from compose.config.config import resolve_environment -from compose.config.config import ServiceConfig from compose.const import LABEL_PROJECT from compose.progress_stream import stream_output from compose.service import Service @@ -39,9 +38,7 @@ class DockerClientTestCase(unittest.TestCase): if 'command' not in kwargs: kwargs['command'] = ["top"] - service_config = ServiceConfig('.', None, name, kwargs) - kwargs['environment'] = resolve_environment(service_config) - + kwargs['environment'] = resolve_environment(kwargs) labels = dict(kwargs.setdefault('labels', {})) labels['com.docker.compose.test-name'] = self.id() diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index a5eeb64f9..2cd26e8f7 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -10,6 +10,7 @@ import py import pytest from compose.config import config +from compose.config.config import resolve_environment from compose.config.errors import ConfigurationError from compose.config.types import VolumeSpec from compose.const import IS_WINDOWS_PLATFORM @@ -973,65 +974,54 @@ class EnvTest(unittest.TestCase): os.environ['FILE_DEF_EMPTY'] = 'E2' os.environ['ENV_DEF'] = 'E3' - service_dict = make_service_dict( - 'foo', { - 'build': '.', - 'environment': { - 'FILE_DEF': 'F1', - 'FILE_DEF_EMPTY': '', - 'ENV_DEF': None, - 'NO_DEF': None - }, + service_dict = { + 'build': '.', + 'environment': { + 'FILE_DEF': 'F1', + 'FILE_DEF_EMPTY': '', + 'ENV_DEF': None, + 'NO_DEF': None }, - 'tests/' - ) - + } self.assertEqual( - service_dict['environment'], + resolve_environment(service_dict), {'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', 'NO_DEF': ''}, ) - def test_env_from_file(self): - service_dict = make_service_dict( - 'foo', - {'build': '.', 'env_file': 'one.env'}, - 'tests/fixtures/env', - ) + def test_resolve_environment_from_env_file(self): self.assertEqual( - service_dict['environment'], + resolve_environment({'env_file': ['tests/fixtures/env/one.env']}), {'ONE': '2', 'TWO': '1', 'THREE': '3', 'FOO': 'bar'}, ) - def test_env_from_multiple_files(self): - service_dict = make_service_dict( - 'foo', - {'build': '.', 'env_file': ['one.env', 'two.env']}, - 'tests/fixtures/env', - ) + def test_resolve_environment_with_multiple_env_files(self): + service_dict = { + 'env_file': [ + 'tests/fixtures/env/one.env', + 'tests/fixtures/env/two.env' + ] + } self.assertEqual( - service_dict['environment'], + resolve_environment(service_dict), {'ONE': '2', 'TWO': '1', 'THREE': '3', 'FOO': 'baz', 'DOO': 'dah'}, ) - def test_env_nonexistent_file(self): - options = {'env_file': 'nonexistent.env'} - self.assertRaises( - ConfigurationError, - lambda: make_service_dict('foo', options, 'tests/fixtures/env'), - ) + def test_resolve_environment_nonexistent_file(self): + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details( + {'foo': {'image': 'example', 'env_file': 'nonexistent.env'}}, + working_dir='tests/fixtures/env')) + + assert 'Couldn\'t find env file' in exc.exconly() + assert 'nonexistent.env' in exc.exconly() @mock.patch.dict(os.environ) - def test_resolve_environment_from_file(self): + def test_resolve_environment_from_env_file_with_empty_values(self): os.environ['FILE_DEF'] = 'E1' os.environ['FILE_DEF_EMPTY'] = 'E2' os.environ['ENV_DEF'] = 'E3' - service_dict = make_service_dict( - 'foo', - {'build': '.', 'env_file': 'resolve.env'}, - 'tests/fixtures/env', - ) self.assertEqual( - service_dict['environment'], + resolve_environment({'env_file': ['tests/fixtures/env/resolve.env']}), { 'FILE_DEF': u'bär', 'FILE_DEF_EMPTY': '', @@ -1378,6 +1368,8 @@ class ExtendsTest(unittest.TestCase): - 'envs' environment: - SECRET + - TEST_ONE=common + - TEST_TWO=common """) tmpdir.join('docker-compose.yml').write(""" ext: @@ -1388,12 +1380,20 @@ class ExtendsTest(unittest.TestCase): - 'envs' environment: - THING + - TEST_ONE=top """) commondir.join('envs').write(""" - COMMON_ENV_FILE=1 + COMMON_ENV_FILE + TEST_ONE=common-env-file + TEST_TWO=common-env-file + TEST_THREE=common-env-file + TEST_FOUR=common-env-file """) tmpdir.join('envs').write(""" - FROM_ENV_FILE=1 + TOP_ENV_FILE + TEST_ONE=top-env-file + TEST_TWO=top-env-file + TEST_THREE=top-env-file """) expected = [ @@ -1402,15 +1402,21 @@ class ExtendsTest(unittest.TestCase): 'image': 'example/app', 'environment': { 'SECRET': 'secret', - 'FROM_ENV_FILE': '1', - 'COMMON_ENV_FILE': '1', + 'TOP_ENV_FILE': 'secret', + 'COMMON_ENV_FILE': 'secret', 'THING': 'thing', + 'TEST_ONE': 'top', + 'TEST_TWO': 'common', + 'TEST_THREE': 'top-env-file', + 'TEST_FOUR': 'common-env-file', }, }, ] with mock.patch.dict(os.environ): os.environ['SECRET'] = 'secret' os.environ['THING'] = 'thing' + os.environ['COMMON_ENV_FILE'] = 'secret' + os.environ['TOP_ENV_FILE'] = 'secret' config = load_from_filename(str(tmpdir.join('docker-compose.yml'))) assert config == expected From 0dbd99bad2c6b3d99beda32e705777a912231053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Eckerstr=C3=B6m?= Date: Wed, 29 Apr 2015 10:22:24 +0200 Subject: [PATCH 27/35] Added support for url buid paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jonas Eckerström --- compose/config/config.py | 27 ++++++++++++++++++++++++--- tests/unit/config/config_test.py | 14 ++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index cbebeca83..05ef8a59d 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -75,6 +75,13 @@ ALLOWED_KEYS = DOCKER_CONFIG_KEYS + [ 'external_links', ] +DOCKER_VALID_URL_PREFIXES = ( + 'http://', + 'https://', + 'git://', + 'github.com/', + 'git@', +) SUPPORTED_FILENAMES = [ 'docker-compose.yml', @@ -376,7 +383,7 @@ def process_service(service_config): service_dict['volumes'] = resolve_volume_paths(working_dir, service_dict) if 'build' in service_dict: - service_dict['build'] = expand_path(working_dir, service_dict['build']) + service_dict['build'] = resolve_build_path(working_dir, service_dict['build']) if 'labels' in service_dict: service_dict['labels'] = parse_labels(service_dict['labels']) @@ -548,11 +555,25 @@ def resolve_volume_path(working_dir, volume): return container_path +def resolve_build_path(working_dir, build_path): + if is_url(build_path): + return build_path + return expand_path(working_dir, build_path) + + +def is_url(build_path): + return build_path.startswith(DOCKER_VALID_URL_PREFIXES) + + def validate_paths(service_dict): if 'build' in service_dict: build_path = service_dict['build'] - if not os.path.exists(build_path) or not os.access(build_path, os.R_OK): - raise ConfigurationError("build path %s either does not exist or is not accessible." % build_path) + if ( + not is_url(build_path) and + (not os.path.exists(build_path) or not os.access(build_path, os.R_OK)) + ): + raise ConfigurationError( + "build path %s either does not exist or is not accessible." % build_path) def merge_path_mappings(base, override): diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 2cd26e8f7..6de794ade 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -1497,6 +1497,20 @@ class BuildPathTest(unittest.TestCase): service_dict = load_from_filename('tests/fixtures/build-path/docker-compose.yml') self.assertEquals(service_dict, [{'name': 'foo', 'build': self.abs_context_path}]) + def test_valid_url_path(self): + valid_urls = [ + 'git://github.com/docker/docker', + 'git@github.com:docker/docker.git', + 'git@bitbucket.org:atlassianlabs/atlassian-docker.git', + 'https://github.com/docker/docker.git', + 'http://github.com/docker/docker.git', + ] + for valid_url in valid_urls: + service_dict = config.load(build_config_details({ + 'validurl': {'build': valid_url}, + }, '.', None)) + assert service_dict[0]['build'] == valid_url + class GetDefaultConfigFilesTestCase(unittest.TestCase): From 69e956ce8b7d278dccf705d74280836f20fbf68a Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 19 Nov 2015 15:46:14 -0500 Subject: [PATCH 28/35] Add integration test and docs for build with a git url. Signed-off-by: Daniel Nephin --- compose/config/config.py | 3 ++- docs/compose-file.md | 11 +++++++---- tests/integration/service_test.py | 7 +++++++ tests/unit/config/config_test.py | 16 +++++++++++++++- 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 05ef8a59d..b5847d2ec 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -573,7 +573,8 @@ def validate_paths(service_dict): (not os.path.exists(build_path) or not os.access(build_path, os.R_OK)) ): raise ConfigurationError( - "build path %s either does not exist or is not accessible." % build_path) + "build path %s either does not exist, is not accessible, " + "or is not a valid URL." % build_path) def merge_path_mappings(base, override): diff --git a/docs/compose-file.md b/docs/compose-file.md index 51d1f5e1a..800d2aa98 100644 --- a/docs/compose-file.md +++ b/docs/compose-file.md @@ -31,15 +31,18 @@ definition. ### build -Path to a directory containing a Dockerfile. When the value supplied is a -relative path, it is interpreted as relative to the location of the yml file -itself. This directory is also the build context that is sent to the Docker daemon. +Either a path to a directory containing a Dockerfile, or a url to a git repository. + +When the value supplied is a relative path, it is interpreted as relative to the +location of the Compose file. This directory is also the build context that is +sent to the Docker daemon. Compose will build and tag it with a generated name, and use that image thereafter. build: /path/to/build/dir -Using `build` together with `image` is not allowed. Attempting to do so results in an error. +Using `build` together with `image` is not allowed. Attempting to do so results in +an error. ### cap_add, cap_drop diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 6808280f0..01133d585 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -501,6 +501,13 @@ class ServiceTest(DockerClientTestCase): self.create_service('web', build=text_type(base_dir)).build() self.assertEqual(len(self.client.images(name='composetest_web')), 1) + def test_build_with_git_url(self): + build_url = "https://github.com/dnephin/docker-build-from-url.git" + service = self.create_service('buildwithurl', build=build_url) + self.addCleanup(self.client.remove_image, service.image_name) + service.build() + assert service.image() + def test_start_container_stays_unpriviliged(self): service = self.create_service('web') container = create_and_start_container(service).inspect() diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 6de794ade..e15ac3502 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -1497,13 +1497,14 @@ class BuildPathTest(unittest.TestCase): service_dict = load_from_filename('tests/fixtures/build-path/docker-compose.yml') self.assertEquals(service_dict, [{'name': 'foo', 'build': self.abs_context_path}]) - def test_valid_url_path(self): + def test_valid_url_in_build_path(self): valid_urls = [ 'git://github.com/docker/docker', 'git@github.com:docker/docker.git', 'git@bitbucket.org:atlassianlabs/atlassian-docker.git', 'https://github.com/docker/docker.git', 'http://github.com/docker/docker.git', + 'github.com/docker/docker.git', ] for valid_url in valid_urls: service_dict = config.load(build_config_details({ @@ -1511,6 +1512,19 @@ class BuildPathTest(unittest.TestCase): }, '.', None)) assert service_dict[0]['build'] == valid_url + def test_invalid_url_in_build_path(self): + invalid_urls = [ + 'example.com/bogus', + 'ftp://example.com/', + '/path/does/not/exist', + ] + for invalid_url in invalid_urls: + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details({ + 'invalidurl': {'build': invalid_url}, + }, '.', None)) + assert 'build path' in exc.exconly() + class GetDefaultConfigFilesTestCase(unittest.TestCase): From e67419065ac55e8e9aae9bce73021e7da571733c Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 26 Nov 2015 15:06:30 +0000 Subject: [PATCH 29/35] Fix ports validation test We were essentially only testing that *at least one* of the invalid values fails the validation check, rather than that *all* of them fail. Signed-off-by: Aanand Prasad --- tests/unit/config/config_test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index e15ac3502..a33944e77 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -264,9 +264,8 @@ class ConfigTest(unittest.TestCase): assert services[0]['name'] == valid_name def test_config_invalid_ports_format_validation(self): - expected_error_msg = "Service 'web' configuration key 'ports' contains an invalid type" - with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): - for invalid_ports in [{"1": "8000"}, False, 0, "8000", 8000, ["8000", "8000"]]: + for invalid_ports in [{"1": "8000"}, False, 0, "8000", 8000, ["8000", "8000"]]: + with pytest.raises(ConfigurationError): config.load( build_config_details( {'web': {'image': 'busybox', 'ports': invalid_ports}}, From ab36c9c6cd5df22f3647b1c619b1ae4c6b604208 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 26 Nov 2015 18:52:14 +0000 Subject: [PATCH 30/35] Refactor ports section of fields schema Signed-off-by: Aanand Prasad --- compose/config/fields_schema.json | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/compose/config/fields_schema.json b/compose/config/fields_schema.json index 9cbcfd1b2..3f1f10fa6 100644 --- a/compose/config/fields_schema.json +++ b/compose/config/fields_schema.json @@ -83,16 +83,8 @@ "ports": { "type": "array", "items": { - "oneOf": [ - { - "type": "string", - "format": "ports" - }, - { - "type": "number", - "format": "ports" - } - ] + "type": ["string", "number"], + "format": "ports" }, "uniqueItems": true }, From 527bf3b0234e28d8de73d385c298ae1e0ecd15e2 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 26 Nov 2015 18:54:30 +0000 Subject: [PATCH 31/35] Fix ports validation message - The `raises` kwarg to the `cls_check` decorator was being used incorrectly (it should be an exception class, not an object). - We need to check for `error.cause` and get the message out of the exception object. NB: The particular case where validation fails in the case of `ports` is only when ranges don't match in length - no further validation is currently performed client-side. Signed-off-by: Aanand Prasad --- compose/config/validation.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/compose/config/validation.py b/compose/config/validation.py index 38020366d..24a45e768 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -36,16 +36,12 @@ DOCKER_CONFIG_HINTS = { VALID_NAME_CHARS = '[a-zA-Z0-9\._\-]' -@FormatChecker.cls_checks( - format="ports", - raises=ValidationError( - "Invalid port formatting, it should be " - "'[[remote_ip:]remote_port:]port[/protocol]'")) +@FormatChecker.cls_checks(format="ports", raises=ValidationError) def format_ports(instance): try: split_port(instance) - except ValueError: - return False + except ValueError as e: + raise ValidationError(six.text_type(e)) return True @@ -184,6 +180,10 @@ def handle_generic_service_error(error, service_name): config_key, required_keys) + elif error.cause: + error_msg = six.text_type(error.cause) + msg_format = "Service '{}' configuration key {} is invalid: {}" + elif error.path: msg_format = "Service '{}' configuration key {} value {}" From e6fbca42a13c6eda9af333f8775b8369ed0ec585 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 26 Nov 2015 19:17:13 +0000 Subject: [PATCH 32/35] Split out ports validation tests into type, uniqueness, format Signed-off-by: Aanand Prasad --- tests/unit/config/config_test.py | 86 ++++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 22 deletions(-) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index a33944e77..863da3b2d 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -263,28 +263,6 @@ class ConfigTest(unittest.TestCase): 'common.yml')) assert services[0]['name'] == valid_name - def test_config_invalid_ports_format_validation(self): - for invalid_ports in [{"1": "8000"}, False, 0, "8000", 8000, ["8000", "8000"]]: - with pytest.raises(ConfigurationError): - config.load( - build_config_details( - {'web': {'image': 'busybox', 'ports': invalid_ports}}, - 'working_dir', - 'filename.yml' - ) - ) - - def test_config_valid_ports_format_validation(self): - valid_ports = [["8000", "9000"], ["8000/8050"], ["8000"], [8000], ["49153-49154:3002-3003"]] - for ports in valid_ports: - config.load( - build_config_details( - {'web': {'image': 'busybox', 'ports': ports}}, - 'working_dir', - 'filename.yml' - ) - ) - def test_config_hint(self): expected_error_msg = "(did you mean 'privileged'?)" with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): @@ -558,6 +536,70 @@ class ConfigTest(unittest.TestCase): assert "which is an invalid type" in exc.exconly() +class PortsTest(unittest.TestCase): + INVALID_PORTS_TYPES = [ + {"1": "8000"}, + False, + "8000", + 8000, + ] + + NON_UNIQUE_SINGLE_PORTS = [ + ["8000", "8000"], + ] + + INVALID_PORT_MAPPINGS = [ + ["8000-8001:8000"], + ] + + VALID_SINGLE_PORTS = [ + ["8000"], + ["8000/tcp"], + ["8000", "9000"], + [8000], + [8000, 9000], + ] + + VALID_PORT_MAPPINGS = [ + ["8000:8050"], + ["49153-49154:3002-3003"], + ] + + def test_config_invalid_ports_type_validation(self): + for invalid_ports in self.INVALID_PORTS_TYPES: + with pytest.raises(ConfigurationError) as exc: + self.check_config({'ports': invalid_ports}) + + assert "contains an invalid type" in exc.value.msg + + def test_config_non_unique_ports_validation(self): + for invalid_ports in self.NON_UNIQUE_SINGLE_PORTS: + with pytest.raises(ConfigurationError) as exc: + self.check_config({'ports': invalid_ports}) + + assert "non-unique" in exc.value.msg + + def test_config_invalid_ports_format_validation(self): + for invalid_ports in self.INVALID_PORT_MAPPINGS: + with pytest.raises(ConfigurationError) as exc: + self.check_config({'ports': invalid_ports}) + + assert "Port ranges don't match in length" in exc.value.msg + + def test_config_valid_ports_format_validation(self): + for valid_ports in self.VALID_SINGLE_PORTS + self.VALID_PORT_MAPPINGS: + self.check_config({'ports': valid_ports}) + + def check_config(self, cfg): + config.load( + build_config_details( + {'web': dict(image='busybox', **cfg)}, + 'working_dir', + 'filename.yml' + ) + ) + + class InterpolationTest(unittest.TestCase): @mock.patch.dict(os.environ) def test_config_file_with_environment_variable(self): From 96f4a42a3572ec05bb37116cd53664fb6df629f9 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 26 Nov 2015 19:17:58 +0000 Subject: [PATCH 33/35] Validate the 'expose' option Signed-off-by: Aanand Prasad --- compose/config/fields_schema.json | 5 ++++- compose/config/validation.py | 14 +++++++++++++- tests/unit/config/config_test.py | 27 +++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/compose/config/fields_schema.json b/compose/config/fields_schema.json index 3f1f10fa6..7d5220e3f 100644 --- a/compose/config/fields_schema.json +++ b/compose/config/fields_schema.json @@ -41,7 +41,10 @@ "expose": { "type": "array", - "items": {"type": ["string", "number"]}, + "items": { + "type": ["string", "number"], + "format": "expose" + }, "uniqueItems": true }, diff --git a/compose/config/validation.py b/compose/config/validation.py index 24a45e768..d16bdb9d3 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -1,6 +1,7 @@ import json import logging import os +import re import sys import six @@ -34,6 +35,7 @@ DOCKER_CONFIG_HINTS = { VALID_NAME_CHARS = '[a-zA-Z0-9\._\-]' +VALID_EXPOSE_FORMAT = r'^\d+(\/[a-zA-Z]+)?$' @FormatChecker.cls_checks(format="ports", raises=ValidationError) @@ -45,6 +47,16 @@ def format_ports(instance): return True +@FormatChecker.cls_checks(format="expose", raises=ValidationError) +def format_expose(instance): + if isinstance(instance, six.string_types): + if not re.match(VALID_EXPOSE_FORMAT, instance): + raise ValidationError( + "should be of the format 'PORT[/PROTOCOL]'") + + return True + + @FormatChecker.cls_checks(format="bool-value-in-mapping") def format_boolean_in_environment(instance): """ @@ -273,7 +285,7 @@ def validate_against_fields_schema(config, filename): _validate_against_schema( config, "fields_schema.json", - format_checker=["ports", "bool-value-in-mapping"], + format_checker=["ports", "expose", "bool-value-in-mapping"], filename=filename) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 863da3b2d..20705d555 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -590,6 +590,33 @@ class PortsTest(unittest.TestCase): for valid_ports in self.VALID_SINGLE_PORTS + self.VALID_PORT_MAPPINGS: self.check_config({'ports': valid_ports}) + def test_config_invalid_expose_type_validation(self): + for invalid_expose in self.INVALID_PORTS_TYPES: + with pytest.raises(ConfigurationError) as exc: + self.check_config({'expose': invalid_expose}) + + assert "contains an invalid type" in exc.value.msg + + def test_config_non_unique_expose_validation(self): + for invalid_expose in self.NON_UNIQUE_SINGLE_PORTS: + with pytest.raises(ConfigurationError) as exc: + self.check_config({'expose': invalid_expose}) + + assert "non-unique" in exc.value.msg + + def test_config_invalid_expose_format_validation(self): + # Valid port mappings ARE NOT valid 'expose' entries + for invalid_expose in self.INVALID_PORT_MAPPINGS + self.VALID_PORT_MAPPINGS: + with pytest.raises(ConfigurationError) as exc: + self.check_config({'expose': invalid_expose}) + + assert "should be of the format" in exc.value.msg + + def test_config_valid_expose_format_validation(self): + # Valid single ports ARE valid 'expose' entries + for valid_expose in self.VALID_SINGLE_PORTS: + self.check_config({'expose': valid_expose}) + def check_config(self, cfg): config.load( build_config_details( From aaf66e34856361cfbf63638fa5bad93d6d75643a Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 21 Oct 2015 20:13:43 -0400 Subject: [PATCH 34/35] FAQ document for Compose Signed-off-by: Daniel Nephin --- docs/faq.md | 139 +++++++++++++++++++++++++ docs/index.md | 1 + script/travis/render-bintray-config.py | 4 +- 3 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 docs/faq.md diff --git a/docs/faq.md b/docs/faq.md new file mode 100644 index 000000000..b36eb5ac5 --- /dev/null +++ b/docs/faq.md @@ -0,0 +1,139 @@ + + +# Frequently asked questions + +If you don’t see your question here, feel free to drop by `#docker-compose` on +freenode IRC and ask the community. + +## Why do my services take 10 seconds to stop? + +Compose stop attempts to stop a container by sending a `SIGTERM`. It then waits +for a [default timeout of 10 seconds](./reference/stop.md). After the timeout, +a `SIGKILL` is sent to the container to forcefully kill it. If you +are waiting for this timeout, it means that your containers aren't shutting down +when they receive the `SIGTERM` signal. + +There has already been a lot written about this problem of +[processes handling signals](https://medium.com/@gchudnov/trapping-signals-in-docker-containers-7a57fdda7d86) +in containers. + +To fix this problem, try the following: + +* Make sure you're using the JSON form of `CMD` and `ENTRYPOINT` +in your Dockerfile. + + For example use `["program", "arg1", "arg2"]` not `"program arg1 arg2"`. + Using the string form causes Docker to run your process using `bash` which + doesn't handle signals properly. Compose always uses the JSON form, so don't + worry if you override the command or entrypoint in your Compose file. + +* If you are able, modify the application that you're running to +add an explicit signal handler for `SIGTERM`. + +* If you can't modify the application, wrap the application in a lightweight init +system (like [s6](http://skarnet.org/software/s6/)) or a signal proxy (like +[dumb-init](https://github.com/Yelp/dumb-init) or +[tini](https://github.com/krallin/tini)). Either of these wrappers take care of +handling `SIGTERM` properly. + +## How do I run multiple copies of a Compose file on the same host? + +Compose uses the project name to create unique identifiers for all of a +project's containers and other resources. To run multiple copies of a project, +set a custom project name using the [`-p` command line +option](./reference/docker-compose.md) or the [`COMPOSE_PROJECT_NAME` +environment variable](./reference/overview.md#compose-project-name). + +## What's the difference between `up`, `run`, and `start`? + +Typically, you want `docker-compose up`. Use `up` to start or restart all the +services defined in a `docker-compose.yml`. In the default "attached" +mode, you'll see all the logs from all the containers. In "detached" mode (`-d`), +Compose exits after starting the containers, but the containers continue to run +in the background. + +The `docker-compose run` command is for running "one-off" or "adhoc" tasks. It +requires the service name you want to run and only starts containers for services +that the running service depends on. Use `run` to run tests or perform +an administrative task such as removing or adding data to a data volume +container. The `run` command acts like `docker run -ti` in that it opens an +interactive terminal to the container and returns an exit status matching the +exit status of the process in the container. + +The `docker-compose start` command is useful only to restart containers +that were previously created, but were stopped. It never creates new +containers. + +## Can I use json instead of yaml for my Compose file? + +Yes. [Yaml is a superset of json](http://stackoverflow.com/a/1729545/444646) so +any JSON file should be valid Yaml. To use a JSON file with Compose, +specify the filename to use, for example: + +```bash +docker-compose -f docker-compose.json up +``` + +## How do I get Compose to wait for my database to be ready before starting my application? + +Unfortunately, Compose won't do that for you but for a good reason. + +The problem of waiting for a database to be ready is really just a subset of a +much larger problem of distributed systems. In production, your database could +become unavailable or move hosts at any time. The application needs to be +resilient to these types of failures. + +To handle this, the application would attempt to re-establish a connection to +the database after a failure. If the application retries the connection, +it should eventually be able to connect to the database. + +To wait for the application to be in a good state, you can implement a +healthcheck. A healthcheck makes a request to the application and checks +the response for a success status code. If it is not successful it waits +for a short period of time, and tries again. After some timeout value, the check +stops trying and report a failure. + +If you need to run tests against your application, you can start by running a +healthcheck. Once the healthcheck gets a successful response, you can start +running your tests. + + +## Should I include my code with `COPY`/`ADD` or a volume? + +You can add your code to the image using `COPY` or `ADD` directive in a +`Dockerfile`. This is useful if you need to relocate your code along with the +Docker image, for example when you're sending code to another environment +(production, CI, etc). + +You should use a `volume` if you want to make changes to your code and see them +reflected immediately, for example when you're developing code and your server +supports hot code reloading or live-reload. + +There may be cases where you'll want to use both. You can have the image +include the code using a `COPY`, and use a `volume` in your Compose file to +include the code from the host during development. The volume overrides +the directory contents of the image. + +## Where can I find example compose files? + +There are [many examples of Compose files on +github](https://github.com/search?q=in%3Apath+docker-compose.yml+extension%3Ayml&type=Code). + + +## Compose documentation + +- [Installing Compose](install.md) +- [Get started with Django](django.md) +- [Get started with Rails](rails.md) +- [Get started with WordPress](wordpress.md) +- [Command line reference](./reference/index.md) +- [Compose file reference](compose-file.md) diff --git a/docs/index.md b/docs/index.md index 279154eef..8b32a7541 100644 --- a/docs/index.md +++ b/docs/index.md @@ -59,6 +59,7 @@ Compose has commands for managing the whole lifecycle of your application: - [Get started with Django](django.md) - [Get started with Rails](rails.md) - [Get started with WordPress](wordpress.md) +- [Frequently asked questions](faq.md) - [Command line reference](./reference/index.md) - [Compose file reference](compose-file.md) diff --git a/script/travis/render-bintray-config.py b/script/travis/render-bintray-config.py index 6aa468d6d..fc5d409a0 100755 --- a/script/travis/render-bintray-config.py +++ b/script/travis/render-bintray-config.py @@ -1,4 +1,6 @@ #!/usr/bin/env python +from __future__ import print_function + import datetime import os.path import sys @@ -6,4 +8,4 @@ import sys os.environ['DATE'] = str(datetime.date.today()) for line in sys.stdin: - print os.path.expandvars(line), + print(os.path.expandvars(line), end='') From 7240ff35eeac36d7fb53892af495bb172e0e00c2 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 2 Dec 2015 17:06:17 -0800 Subject: [PATCH 35/35] Bump 1.5.2 Signed-off-by: Daniel Nephin --- CHANGELOG.md | 22 ++++++++++++++++++++++ compose/__init__.py | 2 +- docs/install.md | 6 +++--- script/run.sh | 2 +- 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50aabcb8e..955184272 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,28 @@ Change log ========== +1.5.2 (2015-12-03) +------------------ + +- Fixed a bug which broke the use of `environment` and `env_file` with + `extends`, and caused environment keys without values to have a `None` + value, instead of a value from the host environment. + +- Fixed a regression in 1.5.1 that caused a warning about volumes to be + raised incorrectly when containers were recreated. + +- Fixed a bug which prevented building a `Dockerfile` that used `ADD ` + +- Fixed a bug with `docker-compose restart` which prevented it from + starting stopped containers. + +- Fixed handling of SIGTERM and SIGINT to properly stop containers + +- Add support for using a url as the value of `build` + +- Improved the validation of the `expose` option + + 1.5.1 (2015-11-12) ------------------ diff --git a/compose/__init__.py b/compose/__init__.py index 5f2b332af..03d61530a 100644 --- a/compose/__init__.py +++ b/compose/__init__.py @@ -1,3 +1,3 @@ from __future__ import unicode_literals -__version__ = '1.5.1' +__version__ = '1.5.2' diff --git a/docs/install.md b/docs/install.md index 5bbd6e595..6635ddf19 100644 --- a/docs/install.md +++ b/docs/install.md @@ -39,7 +39,7 @@ which the release page specifies, in your terminal. The following is an example command illustrating the format: - curl -L https://github.com/docker/compose/releases/download/1.5.1/docker-compose-`uname -s`-`uname -m` > /usr/local/bin/docker-compose + curl -L https://github.com/docker/compose/releases/download/1.5.2/docker-compose-`uname -s`-`uname -m` > /usr/local/bin/docker-compose If you have problems installing with `curl`, see [Alternative Install Options](#alternative-install-options). @@ -54,7 +54,7 @@ which the release page specifies, in your terminal. 7. Test the installation. $ docker-compose --version - docker-compose version: 1.5.1 + docker-compose version: 1.5.2 ## Alternative install options @@ -77,7 +77,7 @@ to get started. Compose can also be run inside a container, from a small bash script wrapper. To install compose as a container run: - $ curl -L https://github.com/docker/compose/releases/download/1.5.1/run.sh > /usr/local/bin/docker-compose + $ curl -L https://github.com/docker/compose/releases/download/1.5.2/run.sh > /usr/local/bin/docker-compose $ chmod +x /usr/local/bin/docker-compose ## Master builds diff --git a/script/run.sh b/script/run.sh index 9563b2e9c..c5f7cc865 100755 --- a/script/run.sh +++ b/script/run.sh @@ -15,7 +15,7 @@ set -e -VERSION="1.5.1" +VERSION="1.5.2" IMAGE="docker/compose:$VERSION"