From bbdbc359248dd3c5f00d92538698317b46be49f4 Mon Sep 17 00:00:00 2001
From: Joffrey F <joffrey@docker.com>
Date: Tue, 25 Apr 2017 14:51:53 -0700
Subject: [PATCH] Avoid rebinding tmpfs data volumes when recreating containers

Signed-off-by: Joffrey F <joffrey@docker.com>
---
 compose/service.py                | 12 +++++++++---
 tests/integration/project_test.py | 18 ++++++++++++++++++
 tests/unit/service_test.py        | 16 ++++++++++------
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/compose/service.py b/compose/service.py
index e903115af..8699372ed 100644
--- a/compose/service.py
+++ b/compose/service.py
@@ -16,6 +16,7 @@ from docker.errors import NotFound
 from docker.types import LogConfig
 from docker.utils.ports import build_port_bindings
 from docker.utils.ports import split_port
+from docker.utils.utils import convert_tmpfs_mounts
 
 from . import __version__
 from . import const
@@ -744,6 +745,7 @@ class Service(object):
 
         binds, affinity = merge_volume_bindings(
             container_options.get('volumes') or [],
+            self.options.get('tmpfs') or [],
             previous_container)
         override_options['binds'] = binds
         container_options['environment'].update(affinity)
@@ -1126,7 +1128,7 @@ def parse_repository_tag(repo_path):
 # Volumes
 
 
-def merge_volume_bindings(volumes, previous_container):
+def merge_volume_bindings(volumes, tmpfs, previous_container):
     """Return a list of volume bindings for a container. Container data volumes
     are replaced by those from the previous container.
     """
@@ -1138,7 +1140,7 @@ def merge_volume_bindings(volumes, previous_container):
         if volume.external)
 
     if previous_container:
-        old_volumes = get_container_data_volumes(previous_container, volumes)
+        old_volumes = get_container_data_volumes(previous_container, volumes, tmpfs)
         warn_on_masked_volume(volumes, old_volumes, previous_container.service)
         volume_bindings.update(
             build_volume_binding(volume) for volume in old_volumes)
@@ -1149,7 +1151,7 @@ def merge_volume_bindings(volumes, previous_container):
     return list(volume_bindings.values()), affinity
 
 
-def get_container_data_volumes(container, volumes_option):
+def get_container_data_volumes(container, volumes_option, tmpfs_option):
     """Find the container data volumes that are in `volumes_option`, and return
     a mapping of volume bindings for those volumes.
     """
@@ -1172,6 +1174,10 @@ def get_container_data_volumes(container, volumes_option):
         if volume.external:
             continue
 
+        # Attempting to rebind tmpfs volumes breaks: https://github.com/docker/compose/issues/4751
+        if volume.internal in convert_tmpfs_mounts(tmpfs_option).keys():
+            continue
+
         mount = container_mounts.get(volume.internal)
 
         # New volume, doesn't exist in the old container
diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py
index b69b04565..69f06b75c 100644
--- a/tests/integration/project_test.py
+++ b/tests/integration/project_test.py
@@ -552,6 +552,24 @@ class ProjectTest(DockerClientTestCase):
         self.assertEqual(len(project.get_service('data').containers(stopped=True)), 1)
         self.assertEqual(len(project.get_service('console').containers()), 0)
 
+    def test_project_up_recreate_with_tmpfs_volume(self):
+        # https://github.com/docker/compose/issues/4751
+        project = Project.from_config(
+            name='composetest',
+            config_data=load_config({
+                'version': '2.1',
+                'services': {
+                    'foo': {
+                        'image': 'busybox:latest',
+                        'tmpfs': ['/dev/shm'],
+                        'volumes': ['/dev/shm']
+                    }
+                }
+            }), client=self.client
+        )
+        project.up()
+        project.up(strategy=ConvergenceStrategy.always)
+
     def test_unscale_after_restart(self):
         web = self.create_service('web')
         project = Project('composetest', [web], self.client)
diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py
index f3f3a2a83..c32c36339 100644
--- a/tests/unit/service_test.py
+++ b/tests/unit/service_test.py
@@ -858,6 +858,7 @@ class ServiceVolumesTest(unittest.TestCase):
             '/new/volume',
             '/existing/volume',
             'named:/named/vol',
+            '/dev/tmpfs'
         ]]
 
         self.mock_client.inspect_image.return_value = {
@@ -903,15 +904,18 @@ class ServiceVolumesTest(unittest.TestCase):
             VolumeSpec.parse('imagedata:/mnt/image/data:rw'),
         ]
 
-        volumes = get_container_data_volumes(container, options)
+        volumes = get_container_data_volumes(container, options, ['/dev/tmpfs'])
         assert sorted(volumes) == sorted(expected)
 
     def test_merge_volume_bindings(self):
         options = [
-            VolumeSpec.parse('/host/volume:/host/volume:ro', True),
-            VolumeSpec.parse('/host/rw/volume:/host/rw/volume', True),
-            VolumeSpec.parse('/new/volume', True),
-            VolumeSpec.parse('/existing/volume', True),
+            VolumeSpec.parse(v, True) for v in [
+                '/host/volume:/host/volume:ro',
+                '/host/rw/volume:/host/rw/volume',
+                '/new/volume',
+                '/existing/volume',
+                '/dev/tmpfs'
+            ]
         ]
 
         self.mock_client.inspect_image.return_value = {
@@ -936,7 +940,7 @@ class ServiceVolumesTest(unittest.TestCase):
             'existingvolume:/existing/volume:rw',
         ]
 
-        binds, affinity = merge_volume_bindings(options, previous_container)
+        binds, affinity = merge_volume_bindings(options, ['/dev/tmpfs'], previous_container)
         assert sorted(binds) == sorted(expected)
         assert affinity == {'affinity:container': '=cdefab'}