Merge pull request #2728 from shin-/2709_service_volumes

Match named volumes in service definitions with declared volumes

compose/config/config.py

@@ -25,6 +25,7 @@ from .types import parse_extra_hosts
 from .types import parse_restart_spec
 from .types import VolumeFromSpec
 from .types import VolumeSpec
+from .validation import match_named_volumes
 from .validation import validate_against_fields_schema
 from .validation import validate_against_service_schema
 from .validation import validate_depends_on
@@ -274,6 +275,11 @@ def load(config_details):
         config_details.working_dir,
         main_file,
         [file.get_service_dicts() for file in config_details.config_files])
+
+    if main_file.version >= 2:
+        for service_dict in service_dicts:
+            match_named_volumes(service_dict, volumes)
+
     return Config(main_file.version, service_dicts, volumes, networks)
 
 

compose/config/types.py

@@ -163,3 +163,7 @@ class VolumeSpec(namedtuple('_VolumeSpec', 'external internal mode')):
     def repr(self):
         external = self.external + ':' if self.external else ''
         return '{ext}{v.internal}:{v.mode}'.format(ext=external, v=self)
+
+    @property
+    def is_named_volume(self):
+        return self.external and not self.external.startswith(('.', '/', '~'))

compose/config/validation.py

@@ -77,6 +77,18 @@ def format_boolean_in_environment(instance):
     return True
 
 
