From 5b55a08846088d6cdbc4aca08d3143f5c9c3d3b7 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Sat, 18 Jul 2015 11:38:46 +0200 Subject: [PATCH] Add support for ro option in volumes_from Fixes #1188 Signed-off-by: Vincent Demeester --- compose/project.py | 24 +++++++++----- compose/service.py | 52 ++++++++++++++++++++++++------- docs/yml.md | 4 ++- tests/integration/project_test.py | 5 +-- tests/integration/service_test.py | 7 +++-- tests/unit/project_test.py | 6 ++-- tests/unit/service_test.py | 34 ++++++++++++++++---- 7 files changed, 97 insertions(+), 35 deletions(-) diff --git a/compose/project.py b/compose/project.py index 4750a7a9a..919a201f1 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,15 @@ 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 [volume_from.split(':')[0] 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 +181,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_config, + 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 c9ca00ae4..79a138aac 100644 --- a/compose/service.py +++ b/compose/service.py @@ -6,7 +6,6 @@ import os import re import sys from collections import namedtuple -from operator import attrgetter import enum import six @@ -82,6 +81,9 @@ class NoSuchImageError(Exception): VolumeSpec = namedtuple('VolumeSpec', 'external internal mode') +VolumeFromSpec = namedtuple('VolumeFromSpec', 'source mode') + + ServiceName = namedtuple('ServiceName', 'project service number') @@ -519,7 +521,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 @@ -559,16 +561,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 @@ -988,6 +983,39 @@ def parse_volume_spec(volume_config): return VolumeSpec(external, internal, mode) + +def build_volume_from(volume_from_spec): + volumes_from = [] + if isinstance(volume_from_spec.source, Service): + containers = volume_from_spec.source.containers(stopped=True) + if not containers: + volumes_from = ["{}:{}".format(volume_from_spec.source.create_container().id, volume_from_spec.mode)] + else: + volumes_from = ["{}:{}".format(container.id, volume_from_spec.mode) for container in containers] + elif isinstance(volume_from_spec.source, Container): + volumes_from = ["{}:{}".format(volume_from_spec.source.id, volume_from_spec.mode)] + return volumes_from + + +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 + + if mode not in ('rw', 'ro'): + raise ConfigError("VolumeFrom %s has invalid mode (%s), should be " + "one of: rw, ro." % (volume_from_config, mode)) + + return VolumeFromSpec(source, mode) + + # Labels diff --git a/docs/yml.md b/docs/yml.md index 81357df3d..12c9b554a 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, with the +supported flags by docker : ``ro``, ``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 2c9c6fc20..306060960 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,12 @@ 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..f3cf9e294 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(), [cid + ':rw' for cid in container_ids]) 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..f85d34d2a 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(), [cid + ":rw" for cid in container_ids]) + + 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) 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):