Don't preserve host volumes on container recreate.

Fixes a regression after the API changed to use Mounts.

Signed-off-by: Daniel Nephin <[email protected]>

compose/service.py

@@ -849,7 +849,13 @@ def get_container_data_volumes(container, volumes_option):
     a mapping of volume bindings for those volumes.
     """
     volumes = []
-    container_volumes = container.get('Volumes') or {}
+    volumes_option = volumes_option or []
+
+    container_mounts = dict(
+        (mount['Destination'], mount)
+        for mount in container.get('Mounts') or {}
+    )
+
     image_volumes = [
         VolumeSpec.parse(volume)
         for volume in
@@ -861,13 +867,13 @@ def get_container_data_volumes(container, volumes_option):
         if volume.external:
             continue
 
-        volume_path = container_volumes.get(volume.internal)
+        mount = container_mounts.get(volume.internal)
         # New volume, doesn't exist in the old container
-        if not volume_path:
+        if not mount:
             continue
 
         # Copy existing volume from old container
-        volume = volume._replace(external=volume_path)
+        volume = volume._replace(external=mount['Source'])
         volumes.append(volume)
 
     return volumes

tests/integration/service_test.py

@@ -312,7 +312,8 @@ class ServiceTest(DockerClientTestCase):
             ConvergencePlan('recreate', [old_container]))
 
         self.assertEqual(
-            [mount['Destination'] for mount in new_container.get('Mounts')], ['/data']
+            [mount['Destination'] for mount in new_container.get('Mounts')],
+            ['/data']
         )
         self.assertEqual(new_container.get_mount('/data')['Source'], volume_path)
 
@@ -323,8 +324,11 @@ class ServiceTest(DockerClientTestCase):
         )
 
         old_container = create_and_start_container(service)
-        self.assertEqual(list(old_container.get('Volumes').keys()), ['/data'])
-        volume_path = old_container.get('Volumes')['/data']
+        self.assertEqual(
+            [mount['Destination'] for mount in old_container.get('Mounts')],
+            ['/data']
+        )
+        volume_path = old_container.get_mount('/data')['Source']
 
         service.options['volumes'] = [VolumeSpec.parse('/tmp:/data')]
 
@@ -338,8 +342,11 @@ class ServiceTest(DockerClientTestCase):
             "Service \"db\" is using volume \"/data\" from the previous container",
             args[0])
 
-        self.assertEqual(list(new_container.get('Volumes')), ['/data'])
-        self.assertEqual(new_container.get('Volumes')['/data'], volume_path)
+        self.assertEqual(
+            [mount['Destination'] for mount in new_container.get('Mounts')],
+            ['/data']
+        )
+        self.assertEqual(new_container.get_mount('/data')['Source'], volume_path)
 
     def test_execute_convergence_plan_without_start(self):
         service = self.create_service(

tests/unit/service_test.py

@@ -234,6 +234,7 @@ class ServiceTest(unittest.TestCase):
         prev_container = mock.Mock(
             id='ababab',
             image_config={'ContainerConfig': {}})
+        prev_container.get.return_value = None
 
         opts = service._get_container_create_options(
             {},
@@ -575,6 +576,10 @@ class NetTestCase(unittest.TestCase):
         self.assertEqual(net.service_name, service_name)
 
 
+def build_mount(destination, source, mode='rw'):
+    return {'Source': source, 'Destination': destination, 'Mode': mode}
+
+
 class ServiceVolumesTest(unittest.TestCase):
 
     def setUp(self):