Merge pull request #1261 from aanand/fix-vars-in-volume-paths

Fix vars in volume paths

compose/config.py

@@ -329,6 +329,8 @@ def resolve_host_paths(volumes, working_dir=None):
 def resolve_host_path(volume, working_dir):
     container_path, host_path = split_volume(volume)
     if host_path is not None:
+        host_path = os.path.expanduser(host_path)
+        host_path = os.path.expandvars(host_path)
         return "%s:%s" % (expand_path(working_dir, host_path), container_path)
     else:
         return container_path

compose/service.py

@@ -3,7 +3,6 @@ from __future__ import absolute_import
 from collections import namedtuple
 import logging
 import re
-import os
 from operator import attrgetter
 import sys
 import six
@@ -589,8 +588,7 @@ def parse_repository_tag(s):
 
 def build_volume_binding(volume_spec):
     internal = {'bind': volume_spec.internal, 'ro': volume_spec.mode == 'ro'}
-    external = os.path.expanduser(volume_spec.external)
+    return volume_spec.external, internal
-    return os.path.abspath(os.path.expandvars(external)), internal
 
 
 def build_port_bindings(ports):

tests/integration/service_test.py

@@ -123,6 +123,24 @@ 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)))
 
+    @mock.patch.dict(os.environ)
+    def test_create_container_with_home_and_env_var_in_volume_path(self):
+        os.environ['VOLUME_NAME'] = 'my-volume'
+        os.environ['HOME'] = '/tmp/home-dir'
+        expected_host_path = os.path.join(os.environ['HOME'], os.environ['VOLUME_NAME'])
+
+        host_path = '~/${VOLUME_NAME}'
+        container_path = '/container-path'
+
+        service = self.create_service('db', volumes=['%s:%s' % (host_path, container_path)])
+        container = service.create_container()
+        service.start_container(container)
+
+        actual_host_path = container.get('Volumes')[container_path]
+        components = actual_host_path.split('/')
+        self.assertTrue(components[-2:] == ['home-dir', 'my-volume'],
+                        msg="Last two components differ: %s, %s" % (actual_host_path, expected_host_path))
+
     def test_create_container_with_volumes_from(self):
         volume_service = self.create_service('data')
         volume_container_1 = volume_service.create_container()

tests/unit/config_test.py

@@ -40,6 +40,20 @@ class ConfigTest(unittest.TestCase):
         config.make_service_dict('foo', {'ports': ['8000']})
 
 
+class VolumePathTest(unittest.TestCase):
+    @mock.patch.dict(os.environ)
+    def test_volume_binding_with_environ(self):
+        os.environ['VOLUME_PATH'] = '/host/path'
+        d = config.make_service_dict('foo', {'volumes': ['${VOLUME_PATH}:/container/path']}, working_dir='.')
+        self.assertEqual(d['volumes'], ['/host/path:/container/path'])
+
+    @mock.patch.dict(os.environ)
+    def test_volume_binding_with_home(self):
+        os.environ['HOME'] = '/home/user'
+        d = config.make_service_dict('foo', {'volumes': ['~:/container/path']}, working_dir='.')
+        self.assertEqual(d['volumes'], ['/home/user:/container/path'])
+
+
 class MergeVolumesTest(unittest.TestCase):
     def test_empty(self):
         service_dict = config.merge_service_dicts({}, {})

tests/unit/service_test.py

@@ -1,6 +1,5 @@
 from __future__ import unicode_literals
 from __future__ import absolute_import
-import os
 
 from .. import unittest
 import mock
@@ -304,17 +303,3 @@ class ServiceVolumesTest(unittest.TestCase):
         self.assertEqual(
             binding,
             ('/outside', dict(bind='/inside', ro=False)))
-
-    @mock.patch.dict(os.environ)
-    def test_build_volume_binding_with_environ(self):
-        os.environ['VOLUME_PATH'] = '/opt'
-        binding = build_volume_binding(parse_volume_spec('${VOLUME_PATH}:/opt'))
-        self.assertEqual(binding, ('/opt', dict(bind='/opt', ro=False)))
-
-    @mock.patch.dict(os.environ)
-    def test_building_volume_binding_with_home(self):
-        os.environ['HOME'] = '/home/user'
-        binding = build_volume_binding(parse_volume_spec('~:/home/user'))
-        self.assertEqual(
-            binding,
-            ('/home/user', dict(bind='/home/user', ro=False)))