From e566a4dc1c722b997890c2ab83bb5ad1e8b8f852 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Mon, 25 Jan 2016 12:45:30 +0000 Subject: [PATCH] Implement network_mode in v2 Signed-off-by: Aanand Prasad --- compose/config/config.py | 18 ++- compose/config/service_schema_v2.json | 1 + compose/config/sort_services.py | 12 +- compose/config/validation.py | 19 +++ compose/project.py | 43 +++--- docs/compose-file.md | 49 +++++-- docs/networking.md | 12 -- tests/acceptance/cli_test.py | 35 ++++- tests/fixtures/extends/invalid-net-v2.yml | 12 ++ tests/fixtures/networks/bridge.yml | 9 ++ tests/fixtures/networks/network-mode.yml | 27 ++++ .../fixtures/networks/predefined-networks.yml | 17 --- tests/integration/project_test.py | 99 +++++++++++-- tests/unit/config/config_test.py | 131 +++++++++++++++++- tests/unit/config/sort_services_test.py | 4 +- tests/unit/project_test.py | 4 +- 16 files changed, 405 insertions(+), 87 deletions(-) create mode 100644 tests/fixtures/extends/invalid-net-v2.yml create mode 100644 tests/fixtures/networks/bridge.yml create mode 100644 tests/fixtures/networks/network-mode.yml delete mode 100644 tests/fixtures/networks/predefined-networks.yml diff --git a/compose/config/config.py b/compose/config/config.py index 8e7d96e26..c1391c2fb 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -19,6 +19,7 @@ from .errors import CircularReference from .errors import ComposeFileNotFound from .errors import ConfigurationError from .interpolation import interpolate_environment_variables +from .sort_services import get_container_name_from_net from .sort_services import get_service_name_from_net from .sort_services import sort_service_dicts from .types import parse_extra_hosts @@ -30,6 +31,7 @@ from .validation import validate_against_fields_schema from .validation import validate_against_service_schema from .validation import validate_depends_on from .validation import validate_extends_file_path +from .validation import validate_network_mode from .validation import validate_top_level_object from .validation import validate_top_level_service_objects from .validation import validate_ulimits @@ -490,10 +492,15 @@ def validate_extended_service_dict(service_dict, filename, service): "%s services with 'volumes_from' cannot be extended" % error_prefix) if 'net' in service_dict: - if get_service_name_from_net(service_dict['net']) is not None: + if get_container_name_from_net(service_dict['net']): raise ConfigurationError( "%s services with 'net: container' cannot be extended" % error_prefix) + if 'network_mode' in service_dict: + if get_service_name_from_net(service_dict['network_mode']): + raise ConfigurationError( + "%s services with 'network_mode: service' cannot be extended" % error_prefix) + if 'depends_on' in service_dict: raise ConfigurationError( "%s services with 'depends_on' cannot be extended" % error_prefix) @@ -505,6 +512,7 @@ def validate_service(service_config, service_names, version): validate_paths(service_dict) validate_ulimits(service_config) + validate_network_mode(service_config, service_names) validate_depends_on(service_config, service_names) if not service_dict.get('image') and has_uppercase(service_name): @@ -565,6 +573,14 @@ def finalize_service(service_config, service_names, version): service_dict['volumes'] = [ VolumeSpec.parse(v) for v in service_dict['volumes']] + if 'net' in service_dict: + network_mode = service_dict.pop('net') + container_name = get_container_name_from_net(network_mode) + if container_name and container_name in service_names: + service_dict['network_mode'] = 'service:{}'.format(container_name) + else: + service_dict['network_mode'] = network_mode + if 'restart' in service_dict: service_dict['restart'] = parse_restart_spec(service_dict['restart']) diff --git a/compose/config/service_schema_v2.json b/compose/config/service_schema_v2.json index 94046d5b4..56c0cbf5c 100644 --- a/compose/config/service_schema_v2.json +++ b/compose/config/service_schema_v2.json @@ -103,6 +103,7 @@ "mac_address": {"type": "string"}, "mem_limit": {"type": ["number", "string"]}, "memswap_limit": {"type": ["number", "string"]}, + "network_mode": {"type": "string"}, "networks": { "type": "array", diff --git a/compose/config/sort_services.py b/compose/config/sort_services.py index ac0fa4585..cf38a6031 100644 --- a/compose/config/sort_services.py +++ b/compose/config/sort_services.py @@ -5,10 +5,18 @@ from compose.config.errors import DependencyError def get_service_name_from_net(net_config): + return get_source_name_from_net(net_config, 'service') + + +def get_container_name_from_net(net_config): + return get_source_name_from_net(net_config, 'container') + + +def get_source_name_from_net(net_config, source_type): if not net_config: return - if not net_config.startswith('container:'): + if not net_config.startswith(source_type+':'): return _, net_name = net_config.split(':', 1) @@ -33,7 +41,7 @@ def sort_service_dicts(services): service for service in services if (name in get_service_names(service.get('links', [])) or name in get_service_names_from_volumes_from(service.get('volumes_from', [])) or - name == get_service_name_from_net(service.get('net')) or + name == get_service_name_from_net(service.get('network_mode')) or name in service.get('depends_on', [])) ] diff --git a/compose/config/validation.py b/compose/config/validation.py index 5c2d69ec3..dfc34d575 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -15,6 +15,7 @@ from jsonschema import RefResolver from jsonschema import ValidationError from .errors import ConfigurationError +from .sort_services import get_service_name_from_net log = logging.getLogger(__name__) @@ -147,6 +148,24 @@ def validate_extends_file_path(service_name, extends_options, filename): ) +def validate_network_mode(service_config, service_names): + network_mode = service_config.config.get('network_mode') + if not network_mode: + return + + if 'networks' in service_config.config: + raise ConfigurationError("'network_mode' and 'networks' cannot be combined") + + dependency = get_service_name_from_net(network_mode) + if not dependency: + return + + if dependency not in service_names: + raise ConfigurationError( + "Service '{s.name}' uses the network stack of service '{dep}' which " + "is undefined.".format(s=service_config, dep=dependency)) + + def validate_depends_on(service_config, service_names): for dependency in service_config.config.get('depends_on', []): if dependency not in service_names: diff --git a/compose/project.py b/compose/project.py index b51fcd7e9..ef913d634 100644 --- a/compose/project.py +++ b/compose/project.py @@ -10,6 +10,7 @@ from docker.errors import NotFound from . import parallel from .config import ConfigurationError +from .config.sort_services import get_container_name_from_net from .config.sort_services import get_service_name_from_net from .const import DEFAULT_TIMEOUT from .const import IMAGE_EVENTS @@ -86,12 +87,11 @@ class Project(object): for service_dict in config_data.services: if use_networking: networks = get_networks(service_dict, all_networks) - net = Net(networks[0]) if networks else Net("none") else: networks = [] - net = project.get_net(service_dict) links = project.get_links(service_dict) + net = project.get_net(service_dict, networks) volumes_from = get_volumes_from(project, service_dict) if config_data.version == 2: @@ -197,27 +197,27 @@ class Project(object): del service_dict['links'] return links - def get_net(self, service_dict): - net = service_dict.pop('net', None) + def get_net(self, service_dict, networks): + net = service_dict.pop('network_mode', None) if not net: + if self.use_networking: + return Net(networks[0]) if networks else Net('none') return Net(None) - net_name = get_service_name_from_net(net) - if not net_name: - return Net(net) + service_name = get_service_name_from_net(net) + if service_name: + return ServiceNet(self.get_service(service_name)) - try: - return ServiceNet(self.get_service(net_name)) - except NoSuchService: - pass - try: - return ContainerNet(Container.from_id(self.client, net_name)) - except APIError: - raise ConfigurationError( - 'Service "%s" is trying to use the network of "%s", ' - 'which is not the name of a service or container.' % ( - service_dict['name'], - net_name)) + container_name = get_container_name_from_net(net) + if container_name: + try: + return ContainerNet(Container.from_id(self.client, container_name)) + except APIError: + raise ConfigurationError( + "Service '{name}' uses the network stack of container '{dep}' which " + "does not exist.".format(name=service_dict['name'], dep=container_name)) + + return Net(net) def start(self, service_names=None, **options): containers = [] @@ -465,9 +465,12 @@ class Project(object): def get_networks(service_dict, network_definitions): + if 'network_mode' in service_dict: + return [] + networks = [] for name in service_dict.pop('networks', ['default']): - if name in ['bridge', 'host']: + if name in ['bridge']: networks.append(name) else: matches = [n for n in network_definitions if n.name == name] diff --git a/docs/compose-file.md b/docs/compose-file.md index b2675ac9b..6b61755f2 100644 --- a/docs/compose-file.md +++ b/docs/compose-file.md @@ -437,14 +437,29 @@ Specify logging options as key-value pairs. An example of `syslog` options: ### net > [Version 1 file format](#version-1) only. In version 2, use -> [networks](#networks). +> [network_mode](#network_mode). -Networking mode. Use the same values as the docker client `--net` parameter. +Network mode. Use the same values as the docker client `--net` parameter. +The `container:...` form can take a service name instead of a container name or +id. net: "bridge" - net: "none" - net: "container:[name or id]" net: "host" + net: "none" + net: "container:[service name or container name/id]" + +### network_mode + +> [Version 2 file format](#version-1) only. In version 1, use [net](#net). + +Network mode. Use the same values as the docker client `--net` parameter, plus +the special form `service:[service name]`. + + network_mode: "bridge" + network_mode: "host" + network_mode: "none" + network_mode: "service:[service name]" + network_mode: "container:[container name/id]" ### networks @@ -457,8 +472,8 @@ Networks to join, referencing entries under the - some-network - other-network -The values `bridge`, `host` and `none` can also be used, and are equivalent to -`net: "bridge"`, `net: "host"` or `net: "none"` in version 1. +The value `bridge` can also be used to make containers join the pre-defined +`bridge` network. There is no equivalent to `net: "container:[name or id]"`. @@ -918,16 +933,22 @@ It's more complicated if you're using particular configuration features: your service's containers to an [external network](networking.md#using-a-pre-existing-network). -- `net`: If you're using `host`, `bridge` or `none`, this is now replaced by - `networks`: +- `net`: This is now replaced by [network_mode](#network_mode): - net: host -> networks: ["host"] - net: bridge -> networks: ["bridge"] - net: none -> networks: ["none"] + net: host -> network_mode: host + net: bridge -> network_mode: bridge + net: none -> network_mode: none - If you're using `net: "container:"`, there is no equivalent to this in - version 2 - you should use [Docker networks](networking.md) for - communication instead. + If you're using `net: "container:[service name]"`, you must now use + `network_mode: "service:[service name]"` instead. + + net: "container:web" -> network_mode: "service:web" + + If you're using `net: "container:[container name/id]"`, the value does not + need to change. + + net: "container:cont-name" -> network_mode: "container:cont-name" + net: "container:abc12345" -> network_mode: "container:abc12345" ## Variable substitution diff --git a/docs/networking.md b/docs/networking.md index 1e662dd2c..93533e9d1 100644 --- a/docs/networking.md +++ b/docs/networking.md @@ -144,15 +144,3 @@ If you want your containers to join a pre-existing network, use the [`external` name: my-pre-existing-network Instead of attemping to create a network called `[projectname]_default`, Compose will look for a network called `my-pre-existing-network` and connect your app's containers to it. - -## Custom container network modes - -The `docker` CLI command allows you to specify a custom network mode for a container with the `--net` option - for example, `--net=host` specifies that the container should use the same network namespace as the Docker host, and `--net=none` specifies that it should have no networking capabilities. - -To make use of this in Compose, specify a `networks` list with a single item `host`, `bridge` or `none`: - - app: - build: ./app - networks: ["host"] - -There is no equivalent to `--net=container:CONTAINER_NAME` in the v2 Compose file format. You should instead use networks to enable communication. diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 7bc72305c..4b560efa1 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -496,8 +496,29 @@ class CLITestCase(DockerClientTestCase): assert 'Service "web" uses an undefined network "foo"' in result.stderr @v2_only() - def test_up_predefined_networks(self): - filename = 'predefined-networks.yml' + def test_up_with_bridge_network_plus_default(self): + filename = 'bridge.yml' + + self.base_dir = 'tests/fixtures/networks' + self._project = get_project(self.base_dir, [filename]) + + self.dispatch(['-f', filename, 'up', '-d'], None) + + container = self.project.containers()[0] + + assert sorted(list(container.get('NetworkSettings.Networks'))) == sorted([ + 'bridge', + self.project.default_network.full_name, + ]) + + @v2_only() + def test_up_with_network_mode(self): + c = self.client.create_container('busybox', 'top', name='composetest_network_mode_container') + self.addCleanup(self.client.remove_container, c, force=True) + self.client.start(c) + container_mode_source = 'container:{}'.format(c['Id']) + + filename = 'network-mode.yml' self.base_dir = 'tests/fixtures/networks' self._project = get_project(self.base_dir, [filename]) @@ -515,6 +536,16 @@ class CLITestCase(DockerClientTestCase): assert list(container.get('NetworkSettings.Networks')) == [name] assert container.get('HostConfig.NetworkMode') == name + service_mode_source = 'container:{}'.format( + self.project.get_service('bridge').containers()[0].id) + service_mode_container = self.project.get_service('service').containers()[0] + assert not service_mode_container.get('NetworkSettings.Networks') + assert service_mode_container.get('HostConfig.NetworkMode') == service_mode_source + + container_mode_container = self.project.get_service('container').containers()[0] + assert not container_mode_container.get('NetworkSettings.Networks') + assert container_mode_container.get('HostConfig.NetworkMode') == container_mode_source + @v2_only() def test_up_external_networks(self): filename = 'external-networks.yml' diff --git a/tests/fixtures/extends/invalid-net-v2.yml b/tests/fixtures/extends/invalid-net-v2.yml new file mode 100644 index 000000000..0a04f4680 --- /dev/null +++ b/tests/fixtures/extends/invalid-net-v2.yml @@ -0,0 +1,12 @@ +version: 2 +services: + myweb: + build: '.' + extends: + service: web + command: top + web: + build: '.' + network_mode: "service:net" + net: + build: '.' diff --git a/tests/fixtures/networks/bridge.yml b/tests/fixtures/networks/bridge.yml new file mode 100644 index 000000000..950983722 --- /dev/null +++ b/tests/fixtures/networks/bridge.yml @@ -0,0 +1,9 @@ +version: 2 + +services: + web: + image: busybox + command: top + networks: + - bridge + - default diff --git a/tests/fixtures/networks/network-mode.yml b/tests/fixtures/networks/network-mode.yml new file mode 100644 index 000000000..7ab63df82 --- /dev/null +++ b/tests/fixtures/networks/network-mode.yml @@ -0,0 +1,27 @@ +version: 2 + +services: + bridge: + image: busybox + command: top + network_mode: bridge + + service: + image: busybox + command: top + network_mode: "service:bridge" + + container: + image: busybox + command: top + network_mode: "container:composetest_network_mode_container" + + host: + image: busybox + command: top + network_mode: host + + none: + image: busybox + command: top + network_mode: none diff --git a/tests/fixtures/networks/predefined-networks.yml b/tests/fixtures/networks/predefined-networks.yml deleted file mode 100644 index d0fac377d..000000000 --- a/tests/fixtures/networks/predefined-networks.yml +++ /dev/null @@ -1,17 +0,0 @@ -version: 2 - -services: - bridge: - image: busybox - command: top - networks: ["bridge"] - - host: - image: busybox - command: top - networks: ["host"] - - none: - image: busybox - command: top - networks: [] diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 586f9444c..0945ebb8e 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -4,10 +4,12 @@ from __future__ import unicode_literals import random import py +import pytest from docker.errors import NotFound from .testcases import DockerClientTestCase from compose.config import config +from compose.config import ConfigurationError from compose.config.types import VolumeFromSpec from compose.config.types import VolumeSpec from compose.const import LABEL_PROJECT @@ -104,7 +106,71 @@ class ProjectTest(DockerClientTestCase): db = project.get_service('db') self.assertEqual(db._get_volumes_from(), [data_container.id + ':rw']) - def test_net_from_service(self): + @v2_only() + def test_network_mode_from_service(self): + project = Project.from_config( + name='composetest', + client=self.client, + config_data=build_service_dicts({ + 'version': 2, + 'services': { + 'net': { + 'image': 'busybox:latest', + 'command': ["top"] + }, + 'web': { + 'image': 'busybox:latest', + 'network_mode': 'service:net', + 'command': ["top"] + }, + }, + }), + ) + + project.up() + + web = project.get_service('web') + net = project.get_service('net') + self.assertEqual(web.net.mode, 'container:' + net.containers()[0].id) + + @v2_only() + def test_network_mode_from_container(self): + def get_project(): + return Project.from_config( + name='composetest', + config_data=build_service_dicts({ + 'version': 2, + 'services': { + 'web': { + 'image': 'busybox:latest', + 'network_mode': 'container:composetest_net_container' + }, + }, + }), + client=self.client, + ) + + with pytest.raises(ConfigurationError) as excinfo: + get_project() + + assert "container 'composetest_net_container' which does not exist" in excinfo.exconly() + + net_container = Container.create( + self.client, + image='busybox:latest', + name='composetest_net_container', + command='top', + labels={LABEL_PROJECT: 'composetest'}, + ) + net_container.start() + + project = get_project() + project.up() + + web = project.get_service('web') + self.assertEqual(web.net.mode, 'container:' + net_container.id) + + def test_net_from_service_v1(self): project = Project.from_config( name='composetest', config_data=build_service_dicts({ @@ -127,7 +193,24 @@ class ProjectTest(DockerClientTestCase): net = project.get_service('net') self.assertEqual(web.net.mode, 'container:' + net.containers()[0].id) - def test_net_from_container(self): + def test_net_from_container_v1(self): + def get_project(): + return Project.from_config( + name='composetest', + config_data=build_service_dicts({ + 'web': { + 'image': 'busybox:latest', + 'net': 'container:composetest_net_container' + }, + }), + client=self.client, + ) + + with pytest.raises(ConfigurationError) as excinfo: + get_project() + + assert "container 'composetest_net_container' which does not exist" in excinfo.exconly() + net_container = Container.create( self.client, image='busybox:latest', @@ -137,17 +220,7 @@ class ProjectTest(DockerClientTestCase): ) net_container.start() - project = Project.from_config( - name='composetest', - config_data=build_service_dicts({ - 'web': { - 'image': 'busybox:latest', - 'net': 'container:composetest_net_container' - }, - }), - client=self.client, - ) - + project = get_project() project.up() web = project.get_service('web') diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 98fe77588..0d8f72249 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -1015,6 +1015,126 @@ class ConfigTest(unittest.TestCase): assert "Service 'one' depends on service 'three'" in exc.exconly() +class NetworkModeTest(unittest.TestCase): + def test_network_mode_standard(self): + config_data = config.load(build_config_details({ + 'version': 2, + 'services': { + 'web': { + 'image': 'busybox', + 'command': "top", + 'network_mode': 'bridge', + }, + }, + })) + + assert config_data.services[0]['network_mode'] == 'bridge' + + def test_network_mode_standard_v1(self): + config_data = config.load(build_config_details({ + 'web': { + 'image': 'busybox', + 'command': "top", + 'net': 'bridge', + }, + })) + + assert config_data.services[0]['network_mode'] == 'bridge' + assert 'net' not in config_data.services[0] + + def test_network_mode_container(self): + config_data = config.load(build_config_details({ + 'version': 2, + 'services': { + 'web': { + 'image': 'busybox', + 'command': "top", + 'network_mode': 'container:foo', + }, + }, + })) + + assert config_data.services[0]['network_mode'] == 'container:foo' + + def test_network_mode_container_v1(self): + config_data = config.load(build_config_details({ + 'web': { + 'image': 'busybox', + 'command': "top", + 'net': 'container:foo', + }, + })) + + assert config_data.services[0]['network_mode'] == 'container:foo' + + def test_network_mode_service(self): + config_data = config.load(build_config_details({ + 'version': 2, + 'services': { + 'web': { + 'image': 'busybox', + 'command': "top", + 'network_mode': 'service:foo', + }, + 'foo': { + 'image': 'busybox', + 'command': "top", + }, + }, + })) + + assert config_data.services[1]['network_mode'] == 'service:foo' + + def test_network_mode_service_v1(self): + config_data = config.load(build_config_details({ + 'web': { + 'image': 'busybox', + 'command': "top", + 'net': 'container:foo', + }, + 'foo': { + 'image': 'busybox', + 'command': "top", + }, + })) + + assert config_data.services[1]['network_mode'] == 'service:foo' + + def test_network_mode_service_nonexistent(self): + with pytest.raises(ConfigurationError) as excinfo: + config.load(build_config_details({ + 'version': 2, + 'services': { + 'web': { + 'image': 'busybox', + 'command': "top", + 'network_mode': 'service:foo', + }, + }, + })) + + assert "service 'foo' which is undefined" in excinfo.exconly() + + def test_network_mode_plus_networks_is_invalid(self): + with pytest.raises(ConfigurationError) as excinfo: + config.load(build_config_details({ + 'version': 2, + 'services': { + 'web': { + 'image': 'busybox', + 'command': "top", + 'network_mode': 'bridge', + 'networks': ['front'], + }, + }, + 'networks': { + 'front': None, + } + })) + + assert "'network_mode' and 'networks' cannot be combined" in excinfo.exconly() + + class PortsTest(unittest.TestCase): INVALID_PORTS_TYPES = [ {"1": "8000"}, @@ -1867,11 +1987,18 @@ class ExtendsTest(unittest.TestCase): load_from_filename('tests/fixtures/extends/invalid-volumes.yml') def test_invalid_net_in_extended_service(self): - expected_error_msg = "services with 'net: container' cannot be extended" + with pytest.raises(ConfigurationError) as excinfo: + load_from_filename('tests/fixtures/extends/invalid-net-v2.yml') - with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): + assert 'network_mode: service' in excinfo.exconly() + assert 'cannot be extended' in excinfo.exconly() + + with pytest.raises(ConfigurationError) as excinfo: load_from_filename('tests/fixtures/extends/invalid-net.yml') + assert 'net: container' in excinfo.exconly() + assert 'cannot be extended' in excinfo.exconly() + @mock.patch.dict(os.environ) def test_load_config_runs_interpolation_in_extended_service(self): os.environ.update(HOSTNAME_VALUE="penguin") diff --git a/tests/unit/config/sort_services_test.py b/tests/unit/config/sort_services_test.py index f59906644..c39ac0225 100644 --- a/tests/unit/config/sort_services_test.py +++ b/tests/unit/config/sort_services_test.py @@ -100,7 +100,7 @@ class TestSortService(object): }, { 'name': 'parent', - 'net': 'container:child' + 'network_mode': 'service:child' }, { 'name': 'child' @@ -137,7 +137,7 @@ class TestSortService(object): def test_sort_service_dicts_7(self): services = [ { - 'net': 'container:three', + 'network_mode': 'service:three', 'name': 'four' }, { diff --git a/tests/unit/project_test.py b/tests/unit/project_test.py index ffd4455f3..3ad131f3c 100644 --- a/tests/unit/project_test.py +++ b/tests/unit/project_test.py @@ -365,7 +365,7 @@ class ProjectTest(unittest.TestCase): { 'name': 'test', 'image': 'busybox:latest', - 'net': 'container:aaa' + 'network_mode': 'container:aaa' }, ], networks=None, @@ -398,7 +398,7 @@ class ProjectTest(unittest.TestCase): { 'name': 'test', 'image': 'busybox:latest', - 'net': 'container:aaa' + 'network_mode': 'service:aaa' }, ], networks=None,