mirror of https://github.com/docker/compose.git
Resolves #447, fix volume logic for recreate container
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
This commit is contained in:
parent
26f45efea2
commit
7eb476e61d
|
@ -181,8 +181,8 @@ class Project(object):
|
|||
|
||||
for container in create_func(
|
||||
insecure_registry=insecure_registry,
|
||||
detach=detach,
|
||||
do_build=do_build):
|
||||
detach=detach,
|
||||
do_build=do_build):
|
||||
running_containers.append(container)
|
||||
|
||||
return running_containers
|
||||
|
|
|
@ -280,6 +280,7 @@ class Service(object):
|
|||
else:
|
||||
raise
|
||||
|
||||
intermediate_options = dict(self.options, **override_options)
|
||||
intermediate_container = Container.create(
|
||||
self.client,
|
||||
image=container.image,
|
||||
|
@ -287,16 +288,21 @@ class Service(object):
|
|||
command=[],
|
||||
detach=True,
|
||||
)
|
||||
intermediate_container.start(volumes_from=container.id)
|
||||
intermediate_container.start(
|
||||
binds=get_container_data_volumes(
|
||||
container, intermediate_options.get('volumes')))
|
||||
intermediate_container.wait()
|
||||
container.remove()
|
||||
|
||||
# TODO: volumes are being passed to both start and create, this is
|
||||
# probably unnecessary
|
||||
options = dict(override_options)
|
||||
new_container = self.create_container(do_build=False, **options)
|
||||
self.start_container(new_container, intermediate_container=intermediate_container)
|
||||
self.start_container(
|
||||
new_container,
|
||||
intermediate_container=intermediate_container)
|
||||
|
||||
intermediate_container.remove()
|
||||
|
||||
return new_container
|
||||
|
||||
def start_container_if_stopped(self, container, **options):
|
||||
|
@ -309,12 +315,6 @@ class Service(object):
|
|||
def start_container(self, container, intermediate_container=None, **override_options):
|
||||
options = dict(self.options, **override_options)
|
||||
port_bindings = build_port_bindings(options.get('ports') or [])
|
||||
|
||||
volume_bindings = dict(
|
||||
build_volume_binding(parse_volume_spec(volume))
|
||||
for volume in options.get('volumes') or []
|
||||
if ':' in volume)
|
||||
|
||||
privileged = options.get('privileged', False)
|
||||
net = options.get('net', 'bridge')
|
||||
dns = options.get('dns', None)
|
||||
|
@ -323,12 +323,14 @@ class Service(object):
|
|||
cap_drop = options.get('cap_drop', None)
|
||||
|
||||
restart = parse_restart_spec(options.get('restart', None))
|
||||
binds = get_volume_bindings(
|
||||
options.get('volumes'), intermediate_container)
|
||||
|
||||
container.start(
|
||||
links=self._get_links(link_to_self=options.get('one_off', False)),
|
||||
port_bindings=port_bindings,
|
||||
binds=volume_bindings,
|
||||
volumes_from=self._get_volumes_from(intermediate_container),
|
||||
binds=binds,
|
||||
volumes_from=self._get_volumes_from(),
|
||||
privileged=privileged,
|
||||
network_mode=net,
|
||||
dns=dns,
|
||||
|
@ -390,7 +392,7 @@ class Service(object):
|
|||
links.append((external_link, link_name))
|
||||
return links
|
||||
|
||||
def _get_volumes_from(self, intermediate_container=None):
|
||||
def _get_volumes_from(self):
|
||||
volumes_from = []
|
||||
for volume_source in self.volumes_from:
|
||||
if isinstance(volume_source, Service):
|
||||
|
@ -404,9 +406,6 @@ class Service(object):
|
|||
elif isinstance(volume_source, Container):
|
||||
volumes_from.append(volume_source.id)
|
||||
|
||||
if intermediate_container:
|
||||
volumes_from.append(intermediate_container.id)
|
||||
|
||||
return volumes_from
|
||||
|
||||
def _get_container_create_options(self, override_options, one_off=False):
|
||||
|
@ -521,6 +520,45 @@ class Service(object):
|
|||
)
|
||||
|
||||
|
||||
def get_container_data_volumes(container, volumes_option):
|
||||
"""Find the container data volumes that are in `volumes_option`, and return
|
||||
a mapping of volume bindings for those volumes.
|
||||
"""
|
||||
volumes = []
|
||||
for volume in volumes_option or []:
|
||||
volume = parse_volume_spec(volume)
|
||||
# No need to preserve host volumes
|
||||
if volume.external:
|
||||
continue
|
||||
|
||||
volume_path = (container.get('Volumes') or {}).get(volume.internal)
|
||||
# New volume, doesn't exist in the old container
|
||||
if not volume_path:
|
||||
continue
|
||||
|
||||
# Copy existing volume from old container
|
||||
volume = volume._replace(external=volume_path)
|
||||
volumes.append(build_volume_binding(volume))
|
||||
|
||||
return dict(volumes)
|
||||
|
||||
|
||||
def get_volume_bindings(volumes_option, intermediate_container):
|
||||
"""Return a list of volume bindings for a container. Container data volume
|
||||
bindings are replaced by those in the intermediate container.
|
||||
"""
|
||||
volume_bindings = dict(
|
||||
build_volume_binding(parse_volume_spec(volume))
|
||||
for volume in volumes_option or []
|
||||
if ':' in volume)
|
||||
|
||||
if intermediate_container:
|
||||
volume_bindings.update(
|
||||
get_container_data_volumes(intermediate_container, volumes_option))
|
||||
|
||||
return volume_bindings
|
||||
|
||||
|
||||
NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')
|
||||
|
||||
|
||||
|
|
|
@ -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.inspect()['Volumes'])
|
||||
self.assertIn('/var/db', container.get('Volumes'))
|
||||
|
||||
def test_create_container_with_cpu_shares(self):
|
||||
service = self.create_service('db', cpu_shares=73)
|
||||
|
@ -179,6 +179,16 @@ 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'})
|
||||
|
|
|
@ -11,13 +11,15 @@ from requests import Response
|
|||
from fig import Service
|
||||
from fig.container import Container
|
||||
from fig.service import (
|
||||
ConfigError,
|
||||
split_port,
|
||||
build_port_bindings,
|
||||
parse_volume_spec,
|
||||
build_volume_binding,
|
||||
APIError,
|
||||
ConfigError,
|
||||
build_port_bindings,
|
||||
build_volume_binding,
|
||||
get_container_data_volumes,
|
||||
get_volume_bindings,
|
||||
parse_repository_tag,
|
||||
parse_volume_spec,
|
||||
split_port,
|
||||
)
|
||||
|
||||
|
||||
|
@ -57,13 +59,6 @@ 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)
|
||||
|
@ -288,6 +283,50 @@ 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):
|
||||
|
|
Loading…
Reference in New Issue