Revert "Merge pull request #711 from dnephin/fix_volumes_on_recreate"

This reverts commit 55095ef488, reversing
changes made to 72095f54b2.

Signed-off-by: Daniel Nephin <dnephin@yelp.com>

tests/integration/service_test.py

@@ -98,7 +98,7 @@ class ServiceTest(DockerClientTestCase):
         service = self.create_service('db', volumes=['/var/db'])
         container = service.create_container()
         service.start_container(container)
-        self.assertIn('/var/db', container.get('Volumes'))
+        self.assertIn('/var/db', container.inspect()['Volumes'])
 
     def test_create_container_with_cpu_shares(self):
         service = self.create_service('db', cpu_shares=73)
@@ -148,23 +148,30 @@ class ServiceTest(DockerClientTestCase):
         self.assertIn('FOO=1', old_container.dictionary['Config']['Env'])
         self.assertEqual(old_container.name, 'figtest_db_1')
         service.start_container(old_container)
-        volume_path = old_container.get('Volumes')['/etc']
+        volume_path = old_container.inspect()['Volumes']['/etc']
 
         num_containers_before = len(self.client.containers(all=True))
 
         service.options['environment']['FOO'] = '2'
-        containers = service.recreate_containers()
-        self.assertEqual(len(containers), 1)
+        tuples = service.recreate_containers()
+        self.assertEqual(len(tuples), 1)
 
-        new_container = containers[0]
-        self.assertEqual(new_container.get('Config.Entrypoint'), ['sleep'])
-        self.assertEqual(new_container.get('Config.Cmd'), ['300'])
-        self.assertIn('FOO=2', new_container.get('Config.Env'))
+        intermediate_container = tuples[0][0]
+        new_container = tuples[0][1]
+        self.assertEqual(intermediate_container.dictionary['Config']['Entrypoint'], ['/bin/echo'])
+
+        self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep'])
+        self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300'])
+        self.assertIn('FOO=2', new_container.dictionary['Config']['Env'])
         self.assertEqual(new_container.name, 'figtest_db_1')
-        self.assertEqual(new_container.get('Volumes')['/etc'], volume_path)
+        self.assertEqual(new_container.inspect()['Volumes']['/etc'], volume_path)
+        self.assertIn(intermediate_container.id, new_container.dictionary['HostConfig']['VolumesFrom'])
 
         self.assertEqual(len(self.client.containers(all=True)), num_containers_before)
         self.assertNotEqual(old_container.id, new_container.id)
+        self.assertRaises(APIError,
+                          self.client.inspect_container,
+                          intermediate_container.id)
 
     def test_recreate_containers_when_containers_are_stopped(self):
         service = self.create_service(
@@ -179,16 +186,6 @@ class ServiceTest(DockerClientTestCase):
         service.recreate_containers()
         self.assertEqual(len(service.containers(stopped=True)), 1)
 
-    def test_recreate_containers_with_volume_changes(self):
-        service = self.create_service('withvolumes', volumes=['/etc'])
-        old_container = create_and_start_container(service)
-        self.assertEqual(old_container.get('Volumes').keys(), ['/etc'])
-
-        service = self.create_service('withvolumes')
-        container, = service.recreate_containers()
-        service.start_container(container)
-        self.assertEqual(container.get('Volumes'), {})
-
     def test_start_container_passes_through_options(self):
         db = self.create_service('db')
         create_and_start_container(db, environment={'FOO': 'BAR'})

tests/unit/service_test.py

@@ -11,15 +11,13 @@ from requests import Response
 from fig import Service
 from fig.container import Container
 from fig.service import (
-    APIError,
     ConfigError,
-    build_port_bindings,
-    build_volume_binding,
-    get_container_data_volumes,
-    get_volume_bindings,
-    parse_repository_tag,
-    parse_volume_spec,
     split_port,
+    build_port_bindings,
+    parse_volume_spec,
+    build_volume_binding,
+    APIError,
+    parse_repository_tag,
 )
 
 
@@ -59,6 +57,13 @@ class ServiceTest(unittest.TestCase):
 
         self.assertEqual(service._get_volumes_from(), [container_id])
 
+    def test_get_volumes_from_intermediate_container(self):
+        container_id = 'aabbccddee'
+        service = Service('test')
+        container = mock.Mock(id=container_id, spec=Container)
+
+        self.assertEqual(service._get_volumes_from(container), [container_id])
+
     def test_get_volumes_from_service_container_exists(self):
         container_ids = ['aabbccddee', '12345']
         from_service = mock.create_autospec(Service)
@@ -283,50 +288,6 @@ class ServiceVolumesTest(unittest.TestCase):
             binding,
             ('/home/user', dict(bind='/home/user', ro=False)))
 
-    def test_get_container_data_volumes(self):
-        options = [
-            '/host/volume:/host/volume:ro',
-            '/new/volume',
-            '/existing/volume',
-        ]
-
-        container = Container(None, {
-            'Volumes': {
-                '/host/volume':     '/host/volume',
-                '/existing/volume': '/var/lib/docker/aaaaaaaa',
-                '/removed/volume':  '/var/lib/docker/bbbbbbbb',
-            },
-        }, has_been_inspected=True)
-
-        expected = {
-            '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
-        }
-
-        binds = get_container_data_volumes(container, options)
-        self.assertEqual(binds, expected)
-
-    def test_get_volume_bindings(self):
-        options = [
-            '/host/volume:/host/volume:ro',
-            '/host/rw/volume:/host/rw/volume',
-            '/new/volume',
-            '/existing/volume',
-        ]
-
-        intermediate_container = Container(None, {
-            'Volumes': {'/existing/volume': '/var/lib/docker/aaaaaaaa'},
-            }, has_been_inspected=True)
-
-        expected = {
-            '/host/volume': {'bind': '/host/volume', 'ro': True},
-            '/host/rw/volume': {'bind': '/host/rw/volume', 'ro': False},
-            '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
-        }
-
-        binds = get_volume_bindings(options, intermediate_container)
-        self.assertEqual(binds, expected)
-
-
 class ServiceEnvironmentTest(unittest.TestCase):
 
     def setUp(self):