From c9ca5e86b0e8e0e1c8cd2345da5a55739b430242 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@docker.com>
Date: Fri, 13 Nov 2015 17:39:02 -0500
Subject: [PATCH 1/7] Remove project name validation
project name is already normalized to a valid name before creating a service.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
---
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 dd2399ee3..9004260f6 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
@@ -122,9 +121,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 808c391cd..78edf3bf7 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 068edfa31345760d334b952d27e546077b077388 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@docker.com>
Date: Fri, 13 Nov 2015 18:20:09 -0500
Subject: [PATCH 2/7] 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 <dnephin@docker.com>
---
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 84b6748c9..8ec352ecc 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 operator
@@ -12,6 +14,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
@@ -198,8 +201,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
@@ -353,6 +360,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)
@@ -370,12 +378,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 e29a2eb5a..5caa1ea37 100644
--- a/compose/project.py
+++ b/compose/project.py
@@ -19,10 +19,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
log = logging.getLogger(__name__)
@@ -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 9004260f6..be0502c27 100644
--- a/compose/service.py
+++ b/compose/service.py
@@ -70,6 +70,7 @@ class BuildError(Exception):
self.reason = reason
+# TODO: remove
class ConfigError(ValueError):
pass
@@ -86,9 +87,6 @@ class NoSuchImageError(Exception):
VolumeSpec = namedtuple('VolumeSpec', 'external internal mode')
-VolumeFromSpec = namedtuple('VolumeFromSpec', 'source mode')
-
-
ServiceName = namedtuple('ServiceName', 'project service number')
@@ -1029,21 +1027,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 34869ab88..34bf93fcb 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 78edf3bf7..efcc58e26 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 12b82a20ff331f420c040dbb9cf1ea44fd74d7d5 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@docker.com>
Date: Fri, 13 Nov 2015 18:29:25 -0500
Subject: [PATCH 3/7] Move restart spec to the config.types module.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
---
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 8ec352ecc..9b03ea4ff 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -14,6 +14,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
@@ -392,6 +393,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 be0502c27..33d9a7bec 100644
--- a/compose/service.py
+++ b/compose/service.py
@@ -648,8 +648,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)
@@ -667,7 +665,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'),
@@ -1043,24 +1041,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 34bf93fcb..15d8ca072 100644
--- a/tests/integration/service_test.py
+++ b/tests/integration/service_test.py
@@ -786,23 +786,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)
@@ -817,17 +815,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 efec2aae6c86b6577f30451d54ac7030dbf39b13 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@docker.com>
Date: Fri, 13 Nov 2015 18:58:24 -0500
Subject: [PATCH 4/7] 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 <dnephin@docker.com>
---
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 9b03ea4ff..55adcaf28 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -14,6 +14,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
@@ -379,6 +380,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 33d9a7bec..2bb0030f0 100644
--- a/compose/service.py
+++ b/compose/service.py
@@ -640,6 +640,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]
@@ -648,9 +649,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))
@@ -672,8 +670,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'),
@@ -1057,31 +1055,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 15d8ca072..27a290050 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
@@ -139,37 +137,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 dac75b07dc16d9b6f08654301dd9ddd5d0b48393 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@docker.com>
Date: Fri, 13 Nov 2015 19:40:10 -0500
Subject: [PATCH 5/7] Move volume parsing to config.types module
This removes the last of the old service.ConfigError
Signed-off-by: Daniel Nephin <dnephin@docker.com>
---
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 55adcaf28..5b1de5efc 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -17,6 +17,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
@@ -397,6 +398,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 2bb0030f0..6340d074e 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
@@ -70,11 +69,6 @@ class BuildError(Exception):
self.reason = reason
-# TODO: remove
-class ConfigError(ValueError):
- pass
-
-
class NeedsBuildError(Exception):
def __init__(self, service):
self.service = service
@@ -84,9 +78,6 @@ class NoSuchImageError(Exception):
pass
-VolumeSpec = namedtuple('VolumeSpec', 'external internal mode')
-
-
ServiceName = namedtuple('ServiceName', 'project service number')
@@ -598,8 +589,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'),
@@ -884,11 +874,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
@@ -910,7 +899,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 {}
]
@@ -957,56 +946,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 7ca6e8194..73a1d66ce 100644
--- a/tests/acceptance/cli_test.py
+++ b/tests/acceptance/cli_test.py
@@ -37,7 +37,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 27a290050..5dd3d2e68 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
@@ -120,7 +121,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'))
@@ -182,7 +183,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()
@@ -195,11 +198,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']
@@ -213,7 +215,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',
@@ -267,7 +269,7 @@ class ServiceTest(DockerClientTestCase):
service = self.create_service(
'db',
environment={'FOO': '1'},
- volumes=['/etc'],
+ volumes=[VolumeSpec.parse('/etc')],
entrypoint=['top'],
command=['-d', '1']
)
@@ -305,7 +307,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']
)
@@ -343,10 +345,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',
)
@@ -354,7 +354,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(
@@ -864,22 +864,11 @@ class ServiceTest(DockerClientTestCase):
for pair in expected.items():
self.assertIn(pair, labels)
- service.kill()
- remove_stopped(service)
-
- 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 de2d1a701..f5de50ee1 100644
--- a/tests/integration/testcases.py
+++ b/tests/integration/testcases.py
@@ -7,9 +7,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
@@ -45,13 +43,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 efcc58e26..a439f0da9 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 effa9834a5722d2f5f5738087f0207c1eca9fd0b Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@docker.com>
Date: Fri, 13 Nov 2015 19:49:14 -0500
Subject: [PATCH 6/7] Remove unnecessary intermediate variables in
get_container_host_config.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
---
compose/service.py | 44 ++++++++++++--------------------------------
1 file changed, 12 insertions(+), 32 deletions(-)
diff --git a/compose/service.py b/compose/service.py
index 6340d074e..08d563e93 100644
--- a/compose/service.py
+++ b/compose/service.py
@@ -496,7 +496,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, [
@@ -618,54 +618,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 533f33271a9be73546673aa9aacd823ca8ea9c38 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@docker.com>
Date: Tue, 17 Nov 2015 13:35:28 -0500
Subject: [PATCH 7/7] Move service sorting to config package.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
---
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 5b1de5efc..9d438ca12 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -14,6 +14,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
@@ -214,10 +216,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)
@@ -638,17 +640,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 5caa1ea37..30e81693c 100644
--- a/compose/project.py
+++ b/compose/project.py
@@ -9,7 +9,7 @@ from docker.errors import NotFound
from . import parallel
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 .service import ServiceNet
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)
@@ -404,7 +361,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 f5de50ee1..a2218d6bc 100644
--- a/tests/integration/testcases.py
+++ b/tests/integration/testcases.py
@@ -8,6 +8,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 a439f0da9..e87ce5920 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