+def match_named_volumes(service_dict, project_volumes):
+    service_volumes = service_dict.get('volumes', [])
+    for volume_spec in service_volumes:
+        if volume_spec.is_named_volume and volume_spec.external not in project_volumes:
+            raise ConfigurationError(
+                'Named volume "{0}" is used in service "{1}" but no'
+                ' declaration was found in the volumes section.'.format(
+                    volume_spec.repr(), service_dict.get('name')
+                )
+            )
+
+
 def validate_top_level_service_objects(filename, service_dicts):
     """Perform some high level validation of the service name and value.
 

compose/project.py

@@ -42,7 +42,7 @@ class Project(object):
         self.use_networking = use_networking
         self.network_driver = network_driver
         self.networks = networks or []
-        self.volumes = volumes or []
+        self.volumes = volumes or {}
 
     def labels(self, one_off=False):
         return [
@@ -74,6 +74,15 @@ class Project(object):
         if 'default' not in network_config:
             all_networks.append(project.default_network)
 
+        if config_data.volumes:
+            for vol_name, data in config_data.volumes.items():
+                project.volumes[vol_name] = Volume(
+                    client=client, project=name, name=vol_name,
+                    driver=data.get('driver'),
+                    driver_opts=data.get('driver_opts'),
+                    external_name=data.get('external_name')
+                )
+
         for service_dict in config_data.services:
             if use_networking:
                 networks = get_networks(service_dict, all_networks)
@@ -85,6 +94,15 @@ class Project(object):
             links = project.get_links(service_dict)
             volumes_from = get_volumes_from(project, service_dict)
 
+            if config_data.version == 2:
+                service_volumes = service_dict.get('volumes', [])
+                for volume_spec in service_volumes:
+                    if volume_spec.is_named_volume:
+                        declared_volume = project.volumes[volume_spec.external]
+                        service_volumes[service_volumes.index(volume_spec)] = (
+                            volume_spec._replace(external=declared_volume.full_name)
+                        )
+
             project.services.append(
                 Service(
                     client=client,
@@ -94,23 +112,13 @@ class Project(object):
                     links=links,
                     net=net,
                     volumes_from=volumes_from,
-                    **service_dict))
+                    **service_dict)
+            )
 
         project.networks += custom_networks
         if 'default' not in network_config and project.uses_default_network():
             project.networks.append(project.default_network)
 
-        if config_data.volumes:
-            for vol_name, data in config_data.volumes.items():
-                project.volumes.append(
-                    Volume(
-                        client=client, project=name, name=vol_name,
-                        driver=data.get('driver'),
-                        driver_opts=data.get('driver_opts'),
-                        external_name=data.get('external_name')
-                    )
-                )
-
         return project
 
     @property
@@ -239,7 +247,7 @@ class Project(object):
 
     def initialize_volumes(self):
         try:
-            for volume in self.volumes:
+            for volume in self.volumes.values():
                 if volume.external:
                     log.debug(
                         'Volume {0} declared as external. No new '
@@ -294,7 +302,7 @@ class Project(object):
             network.remove()
 
     def remove_volumes(self):
-        for volume in self.volumes:
+        for volume in self.volumes.values():
             volume.remove()
 
     def initialize_networks(self):

tests/integration/project_test.py

@@ -813,3 +813,40 @@ class ProjectTest(DockerClientTestCase):
         assert 'Volume {0} declared as external'.format(
             vol_name
         ) in str(e.exception)
+
+    @v2_only()
+    def test_project_up_named_volumes_in_binds(self):
+        vol_name = '{0:x}'.format(random.getrandbits(32))
+        full_vol_name = 'composetest_{0}'.format(vol_name)
+
+        base_file = config.ConfigFile(
+            'base.yml',
+            {
+                'version': 2,
+                'services': {
+                    'simple': {
+                        'image': 'busybox:latest',
+                        'command': 'top',
+                        'volumes': ['{0}:/data'.format(vol_name)]
+                    },
+                },
+                'volumes': {
+                    vol_name: {'driver': 'local'}
+                }
+
+            })
+        config_details = config.ConfigDetails('.', [base_file])
+        config_data = config.load(config_details)
+        project = Project.from_config(
+            name='composetest', config_data=config_data, client=self.client
+        )
+        service = project.services[0]
+        self.assertEqual(service.name, 'simple')
+        volumes = service.options.get('volumes')
+        self.assertEqual(len(volumes), 1)
+        self.assertEqual(volumes[0].external, full_vol_name)
+        project.up()
+        engine_volumes = self.client.volumes()['Volumes']
+        container = service.get_container()
+        assert [mount['Name'] for mount in container.get('Mounts')] == [full_vol_name]
+        assert next((v for v in engine_volumes if v['Name'] == vol_name), None) is None

tests/unit/config/config_test.py

@@ -564,6 +564,56 @@ class ConfigTest(unittest.TestCase):
         ]
         assert service_sort(service_dicts) == service_sort(expected)
 
+    def test_undeclared_volume_v2(self):
+        base_file = config.ConfigFile(
+            'base.yaml',
+            {
+                'version': 2,
+                'services': {
+                    'web': {
+                        'image': 'busybox:latest',
+                        'volumes': ['data0028:/data:ro'],
+                    },
+                },
+            }
+        )
+        details = config.ConfigDetails('.', [base_file])
+        with self.assertRaises(ConfigurationError):
+            config.load(details)
+
+        base_file = config.ConfigFile(
+            'base.yaml',
+            {
+                'version': 2,
+                'services': {
+                    'web': {
+                        'image': 'busybox:latest',
+                        'volumes': ['./data0028:/data:ro'],
+                    },
+                },
+            }
+        )
+        details = config.ConfigDetails('.', [base_file])
+        config_data = config.load(details)
+        volume = config_data.services[0].get('volumes')[0]
+        assert not volume.is_named_volume
+
+    def test_undeclared_volume_v1(self):
+        base_file = config.ConfigFile(
+            'base.yaml',
+            {
+                'web': {
+                    'image': 'busybox:latest',
+                    'volumes': ['data0028:/data:ro'],
+                },
+            }
+        )
+        details = config.ConfigDetails('.', [base_file])
+        config_data = config.load(details)
+        volume = config_data.services[0].get('volumes')[0]
+        assert volume.external == 'data0028'
+        assert volume.is_named_volume
+
     def test_config_valid_service_names(self):
         for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']:
             services = config.load(