From 79b20eb53f3c580b5202000708e9c39fae8f2c92 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Mon, 4 Dec 2017 22:47:33 -0800 Subject: [PATCH] Add support for mount syntax Signed-off-by: Joffrey F --- compose/config/config.py | 40 +++++++------ compose/config/config_schema_v2.3.json | 34 ++++++++++- compose/config/serialize.py | 6 ++ compose/config/types.py | 13 ++++ compose/service.py | 63 ++++++++++++++------ compose/utils.py | 2 +- compose/volume.py | 9 ++- tests/acceptance/cli_test.py | 22 ++++--- tests/helpers.py | 13 ++-- tests/integration/project_test.py | 21 +++++++ tests/integration/service_test.py | 82 ++++++++++++++++++++++++++ tests/integration/testcases.py | 6 +- tests/unit/config/config_test.py | 53 ++++++++++++++++- tests/unit/service_test.py | 4 +- 14 files changed, 309 insertions(+), 59 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 864bc7e90..9b4130536 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -35,6 +35,7 @@ from .interpolation import interpolate_environment_variables from .sort_services import get_container_name_from_network_mode from .sort_services import get_service_name_from_network_mode from .sort_services import sort_service_dicts +from .types import MountSpec from .types import parse_extra_hosts from .types import parse_restart_spec from .types import ServiceLink @@ -809,6 +810,20 @@ def process_healthcheck(service_dict): return service_dict +def finalize_service_volumes(service_dict, environment): + if 'volumes' in service_dict: + finalized_volumes = [] + normalize = environment.get_boolean('COMPOSE_CONVERT_WINDOWS_PATHS') + for v in service_dict['volumes']: + if isinstance(v, dict): + finalized_volumes.append(MountSpec.parse(v, normalize)) + else: + finalized_volumes.append(VolumeSpec.parse(v, normalize)) + service_dict['volumes'] = finalized_volumes + + return service_dict + + def finalize_service(service_config, service_names, version, environment): service_dict = dict(service_config.config) @@ -822,12 +837,7 @@ def finalize_service(service_config, service_names, version, environment): for vf in service_dict['volumes_from'] ] - if 'volumes' in service_dict: - service_dict['volumes'] = [ - VolumeSpec.parse( - v, environment.get_boolean('COMPOSE_CONVERT_WINDOWS_PATHS') - ) for v in service_dict['volumes'] - ] + service_dict = finalize_service_volumes(service_dict, environment) if 'net' in service_dict: network_mode = service_dict.pop('net') @@ -1143,19 +1153,13 @@ def resolve_volume_paths(working_dir, service_dict): def resolve_volume_path(working_dir, volume): - mount_params = None if isinstance(volume, dict): - container_path = volume.get('target') - host_path = volume.get('source') - mode = None - if host_path: - if volume.get('read_only'): - mode = 'ro' - if volume.get('volume', {}).get('nocopy'): - mode = 'nocopy' - mount_params = (host_path, mode) - else: - container_path, mount_params = split_path_mapping(volume) + if volume.get('source', '').startswith('.') and volume['type'] == 'mount': + volume['source'] = expand_path(working_dir, volume['source']) + return volume + + mount_params = None + container_path, mount_params = split_path_mapping(volume) if mount_params is not None: host_path, mode = mount_params diff --git a/compose/config/config_schema_v2.3.json b/compose/config/config_schema_v2.3.json index 6f923871b..d50df3e81 100644 --- a/compose/config/config_schema_v2.3.json +++ b/compose/config/config_schema_v2.3.json @@ -293,7 +293,39 @@ }, "user": {"type": "string"}, "userns_mode": {"type": "string"}, - "volumes": {"type": "array", "items": {"type": "string"}, "uniqueItems": true}, + "volumes": { + "type": "array", + "items": { + "oneOf": [ + {"type": "string"}, + { + "type": "object", + "required": ["type"], + "additionalProperties": false, + "properties": { + "type": {"type": "string"}, + "source": {"type": "string"}, + "target": {"type": "string"}, + "read_only": {"type": "boolean"}, + "consistency": {"type": "string"}, + "bind": { + "type": "object", + "properties": { + "propagation": {"type": "string"} + } + }, + "volume": { + "type": "object", + "properties": { + "nocopy": {"type": "boolean"} + } + } + } + } + ], + "uniqueItems": true + } + }, "volume_driver": {"type": "string"}, "volumes_from": {"type": "array", "items": {"type": "string"}, "uniqueItems": true}, "working_dir": {"type": "string"} diff --git a/compose/config/serialize.py b/compose/config/serialize.py index 44878a47c..4982f8e34 100644 --- a/compose/config/serialize.py +++ b/compose/config/serialize.py @@ -7,6 +7,7 @@ import yaml from compose.config import types from compose.const import COMPOSEFILE_V1 as V1 from compose.const import COMPOSEFILE_V2_1 as V2_1 +from compose.const import COMPOSEFILE_V2_3 as V2_3 from compose.const import COMPOSEFILE_V3_0 as V3_0 from compose.const import COMPOSEFILE_V3_2 as V3_2 from compose.const import COMPOSEFILE_V3_4 as V3_4 @@ -34,6 +35,7 @@ def serialize_string(dumper, data): return representer(data) +yaml.SafeDumper.add_representer(types.MountSpec, serialize_dict_type) yaml.SafeDumper.add_representer(types.VolumeFromSpec, serialize_config_type) yaml.SafeDumper.add_representer(types.VolumeSpec, serialize_config_type) yaml.SafeDumper.add_representer(types.ServiceSecret, serialize_dict_type) @@ -140,5 +142,9 @@ def denormalize_service_dict(service_dict, version, image_digest=None): p.legacy_repr() if isinstance(p, types.ServicePort) else p for p in service_dict['ports'] ] + if 'volumes' in service_dict and (version < V2_3 or (version > V3_0 and version < V3_2)): + service_dict['volumes'] = [ + v.legacy_repr() if isinstance(v, types.MountSpec) else v for v in service_dict['volumes'] + ] return service_dict diff --git a/compose/config/types.py b/compose/config/types.py index d3b3cfc53..c134bd7ca 100644 --- a/compose/config/types.py +++ b/compose/config/types.py @@ -144,6 +144,15 @@ class MountSpec(object): } _fields = ['type', 'source', 'target', 'read_only', 'consistency'] + @classmethod + def parse(cls, mount_dict, normalize=False): + if mount_dict.get('source'): + mount_dict['source'] = os.path.normpath(mount_dict['source']) + if normalize: + mount_dict['source'] = normalize_path_for_engine(mount_dict['source']) + + return cls(**mount_dict) + def __init__(self, type, source=None, target=None, read_only=None, consistency=None, **kwargs): self.type = type self.source = source @@ -174,6 +183,10 @@ class MountSpec(object): def is_named_volume(self): return self.type == 'volume' and self.source + @property + def external(self): + return self.source + class VolumeSpec(namedtuple('_VolumeSpec', 'external internal mode')): diff --git a/compose/service.py b/compose/service.py index bfc2e5940..f51f0e5af 100644 --- a/compose/service.py +++ b/compose/service.py @@ -785,15 +785,23 @@ class Service(object): self.options.get('labels'), override_options.get('labels')) + container_volumes = [] + container_mounts = [] + if 'volumes' in container_options: + container_volumes = [ + v for v in container_options.get('volumes') if isinstance(v, VolumeSpec) + ] + container_mounts = [v for v in container_options.get('volumes') if isinstance(v, MountSpec)] + binds, affinity = merge_volume_bindings( - container_options.get('volumes') or [], - self.options.get('tmpfs') or [], - previous_container) + container_volumes, self.options.get('tmpfs') or [], previous_container, + container_mounts + ) override_options['binds'] = binds container_options['environment'].update(affinity) - container_options['volumes'] = dict( - (v.internal, {}) for v in container_options.get('volumes') or {}) + container_options['volumes'] = dict((v.internal, {}) for v in container_volumes or {}) + override_options['mounts'] = [build_mount(v) for v in container_mounts] or None secret_volumes = self.get_secret_volumes() if secret_volumes: @@ -803,7 +811,8 @@ class Service(object): (v.target, {}) for v in secret_volumes ) else: - override_options['mounts'] = [build_mount(v) for v in secret_volumes] + override_options['mounts'] = override_options.get('mounts') or [] + override_options['mounts'].extend([build_mount(v) for v in secret_volumes]) container_options['image'] = self.image_name @@ -1245,32 +1254,40 @@ def parse_repository_tag(repo_path): # Volumes -def merge_volume_bindings(volumes, tmpfs, previous_container): - """Return a list of volume bindings for a container. Container data volumes - are replaced by those from the previous container. +def merge_volume_bindings(volumes, tmpfs, previous_container, mounts): + """ + Return a list of volume bindings for a container. Container data volumes + are replaced by those from the previous container. + Anonymous mounts are updated in place. """ affinity = {} volume_bindings = dict( build_volume_binding(volume) for volume in volumes - if volume.external) + if volume.external + ) if previous_container: - old_volumes = get_container_data_volumes(previous_container, volumes, tmpfs) + old_volumes, old_mounts = get_container_data_volumes( + previous_container, volumes, tmpfs, mounts + ) warn_on_masked_volume(volumes, old_volumes, previous_container.service) volume_bindings.update( - build_volume_binding(volume) for volume in old_volumes) + build_volume_binding(volume) for volume in old_volumes + ) - if old_volumes: + if old_volumes or old_mounts: affinity = {'affinity:container': '=' + previous_container.id} return list(volume_bindings.values()), affinity -def get_container_data_volumes(container, volumes_option, tmpfs_option): - """Find the container data volumes that are in `volumes_option`, and return - a mapping of volume bindings for those volumes. +def get_container_data_volumes(container, volumes_option, tmpfs_option, mounts_option): + """ + Find the container data volumes that are in `volumes_option`, and return + a mapping of volume bindings for those volumes. + Anonymous volume mounts are updated in place instead. """ volumes = [] volumes_option = volumes_option or [] @@ -1309,7 +1326,19 @@ def get_container_data_volumes(container, volumes_option, tmpfs_option): volume = volume._replace(external=mount['Name']) volumes.append(volume) - return volumes + updated_mounts = False + for mount in mounts_option: + if mount.type != 'volume': + continue + + ctnr_mount = container_mounts.get(mount.target) + if not ctnr_mount.get('Name'): + continue + + mount.source = ctnr_mount['Name'] + updated_mounts = True + + return volumes, updated_mounts def warn_on_masked_volume(volumes_option, container_volumes, service): diff --git a/compose/utils.py b/compose/utils.py index 197ae6eb2..00b01df2e 100644 --- a/compose/utils.py +++ b/compose/utils.py @@ -101,7 +101,7 @@ def json_stream(stream): def json_hash(obj): - dump = json.dumps(obj, sort_keys=True, separators=(',', ':')) + dump = json.dumps(obj, sort_keys=True, separators=(',', ':'), default=lambda x: x.repr()) h = hashlib.sha256() h.update(dump.encode('utf8')) return h.hexdigest() diff --git a/compose/volume.py b/compose/volume.py index da8ba25ca..0b148620f 100644 --- a/compose/volume.py +++ b/compose/volume.py @@ -7,6 +7,7 @@ from docker.errors import NotFound from docker.utils import version_lt from .config import ConfigurationError +from .config.types import VolumeSpec from .const import LABEL_PROJECT from .const import LABEL_VOLUME @@ -145,5 +146,9 @@ class ProjectVolumes(object): if not volume_spec.is_named_volume: return volume_spec - volume = self.volumes[volume_spec.external] - return volume_spec._replace(external=volume.full_name) + if isinstance(volume_spec, VolumeSpec): + volume = self.volumes[volume_spec.external] + return volume_spec._replace(external=volume.full_name) + else: + volume_spec.source = self.volumes[volume_spec.source].full_name + return volume_spec diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 8ec3f9cdd..2ce5bf83e 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -428,13 +428,21 @@ class CLITestCase(DockerClientTestCase): 'timeout': '1s', 'retries': 5, }, - 'volumes': [ - '/host/path:/container/path:ro', - 'foobar:/container/volumepath:rw', - '/anonymous', - 'foobar:/container/volumepath2:nocopy' - ], - + 'volumes': [{ + 'read_only': True, + 'source': '/host/path', + 'target': '/container/path', + 'type': 'bind' + }, { + 'source': 'foobar', 'target': '/container/volumepath', 'type': 'volume' + }, { + 'target': '/anonymous', 'type': 'volume' + }, { + 'source': 'foobar', + 'target': '/container/volumepath2', + 'type': 'volume', + 'volume': {'nocopy': True} + }], 'stop_grace_period': '20s', }, }, diff --git a/tests/helpers.py b/tests/helpers.py index a93de993f..f151f9cde 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -19,12 +19,8 @@ def build_config_details(contents, working_dir='working_dir', filename='filename ) -def create_host_file(client, filename): +def create_custom_host_file(client, filename, content): dirname = os.path.dirname(filename) - - with open(filename, 'r') as fh: - content = fh.read() - container = client.create_container( 'busybox:latest', ['sh', '-c', 'echo -n "{}" > {}'.format(content, filename)], @@ -48,3 +44,10 @@ def create_host_file(client, filename): return container_info['Node']['Name'] finally: client.remove_container(container, force=True) + + +def create_host_file(client, filename): + with open(filename, 'r') as fh: + content = fh.read() + + return create_custom_host_file(client, filename, content) diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 953dd52be..6686d96cc 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -35,6 +35,7 @@ from tests.integration.testcases import is_cluster from tests.integration.testcases import no_cluster from tests.integration.testcases import v2_1_only from tests.integration.testcases import v2_2_only +from tests.integration.testcases import v2_3_only from tests.integration.testcases import v2_only from tests.integration.testcases import v3_only @@ -436,6 +437,26 @@ class ProjectTest(DockerClientTestCase): self.assertNotEqual(db_container.id, old_db_id) self.assertEqual(db_container.get('Volumes./etc'), db_volume_path) + @v2_3_only() + def test_recreate_preserves_mounts(self): + web = self.create_service('web') + db = self.create_service('db', volumes=[types.MountSpec(type='volume', target='/etc')]) + project = Project('composetest', [web, db], self.client) + project.start() + assert len(project.containers()) == 0 + + project.up(['db']) + assert len(project.containers()) == 1 + old_db_id = project.containers()[0].id + db_volume_path = project.containers()[0].get_mount('/etc')['Source'] + + project.up(strategy=ConvergenceStrategy.always) + assert len(project.containers()) == 2 + + db_container = [c for c in project.containers() if 'db' in c.name][0] + assert db_container.id != old_db_id + assert db_container.get_mount('/etc')['Source'] == db_volume_path + def test_project_up_with_no_recreate_running(self): web = self.create_service('web') db = self.create_service('db', volumes=[VolumeSpec.parse('/var/db')]) diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 00bacebf5..b9005b8e1 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -19,6 +19,7 @@ from .testcases import pull_busybox from .testcases import SWARM_SKIP_CONTAINERS_ALL from .testcases import SWARM_SKIP_CPU_SHARES from compose import __version__ +from compose.config.types import MountSpec from compose.config.types import VolumeFromSpec from compose.config.types import VolumeSpec from compose.const import IS_WINDOWS_PLATFORM @@ -37,6 +38,7 @@ from compose.service import NetworkMode from compose.service import PidMode from compose.service import Service from compose.utils import parse_nanoseconds_int +from tests.helpers import create_custom_host_file from tests.integration.testcases import is_cluster from tests.integration.testcases import no_cluster from tests.integration.testcases import v2_1_only @@ -276,6 +278,54 @@ class ServiceTest(DockerClientTestCase): self.assertTrue(path.basename(actual_host_path) == path.basename(host_path), msg=("Last component differs: %s, %s" % (actual_host_path, host_path))) + @v2_3_only() + def test_create_container_with_host_mount(self): + host_path = '/tmp/host-path' + container_path = '/container-path' + + create_custom_host_file(self.client, path.join(host_path, 'a.txt'), 'test') + + service = self.create_service( + 'db', + volumes=[ + MountSpec(type='bind', source=host_path, target=container_path, read_only=True) + ] + ) + container = service.create_container() + service.start_container(container) + mount = container.get_mount(container_path) + assert mount + assert path.basename(mount['Source']) == path.basename(host_path) + assert mount['RW'] is False + + @v2_3_only() + def test_create_container_with_tmpfs_mount(self): + container_path = '/container-tmpfs' + service = self.create_service( + 'db', + volumes=[MountSpec(type='tmpfs', target=container_path)] + ) + container = service.create_container() + service.start_container(container) + mount = container.get_mount(container_path) + assert mount + assert mount['Type'] == 'tmpfs' + + @v2_3_only() + def test_create_container_with_volume_mount(self): + container_path = '/container-volume' + volume_name = 'composetest_abcde' + self.client.create_volume(volume_name) + service = self.create_service( + 'db', + volumes=[MountSpec(type='volume', source=volume_name, target=container_path)] + ) + container = service.create_container() + service.start_container(container) + mount = container.get_mount(container_path) + assert mount + assert mount['Name'] == volume_name + def test_create_container_with_healthcheck_config(self): one_second = parse_nanoseconds_int('1s') healthcheck = { @@ -439,6 +489,38 @@ class ServiceTest(DockerClientTestCase): orig_container = new_container + @v2_3_only() + def test_execute_convergence_plan_recreate_twice_with_mount(self): + service = self.create_service( + 'db', + volumes=[MountSpec(target='/etc', type='volume')], + entrypoint=['top'], + command=['-d', '1'] + ) + + orig_container = service.create_container() + service.start_container(orig_container) + + orig_container.inspect() # reload volume data + volume_path = orig_container.get_mount('/etc')['Source'] + + # Do this twice to reproduce the bug + for _ in range(2): + new_container, = service.execute_convergence_plan( + ConvergencePlan('recreate', [orig_container]) + ) + + assert new_container.get_mount('/etc')['Source'] == volume_path + if not is_cluster(self.client): + assert ('affinity:container==%s' % orig_container.id in + new_container.get('Config.Env')) + else: + # In Swarm, the env marker is consumed and the container should be deployed + # on the same node. + assert orig_container.get('Node.Name') == new_container.get('Node.Name') + + orig_container = new_container + def test_execute_convergence_plan_when_containers_are_stopped(self): service = self.create_service( 'db', diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index b72fb53a8..9427f3d0d 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -20,7 +20,7 @@ from compose.const import COMPOSEFILE_V2_2 as V2_2 from compose.const import COMPOSEFILE_V2_3 as V2_3 from compose.const import COMPOSEFILE_V3_0 as V3_0 from compose.const import COMPOSEFILE_V3_2 as V3_2 -from compose.const import COMPOSEFILE_V3_3 as V3_3 +from compose.const import COMPOSEFILE_V3_5 as V3_5 from compose.const import LABEL_PROJECT from compose.progress_stream import stream_output from compose.service import Service @@ -47,7 +47,7 @@ def get_links(container): def engine_max_version(): if 'DOCKER_VERSION' not in os.environ: - return V3_3 + return V3_5 version = os.environ['DOCKER_VERSION'].partition('-')[0] if version_lt(version, '1.10'): return V1 @@ -57,7 +57,7 @@ def engine_max_version(): return V2_1 if version_lt(version, '17.06'): return V3_2 - return V3_3 + return V3_5 def min_version_skip(version): diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 00ba6c2c6..d519deb90 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -1137,9 +1137,12 @@ class ConfigTest(unittest.TestCase): details = config.ConfigDetails('.', [base_file, override_file]) service_dicts = config.load(details).services svc_volumes = map(lambda v: v.repr(), service_dicts[0]['volumes']) - assert sorted(svc_volumes) == sorted( - ['/anonymous', '/c:/b:rw', 'vol:/x:ro'] - ) + for vol in svc_volumes: + assert vol in [ + '/anonymous', + '/c:/b:rw', + {'source': 'vol', 'target': '/x', 'type': 'volume', 'read_only': True} + ] @mock.patch.dict(os.environ) def test_volume_mode_override(self): @@ -1223,6 +1226,50 @@ class ConfigTest(unittest.TestCase): assert volume.external == 'data0028' assert volume.is_named_volume + def test_volumes_long_syntax(self): + base_file = config.ConfigFile( + 'base.yaml', { + 'version': '2.3', + 'services': { + 'web': { + 'image': 'busybox:latest', + 'volumes': [ + { + 'target': '/anonymous', 'type': 'volume' + }, { + 'source': '/abc', 'target': '/xyz', 'type': 'bind' + }, { + 'source': '\\\\.\\pipe\\abcd', 'target': '/named_pipe', 'type': 'npipe' + }, { + 'type': 'tmpfs', 'target': '/tmpfs' + } + ] + }, + }, + }, + ) + details = config.ConfigDetails('.', [base_file]) + config_data = config.load(details) + volumes = config_data.services[0].get('volumes') + anon_volume = [v for v in volumes if v.target == '/anonymous'][0] + tmpfs_mount = [v for v in volumes if v.type == 'tmpfs'][0] + host_mount = [v for v in volumes if v.type == 'bind'][0] + npipe_mount = [v for v in volumes if v.type == 'npipe'][0] + + assert anon_volume.type == 'volume' + assert not anon_volume.is_named_volume + + assert tmpfs_mount.target == '/tmpfs' + assert not tmpfs_mount.is_named_volume + + assert host_mount.source == os.path.normpath('/abc') + assert host_mount.target == '/xyz' + assert not host_mount.is_named_volume + + assert npipe_mount.source == '\\\\.\\pipe\\abcd' + assert npipe_mount.target == '/named_pipe' + assert not npipe_mount.is_named_volume + def test_config_valid_service_names(self): for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']: services = config.load( diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 16670cff5..24ed60e94 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -939,7 +939,7 @@ class ServiceVolumesTest(unittest.TestCase): VolumeSpec.parse('imagedata:/mnt/image/data:rw'), ] - volumes = get_container_data_volumes(container, options, ['/dev/tmpfs']) + volumes, _ = get_container_data_volumes(container, options, ['/dev/tmpfs'], []) assert sorted(volumes) == sorted(expected) def test_merge_volume_bindings(self): @@ -975,7 +975,7 @@ class ServiceVolumesTest(unittest.TestCase): 'existingvolume:/existing/volume:rw', ] - binds, affinity = merge_volume_bindings(options, ['/dev/tmpfs'], previous_container) + binds, affinity = merge_volume_bindings(options, ['/dev/tmpfs'], previous_container, []) assert sorted(binds) == sorted(expected) assert affinity == {'affinity:container': '=cdefab'}