Move named volumes matching to config validation phase

Signed-off-by: Joffrey F <joffrey@docker.com>

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
@@ -271,6 +272,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/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 [
@@ -76,13 +76,11 @@ class Project(object):
 
         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')
-                    )
+                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:
@@ -98,7 +96,13 @@ class Project(object):
             volumes_from = get_volumes_from(project, service_dict)
 
             if config_data.version == 2:
-                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:
+                        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(
@@ -244,7 +248,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 '
@@ -299,7 +303,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):
@@ -477,26 +481,6 @@ def get_networks(service_dict, network_definitions):
     return networks
 
 
-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:
-            declared_volume = next(
-                (v for v in project_volumes if v.name == volume_spec.external),
-                None
-            )
-            if not declared_volume:
-                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')
-                    )
-                )
-            service_volumes[service_volumes.index(volume_spec)] = (
-                volume_spec._replace(external=declared_volume.full_name)
-            )
-
-
 def get_volumes_from(project, service_dict):
     volumes_from = service_dict.pop('volumes_from', None)
     if not volumes_from:

tests/unit/config/config_test.py

@@ -523,6 +523,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(

tests/unit/project_test.py

@@ -7,10 +7,8 @@ import docker
 
 from .. import mock
 from .. import unittest
-from compose.config import ConfigurationError
 from compose.config.config import Config
 from compose.config.types import VolumeFromSpec
-from compose.config.types import VolumeSpec
 from compose.const import LABEL_SERVICE
 from compose.container import Container
 from compose.project import Project
@@ -478,44 +476,3 @@ class ProjectTest(unittest.TestCase):
             ),
         )
         self.assertEqual([c.id for c in project.containers()], ['1'])
-
-    def test_undeclared_volume_v2(self):
-        config = Config(
-            version=2,
-            services=[
-                {
-                    'name': 'web',
-                    'image': 'busybox:latest',
-                    'volumes': [VolumeSpec.parse('data0028:/data:ro')],
-                },
-            ],
-            networks=None,
-            volumes=None,
-        )
-        with self.assertRaises(ConfigurationError):
-            Project.from_config('composetest', config, None)
-
-        config = Config(
-            version=2,
-            services=[
-                {
-                    'name': 'web',
-                    'image': 'busybox:latest',
-                    'volumes': [VolumeSpec.parse('./data0028:/data:ro')],
-                },
-            ], networks=None, volumes=None,
-        )
-        Project.from_config('composetest', config, None)
-
-    def test_undeclared_volume_v1(self):
-        config = Config(
-            version=1,
-            services=[
-                {
-                    'name': 'web',
-                    'image': 'busybox:latest',
-                    'volumes': [VolumeSpec.parse('data0028:/data:ro')],
-                },
-            ], networks=None, volumes=None,
-        )
-        Project.from_config('composetest', config, None)