Merge pull request #6221 from Dimrok/feature/volumes-order

Preserve volumes order as declared in the compose file.

compose/service.py

@@ -56,6 +56,7 @@ from .utils import json_hash
 from .utils import parse_bytes
 from .utils import parse_seconds_float
 from .utils import truncate_id
+from .utils import unique_everseen
 
 
 log = logging.getLogger(__name__)
@@ -940,8 +941,9 @@ class Service(object):
                 override_options['mounts'] = override_options.get('mounts') or []
                 override_options['mounts'].extend([build_mount(v) for v in secret_volumes])
 
-        # Remove possible duplicates (see e.g. https://github.com/docker/compose/issues/5885)
-        override_options['binds'] = list(set(binds))
+        # Remove possible duplicates (see e.g. https://github.com/docker/compose/issues/5885).
+        # unique_everseen preserves order. (see https://github.com/docker/compose/issues/6091).
+        override_options['binds'] = list(unique_everseen(binds))
         return container_options, override_options
 
     def _get_container_host_config(self, override_options, one_off=False):
@@ -1427,7 +1429,7 @@ def merge_volume_bindings(volumes, tmpfs, previous_container, mounts):
     """
     affinity = {}
 
-    volume_bindings = dict(
+    volume_bindings = OrderedDict(
         build_volume_binding(volume)
         for volume in volumes
         if volume.external

compose/utils.py

@@ -170,3 +170,13 @@ def truncate_id(value):
     if len(value) > 12:
         return value[:12]
     return value
+
+
+def unique_everseen(iterable, key=lambda x: x):
+    "List unique elements, preserving order. Remember all elements ever seen."
+    seen = set()
+    for element in iterable:
+        unique_key = key(element)
+        if unique_key not in seen:
+            seen.add(unique_key)
+            yield element

tests/unit/config/config_test.py

@@ -8,6 +8,7 @@ import os
 import shutil
 import tempfile
 from operator import itemgetter
+from random import shuffle
 
 import py
 import pytest
@@ -42,7 +43,7 @@ from tests import unittest
 DEFAULT_VERSION = V2_0
 
 
-def make_service_dict(name, service_dict, working_dir, filename=None):
+def make_service_dict(name, service_dict, working_dir='.', filename=None):
     """Test helper function to construct a ServiceExtendsResolver
     """
     resolver = config.ServiceExtendsResolver(
@@ -3536,6 +3537,13 @@ class VolumeConfigTest(unittest.TestCase):
         ).services[0]
         assert d['volumes'] == [VolumeSpec.parse('/host/path:/container/path')]
 
+    @pytest.mark.skipif(IS_WINDOWS_PLATFORM, reason='posix paths')
+    def test_volumes_order_is_preserved(self):
+        volumes = ['/{0}:/{0}'.format(i) for i in range(0, 6)]
+        shuffle(volumes)
+        cfg = make_service_dict('foo', {'build': '.', 'volumes': volumes})
+        assert cfg['volumes'] == volumes
+
     @pytest.mark.skipif(IS_WINDOWS_PLATFORM, reason='posix paths')
     @mock.patch.dict(os.environ)
     def test_volume_binding_with_home(self):

tests/unit/service_test.py

@@ -1037,6 +1037,23 @@ class ServiceTest(unittest.TestCase):
         assert len(override_opts['binds']) == 1
         assert override_opts['binds'][0] == 'vol:/data:rw'
 
+    def test_volumes_order_is_preserved(self):
+        service = Service('foo', client=self.mock_client)
+        volumes = [
+            VolumeSpec.parse(cfg) for cfg in [
+                '/v{0}:/v{0}:rw'.format(i) for i in range(6)
+            ]
+        ]
+        ctnr_opts, override_opts = service._build_container_volume_options(
+            previous_container=None,
+            container_options={
+                'volumes': volumes,
+                'environment': {},
+            },
+            override_options={},
+        )
+        assert override_opts['binds'] == [vol.repr() for vol in volumes]
+
 
 class TestServiceNetwork(unittest.TestCase):
     def setUp(self):

tests/unit/utils_test.py

@@ -68,3 +68,11 @@ class TestParseBytes(object):
         assert utils.parse_bytes(123) == 123
         assert utils.parse_bytes('foobar') is None
         assert utils.parse_bytes('123') == 123
+
+
+class TestMoreItertools(object):
+    def test_unique_everseen(self):
+        unique = utils.unique_everseen
+        assert list(unique([2, 1, 2, 1])) == [2, 1]
+        assert list(unique([2, 1, 2, 1], hash)) == [2, 1]
+        assert list(unique([2, 1, 2, 1], lambda x: 'key_%s' % x)) == [2, 1]