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/__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 84b6748c9..9d438ca12 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,12 @@ 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 +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 @@ -198,16 +206,20 @@ 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 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) @@ -353,6 +365,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 +383,33 @@ 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']) 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']] + + 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']) + + 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 @@ -606,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/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/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/config/types.py b/compose/config/types.py new file mode 100644 index 000000000..cec1f6cfd --- /dev/null +++ b/compose/config/types.py @@ -0,0 +1,120 @@ +""" +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')): + + @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) + + +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)} + + +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 + + +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/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/project.py b/compose/project.py index e29a2eb5a..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 @@ -19,61 +19,13 @@ 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__) -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 [ - parse_volume_from_spec(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. @@ -101,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) @@ -192,16 +144,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 ' @@ -410,7 +361,3 @@ class NoSuchService(Exception): def __str__(self): return self.msg - - -class DependencyError(ConfigurationError): - pass diff --git a/compose/service.py b/compose/service.py index dd2399ee3..08d563e93 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,9 +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.validation import VALID_NAME_CHARS +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 @@ -71,10 +69,6 @@ class BuildError(Exception): self.reason = reason -class ConfigError(ValueError): - pass - - class NeedsBuildError(Exception): def __init__(self, service): self.service = service @@ -84,12 +78,6 @@ class NoSuchImageError(Exception): pass -VolumeSpec = namedtuple('VolumeSpec', 'external internal mode') - - -VolumeFromSpec = namedtuple('VolumeFromSpec', 'source mode') - - ServiceName = namedtuple('ServiceName', 'project service number') @@ -122,9 +110,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 @@ -511,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, [ @@ -604,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'), @@ -634,58 +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) - - 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] - - 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) - - 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, - restart_policy=restart, - cap_add=cap_add, - cap_drop=cap_drop, + devices=options.get('devices'), + dns=options.get('dns'), + dns_search=options.get('dns_search'), + restart_policy=options.get('restart'), + 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=extra_hosts, - read_only=read_only, - pid_mode=pid, - security_opt=security_opt, + extra_hosts=options.get('extra_hosts'), + read_only=options.get('read_only'), + 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): @@ -894,11 +854,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 @@ -920,7 +879,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 {} ] @@ -967,56 +926,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 @@ -1033,21 +942,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 @@ -1064,24 +958,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 @@ -1098,31 +974,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/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 2ce319005..443ff9783 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -3,12 +3,13 @@ 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.config.types import VolumeSpec 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): @@ -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 34869ab88..5dd3d2e68 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -14,6 +14,8 @@ from .. import mock 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 @@ -21,13 +23,10 @@ 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 from compose.service import Service -from compose.service import VolumeFromSpec def create_and_start_container(service, **override_options): @@ -122,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')) @@ -139,37 +138,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) @@ -215,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() @@ -228,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'] @@ -246,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', @@ -300,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'] ) @@ -338,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'] ) @@ -376,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', ) @@ -387,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( @@ -786,23 +753,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 +782,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']) @@ -909,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 2c5ca9fdd..334693f70 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -6,7 +6,6 @@ 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 @@ -41,13 +40,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/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', diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index c69e34306..a5eeb64f9 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 @@ -32,7 +33,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)]) @@ -76,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( @@ -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'}, }, ] @@ -231,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( @@ -512,6 +534,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) @@ -603,14 +648,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) @@ -1008,19 +1050,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): @@ -1267,8 +1311,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/sort_service_test.py b/tests/unit/config/sort_services_test.py similarity index 94% rename from tests/unit/sort_service_test.py rename to tests/unit/config/sort_services_test.py index a7e522a1d..8d0c3ae40 100644 --- a/tests/unit/sort_service_test.py +++ b/tests/unit/config/sort_services_test.py @@ -1,6 +1,7 @@ -from .. import unittest -from compose.project import DependencyError -from compose.project import sort_service_dicts +from compose.config.errors import DependencyError +from compose.config.sort_services import sort_service_dicts +from compose.config.types import VolumeFromSpec +from tests import unittest class SortServiceTest(unittest.TestCase): @@ -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' diff --git a/tests/unit/config/types_test.py b/tests/unit/config/types_test.py new file mode 100644 index 000000000..4df665485 --- /dev/null +++ b/tests/unit/config/types_test.py @@ -0,0 +1,66 @@ +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(): + 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' + } + + +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/project_test.py b/tests/unit/project_test.py index b38f5c783..f4c6f8ca1 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 @@ -33,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': ['volume'] - }, - { - '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 = [ { @@ -167,7 +145,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 +168,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 +184,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 85d1479d5..1c8b441f3 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -2,11 +2,11 @@ from __future__ import absolute_import from __future__ import unicode_literals import docker -import pytest from .. import mock from .. import unittest -from compose.const import IS_WINDOWS_PLATFORM +from compose.config.types import VolumeFromSpec +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 @@ -14,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 @@ -22,11 +21,8 @@ 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 -from compose.service import VolumeSpec from compose.service import warn_on_masked_volume @@ -35,11 +31,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 = [] @@ -589,46 +580,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', @@ -652,19 +609,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 = { @@ -690,8 +647,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, ) @@ -720,7 +677,7 @@ class ServiceVolumesTest(unittest.TestCase): service = Service( 'web', image='busybox', - volumes=['/host/path:/data'], + volumes=[VolumeSpec.parse('/host/path:/data')], client=self.mock_client, ) @@ -788,22 +745,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])