Merge pull request #2934 from dnephin/fix_volume_copy

Fix copying of volumes by using the name of the volume

compose/service.py

@@ -925,7 +925,7 @@ def get_container_data_volumes(container, volumes_option):
             continue
 
         # Copy existing volume from old container
-        volume = volume._replace(external=mount['Source'])
+        volume = volume._replace(external=mount['Name'])
         volumes.append(volume)
 
     return volumes

tests/integration/service_test.py

@@ -273,6 +273,30 @@ class ServiceTest(DockerClientTestCase):
                           self.client.inspect_container,
                           old_container.id)
 
+    def test_execute_convergence_plan_recreate_twice(self):
+        service = self.create_service(
+            'db',
+            volumes=[VolumeSpec.parse('/etc')],
+            entrypoint=['top'],
+            command=['-d', '1'])
+
+        orig_container = service.create_container()
+        service.start_container(orig_container)
+
+        orig_container.inspect()  # reload volume data
+        volume_path = orig_container.get_mount('/etc')['Source']
+
+        # Do this twice to reproduce the bug
+        for _ in range(2):
+            new_container, = service.execute_convergence_plan(
+                ConvergencePlan('recreate', [orig_container]))
+
+            assert new_container.get_mount('/etc')['Source'] == volume_path
+            assert ('affinity:container==%s' % orig_container.id in
+                    new_container.get('Config.Env'))
+
+            orig_container = new_container
+
     def test_execute_convergence_plan_when_containers_are_stopped(self):
         service = self.create_service(
             'db',

tests/unit/service_test.py

@@ -691,8 +691,8 @@ class ServiceVolumesTest(unittest.TestCase):
         }, has_been_inspected=True)
 
         expected = [
-            VolumeSpec.parse('/var/lib/docker/aaaaaaaa:/existing/volume:rw'),
+            VolumeSpec.parse('existingvolume:/existing/volume:rw'),
-            VolumeSpec.parse('/var/lib/docker/cccccccc:/mnt/image/data:rw'),
+            VolumeSpec.parse('imagedata:/mnt/image/data:rw'),
         ]
 
         volumes = get_container_data_volumes(container, options)
@@ -724,11 +724,11 @@ class ServiceVolumesTest(unittest.TestCase):
         expected = [
             '/host/volume:/host/volume:ro',
             '/host/rw/volume:/host/rw/volume:rw',
-            '/var/lib/docker/aaaaaaaa:/existing/volume:rw',
+            'existingvolume:/existing/volume:rw',
         ]
 
         binds = merge_volume_bindings(options, intermediate_container)
-        self.assertEqual(set(binds), set(expected))
+        assert sorted(binds) == sorted(expected)
 
     def test_mount_same_host_path_to_two_volumes(self):
         service = Service(
@@ -761,13 +761,14 @@ class ServiceVolumesTest(unittest.TestCase):
             ]),
         )
 
-    def test_different_host_path_in_container_json(self):
+    def test_get_container_create_options_with_different_host_path_in_container_json(self):
         service = Service(
             'web',
             image='busybox',
             volumes=[VolumeSpec.parse('/host/path:/data')],
             client=self.mock_client,
         )
+        volume_name = 'abcdefff1234'
 
         self.mock_client.inspect_image.return_value = {
             'Id': 'ababab',
@@ -788,7 +789,7 @@ class ServiceVolumesTest(unittest.TestCase):
                     'Mode': '',
                     'RW': True,
                     'Driver': 'local',
-                    'Name': 'abcdefff1234'
+                    'Name': volume_name,
                 },
             ]
         }
@@ -799,9 +800,9 @@ class ServiceVolumesTest(unittest.TestCase):
             previous_container=Container(self.mock_client, {'Id': '123123123'}),
         )
 
-        self.assertEqual(
+        assert (
-            self.mock_client.create_host_config.call_args[1]['binds'],
+            self.mock_client.create_host_config.call_args[1]['binds'] ==
-            ['/mnt/sda1/host/path:/data:rw'],
+            ['{}:/data:rw'.format(volume_name)]
         )
 
     def test_warn_on_masked_volume_no_warning_when_no_container_volumes(self):