Merge pull request #5436 from docker/5244-advanced-mount-support

Add support for mount syntax

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

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"}

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

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')):
 

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):

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()

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

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',
                 },
             },

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)

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')])

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',

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):

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(

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'}