Merge pull request #5776 from docker/5772-catch-volume-divergence

Check volume config against remote and error out if diverged

.circleci/config.yml

@@ -6,8 +6,8 @@ jobs:
     steps:
     - checkout
     - run:
-        name: install python3
-        command: brew update > /dev/null && brew upgrade python
+        name: setup script
+        command: ./script/setup/osx
     - run:
         name: install tox
         command: sudo pip install --upgrade tox==2.1.1

compose/volume.py

@@ -124,19 +124,7 @@ class ProjectVolumes(object):
                     )
                     volume.create()
                 else:
-                    driver = volume.inspect()['Driver']
-                    if volume.driver is not None and driver != volume.driver:
-                        raise ConfigurationError(
-                            'Configuration for volume {0} specifies driver '
-                            '{1}, but a volume with the same name uses a '
-                            'different driver ({3}). If you wish to use the '
-                            'new configuration, please remove the existing '
-                            'volume "{2}" first:\n'
-                            '$ docker volume rm {2}'.format(
-                                volume.name, volume.driver, volume.full_name,
-                                volume.inspect()['Driver']
-                            )
-                        )
+                    check_remote_volume_config(volume.inspect(), volume)
         except NotFound:
             raise ConfigurationError(
                 'Volume %s specifies nonexistent driver %s' % (volume.name, volume.driver)
@@ -152,3 +140,43 @@ class ProjectVolumes(object):
         else:
             volume_spec.source = self.volumes[volume_spec.source].full_name
             return volume_spec
+
+
+class VolumeConfigChangedError(ConfigurationError):
+    def __init__(self, local, property_name, local_value, remote_value):
+        super(VolumeConfigChangedError, self).__init__(
+            'Configuration for volume {vol_name} specifies {property_name} '
+            '{local_value}, but a volume with the same name uses a different '
+            '{property_name} ({remote_value}). If you wish to use the new '
+            'configuration, please remove the existing volume "{full_name}" '
+            'first:\n$ docker volume rm {full_name}'.format(
+                vol_name=local.name, property_name=property_name,
+                local_value=local_value, remote_value=remote_value,
+                full_name=local.full_name
+            )
+        )
+
+
+def check_remote_volume_config(remote, local):
+    if local.driver and remote.get('Driver') != local.driver:
+        raise VolumeConfigChangedError(local, 'driver', local.driver, remote.get('Driver'))
+    local_opts = local.driver_opts or {}
+    remote_opts = remote.get('Options') or {}
+    for k in set.union(set(remote_opts.keys()), set(local_opts.keys())):
+        if k.startswith('com.docker.'):  # These options are set internally
+            continue
+        if remote_opts.get(k) != local_opts.get(k):
+            raise VolumeConfigChangedError(
+                local, '"{}" driver_opt'.format(k), local_opts.get(k), remote_opts.get(k),
+            )
+
+    local_labels = local.labels or {}
+    remote_labels = remote.get('Labels') or {}
+    for k in set.union(set(remote_labels.keys()), set(local_labels.keys())):
+        if k.startswith('com.docker.'):  # We are only interested in user-specified labels
+            continue
+        if remote_labels.get(k) != local_labels.get(k):
+            log.warn(
+                'Volume {}: label "{}" has changed. It may need to be'
+                ' recreated.'.format(local.name, k)
+            )

tests/integration/project_test.py

@@ -4,6 +4,7 @@ from __future__ import unicode_literals
 import json
 import os
 import random
+import shutil
 import tempfile
 
 import py
@@ -1537,6 +1538,52 @@ class ProjectTest(DockerClientTestCase):
             vol_name
         ) in str(e.value)
 
+    @v2_only()
+    @no_cluster('inspect volume by name defect on Swarm Classic')
+    def test_initialize_volumes_updated_driver_opts(self):
+        vol_name = '{0:x}'.format(random.getrandbits(32))
+        full_vol_name = 'composetest_{0}'.format(vol_name)
+        tmpdir = tempfile.mkdtemp(prefix='compose_test_')
+        self.addCleanup(shutil.rmtree, tmpdir)
+        driver_opts = {'o': 'bind', 'device': tmpdir, 'type': 'none'}
+
+        config_data = build_config(
+            version=V2_0,
+            services=[{
+                'name': 'web',
+                'image': 'busybox:latest',
+                'command': 'top'
+            }],
+            volumes={
+                vol_name: {
+                    'driver': 'local',
+                    'driver_opts': driver_opts
+                }
+            },
+        )
+        project = Project.from_config(
+            name='composetest',
+            config_data=config_data, client=self.client
+        )
+        project.volumes.initialize()
+
+        volume_data = self.get_volume_data(full_vol_name)
+        assert volume_data['Name'].split('/')[-1] == full_vol_name
+        assert volume_data['Driver'] == 'local'
+        assert volume_data['Options'] == driver_opts
+
+        driver_opts['device'] = '/opt/data/localdata'
+        project = Project.from_config(
+            name='composetest',
+            config_data=config_data,
+            client=self.client
+        )
+        with pytest.raises(config.ConfigurationError) as e:
+            project.volumes.initialize()
+        assert 'Configuration for volume {0} specifies "device" driver_opt {1}'.format(
+            vol_name, driver_opts['device']
+        ) in str(e.value)
+
     @v2_only()
     def test_initialize_volumes_updated_blank_driver(self):
         vol_name = '{0:x}'.format(random.getrandbits(32))