From 69ed5f9c48a5fe419ac041239659fe69304fe4a9 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 17 Dec 2015 17:49:48 +0000 Subject: [PATCH 01/11] Specify networks in Compose file There's not yet a proper way for services to join networks Signed-off-by: Aanand Prasad --- compose/cli/main.py | 3 +- compose/config/config.py | 43 +-- compose/config/fields_schema_v2.json | 13 + compose/network.py | 57 ++++ compose/project.py | 127 ++++----- tests/acceptance/cli_test.py | 19 +- tests/fixtures/networks/docker-compose.yml | 7 + .../no-links-composefile/docker-compose.yml | 9 + tests/integration/project_test.py | 113 ++++---- tests/unit/project_test.py | 248 +++++++++++------- 10 files changed, 400 insertions(+), 239 deletions(-) create mode 100644 compose/network.py create mode 100644 tests/fixtures/networks/docker-compose.yml create mode 100644 tests/fixtures/no-links-composefile/docker-compose.yml diff --git a/compose/cli/main.py b/compose/cli/main.py index 9957d391f..62abcb10a 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -696,8 +696,7 @@ def run_one_off_container(container_options, project, service, options): start_deps=True, strategy=ConvergenceStrategy.never) - if project.use_networking: - project.ensure_network_exists() + project.initialize_networks() container = service.create_container( quiet=True, diff --git a/compose/config/config.py b/compose/config/config.py index 8383fb5c9..c8d93faf6 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -139,14 +139,16 @@ class ConfigFile(namedtuple('_ConfigFile', 'filename config')): return {} if self.version == 1 else self.config.get('volumes', {}) -class Config(namedtuple('_Config', 'version services volumes')): +class Config(namedtuple('_Config', 'version services volumes networks')): """ :param version: configuration version :type version: int :param services: List of service description dictionaries :type services: :class:`list` - :param volumes: List of volume description dictionaries - :type volumes: :class:`list` + :param volumes: Dictionary mapping volume names to description dictionaries + :type volumes: :class:`dict` + :param networks: Dictionary mapping network names to description dictionaries + :type networks: :class:`dict` """ @@ -256,39 +258,44 @@ def load(config_details): config_details = config_details._replace(config_files=processed_files) main_file = config_details.config_files[0] - volumes = load_volumes(config_details.config_files) + volumes = load_mapping(config_details.config_files, 'volumes', 'Volume') + networks = load_mapping(config_details.config_files, 'networks', 'Network') service_dicts = load_services( config_details.working_dir, main_file.filename, [file.get_service_dicts() for file in config_details.config_files], main_file.version) - return Config(main_file.version, service_dicts, volumes) + return Config(main_file.version, service_dicts, volumes, networks) -def load_volumes(config_files): - volumes = {} +def load_mapping(config_files, key, entity_type): + mapping = {} + for config_file in config_files: - for name, volume_config in config_file.get_volumes().items(): - volumes[name] = volume_config or {} - if not volume_config: + for name, config in config_file.config.get(key, {}).items(): + mapping[name] = config or {} + if not config: continue - external = volume_config.get('external') + external = config.get('external') if external: - if len(volume_config.keys()) > 1: + if len(config.keys()) > 1: raise ConfigurationError( - 'Volume {0} declared as external but specifies' - ' additional attributes ({1}). '.format( + '{} {} declared as external but specifies' + ' additional attributes ({}). '.format( + entity_type, name, - ', '.join([k for k in volume_config.keys() if k != 'external']) + ', '.join([k for k in config.keys() if k != 'external']) ) ) if isinstance(external, dict): - volume_config['external_name'] = external.get('name') + config['external_name'] = external.get('name') else: - volume_config['external_name'] = name + config['external_name'] = name - return volumes + mapping[name] = config + + return mapping def load_services(working_dir, filename, service_configs, version): diff --git a/compose/config/fields_schema_v2.json b/compose/config/fields_schema_v2.json index 25126ed16..b0f304e86 100644 --- a/compose/config/fields_schema_v2.json +++ b/compose/config/fields_schema_v2.json @@ -17,6 +17,15 @@ }, "additionalProperties": false }, + "networks": { + "id": "#/properties/networks", + "type": "object", + "patternProperties": { + "^[a-zA-Z0-9._-]+$": { + "$ref": "#/definitions/network" + } + } + }, "volumes": { "id": "#/properties/volumes", "type": "object", @@ -30,6 +39,10 @@ }, "definitions": { + "network": { + "id": "#/definitions/network", + "type": "object" + }, "volume": { "id": "#/definitions/volume", "type": ["object", "null"], diff --git a/compose/network.py b/compose/network.py new file mode 100644 index 000000000..0b4e40c60 --- /dev/null +++ b/compose/network.py @@ -0,0 +1,57 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + +import logging + +from docker.errors import NotFound + +from .config import ConfigurationError + + +log = logging.getLogger(__name__) + + +class Network(object): + def __init__(self, client, project, name, driver=None, driver_opts=None): + self.client = client + self.project = project + self.name = name + self.driver = driver + self.driver_opts = driver_opts + + def ensure(self): + try: + data = self.inspect() + if self.driver and data['Driver'] != self.driver: + raise ConfigurationError( + 'Network {} needs to be recreated - driver has changed' + .format(self.full_name)) + if data['Options'] != (self.driver_opts or {}): + raise ConfigurationError( + 'Network {} needs to be recreated - options have changed' + .format(self.full_name)) + except NotFound: + driver_name = 'the default driver' + if self.driver: + driver_name = 'driver "{}"'.format(self.driver) + + log.info( + 'Creating network "{}" with {}' + .format(self.full_name, driver_name) + ) + + self.client.create_network( + self.full_name, self.driver, self.driver_opts + ) + + def remove(self): + # TODO: don't remove external networks + log.info("Removing network {}".format(self.full_name)) + self.client.remove_network(self.full_name) + + def inspect(self): + return self.client.inspect_network(self.full_name) + + @property + def full_name(self): + return '{0}_{1}'.format(self.project, self.name) diff --git a/compose/project.py b/compose/project.py index 3e7c5afda..10d457a7f 100644 --- a/compose/project.py +++ b/compose/project.py @@ -17,6 +17,7 @@ from .const import LABEL_ONE_OFF from .const import LABEL_PROJECT from .const import LABEL_SERVICE from .container import Container +from .network import Network from .service import ContainerNet from .service import ConvergenceStrategy from .service import Net @@ -33,12 +34,14 @@ class Project(object): """ A collection of services. """ - def __init__(self, name, services, client, volumes=None, use_networking=False, network_driver=None): + def __init__(self, name, services, client, networks=None, volumes=None, + use_networking=False, network_driver=None): self.name = name self.services = services self.client = client self.use_networking = use_networking self.network_driver = network_driver + self.networks = networks or [] self.volumes = volumes or [] def labels(self, one_off=False): @@ -55,9 +58,6 @@ class Project(object): use_networking = (config_data.version and config_data.version >= 2) project = cls(name, [], client, use_networking=use_networking) - if use_networking: - remove_links(config_data.services) - for service_dict in config_data.services: links = project.get_links(service_dict) volumes_from = get_volumes_from(project, service_dict) @@ -72,6 +72,16 @@ class Project(object): net=net, volumes_from=volumes_from, **service_dict)) + + if config_data.networks: + for network_name, data in config_data.networks.items(): + project.networks.append( + Network( + client=client, project=name, name=network_name, + driver=data.get('driver'), driver_opts=data.get('driver_opts') + ) + ) + if config_data.volumes: for vol_name, data in config_data.volumes.items(): project.volumes.append( @@ -82,6 +92,7 @@ class Project(object): external_name=data.get('external_name') ) ) + return project @property @@ -124,20 +135,18 @@ class Project(object): Raises NoSuchService if any of the named services do not exist. """ if service_names is None or len(service_names) == 0: - return self.get_services( - service_names=self.service_names, - include_deps=include_deps - ) - else: - unsorted = [self.get_service(name) for name in service_names] - services = [s for s in self.services if s in unsorted] + service_names = self.service_names - if include_deps: - services = reduce(self._inject_deps, services, []) + unsorted = [self.get_service(name) for name in service_names] + services = [s for s in self.services if s in unsorted] - uniques = [] - [uniques.append(s) for s in services if s not in uniques] - return uniques + if include_deps: + services = reduce(self._inject_deps, services, []) + + uniques = [] + [uniques.append(s) for s in services if s not in uniques] + + return uniques def get_services_without_duplicate(self, service_names=None, include_deps=False): services = self.get_services(service_names, include_deps) @@ -166,7 +175,7 @@ class Project(object): net = service_dict.pop('net', None) if not net: if self.use_networking: - return Net(self.default_network_name) + return Net(self.default_network.full_name) return Net(None) net_name = get_service_name_from_net(net) @@ -251,7 +260,7 @@ class Project(object): def down(self, remove_image_type, include_volumes): self.stop() self.remove_stopped(v=include_volumes) - self.remove_network() + self.remove_default_network() if include_volumes: self.remove_volumes() @@ -262,10 +271,34 @@ class Project(object): for service in self.get_services(): service.remove_image(remove_image_type) + def remove_default_network(self): + if not self.use_networking: + return + if self.uses_default_network(): + self.default_network.remove() + def remove_volumes(self): for volume in self.volumes: volume.remove() + def initialize_networks(self): + networks = self.networks + if self.uses_default_network(): + networks.append(self.default_network) + + for network in networks: + network.ensure() + + def uses_default_network(self): + return any( + service.net.mode == self.default_network.full_name + for service in self.services + ) + + @property + def default_network(self): + return Network(client=self.client, project=self.name, name='default') + def restart(self, service_names=None, **options): containers = self.containers(service_names, stopped=True) parallel.parallel_restart(containers, options) @@ -335,9 +368,7 @@ class Project(object): plans = self._get_convergence_plans(services, strategy) - if self.use_networking and self.uses_default_network(): - self.ensure_network_exists() - + self.initialize_networks() self.initialize_volumes() return [ @@ -395,40 +426,6 @@ class Project(object): return [c for c in containers if matches_service_names(c)] - def get_network(self): - try: - return self.client.inspect_network(self.default_network_name) - except NotFound: - return None - - def ensure_network_exists(self): - # TODO: recreate network if driver has changed? - if self.get_network() is None: - driver_name = 'the default driver' - if self.network_driver: - driver_name = 'driver "{}"'.format(self.network_driver) - - log.info( - 'Creating network "{}" with {}' - .format(self.default_network_name, driver_name) - ) - self.client.create_network(self.default_network_name, driver=self.network_driver) - - def remove_network(self): - if not self.use_networking: - return - network = self.get_network() - if network: - log.info("Removing network %s", self.default_network_name) - self.client.remove_network(network['Id']) - - def uses_default_network(self): - return any(service.net.mode == self.default_network_name for service in self.services) - - @property - def default_network_name(self): - return '{}_default'.format(self.name) - def _inject_deps(self, acc, service): dep_names = service.get_dependency_names() @@ -444,26 +441,6 @@ class Project(object): return acc + dep_services -def remove_links(service_dicts): - services_with_links = [s for s in service_dicts if 'links' in s] - if not services_with_links: - return - - if len(services_with_links) == 1: - prefix = '"{}" defines'.format(services_with_links[0]['name']) - else: - prefix = 'Some services ({}) define'.format( - ", ".join('"{}"'.format(s['name']) for s in services_with_links)) - - log.warn( - '\n{} links, which are not compatible with Docker networking and will be ignored.\n' - 'Future versions of Docker will not support links - you should remove them for ' - 'forwards-compatibility.\n'.format(prefix)) - - for s in services_with_links: - del s['links'] - - def get_volumes_from(project, service_dict): volumes_from = service_dict.pop('volumes_from', None) if not volumes_from: diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index cb04918bd..1e31988b2 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -354,7 +354,7 @@ class CLITestCase(DockerClientTestCase): self.base_dir = 'tests/fixtures/links-composefile' self.dispatch(['up', '-d'], None) - networks = self.client.networks(names=[self.project.default_network_name]) + networks = self.client.networks(names=[self.project.default_network.full_name]) self.assertEqual(len(networks), 0) for service in self.project.get_services(): @@ -371,7 +371,7 @@ class CLITestCase(DockerClientTestCase): services = self.project.get_services() - networks = self.client.networks(names=[self.project.default_network_name]) + networks = self.client.networks(names=[self.project.default_network.full_name]) for n in networks: self.addCleanup(self.client.remove_network, n['Id']) self.assertEqual(len(networks), 1) @@ -388,6 +388,19 @@ class CLITestCase(DockerClientTestCase): web_container = self.project.get_service('simple').containers()[0] self.assertFalse(web_container.get('HostConfig.Links')) + def test_up_with_networks(self): + self.base_dir = 'tests/fixtures/networks' + self.dispatch(['up', '-d'], None) + + networks = self.client.networks(names=[ + '{}_{}'.format(self.project.name, n) + for n in ['foo', 'bar']]) + + self.assertEqual(len(networks), 2) + + for net in networks: + self.assertEqual(net['Driver'], 'bridge') + def test_up_with_links_is_invalid(self): self.base_dir = 'tests/fixtures/v2-simple' @@ -698,7 +711,7 @@ class CLITestCase(DockerClientTestCase): self.dispatch(['run', 'simple', 'true'], None) service = self.project.get_service('simple') container, = service.containers(stopped=True, one_off=True) - networks = self.client.networks(names=[self.project.default_network_name]) + networks = self.client.networks(names=[self.project.default_network.full_name]) for n in networks: self.addCleanup(self.client.remove_network, n['Id']) self.assertEqual(len(networks), 1) diff --git a/tests/fixtures/networks/docker-compose.yml b/tests/fixtures/networks/docker-compose.yml new file mode 100644 index 000000000..c0795526f --- /dev/null +++ b/tests/fixtures/networks/docker-compose.yml @@ -0,0 +1,7 @@ +version: 2 + +networks: + foo: + driver: + + bar: {} diff --git a/tests/fixtures/no-links-composefile/docker-compose.yml b/tests/fixtures/no-links-composefile/docker-compose.yml new file mode 100644 index 000000000..75a6a085c --- /dev/null +++ b/tests/fixtures/no-links-composefile/docker-compose.yml @@ -0,0 +1,9 @@ +db: + image: busybox:latest + command: top +web: + image: busybox:latest + command: top +console: + image: busybox:latest + command: top diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 535a97750..ef8a084b7 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -14,7 +14,6 @@ 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 Net def build_service_dicts(service_config): @@ -104,21 +103,6 @@ class ProjectTest(DockerClientTestCase): db = project.get_service('db') self.assertEqual(db._get_volumes_from(), [data_container.id + ':rw']) - def test_get_network_does_not_exist(self): - project = Project('composetest', [], self.client) - assert project.get_network() is None - - def test_get_network(self): - project_name = 'network_does_exist' - network_name = '{}_default'.format(project_name) - - project = Project(project_name, [], self.client) - self.client.create_network(network_name) - self.addCleanup(self.client.remove_network, network_name) - - assert isinstance(project.get_network(), dict) - assert project.get_network()['Name'] == network_name - def test_net_from_service(self): project = Project.from_config( name='composetest', @@ -473,18 +457,6 @@ 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_with_custom_network(self): - network_name = 'composetest-custom' - - self.client.create_network(network_name) - self.addCleanup(self.client.remove_network, network_name) - - web = self.create_service('web', net=Net(network_name)) - project = Project('composetest', [web], self.client, use_networking=True) - project.up() - - assert project.get_network() is None - def test_unscale_after_restart(self): web = self.create_service('web') project = Project('composetest', [web], self.client) @@ -510,15 +482,50 @@ class ProjectTest(DockerClientTestCase): service = project.get_service('web') self.assertEqual(len(service.containers()), 1) + def test_project_up_networks(self): + config_data = config.Config( + version=2, + services=[{ + 'name': 'web', + 'image': 'busybox:latest', + 'command': 'top', + }], + volumes={}, + networks={ + 'foo': {'driver': 'bridge'}, + 'bar': {'driver': None}, + 'baz': {}, + }, + ) + + project = Project.from_config( + client=self.client, + name='composetest', + config_data=config_data, + ) + project.up() + self.assertEqual(len(project.containers()), 1) + + for net_name in ['foo', 'bar', 'baz']: + full_net_name = 'composetest_{}'.format(net_name) + network_data = self.client.inspect_network(full_net_name) + self.assertEqual(network_data['Name'], full_net_name) + + foo_data = self.client.inspect_network('composetest_foo') + self.assertEqual(foo_data['Driver'], 'bridge') + def test_project_up_volumes(self): vol_name = '{0:x}'.format(random.getrandbits(32)) full_vol_name = 'composetest_{0}'.format(vol_name) config_data = config.Config( - version=2, services=[{ + version=2, + services=[{ 'name': 'web', 'image': 'busybox:latest', 'command': 'top' - }], volumes={vol_name: {'driver': 'local'}} + }], + volumes={vol_name: {'driver': 'local'}}, + networks={}, ) project = Project.from_config( @@ -587,11 +594,14 @@ class ProjectTest(DockerClientTestCase): vol_name = '{0:x}'.format(random.getrandbits(32)) full_vol_name = 'composetest_{0}'.format(vol_name) config_data = config.Config( - version=2, services=[{ + version=2, + services=[{ 'name': 'web', 'image': 'busybox:latest', 'command': 'top' - }], volumes={vol_name: {}} + }], + volumes={vol_name: {}}, + networks={}, ) project = Project.from_config( @@ -608,11 +618,14 @@ class ProjectTest(DockerClientTestCase): vol_name = '{0:x}'.format(random.getrandbits(32)) full_vol_name = 'composetest_{0}'.format(vol_name) config_data = config.Config( - version=2, services=[{ + version=2, + services=[{ 'name': 'web', 'image': 'busybox:latest', 'command': 'top' - }], volumes={vol_name: {}} + }], + volumes={vol_name: {}}, + networks={}, ) project = Project.from_config( @@ -629,11 +642,14 @@ class ProjectTest(DockerClientTestCase): vol_name = '{0:x}'.format(random.getrandbits(32)) config_data = config.Config( - version=2, services=[{ + version=2, + services=[{ 'name': 'web', 'image': 'busybox:latest', 'command': 'top' - }], volumes={vol_name: {'driver': 'foobar'}} + }], + volumes={vol_name: {'driver': 'foobar'}}, + networks={}, ) project = Project.from_config( @@ -648,11 +664,14 @@ class ProjectTest(DockerClientTestCase): full_vol_name = 'composetest_{0}'.format(vol_name) config_data = config.Config( - version=2, services=[{ + version=2, + services=[{ 'name': 'web', 'image': 'busybox:latest', 'command': 'top' - }], volumes={vol_name: {'driver': 'local'}} + }], + volumes={vol_name: {'driver': 'local'}}, + networks={}, ) project = Project.from_config( name='composetest', @@ -683,13 +702,16 @@ class ProjectTest(DockerClientTestCase): full_vol_name = 'composetest_{0}'.format(vol_name) self.client.create_volume(vol_name) config_data = config.Config( - version=2, services=[{ + version=2, + services=[{ 'name': 'web', 'image': 'busybox:latest', 'command': 'top' - }], volumes={ + }], + volumes={ vol_name: {'external': True, 'external_name': vol_name} - } + }, + networks=None, ) project = Project.from_config( name='composetest', @@ -704,13 +726,16 @@ class ProjectTest(DockerClientTestCase): vol_name = '{0:x}'.format(random.getrandbits(32)) config_data = config.Config( - version=2, services=[{ + version=2, + services=[{ 'name': 'web', 'image': 'busybox:latest', 'command': 'top' - }], volumes={ + }], + volumes={ vol_name: {'external': True, 'external_name': vol_name} - } + }, + networks=None, ) project = Project.from_config( name='composetest', diff --git a/tests/unit/project_test.py b/tests/unit/project_test.py index 861f96562..470e51ad3 100644 --- a/tests/unit/project_test.py +++ b/tests/unit/project_test.py @@ -21,35 +21,27 @@ class ProjectTest(unittest.TestCase): def setUp(self): self.mock_client = mock.create_autospec(docker.Client) - def test_from_dict(self): - project = Project.from_config('composetest', Config(None, [ - { - 'name': 'web', - 'image': 'busybox:latest' - }, - { - 'name': 'db', - 'image': 'busybox:latest' - }, - ], None), None) - self.assertEqual(len(project.services), 2) - self.assertEqual(project.get_service('web').name, 'web') - self.assertEqual(project.get_service('web').options['image'], 'busybox:latest') - self.assertEqual(project.get_service('db').name, 'db') - self.assertEqual(project.get_service('db').options['image'], 'busybox:latest') - def test_from_config(self): - config = Config(None, [ - { - 'name': 'web', - 'image': 'busybox:latest', - }, - { - 'name': 'db', - 'image': 'busybox:latest', - }, - ], None) - project = Project.from_config('composetest', config, None) + config = Config( + version=None, + services=[ + { + 'name': 'web', + 'image': 'busybox:latest', + }, + { + 'name': 'db', + 'image': 'busybox:latest', + }, + ], + networks=None, + volumes=None, + ) + project = Project.from_config( + name='composetest', + config_data=config, + client=None, + ) self.assertEqual(len(project.services), 2) self.assertEqual(project.get_service('web').name, 'web') self.assertEqual(project.get_service('web').options['image'], 'busybox:latest') @@ -58,16 +50,21 @@ class ProjectTest(unittest.TestCase): self.assertFalse(project.use_networking) def test_from_config_v2(self): - config = Config(2, [ - { - 'name': 'web', - 'image': 'busybox:latest', - }, - { - 'name': 'db', - 'image': 'busybox:latest', - }, - ], None) + config = Config( + version=2, + services=[ + { + 'name': 'web', + 'image': 'busybox:latest', + }, + { + 'name': 'db', + 'image': 'busybox:latest', + }, + ], + networks=None, + volumes=None, + ) project = Project.from_config('composetest', config, None) self.assertEqual(len(project.services), 2) self.assertTrue(project.use_networking) @@ -161,13 +158,20 @@ class ProjectTest(unittest.TestCase): container_id = 'aabbccddee' container_dict = dict(Name='aaa', Id=container_id) self.mock_client.inspect_container.return_value = container_dict - project = Project.from_config('test', Config(None, [ - { - 'name': 'test', - 'image': 'busybox:latest', - 'volumes_from': [VolumeFromSpec('aaa', 'rw', 'container')] - } - ], None), self.mock_client) + project = Project.from_config( + name='test', + client=self.mock_client, + config_data=Config( + version=None, + services=[{ + 'name': 'test', + 'image': 'busybox:latest', + 'volumes_from': [VolumeFromSpec('aaa', 'rw', 'container')] + }], + networks=None, + volumes=None, + ), + ) assert project.get_service('test')._get_volumes_from() == [container_id + ":rw"] def test_use_volumes_from_service_no_container(self): @@ -180,33 +184,51 @@ class ProjectTest(unittest.TestCase): "Image": 'busybox:latest' } ] - project = Project.from_config('test', Config(None, [ - { - 'name': 'vol', - 'image': 'busybox:latest' - }, - { - 'name': 'test', - 'image': 'busybox:latest', - 'volumes_from': [VolumeFromSpec('vol', 'rw', 'service')] - } - ], None), self.mock_client) + project = Project.from_config( + name='test', + client=self.mock_client, + config_data=Config( + version=None, + services=[ + { + 'name': 'vol', + 'image': 'busybox:latest' + }, + { + 'name': 'test', + 'image': 'busybox:latest', + 'volumes_from': [VolumeFromSpec('vol', 'rw', 'service')] + } + ], + networks=None, + volumes=None, + ), + ) assert project.get_service('test')._get_volumes_from() == [container_name + ":rw"] def test_use_volumes_from_service_container(self): container_ids = ['aabbccddee', '12345'] - project = Project.from_config('test', Config(None, [ - { - 'name': 'vol', - 'image': 'busybox:latest' - }, - { - 'name': 'test', - 'image': 'busybox:latest', - 'volumes_from': [VolumeFromSpec('vol', 'rw', 'service')] - } - ], None), None) + project = Project.from_config( + name='test', + client=None, + config_data=Config( + version=None, + services=[ + { + 'name': 'vol', + 'image': 'busybox:latest' + }, + { + 'name': 'test', + 'image': 'busybox:latest', + 'volumes_from': [VolumeFromSpec('vol', 'rw', 'service')] + } + ], + networks=None, + volumes=None, + ), + ) with mock.patch.object(Service, 'containers') as mock_return: mock_return.return_value = [ mock.Mock(id=container_id, spec=Container) @@ -313,12 +335,21 @@ class ProjectTest(unittest.TestCase): ] def test_net_unset(self): - project = Project.from_config('test', Config(None, [ - { - 'name': 'test', - 'image': 'busybox:latest', - } - ], None), self.mock_client) + project = Project.from_config( + name='test', + client=self.mock_client, + config_data=Config( + version=None, + services=[ + { + 'name': 'test', + 'image': 'busybox:latest', + } + ], + networks=None, + volumes=None, + ), + ) service = project.get_service('test') self.assertEqual(service.net.id, None) self.assertNotIn('NetworkMode', service._get_container_host_config({})) @@ -327,13 +358,22 @@ class ProjectTest(unittest.TestCase): container_id = 'aabbccddee' container_dict = dict(Name='aaa', Id=container_id) self.mock_client.inspect_container.return_value = container_dict - project = Project.from_config('test', Config(None, [ - { - 'name': 'test', - 'image': 'busybox:latest', - 'net': 'container:aaa' - } - ], None), self.mock_client) + project = Project.from_config( + name='test', + client=self.mock_client, + config_data=Config( + version=None, + services=[ + { + 'name': 'test', + 'image': 'busybox:latest', + 'net': 'container:aaa' + }, + ], + networks=None, + volumes=None, + ), + ) service = project.get_service('test') self.assertEqual(service.net.mode, 'container:' + container_id) @@ -347,17 +387,26 @@ class ProjectTest(unittest.TestCase): "Image": 'busybox:latest' } ] - project = Project.from_config('test', Config(None, [ - { - 'name': 'aaa', - 'image': 'busybox:latest' - }, - { - 'name': 'test', - 'image': 'busybox:latest', - 'net': 'container:aaa' - } - ], None), self.mock_client) + project = Project.from_config( + name='test', + client=self.mock_client, + config_data=Config( + version=None, + services=[ + { + 'name': 'aaa', + 'image': 'busybox:latest' + }, + { + 'name': 'test', + 'image': 'busybox:latest', + 'net': 'container:aaa' + }, + ], + networks=None, + volumes=None, + ), + ) service = project.get_service('test') self.assertEqual(service.net.mode, 'container:' + container_name) @@ -403,11 +452,16 @@ class ProjectTest(unittest.TestCase): }, } project = Project.from_config( - 'test', - Config(None, [{ - 'name': 'web', - 'image': 'busybox:latest', - }], None), - self.mock_client, + name='test', + client=self.mock_client, + config_data=Config( + version=None, + services=[{ + 'name': 'web', + 'image': 'busybox:latest', + }], + networks=None, + volumes=None, + ), ) self.assertEqual([c.id for c in project.containers()], ['1']) From 35e347cf92b13a374ff767653aa8629ccdb64a71 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Wed, 13 Jan 2016 14:05:30 +0000 Subject: [PATCH 02/11] Disable the use of 'net' in v2 Signed-off-by: Aanand Prasad --- compose/config/service_schema_v2.json | 1 - tests/acceptance/cli_test.py | 24 +++++++++++++++++++ .../fixtures/net-container/docker-compose.yml | 7 ++++++ tests/fixtures/net-container/v2-invalid.yml | 10 ++++++++ 4 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/net-container/docker-compose.yml create mode 100644 tests/fixtures/net-container/v2-invalid.yml diff --git a/compose/config/service_schema_v2.json b/compose/config/service_schema_v2.json index c911502fe..1e54c666b 100644 --- a/compose/config/service_schema_v2.json +++ b/compose/config/service_schema_v2.json @@ -89,7 +89,6 @@ "mac_address": {"type": "string"}, "mem_limit": {"type": ["number", "string"]}, "memswap_limit": {"type": ["number", "string"]}, - "net": {"type": "string"}, "pid": {"type": ["string", "null"]}, "ports": { diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 1e31988b2..ceec400c9 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -422,6 +422,30 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(len(db.containers()), 1) self.assertEqual(len(console.containers()), 0) + def test_up_with_net_is_invalid(self): + self.base_dir = 'tests/fixtures/net-container' + + result = self.dispatch( + ['-f', 'v2-invalid.yml', 'up', '-d'], + returncode=1) + + # TODO: fix validation error messages for v2 files + # assert "Unsupported config option for service 'web': 'net'" in exc.exconly() + assert "Unsupported config option" in result.stderr + + def test_up_with_net_v1(self): + self.base_dir = 'tests/fixtures/net-container' + self.dispatch(['up', '-d'], None) + + bar = self.project.get_service('bar') + bar_container = bar.containers()[0] + + foo = self.project.get_service('foo') + foo_container = foo.containers()[0] + + assert foo_container.get('HostConfig.NetworkMode') == \ + 'container:{}'.format(bar_container.id) + def test_up_with_no_deps(self): self.base_dir = 'tests/fixtures/links-composefile' self.dispatch(['up', '-d', '--no-deps', 'web'], None) diff --git a/tests/fixtures/net-container/docker-compose.yml b/tests/fixtures/net-container/docker-compose.yml new file mode 100644 index 000000000..b5506e0e1 --- /dev/null +++ b/tests/fixtures/net-container/docker-compose.yml @@ -0,0 +1,7 @@ +foo: + image: busybox + command: top + net: "container:bar" +bar: + image: busybox + command: top diff --git a/tests/fixtures/net-container/v2-invalid.yml b/tests/fixtures/net-container/v2-invalid.yml new file mode 100644 index 000000000..eac4b5f18 --- /dev/null +++ b/tests/fixtures/net-container/v2-invalid.yml @@ -0,0 +1,10 @@ +version: 2 + +services: + foo: + image: busybox + command: top + bar: + image: busybox + command: top + net: "container:foo" From 3f9038aea9abfde49ee2b8745369d7d8647f3262 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Wed, 13 Jan 2016 14:15:02 +0000 Subject: [PATCH 03/11] Remove test duplication Signed-off-by: Aanand Prasad --- tests/acceptance/cli_test.py | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index ceec400c9..52e3d354e 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -350,22 +350,7 @@ class CLITestCase(DockerClientTestCase): assert 'simple_1 | simple' in result.stdout assert 'another_1 | another' in result.stdout - def test_up_without_networking(self): - self.base_dir = 'tests/fixtures/links-composefile' - self.dispatch(['up', '-d'], None) - - networks = self.client.networks(names=[self.project.default_network.full_name]) - self.assertEqual(len(networks), 0) - - for service in self.project.get_services(): - containers = service.containers() - self.assertEqual(len(containers), 1) - self.assertNotEqual(containers[0].get('Config.Hostname'), service.name) - - web_container = self.project.get_service('web').containers()[0] - self.assertTrue(web_container.get('HostConfig.Links')) - - def test_up_with_networking(self): + def test_up(self): self.base_dir = 'tests/fixtures/v2-simple' self.dispatch(['up', '-d'], None) @@ -385,9 +370,6 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(len(containers), 1) self.assertIn(containers[0].id, network['Containers']) - web_container = self.project.get_service('simple').containers()[0] - self.assertFalse(web_container.get('HostConfig.Links')) - def test_up_with_networks(self): self.base_dir = 'tests/fixtures/networks' self.dispatch(['up', '-d'], None) @@ -415,13 +397,26 @@ class CLITestCase(DockerClientTestCase): def test_up_with_links_v1(self): self.base_dir = 'tests/fixtures/links-composefile' self.dispatch(['up', '-d', 'web'], None) + + # No network was created + networks = self.client.networks(names=[self.project.default_network.full_name]) + for n in networks: + self.addCleanup(self.client.remove_network, n['Id']) + self.assertEqual(len(networks), 0) + web = self.project.get_service('web') db = self.project.get_service('db') console = self.project.get_service('console') + + # console was not started self.assertEqual(len(web.containers()), 1) self.assertEqual(len(db.containers()), 1) self.assertEqual(len(console.containers()), 0) + # web has links + web_container = web.containers()[0] + self.assertTrue(web_container.get('HostConfig.Links')) + def test_up_with_net_is_invalid(self): self.base_dir = 'tests/fixtures/net-container' From 3eafdbb01bc9c2e72098aa30eb16f5e29d2228ab Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Wed, 13 Jan 2016 17:00:31 +0000 Subject: [PATCH 04/11] Connect services to networks with the 'networks' key Signed-off-by: Aanand Prasad --- compose/cli/main.py | 3 +- compose/config/service_schema_v2.json | 7 ++ compose/network.py | 1 + compose/project.py | 48 ++++++++++--- compose/service.py | 19 ++++-- tests/acceptance/cli_test.py | 68 +++++++++++++++---- tests/fixtures/networks/docker-compose.yml | 20 ++++-- tests/fixtures/networks/missing-network.yml | 10 +++ tests/fixtures/no-services/docker-compose.yml | 5 ++ tests/integration/resilience_test.py | 4 +- tests/integration/service_test.py | 33 ++++----- tests/unit/project_test.py | 55 +++++++++------ 12 files changed, 195 insertions(+), 78 deletions(-) create mode 100644 tests/fixtures/networks/missing-network.yml create mode 100644 tests/fixtures/no-services/docker-compose.yml diff --git a/compose/cli/main.py b/compose/cli/main.py index 62abcb10a..473c6d605 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -704,7 +704,7 @@ def run_one_off_container(container_options, project, service, options): **container_options) if options['-d']: - container.start() + service.start_container(container) print(container.name) return @@ -716,6 +716,7 @@ def run_one_off_container(container_options, project, service, options): try: try: dockerpty.start(project.client, container.id, interactive=not options['-T']) + service.connect_container_to_networks(container) exit_code = container.wait() except signals.ShutdownException: project.client.stop(container.id) diff --git a/compose/config/service_schema_v2.json b/compose/config/service_schema_v2.json index 1e54c666b..5f4e04781 100644 --- a/compose/config/service_schema_v2.json +++ b/compose/config/service_schema_v2.json @@ -89,6 +89,13 @@ "mac_address": {"type": "string"}, "mem_limit": {"type": ["number", "string"]}, "memswap_limit": {"type": ["number", "string"]}, + + "networks": { + "type": "array", + "items": {"type": "string"}, + "uniqueItems": true + }, + "pid": {"type": ["string", "null"]}, "ports": { diff --git a/compose/network.py b/compose/network.py index 0b4e40c60..a8f7e918d 100644 --- a/compose/network.py +++ b/compose/network.py @@ -11,6 +11,7 @@ from .config import ConfigurationError log = logging.getLogger(__name__) +# TODO: support external networks class Network(object): def __init__(self, client, project, name, driver=None, driver_opts=None): self.client = client diff --git a/compose/project.py b/compose/project.py index 10d457a7f..292bf2f2e 100644 --- a/compose/project.py +++ b/compose/project.py @@ -58,7 +58,21 @@ class Project(object): use_networking = (config_data.version and config_data.version >= 2) project = cls(name, [], client, use_networking=use_networking) + custom_networks = [] + if config_data.networks: + for network_name, data in config_data.networks.items(): + custom_networks.append( + Network( + client=client, project=name, name=network_name, + driver=data.get('driver'), driver_opts=data.get('driver_opts') + ) + ) + for service_dict in config_data.services: + networks = project.get_networks( + service_dict, + custom_networks + [project.default_network]) + links = project.get_links(service_dict) volumes_from = get_volumes_from(project, service_dict) net = project.get_net(service_dict) @@ -68,19 +82,15 @@ class Project(object): client=client, project=name, use_networking=use_networking, + networks=networks, links=links, net=net, volumes_from=volumes_from, **service_dict)) - if config_data.networks: - for network_name, data in config_data.networks.items(): - project.networks.append( - Network( - client=client, project=name, name=network_name, - driver=data.get('driver'), driver_opts=data.get('driver_opts') - ) - ) + project.networks += custom_networks + if project.uses_default_network(): + project.networks.append(project.default_network) if config_data.volumes: for vol_name, data in config_data.volumes.items(): @@ -154,6 +164,18 @@ class Project(object): service.remove_duplicate_containers() return services + def get_networks(self, service_dict, network_definitions): + networks = [] + for name in service_dict.pop('networks', ['default']): + matches = [n for n in network_definitions if n.name == name] + if matches: + networks.append(matches[0].full_name) + else: + raise ConfigurationError( + 'Service "{}" uses an undefined network "{}"' + .format(service_dict['name'], name)) + return networks + def get_links(self, service_dict): links = [] if 'links' in service_dict: @@ -172,10 +194,11 @@ class Project(object): return links def get_net(self, service_dict): + if self.use_networking: + return Net(None) + net = service_dict.pop('net', None) if not net: - if self.use_networking: - return Net(self.default_network.full_name) return Net(None) net_name = get_service_name_from_net(net) @@ -282,6 +305,9 @@ class Project(object): volume.remove() def initialize_networks(self): + if not self.use_networking: + return + networks = self.networks if self.uses_default_network(): networks.append(self.default_network) @@ -291,7 +317,7 @@ class Project(object): def uses_default_network(self): return any( - service.net.mode == self.default_network.full_name + self.default_network.full_name in service.networks for service in self.services ) diff --git a/compose/service.py b/compose/service.py index 1c848ca39..0a7f0d8e8 100644 --- a/compose/service.py +++ b/compose/service.py @@ -116,6 +116,7 @@ class Service(object): links=None, volumes_from=None, net=None, + networks=None, **options ): self.name = name @@ -125,6 +126,7 @@ class Service(object): self.links = links or [] self.volumes_from = volumes_from or [] self.net = net or Net(None) + self.networks = networks or [] self.options = options def containers(self, stopped=False, one_off=False, filters={}): @@ -175,7 +177,7 @@ class Service(object): def create_and_start(service, number): container = service.create_container(number=number, quiet=True) - container.start() + service.start_container(container) return container running_containers = self.containers(stopped=False) @@ -348,7 +350,7 @@ class Service(object): container.attach_log_stream() if start: - container.start() + self.start_container(container) return [container] @@ -406,7 +408,7 @@ class Service(object): if attach_logs: new_container.attach_log_stream() if start_new_container: - new_container.start() + self.start_container(new_container) container.remove() return new_container @@ -415,9 +417,18 @@ class Service(object): log.info("Starting %s" % container.name) if attach_logs: container.attach_log_stream() - container.start() + return self.start_container(container) + + def start_container(self, container): + container.start() + self.connect_container_to_networks(container) return container + def connect_container_to_networks(self, container): + for network in self.networks: + log.debug('Connecting "{}" to "{}"'.format(container.name, network)) + self.client.connect_container_to_network(container.id, network) + def remove_duplicate_containers(self, timeout=DEFAULT_TIMEOUT): for c in self.duplicate_containers(): log.info('Removing %s' % c.name) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 52e3d354e..ff9c34f1f 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -103,8 +103,15 @@ class CLITestCase(DockerClientTestCase): if self.base_dir: self.project.kill() self.project.remove_stopped() + for container in self.project.containers(stopped=True, one_off=True): container.remove(force=True) + + networks = self.client.networks() + for n in networks: + if n['Name'].startswith('{}_'.format(self.project.name)): + self.client.remove_network(n['Name']) + super(CLITestCase, self).tearDown() @property @@ -357,12 +364,11 @@ class CLITestCase(DockerClientTestCase): services = self.project.get_services() networks = self.client.networks(names=[self.project.default_network.full_name]) - for n in networks: - self.addCleanup(self.client.remove_network, n['Id']) self.assertEqual(len(networks), 1) self.assertEqual(networks[0]['Driver'], 'bridge') network = self.client.inspect_network(networks[0]['Id']) + # print self.project.services[0].containers()[0].get('NetworkSettings') self.assertEqual(len(network['Containers']), len(services)) for service in services: @@ -374,14 +380,52 @@ class CLITestCase(DockerClientTestCase): self.base_dir = 'tests/fixtures/networks' self.dispatch(['up', '-d'], None) - networks = self.client.networks(names=[ - '{}_{}'.format(self.project.name, n) - for n in ['foo', 'bar']]) + back_name = '{}_back'.format(self.project.name) + front_name = '{}_front'.format(self.project.name) - self.assertEqual(len(networks), 2) + networks = [ + n for n in self.client.networks() + if n['Name'].startswith('{}_'.format(self.project.name)) + ] - for net in networks: - self.assertEqual(net['Driver'], 'bridge') + # Two networks were created: back and front + assert sorted(n['Name'] for n in networks) == [back_name, front_name] + + back_network = [n for n in networks if n['Name'] == back_name][0] + front_network = [n for n in networks if n['Name'] == front_name][0] + + web_container = self.project.get_service('web').containers()[0] + app_container = self.project.get_service('app').containers()[0] + db_container = self.project.get_service('db').containers()[0] + + # db and app joined the back network + assert sorted(back_network['Containers']) == sorted([db_container.id, app_container.id]) + + # web and app joined the front network + assert sorted(front_network['Containers']) == sorted([web_container.id, app_container.id]) + + def test_up_missing_network(self): + self.base_dir = 'tests/fixtures/networks' + + result = self.dispatch( + ['-f', 'missing-network.yml', 'up', '-d'], + returncode=1) + + assert 'Service "web" uses an undefined network "foo"' in result.stderr + + def test_up_no_services(self): + self.base_dir = 'tests/fixtures/no-services' + self.dispatch(['up', '-d'], None) + + network_names = [ + n['Name'] for n in self.client.networks() + if n['Name'].startswith('{}_'.format(self.project.name)) + ] + + assert sorted(network_names) == [ + '{}_{}'.format(self.project.name, name) + for name in ['bar', 'foo'] + ] def test_up_with_links_is_invalid(self): self.base_dir = 'tests/fixtures/v2-simple' @@ -400,9 +444,7 @@ class CLITestCase(DockerClientTestCase): # No network was created networks = self.client.networks(names=[self.project.default_network.full_name]) - for n in networks: - self.addCleanup(self.client.remove_network, n['Id']) - self.assertEqual(len(networks), 0) + assert networks == [] web = self.project.get_service('web') db = self.project.get_service('db') @@ -731,8 +773,6 @@ class CLITestCase(DockerClientTestCase): service = self.project.get_service('simple') container, = service.containers(stopped=True, one_off=True) networks = self.client.networks(names=[self.project.default_network.full_name]) - for n in networks: - self.addCleanup(self.client.remove_network, n['Id']) self.assertEqual(len(networks), 1) self.assertEqual(container.human_readable_command, u'true') @@ -890,7 +930,7 @@ class CLITestCase(DockerClientTestCase): def test_restart(self): service = self.project.get_service('simple') container = service.create_container() - container.start() + service.start_container(container) started_at = container.dictionary['State']['StartedAt'] self.dispatch(['restart', '-t', '1'], None) container.inspect() diff --git a/tests/fixtures/networks/docker-compose.yml b/tests/fixtures/networks/docker-compose.yml index c0795526f..f1b79df09 100644 --- a/tests/fixtures/networks/docker-compose.yml +++ b/tests/fixtures/networks/docker-compose.yml @@ -1,7 +1,19 @@ version: 2 -networks: - foo: - driver: +services: + web: + image: busybox + command: top + networks: ["front"] + app: + image: busybox + command: top + networks: ["front", "back"] + db: + image: busybox + command: top + networks: ["back"] - bar: {} +networks: + front: {} + back: {} diff --git a/tests/fixtures/networks/missing-network.yml b/tests/fixtures/networks/missing-network.yml new file mode 100644 index 000000000..666f7d34b --- /dev/null +++ b/tests/fixtures/networks/missing-network.yml @@ -0,0 +1,10 @@ +version: 2 + +services: + web: + image: busybox + command: top + networks: ["foo"] + +networks: + bar: {} diff --git a/tests/fixtures/no-services/docker-compose.yml b/tests/fixtures/no-services/docker-compose.yml new file mode 100644 index 000000000..fa4987846 --- /dev/null +++ b/tests/fixtures/no-services/docker-compose.yml @@ -0,0 +1,5 @@ +version: 2 + +networks: + foo: {} + bar: {} diff --git a/tests/integration/resilience_test.py b/tests/integration/resilience_test.py index 5df751c77..b544783a4 100644 --- a/tests/integration/resilience_test.py +++ b/tests/integration/resilience_test.py @@ -17,7 +17,7 @@ class ResilienceTest(DockerClientTestCase): self.project = Project('composetest', [self.db], self.client) container = self.db.create_container() - container.start() + self.db.start_container(container) self.host_path = container.get_mount('/var/db')['Source'] def test_successful_recreate(self): @@ -35,7 +35,7 @@ class ResilienceTest(DockerClientTestCase): self.assertEqual(container.get_mount('/var/db')['Source'], self.host_path) def test_start_failure(self): - with mock.patch('compose.container.Container.start', crash): + with mock.patch('compose.service.Service.start_container', crash): with self.assertRaises(Crash): self.project.up(strategy=ConvergenceStrategy.always) diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 0ce4103e5..37ceb65c4 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -32,14 +32,7 @@ from compose.service import Service def create_and_start_container(service, **override_options): container = service.create_container(**override_options) - container.start() - return container - - -def remove_stopped(service): - containers = [c for c in service.containers(stopped=True) if not c.is_running] - for container in containers: - container.remove() + return service.start_container(container) class ServiceTest(DockerClientTestCase): @@ -88,19 +81,19 @@ class ServiceTest(DockerClientTestCase): def test_create_container_with_unspecified_volume(self): service = self.create_service('db', volumes=[VolumeSpec.parse('/var/db')]) container = service.create_container() - container.start() + service.start_container(container) assert container.get_mount('/var/db') def test_create_container_with_volume_driver(self): service = self.create_service('db', volume_driver='foodriver') container = service.create_container() - container.start() + service.start_container(container) self.assertEqual('foodriver', container.get('HostConfig.VolumeDriver')) def test_create_container_with_cpu_shares(self): service = self.create_service('db', cpu_shares=73) container = service.create_container() - container.start() + service.start_container(container) self.assertEqual(container.get('HostConfig.CpuShares'), 73) def test_create_container_with_cpu_quota(self): @@ -113,7 +106,7 @@ class ServiceTest(DockerClientTestCase): extra_hosts = ['somehost:162.242.195.82', 'otherhost:50.31.209.229'] service = self.create_service('db', extra_hosts=extra_hosts) container = service.create_container() - container.start() + service.start_container(container) self.assertEqual(set(container.get('HostConfig.ExtraHosts')), set(extra_hosts)) def test_create_container_with_extra_hosts_dicts(self): @@ -121,33 +114,33 @@ class ServiceTest(DockerClientTestCase): extra_hosts_list = ['somehost:162.242.195.82', 'otherhost:50.31.209.229'] service = self.create_service('db', extra_hosts=extra_hosts) container = service.create_container() - container.start() + service.start_container(container) self.assertEqual(set(container.get('HostConfig.ExtraHosts')), set(extra_hosts_list)) def test_create_container_with_cpu_set(self): service = self.create_service('db', cpuset='0') container = service.create_container() - container.start() + service.start_container(container) self.assertEqual(container.get('HostConfig.CpusetCpus'), '0') def test_create_container_with_read_only_root_fs(self): read_only = True service = self.create_service('db', read_only=read_only) container = service.create_container() - container.start() + service.start_container(container) self.assertEqual(container.get('HostConfig.ReadonlyRootfs'), read_only, container.get('HostConfig')) def test_create_container_with_security_opt(self): security_opt = ['label:disable'] service = self.create_service('db', security_opt=security_opt) container = service.create_container() - container.start() + service.start_container(container) self.assertEqual(set(container.get('HostConfig.SecurityOpt')), set(security_opt)) def test_create_container_with_mac_address(self): service = self.create_service('db', mac_address='02:42:ac:11:65:43') container = service.create_container() - container.start() + service.start_container(container) self.assertEqual(container.inspect()['Config']['MacAddress'], '02:42:ac:11:65:43') def test_create_container_with_specified_volume(self): @@ -158,7 +151,7 @@ class ServiceTest(DockerClientTestCase): 'db', volumes=[VolumeSpec(host_path, container_path, 'rw')]) container = service.create_container() - container.start() + service.start_container(container) assert container.get_mount(container_path) # Match the last component ("host-path"), because boot2docker symlinks /tmp @@ -229,7 +222,7 @@ class ServiceTest(DockerClientTestCase): ] ) host_container = host_service.create_container() - host_container.start() + host_service.start_container(host_container) self.assertIn(volume_container_1.id + ':rw', host_container.get('HostConfig.VolumesFrom')) self.assertIn(volume_container_2.id + ':rw', @@ -248,7 +241,7 @@ class ServiceTest(DockerClientTestCase): self.assertEqual(old_container.get('Config.Cmd'), ['-d', '1']) self.assertIn('FOO=1', old_container.get('Config.Env')) self.assertEqual(old_container.name, 'composetest_db_1') - old_container.start() + service.start_container(old_container) old_container.inspect() # reload volume data volume_path = old_container.get_mount('/etc')['Source'] diff --git a/tests/unit/project_test.py b/tests/unit/project_test.py index 470e51ad3..ffd4455f3 100644 --- a/tests/unit/project_test.py +++ b/tests/unit/project_test.py @@ -12,8 +12,6 @@ from compose.config.types import VolumeFromSpec from compose.const import LABEL_SERVICE from compose.container import Container from compose.project import Project -from compose.service import ContainerNet -from compose.service import Net from compose.service import Service @@ -412,29 +410,42 @@ class ProjectTest(unittest.TestCase): self.assertEqual(service.net.mode, 'container:' + container_name) def test_uses_default_network_true(self): - web = Service('web', project='test', image="alpine", net=Net('test_default')) - db = Service('web', project='test', image="alpine", net=Net('other')) - project = Project('test', [web, db], None) + project = Project.from_config( + name='test', + client=self.mock_client, + config_data=Config( + version=2, + services=[ + { + 'name': 'foo', + 'image': 'busybox:latest' + }, + ], + networks=None, + volumes=None, + ), + ) + assert project.uses_default_network() - def test_uses_default_network_custom_name(self): - web = Service('web', project='test', image="alpine", net=Net('other')) - project = Project('test', [web], None) - assert not project.uses_default_network() + def test_uses_default_network_false(self): + project = Project.from_config( + name='test', + client=self.mock_client, + config_data=Config( + version=2, + services=[ + { + 'name': 'foo', + 'image': 'busybox:latest', + 'networks': ['custom'] + }, + ], + networks={'custom': {}}, + volumes=None, + ), + ) - def test_uses_default_network_host(self): - web = Service('web', project='test', image="alpine", net=Net('host')) - project = Project('test', [web], None) - assert not project.uses_default_network() - - def test_uses_default_network_container(self): - container = mock.Mock(id='test') - web = Service( - 'web', - project='test', - image="alpine", - net=ContainerNet(container)) - project = Project('test', [web], None) assert not project.uses_default_network() def test_container_without_name(self): From 9c91cf29674f4e294f9d24a135933d2e7df4e546 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 14 Jan 2016 12:18:20 +0000 Subject: [PATCH 05/11] Test discoverability across multiple networks Signed-off-by: Aanand Prasad --- tests/acceptance/cli_test.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index ff9c34f1f..4549a3cb7 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -126,6 +126,20 @@ class CLITestCase(DockerClientTestCase): proc = start_process(self.base_dir, project_options + options) return wait_on_process(proc, returncode=returncode) + def execute(self, container, cmd): + # Remove once Hijack and CloseNotifier sign a peace treaty + self.client.close() + exc = self.client.exec_create(container.id, cmd) + self.client.exec_start(exc) + return self.client.exec_inspect(exc)['ExitCode'] + + def lookup(self, container, service_name): + exit_code = self.execute(container, [ + "nslookup", + "{}_{}_1".format(self.project.name, service_name) + ]) + return exit_code == 0 + def test_help(self): self.base_dir = 'tests/fixtures/no-composefile' result = self.dispatch(['help', 'up'], returncode=1) @@ -404,6 +418,13 @@ class CLITestCase(DockerClientTestCase): # web and app joined the front network assert sorted(front_network['Containers']) == sorted([web_container.id, app_container.id]) + # web can see app but not db + assert self.lookup(web_container, "app") + assert not self.lookup(web_container, "db") + + # app can see db + assert self.lookup(app_container, "db") + def test_up_missing_network(self): self.base_dir = 'tests/fixtures/networks' From e75629392d5274697a668e3f405cbb5e82d99676 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 14 Jan 2016 13:01:17 +0000 Subject: [PATCH 06/11] Don't join the bridge network by default in v2 Signed-off-by: Aanand Prasad --- compose/project.py | 18 ++++++++++-------- tests/acceptance/cli_test.py | 9 ++++++--- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/compose/project.py b/compose/project.py index 292bf2f2e..8606c11e2 100644 --- a/compose/project.py +++ b/compose/project.py @@ -69,13 +69,18 @@ class Project(object): ) for service_dict in config_data.services: - networks = project.get_networks( - service_dict, - custom_networks + [project.default_network]) + if use_networking: + networks = project.get_networks( + service_dict, + custom_networks + [project.default_network]) + net = Net(networks[0]) if networks else Net("none") + links = [] + else: + networks = [] + net = project.get_net(service_dict) + links = project.get_links(service_dict) - links = project.get_links(service_dict) volumes_from = get_volumes_from(project, service_dict) - net = project.get_net(service_dict) project.services.append( Service( @@ -194,9 +199,6 @@ class Project(object): return links def get_net(self, service_dict): - if self.use_networking: - return Net(None) - net = service_dict.pop('net', None) if not net: return Net(None) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 4549a3cb7..8db9727be 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -382,13 +382,16 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(networks[0]['Driver'], 'bridge') network = self.client.inspect_network(networks[0]['Id']) - # print self.project.services[0].containers()[0].get('NetworkSettings') - self.assertEqual(len(network['Containers']), len(services)) for service in services: containers = service.containers() self.assertEqual(len(containers), 1) - self.assertIn(containers[0].id, network['Containers']) + + container = containers[0] + self.assertIn(container.id, network['Containers']) + + networks = container.get('NetworkSettings.Networks').keys() + self.assertEqual(networks, [network['Name']]) def test_up_with_networks(self): self.base_dir = 'tests/fixtures/networks' From ca68c9faa4d4a4e217c948c2dd5abf25f595c8d7 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 14 Jan 2016 13:16:02 +0000 Subject: [PATCH 07/11] Services can join 'bridge' or 'host' Signed-off-by: Aanand Prasad --- compose/project.py | 15 +++++++++------ tests/acceptance/cli_test.py | 19 +++++++++++++++++++ .../fixtures/networks/predefined-networks.yml | 17 +++++++++++++++++ 3 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 tests/fixtures/networks/predefined-networks.yml diff --git a/compose/project.py b/compose/project.py index 8606c11e2..1b5d2eb94 100644 --- a/compose/project.py +++ b/compose/project.py @@ -172,13 +172,16 @@ class Project(object): def get_networks(self, service_dict, network_definitions): networks = [] for name in service_dict.pop('networks', ['default']): - matches = [n for n in network_definitions if n.name == name] - if matches: - networks.append(matches[0].full_name) + if name in ['bridge', 'host']: + networks.append(name) else: - raise ConfigurationError( - 'Service "{}" uses an undefined network "{}"' - .format(service_dict['name'], name)) + matches = [n for n in network_definitions if n.name == name] + if matches: + networks.append(matches[0].full_name) + else: + raise ConfigurationError( + 'Service "{}" uses an undefined network "{}"' + .format(service_dict['name'], name)) return networks def get_links(self, service_dict): diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 8db9727be..90c507698 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -437,6 +437,25 @@ class CLITestCase(DockerClientTestCase): assert 'Service "web" uses an undefined network "foo"' in result.stderr + def test_up_predefined_networks(self): + filename = 'predefined-networks.yml' + + self.base_dir = 'tests/fixtures/networks' + self._project = get_project(self.base_dir, [filename]) + + self.dispatch(['-f', filename, 'up', '-d'], None) + + networks = [ + n for n in self.client.networks() + if n['Name'].startswith('{}_'.format(self.project.name)) + ] + assert not networks + + for name in ['bridge', 'host', 'none']: + container = self.project.get_service(name).containers()[0] + assert container.get('NetworkSettings.Networks').keys() == [name] + assert container.get('HostConfig.NetworkMode') == name + def test_up_no_services(self): self.base_dir = 'tests/fixtures/no-services' self.dispatch(['up', '-d'], None) diff --git a/tests/fixtures/networks/predefined-networks.yml b/tests/fixtures/networks/predefined-networks.yml new file mode 100644 index 000000000..d0fac377d --- /dev/null +++ b/tests/fixtures/networks/predefined-networks.yml @@ -0,0 +1,17 @@ +version: 2 + +services: + bridge: + image: busybox + command: top + networks: ["bridge"] + + host: + image: busybox + command: top + networks: ["host"] + + none: + image: busybox + command: top + networks: [] From 73fbd01cfe7c1e8ad7d297d7f7cfb8704aeb501d Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 14 Jan 2016 13:39:44 +0000 Subject: [PATCH 08/11] Support the 'external' option for networks Signed-off-by: Aanand Prasad --- compose/network.py | 25 +++++++++++++++++-- compose/project.py | 4 ++- tests/acceptance/cli_test.py | 23 +++++++++++++++++ tests/fixtures/networks/external-networks.yml | 16 ++++++++++++ 4 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 tests/fixtures/networks/external-networks.yml diff --git a/compose/network.py b/compose/network.py index a8f7e918d..b2ba2e9b7 100644 --- a/compose/network.py +++ b/compose/network.py @@ -11,16 +11,35 @@ from .config import ConfigurationError log = logging.getLogger(__name__) -# TODO: support external networks class Network(object): - def __init__(self, client, project, name, driver=None, driver_opts=None): + def __init__(self, client, project, name, driver=None, driver_opts=None, + external_name=None): self.client = client self.project = project self.name = name self.driver = driver self.driver_opts = driver_opts + self.external_name = external_name def ensure(self): + if self.external_name: + try: + self.inspect() + log.debug( + 'Network {0} declared as external. No new ' + 'network will be created.'.format(self.name) + ) + except NotFound: + raise ConfigurationError( + 'Network {name} declared as external, but could' + ' not be found. Please create the network manually' + ' using `{command} {name}` and try again.'.format( + name=self.external_name, + command='docker network create' + ) + ) + return + try: data = self.inspect() if self.driver and data['Driver'] != self.driver: @@ -55,4 +74,6 @@ class Network(object): @property def full_name(self): + if self.external_name: + return self.external_name return '{0}_{1}'.format(self.project, self.name) diff --git a/compose/project.py b/compose/project.py index 1b5d2eb94..6a171d514 100644 --- a/compose/project.py +++ b/compose/project.py @@ -64,7 +64,9 @@ class Project(object): custom_networks.append( Network( client=client, project=name, name=network_name, - driver=data.get('driver'), driver_opts=data.get('driver_opts') + driver=data.get('driver'), + driver_opts=data.get('driver_opts'), + external_name=data.get('external_name'), ) ) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 90c507698..a7d5dbaae 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -456,6 +456,29 @@ class CLITestCase(DockerClientTestCase): assert container.get('NetworkSettings.Networks').keys() == [name] assert container.get('HostConfig.NetworkMode') == name + def test_up_external_networks(self): + filename = 'external-networks.yml' + + self.base_dir = 'tests/fixtures/networks' + self._project = get_project(self.base_dir, [filename]) + + result = self.dispatch(['-f', filename, 'up', '-d'], returncode=1) + assert 'declared as external, but could not be found' in result.stderr + + networks = [ + n['Name'] for n in self.client.networks() + if n['Name'].startswith('{}_'.format(self.project.name)) + ] + assert not networks + + network_names = ['{}_{}'.format(self.project.name, n) for n in ['foo', 'bar']] + for name in network_names: + self.client.create_network(name) + + self.dispatch(['-f', filename, 'up', '-d']) + container = self.project.containers()[0] + assert sorted(container.get('NetworkSettings.Networks').keys()) == sorted(network_names) + def test_up_no_services(self): self.base_dir = 'tests/fixtures/no-services' self.dispatch(['up', '-d'], None) diff --git a/tests/fixtures/networks/external-networks.yml b/tests/fixtures/networks/external-networks.yml new file mode 100644 index 000000000..644e3dda9 --- /dev/null +++ b/tests/fixtures/networks/external-networks.yml @@ -0,0 +1,16 @@ +version: 2 + +services: + web: + image: busybox + command: top + networks: + - networks_foo + - bar + +networks: + networks_foo: + external: true + bar: + external: + name: networks_bar From 87326c00ebc8380729f1999d5f20eddc265164e8 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 14 Jan 2016 14:51:42 +0000 Subject: [PATCH 09/11] Python 3 fixes Signed-off-by: Aanand Prasad --- tests/acceptance/cli_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index a7d5dbaae..9c9ced8ce 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -390,7 +390,7 @@ class CLITestCase(DockerClientTestCase): container = containers[0] self.assertIn(container.id, network['Containers']) - networks = container.get('NetworkSettings.Networks').keys() + networks = list(container.get('NetworkSettings.Networks')) self.assertEqual(networks, [network['Name']]) def test_up_with_networks(self): @@ -453,7 +453,7 @@ class CLITestCase(DockerClientTestCase): for name in ['bridge', 'host', 'none']: container = self.project.get_service(name).containers()[0] - assert container.get('NetworkSettings.Networks').keys() == [name] + assert list(container.get('NetworkSettings.Networks')) == [name] assert container.get('HostConfig.NetworkMode') == name def test_up_external_networks(self): @@ -477,7 +477,7 @@ class CLITestCase(DockerClientTestCase): self.dispatch(['-f', filename, 'up', '-d']) container = self.project.containers()[0] - assert sorted(container.get('NetworkSettings.Networks').keys()) == sorted(network_names) + assert sorted(list(container.get('NetworkSettings.Networks'))) == sorted(network_names) def test_up_no_services(self): self.base_dir = 'tests/fixtures/no-services' From 4e61377c6d912a4f5e454b6afcb7bde0416a83b3 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 14 Jan 2016 18:01:44 +0000 Subject: [PATCH 10/11] Move get_networks() out of Project class Signed-off-by: Aanand Prasad --- compose/project.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/compose/project.py b/compose/project.py index 6a171d514..933849c29 100644 --- a/compose/project.py +++ b/compose/project.py @@ -72,7 +72,7 @@ class Project(object): for service_dict in config_data.services: if use_networking: - networks = project.get_networks( + networks = get_networks( service_dict, custom_networks + [project.default_network]) net = Net(networks[0]) if networks else Net("none") @@ -171,21 +171,6 @@ class Project(object): service.remove_duplicate_containers() return services - def get_networks(self, service_dict, network_definitions): - networks = [] - for name in service_dict.pop('networks', ['default']): - if name in ['bridge', 'host']: - networks.append(name) - else: - matches = [n for n in network_definitions if n.name == name] - if matches: - networks.append(matches[0].full_name) - else: - raise ConfigurationError( - 'Service "{}" uses an undefined network "{}"' - .format(service_dict['name'], name)) - return networks - def get_links(self, service_dict): links = [] if 'links' in service_dict: @@ -474,6 +459,22 @@ class Project(object): return acc + dep_services +def get_networks(service_dict, network_definitions): + networks = [] + for name in service_dict.pop('networks', ['default']): + if name in ['bridge', 'host']: + networks.append(name) + else: + matches = [n for n in network_definitions if n.name == name] + if matches: + networks.append(matches[0].full_name) + else: + raise ConfigurationError( + 'Service "{}" uses an undefined network "{}"' + .format(service_dict['name'], name)) + return networks + + def get_volumes_from(project, service_dict): volumes_from = service_dict.pop('volumes_from', None) if not volumes_from: From d98b64f6e7f75e801973e59a6723b5f75e724988 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 14 Jan 2016 18:09:45 +0000 Subject: [PATCH 11/11] Remove duplicated logic from initialize_networks() Signed-off-by: Aanand Prasad --- compose/project.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/compose/project.py b/compose/project.py index 933849c29..12d52cc26 100644 --- a/compose/project.py +++ b/compose/project.py @@ -300,11 +300,7 @@ class Project(object): if not self.use_networking: return - networks = self.networks - if self.uses_default_network(): - networks.append(self.default_network) - - for network in networks: + for network in self.networks: network.ensure() def uses_default_network(self):