Merge pull request #2894 from dnephin/only_set_constraint_with_volumes

Only set a container affinity if there are volumes to copy over

compose/service.py

@@ -592,20 +592,19 @@ class Service(object):
                     ports.append(port)
             container_options['ports'] = ports
 
-        override_options['binds'] = merge_volume_bindings(
-            container_options.get('volumes') or [],
-            previous_container)
-
-        if 'volumes' in container_options:
-            container_options['volumes'] = dict(
-                (v.internal, {}) for v in container_options['volumes'])
-
         container_options['environment'] = merge_environment(
             self.options.get('environment'),
             override_options.get('environment'))
 
-        if previous_container:
+        binds, affinity = merge_volume_bindings(
-            container_options['environment']['affinity:container'] = ('=' + previous_container.id)
+            container_options.get('volumes') or [],
+            previous_container)
+        override_options['binds'] = binds
+        container_options['environment'].update(affinity)
+
+        if 'volumes' in container_options:
+            container_options['volumes'] = dict(
+                (v.internal, {}) for v in container_options['volumes'])
 
         container_options['image'] = self.image_name
 
@@ -877,18 +876,23 @@ def merge_volume_bindings(volumes, previous_container):
     """Return a list of volume bindings for a container. Container data volumes
     are replaced by those from the previous container.
     """
+    affinity = {}
+
     volume_bindings = dict(
         build_volume_binding(volume)
         for volume in volumes
         if volume.external)
 
     if previous_container:
-        data_volumes = get_container_data_volumes(previous_container, volumes)
+        old_volumes = get_container_data_volumes(previous_container, volumes)
-        warn_on_masked_volume(volumes, data_volumes, previous_container.service)
+        warn_on_masked_volume(volumes, old_volumes, previous_container.service)
         volume_bindings.update(
-            build_volume_binding(volume) for volume in data_volumes)
+            build_volume_binding(volume) for volume in old_volumes)
 
-    return list(volume_bindings.values())
+        if old_volumes:
+            affinity = {'affinity:container': '=' + previous_container.id}
+
+    return list(volume_bindings.values()), affinity
 
 
 def get_container_data_volumes(container, volumes_option):

tests/unit/service_test.py

@@ -267,13 +267,52 @@ class ServiceTest(unittest.TestCase):
         self.assertEqual(
             opts['labels'][LABEL_CONFIG_HASH],
             'f8bfa1058ad1f4231372a0b1639f0dfdb574dafff4e8d7938049ae993f7cf1fc')
-        self.assertEqual(
+        assert opts['environment'] == {'also': 'real'}
-            opts['environment'],
+
-            {
+    def test_get_container_create_options_sets_affinity_with_binds(self):
-                'affinity:container': '=ababab',
+        service = Service(
-                'also': 'real',
+            'foo',
-            }
+            image='foo',
+            client=self.mock_client,
         )
+        self.mock_client.inspect_image.return_value = {'Id': 'abcd'}
+        prev_container = mock.Mock(
+            id='ababab',
+            image_config={'ContainerConfig': {'Volumes': ['/data']}})
+
+        def container_get(key):
+            return {
+                'Mounts': [
+                    {
+                        'Destination': '/data',
+                        'Source': '/some/path',
+                        'Name': 'abab1234',
+                    },
+                ]
+            }.get(key, None)
+
+        prev_container.get.side_effect = container_get
+
+        opts = service._get_container_create_options(
+            {},
+            1,
+            previous_container=prev_container)
+
+        assert opts['environment'] == {'affinity:container': '=ababab'}
+
+    def test_get_container_create_options_no_affinity_without_binds(self):
+        service = Service('foo', image='foo', client=self.mock_client)
+        self.mock_client.inspect_image.return_value = {'Id': 'abcd'}
+        prev_container = mock.Mock(
+            id='ababab',
+            image_config={'ContainerConfig': {}})
+        prev_container.get.return_value = None
+
+        opts = service._get_container_create_options(
+            {},
+            1,
+            previous_container=prev_container)
+        assert opts['environment'] == {}
 
     def test_get_container_not_found(self):
         self.mock_client.containers.return_value = []
@@ -650,6 +689,7 @@ class ServiceVolumesTest(unittest.TestCase):
             '/host/volume:/host/volume:ro',
             '/new/volume',
             '/existing/volume',
+            'named:/named/vol',
         ]]
 
         self.mock_client.inspect_image.return_value = {
@@ -710,7 +750,8 @@ class ServiceVolumesTest(unittest.TestCase):
             'ContainerConfig': {'Volumes': {}}
         }
 
-        intermediate_container = Container(self.mock_client, {
+        previous_container = Container(self.mock_client, {
+            'Id': 'cdefab',
             'Image': 'ababab',
             'Mounts': [{
                 'Source': '/var/lib/docker/aaaaaaaa',
@@ -727,8 +768,9 @@ class ServiceVolumesTest(unittest.TestCase):
             'existingvolume:/existing/volume:rw',
         ]
 
-        binds = merge_volume_bindings(options, intermediate_container)
+        binds, affinity = merge_volume_bindings(options, previous_container)
         assert sorted(binds) == sorted(expected)
+        assert affinity == {'affinity:container': '=cdefab'}
 
     def test_mount_same_host_path_to_two_volumes(self):
         service = Service(