diff --git a/compose/project.py b/compose/project.py index 4750a7a9a..999c28904 100644 --- a/compose/project.py +++ b/compose/project.py @@ -17,8 +17,10 @@ from .legacy import check_for_legacy_containers from .service import ContainerNet from .service import ConvergenceStrategy from .service import Net +from .service import parse_volume_from_spec from .service import Service from .service import ServiceNet +from .service import VolumeFromSpec from .utils import parallel_execute @@ -34,12 +36,18 @@ def sort_service_dicts(services): def get_service_names(links): return [link.split(':')[0] for link in links] + def get_service_names_from_volumes_from(volumes_from): + return [ + parse_volume_from_spec(volume_from).source + for volume_from in volumes_from + ] + def get_service_dependents(service_dict, services): name = service_dict['name'] return [ service for service in services if (name in get_service_names(service.get('links', [])) or - name in service.get('volumes_from', []) or + name in get_service_names_from_volumes_from(service.get('volumes_from', [])) or name == get_service_name_from_net(service.get('net'))) ] @@ -176,20 +184,23 @@ class Project(object): def get_volumes_from(self, service_dict): volumes_from = [] if 'volumes_from' in service_dict: - for volume_name in service_dict.get('volumes_from', []): + for volume_from_config in service_dict.get('volumes_from', []): + volume_from_spec = parse_volume_from_spec(volume_from_config) + # Get service try: - service = self.get_service(volume_name) - volumes_from.append(service) + service_name = self.get_service(volume_from_spec.source) + volume_from_spec = VolumeFromSpec(service_name, volume_from_spec.mode) except NoSuchService: try: - container = Container.from_id(self.client, volume_name) - volumes_from.append(container) + container_name = Container.from_id(self.client, volume_from_spec.source) + volume_from_spec = VolumeFromSpec(container_name, volume_from_spec.mode) except APIError: raise ConfigurationError( 'Service "%s" mounts volumes from "%s", which is ' 'not the name of a service or container.' % ( service_dict['name'], - volume_name)) + volume_from_spec.source)) + volumes_from.append(volume_from_spec) del service_dict['volumes_from'] return volumes_from diff --git a/compose/service.py b/compose/service.py index 698ab4844..044b34ad5 100644 --- a/compose/service.py +++ b/compose/service.py @@ -83,6 +83,9 @@ class NoSuchImageError(Exception): VolumeSpec = namedtuple('VolumeSpec', 'external internal mode') +VolumeFromSpec = namedtuple('VolumeFromSpec', 'source mode') + + ServiceName = namedtuple('ServiceName', 'project service number') @@ -520,7 +523,7 @@ class Service(object): return [(service.name, alias) for service, alias in self.links] def get_volumes_from_names(self): - return [s.name for s in self.volumes_from if isinstance(s, Service)] + return [s.source.name for s in self.volumes_from if isinstance(s.source, Service)] def get_container_name(self, number, one_off=False): # TODO: Implement issue #652 here @@ -560,16 +563,9 @@ class Service(object): def _get_volumes_from(self): volumes_from = [] - for volume_source in self.volumes_from: - if isinstance(volume_source, Service): - containers = volume_source.containers(stopped=True) - if not containers: - volumes_from.append(volume_source.create_container().id) - else: - volumes_from.extend(map(attrgetter('id'), containers)) - - elif isinstance(volume_source, Container): - volumes_from.append(volume_source.id) + for volume_from_spec in self.volumes_from: + volumes = build_volume_from(volume_from_spec) + volumes_from.extend(volumes) return volumes_from @@ -989,6 +985,38 @@ def parse_volume_spec(volume_config): return VolumeSpec(external, internal, mode) + +def build_volume_from(volume_from_spec): + """ + volume_from can be either a service or a container. We want to return the + container.id and format it into a string complete with the mode. + """ + if isinstance(volume_from_spec.source, Service): + containers = volume_from_spec.source.containers(stopped=True) + if not containers: + return ["{}:{}".format(volume_from_spec.source.create_container().id, volume_from_spec.mode)] + + container = containers[0] + return ["{}:{}".format(container.id, volume_from_spec.mode)] + elif isinstance(volume_from_spec.source, Container): + return ["{}:{}".format(volume_from_spec.source.id, volume_from_spec.mode)] + + +def parse_volume_from_spec(volume_from_config): + parts = volume_from_config.split(':') + if len(parts) > 2: + raise ConfigError("Volume %s has incorrect format, should be " + "external:internal[:mode]" % volume_from_config) + + if len(parts) == 1: + source = parts[0] + mode = 'rw' + else: + source, mode = parts + + return VolumeFromSpec(source, mode) + + # Labels diff --git a/docs/yml.md b/docs/yml.md index 81357df3d..a476fd33f 100644 --- a/docs/yml.md +++ b/docs/yml.md @@ -346,11 +346,13 @@ should always begin with `.` or `..`. ### volumes_from -Mount all of the volumes from another service or container. +Mount all of the volumes from another service or container, optionally +specifying read-only access(``ro``) or read-write(``rw``). volumes_from: - service_name - container_name + - service_name:rw ### cpu\_shares, cpuset, domainname, entrypoint, hostname, ipc, mac\_address, mem\_limit, memswap\_limit, privileged, read\_only, restart, stdin\_open, tty, user, volume\_driver, working\_dir diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index bd7ecccbe..ff50c80b2 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -6,6 +6,7 @@ from compose.const import LABEL_PROJECT from compose.container import Container from compose.project import Project from compose.service import ConvergenceStrategy +from compose.service import VolumeFromSpec def build_service_dicts(service_config): @@ -72,7 +73,7 @@ class ProjectTest(DockerClientTestCase): ) db = project.get_service('db') data = project.get_service('data') - self.assertEqual(db.volumes_from, [data]) + self.assertEqual(db.volumes_from, [VolumeFromSpec(data, 'rw')]) def test_volumes_from_container(self): data_container = Container.create( @@ -93,7 +94,7 @@ class ProjectTest(DockerClientTestCase): client=self.client, ) db = project.get_service('db') - self.assertEqual(db.volumes_from, [data_container]) + self.assertEqual(db._get_volumes_from(), [data_container.id + ':rw']) def test_net_from_service(self): project = Project.from_dicts( diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 7ea4aae51..8a8e4d54d 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -25,6 +25,7 @@ from compose.service import ConfigError from compose.service import ConvergencePlan from compose.service import Net from compose.service import Service +from compose.service import VolumeFromSpec def create_and_start_container(service, **override_options): @@ -272,12 +273,18 @@ class ServiceTest(DockerClientTestCase): command=["top"], labels={LABEL_PROJECT: 'composetest'}, ) - host_service = self.create_service('host', volumes_from=[volume_service, volume_container_2]) + host_service = self.create_service( + 'host', + volumes_from=[ + VolumeFromSpec(volume_service, 'rw'), + VolumeFromSpec(volume_container_2, 'rw') + ] + ) host_container = host_service.create_container() host_service.start_container(host_container) - self.assertIn(volume_container_1.id, + self.assertIn(volume_container_1.id + ':rw', host_container.get('HostConfig.VolumesFrom')) - self.assertIn(volume_container_2.id, + self.assertIn(volume_container_2.id + ':rw', host_container.get('HostConfig.VolumesFrom')) def test_execute_convergence_plan_recreate(self): diff --git a/tests/unit/project_test.py b/tests/unit/project_test.py index ce74eb30b..fc189fbb1 100644 --- a/tests/unit/project_test.py +++ b/tests/unit/project_test.py @@ -168,7 +168,7 @@ class ProjectTest(unittest.TestCase): 'volumes_from': ['aaa'] } ], self.mock_client) - self.assertEqual(project.get_service('test')._get_volumes_from(), [container_id]) + self.assertEqual(project.get_service('test')._get_volumes_from(), [container_id + ":rw"]) def test_use_volumes_from_service_no_container(self): container_name = 'test_vol_1' @@ -191,7 +191,7 @@ class ProjectTest(unittest.TestCase): 'volumes_from': ['vol'] } ], self.mock_client) - self.assertEqual(project.get_service('test')._get_volumes_from(), [container_name]) + self.assertEqual(project.get_service('test')._get_volumes_from(), [container_name + ":rw"]) @mock.patch.object(Service, 'containers') def test_use_volumes_from_service_container(self, mock_return): @@ -211,7 +211,7 @@ class ProjectTest(unittest.TestCase): 'volumes_from': ['vol'] } ], None) - self.assertEqual(project.get_service('test')._get_volumes_from(), container_ids) + self.assertEqual(project.get_service('test')._get_volumes_from(), [container_ids[0] + ':rw']) def test_net_unset(self): project = Project.from_dicts('test', [ diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index c682b8237..19d25e2ed 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -24,6 +24,7 @@ from compose.service import parse_repository_tag from compose.service import parse_volume_spec from compose.service import Service from compose.service import ServiceNet +from compose.service import VolumeFromSpec class ServiceTest(unittest.TestCase): @@ -75,9 +76,18 @@ class ServiceTest(unittest.TestCase): service = Service( 'test', image='foo', - volumes_from=[mock.Mock(id=container_id, spec=Container)]) + volumes_from=[VolumeFromSpec(mock.Mock(id=container_id, spec=Container), 'rw')]) - self.assertEqual(service._get_volumes_from(), [container_id]) + self.assertEqual(service._get_volumes_from(), [container_id + ':rw']) + + def test_get_volumes_from_container_read_only(self): + container_id = 'aabbccddee' + service = Service( + 'test', + image='foo', + volumes_from=[VolumeFromSpec(mock.Mock(id=container_id, spec=Container), 'ro')]) + + self.assertEqual(service._get_volumes_from(), [container_id + ':ro']) def test_get_volumes_from_service_container_exists(self): container_ids = ['aabbccddee', '12345'] @@ -86,9 +96,21 @@ class ServiceTest(unittest.TestCase): mock.Mock(id=container_id, spec=Container) for container_id in container_ids ] - service = Service('test', volumes_from=[from_service], image='foo') + service = Service('test', volumes_from=[VolumeFromSpec(from_service, 'rw')], image='foo') - self.assertEqual(service._get_volumes_from(), container_ids) + self.assertEqual(service._get_volumes_from(), [container_ids[0] + ":rw"]) + + def test_get_volumes_from_service_container_exists_with_flags(self): + for mode in ['ro', 'rw', 'z', 'rw,z', 'z,rw']: + container_ids = ['aabbccddee:' + mode, '12345:' + mode] + from_service = mock.create_autospec(Service) + from_service.containers.return_value = [ + mock.Mock(id=container_id.split(':')[0], spec=Container) + for container_id in container_ids + ] + service = Service('test', volumes_from=[VolumeFromSpec(from_service, mode)], image='foo') + + self.assertEqual(service._get_volumes_from(), [container_ids[0]]) def test_get_volumes_from_service_no_container(self): container_id = 'abababab' @@ -97,9 +119,9 @@ class ServiceTest(unittest.TestCase): from_service.create_container.return_value = mock.Mock( id=container_id, spec=Container) - service = Service('test', image='foo', volumes_from=[from_service]) + service = Service('test', image='foo', volumes_from=[VolumeFromSpec(from_service, 'rw')]) - self.assertEqual(service._get_volumes_from(), [container_id]) + self.assertEqual(service._get_volumes_from(), [container_id + ':rw']) from_service.create_container.assert_called_once_with() def test_split_domainname_none(self): @@ -357,7 +379,7 @@ class ServiceTest(unittest.TestCase): client=self.mock_client, net=ServiceNet(Service('other')), links=[(Service('one'), 'one')], - volumes_from=[Service('two')]) + volumes_from=[VolumeFromSpec(Service('two'), 'rw')]) config_dict = service.config_dict() expected = {