Browse Source

Avoid rebinding tmpfs data volumes when recreating containers

Signed-off-by: Joffrey F <[email protected]>
Joffrey F 8 years ago
parent
commit
a0cf0a2009
3 changed files with 37 additions and 9 deletions
  1. 9 3
      compose/service.py
  2. 18 0
      tests/integration/project_test.py
  3. 10 6
      tests/unit/service_test.py

+ 9 - 3
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
@@ -709,6 +710,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)
@@ -1091,7 +1093,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.
     """
@@ -1103,7 +1105,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)
@@ -1114,7 +1116,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.
     """
@@ -1137,6 +1139,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

+ 18 - 0
tests/integration/project_test.py

@@ -551,6 +551,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)

+ 10 - 6
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'}