From 297941e460bbd1556e4fa5b81ee5816f3f4b0773 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Mon, 6 Jul 2015 12:15:50 -0400 Subject: [PATCH 1/4] rebasing port range changes Signed-off-by: Yuval Kohavi --- compose/service.py | 47 ++----- .../ports-composefile/docker-compose.yml | 1 + tests/integration/cli_test.py | 5 +- tests/unit/service_test.py | 127 ++++-------------- 4 files changed, 40 insertions(+), 140 deletions(-) diff --git a/compose/service.py b/compose/service.py index 2e0490a5086..07f268c267e 100644 --- a/compose/service.py +++ b/compose/service.py @@ -10,6 +10,7 @@ import six from docker.errors import APIError from docker.utils import create_host_config, LogConfig +from docker.utils.ports import build_port_bindings, split_port from . import __version__ from .config import DOCKER_CONFIG_KEYS, merge_environment @@ -599,13 +600,13 @@ def _get_container_create_options( if 'ports' in container_options or 'expose' in self.options: ports = [] all_ports = container_options.get('ports', []) + self.options.get('expose', []) - for port in all_ports: - port = str(port) - if ':' in port: - port = port.split(':')[-1] - if '/' in port: - port = tuple(port.split('/')) - ports.append(port) + for port_range in all_ports: + internal_range, _ = split_port(port_range) + for port in internal_range: + port = str(port) + if '/' in port: + port = tuple(port.split('/')) + ports.append(port) container_options['ports'] = ports override_options['binds'] = merge_volume_bindings( @@ -859,38 +860,6 @@ def parse_volume_spec(volume_config): return VolumeSpec(external, internal, mode) - -# Ports - - -def build_port_bindings(ports): - port_bindings = {} - for port in ports: - internal_port, external = split_port(port) - if internal_port in port_bindings: - port_bindings[internal_port].append(external) - else: - port_bindings[internal_port] = [external] - return port_bindings - - -def split_port(port): - parts = str(port).split(':') - if not 1 <= len(parts) <= 3: - raise ConfigError('Invalid port "%s", should be ' - '[[remote_ip:]remote_port:]port[/protocol]' % port) - - if len(parts) == 1: - internal_port, = parts - return internal_port, None - if len(parts) == 2: - external_port, internal_port = parts - return internal_port, external_port - - external_ip, external_port, internal_port = parts - return internal_port, (external_ip, external_port or None) - - # Labels diff --git a/tests/fixtures/ports-composefile/docker-compose.yml b/tests/fixtures/ports-composefile/docker-compose.yml index 9496ee08260..c213068defb 100644 --- a/tests/fixtures/ports-composefile/docker-compose.yml +++ b/tests/fixtures/ports-composefile/docker-compose.yml @@ -5,3 +5,4 @@ simple: ports: - '3000' - '49152:3001' + - '49153-49154:3002-3003' diff --git a/tests/integration/cli_test.py b/tests/integration/cli_test.py index 0e86c2792f0..e844fa2a3bf 100644 --- a/tests/integration/cli_test.py +++ b/tests/integration/cli_test.py @@ -334,6 +334,7 @@ def test_run_service_with_map_ports(self, __): # get port information port_random = container.get_local_port(3000) port_assigned = container.get_local_port(3001) + port_range = container.get_local_port(3002), container.get_local_port(3003) # close all one off containers we just created container.stop() @@ -342,6 +343,8 @@ def test_run_service_with_map_ports(self, __): self.assertNotEqual(port_random, None) self.assertIn("0.0.0.0", port_random) self.assertEqual(port_assigned, "0.0.0.0:49152") + self.assertEqual(port_range[0], "0.0.0.0:49153") + self.assertEqual(port_range[1], "0.0.0.0:49154") def test_rm(self): service = self.project.get_service('simple') @@ -456,7 +459,7 @@ def get_port(number, mock_stdout): self.assertEqual(get_port(3000), container.get_local_port(3000)) self.assertEqual(get_port(3001), "0.0.0.0:49152") - self.assertEqual(get_port(3002), "") + self.assertEqual(get_port(3002), "0.0.0.0:49153") def test_port_with_scale(self): diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index bc6b9e485e4..151fcee94bb 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -5,7 +5,6 @@ import mock import docker -from docker.utils import LogConfig from compose.service import Service from compose.container import Container @@ -13,14 +12,11 @@ from compose.service import ( ConfigError, NeedsBuildError, - NoSuchImageError, - build_port_bindings, build_volume_binding, get_container_data_volumes, merge_volume_bindings, parse_repository_tag, parse_volume_spec, - split_port, ) @@ -108,48 +104,6 @@ def test_get_volumes_from_service_no_container(self): self.assertEqual(service._get_volumes_from(), [container_id]) from_service.create_container.assert_called_once_with() - def test_split_port_with_host_ip(self): - internal_port, external_port = split_port("127.0.0.1:1000:2000") - self.assertEqual(internal_port, "2000") - self.assertEqual(external_port, ("127.0.0.1", "1000")) - - def test_split_port_with_protocol(self): - internal_port, external_port = split_port("127.0.0.1:1000:2000/udp") - self.assertEqual(internal_port, "2000/udp") - self.assertEqual(external_port, ("127.0.0.1", "1000")) - - def test_split_port_with_host_ip_no_port(self): - internal_port, external_port = split_port("127.0.0.1::2000") - self.assertEqual(internal_port, "2000") - self.assertEqual(external_port, ("127.0.0.1", None)) - - def test_split_port_with_host_port(self): - internal_port, external_port = split_port("1000:2000") - self.assertEqual(internal_port, "2000") - self.assertEqual(external_port, "1000") - - def test_split_port_no_host_port(self): - internal_port, external_port = split_port("2000") - self.assertEqual(internal_port, "2000") - self.assertEqual(external_port, None) - - def test_split_port_invalid(self): - with self.assertRaises(ConfigError): - split_port("0.0.0.0:1000:2000:tcp") - - def test_build_port_bindings_with_one_port(self): - port_bindings = build_port_bindings(["127.0.0.1:1000:1000"]) - self.assertEqual(port_bindings["1000"], [("127.0.0.1", "1000")]) - - def test_build_port_bindings_with_matching_internal_ports(self): - port_bindings = build_port_bindings(["127.0.0.1:1000:1000", "127.0.0.1:2000:1000"]) - self.assertEqual(port_bindings["1000"], [("127.0.0.1", "1000"), ("127.0.0.1", "2000")]) - - def test_build_port_bindings_with_nonmatching_internal_ports(self): - port_bindings = build_port_bindings(["127.0.0.1:1000:1000", "127.0.0.1:2000:2000"]) - self.assertEqual(port_bindings["1000"], [("127.0.0.1", "1000")]) - self.assertEqual(port_bindings["2000"], [("127.0.0.1", "2000")]) - def test_split_domainname_none(self): service = Service('foo', image='foo', hostname='name', client=self.mock_client) self.mock_client.containers.return_value = [] @@ -157,23 +111,6 @@ def test_split_domainname_none(self): self.assertEqual(opts['hostname'], 'name', 'hostname') self.assertFalse('domainname' in opts, 'domainname') - def test_memory_swap_limit(self): - service = Service(name='foo', image='foo', hostname='name', client=self.mock_client, mem_limit=1000000000, memswap_limit=2000000000) - self.mock_client.containers.return_value = [] - opts = service._get_container_create_options({'some': 'overrides'}, 1) - self.assertEqual(opts['memswap_limit'], 2000000000) - self.assertEqual(opts['mem_limit'], 1000000000) - - def test_log_opt(self): - log_opt = {'address': 'tcp://192.168.0.42:123'} - service = Service(name='foo', image='foo', hostname='name', client=self.mock_client, log_driver='syslog', log_opt=log_opt) - self.mock_client.containers.return_value = [] - opts = service._get_container_create_options({'some': 'overrides'}, 1) - - self.assertIsInstance(opts['host_config']['LogConfig'], LogConfig) - self.assertEqual(opts['host_config']['LogConfig'].type, 'syslog') - self.assertEqual(opts['host_config']['LogConfig'].config, log_opt) - def test_split_domainname_fqdn(self): service = Service( 'foo', @@ -229,10 +166,11 @@ def test_get_container(self, mock_container_class): @mock.patch('compose.service.log', autospec=True) def test_pull_image(self, mock_log): service = Service('foo', client=self.mock_client, image='someimage:sometag') - service.pull() + service.pull(insecure_registry=True) self.mock_client.pull.assert_called_once_with( 'someimage', tag='sometag', + insecure_registry=True, stream=True) mock_log.info.assert_called_once_with('Pulling foo (someimage:sometag)...') @@ -242,8 +180,26 @@ def test_pull_image_no_tag(self): self.mock_client.pull.assert_called_once_with( 'ababab', tag='latest', + insecure_registry=False, stream=True) + def test_create_container_from_insecure_registry(self): + service = Service('foo', client=self.mock_client, image='someimage:sometag') + images = [] + + def pull(repo, tag=None, insecure_registry=False, **kwargs): + self.assertEqual('someimage', repo) + self.assertEqual('sometag', tag) + self.assertTrue(insecure_registry) + images.append({'Id': 'abc123'}) + return [] + + service.image = lambda: images[0] if images else None + self.mock_client.pull = pull + + service.create_container(insecure_registry=True) + self.assertEqual(1, len(images)) + @mock.patch('compose.service.Container', autospec=True) def test_recreate_container(self, _): mock_container = mock.create_autospec(Container) @@ -287,7 +243,7 @@ def pull(repo, tag=None, **kwargs): images.append({'Id': 'abc123'}) return [] - service.image = lambda *args, **kwargs: mock_get_image(images) + service.image = lambda: images[0] if images else None self.mock_client.pull = pull service.create_container() @@ -297,7 +253,7 @@ def test_create_container_with_build(self): service = Service('foo', client=self.mock_client, build='.') images = [] - service.image = lambda *args, **kwargs: mock_get_image(images) + service.image = lambda *args, **kwargs: images[0] if images else None service.build = lambda: images.append({'Id': 'abc123'}) service.create_container(do_build=True) @@ -312,7 +268,7 @@ def test_create_container_no_build(self): def test_create_container_no_build_but_needs_build(self): service = Service('foo', client=self.mock_client, build='.') - service.image = lambda *args, **kwargs: mock_get_image([]) + service.image = lambda: None with self.assertRaises(NeedsBuildError): service.create_container(do_build=False) @@ -329,13 +285,6 @@ def test_build_does_not_pull(self): self.assertFalse(self.mock_client.build.call_args[1]['pull']) -def mock_get_image(images): - if images: - return images[0] - else: - raise NoSuchImageError() - - class ServiceVolumesTest(unittest.TestCase): def setUp(self): @@ -353,13 +302,14 @@ def test_parse_volume_spec_with_mode(self): spec = parse_volume_spec('external:interval:ro') self.assertEqual(spec, ('external', 'interval', 'ro')) - spec = parse_volume_spec('external:interval:z') - self.assertEqual(spec, ('external', 'interval', 'z')) - def test_parse_volume_spec_too_many_parts(self): with self.assertRaises(ConfigError): parse_volume_spec('one:two:three:four') + def test_parse_volume_bad_mode(self): + with self.assertRaises(ConfigError): + parse_volume_spec('one:two:notrw') + def test_build_volume_binding(self): binding = build_volume_binding(parse_volume_spec('/outside:/inside')) self.assertEqual(binding, ('/inside', '/outside:/inside:rw')) @@ -488,26 +438,3 @@ def test_different_host_path_in_container_json(self): create_options['host_config']['Binds'], ['/mnt/sda1/host/path:/data:rw'], ) - - def test_create_with_special_volume_mode(self): - self.mock_client.inspect_image.return_value = {'Id': 'imageid'} - - create_calls = [] - - def create_container(*args, **kwargs): - create_calls.append((args, kwargs)) - return {'Id': 'containerid'} - - self.mock_client.create_container = create_container - - volumes = ['/tmp:/foo:z'] - - Service( - 'web', - client=self.mock_client, - image='busybox', - volumes=volumes, - ).create_container() - - self.assertEqual(len(create_calls), 1) - self.assertEqual(create_calls[0][1]['host_config']['Binds'], volumes) From 0fdd977b06f95d3eadf04aa975ade85b8cc00e5f Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Tue, 28 Jul 2015 17:00:24 -0400 Subject: [PATCH 2/4] fixed merge issue from previous commit Signed-off-by: Yuval Kohavi --- tests/unit/service_test.py | 83 +++++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 27 deletions(-) diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 151fcee94bb..77a8138d0bb 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -5,6 +5,7 @@ import mock import docker +from docker.utils import LogConfig from compose.service import Service from compose.container import Container @@ -12,6 +13,7 @@ from compose.service import ( ConfigError, NeedsBuildError, + NoSuchImageError, build_volume_binding, get_container_data_volumes, merge_volume_bindings, @@ -111,6 +113,23 @@ def test_split_domainname_none(self): self.assertEqual(opts['hostname'], 'name', 'hostname') self.assertFalse('domainname' in opts, 'domainname') + def test_memory_swap_limit(self): + service = Service(name='foo', image='foo', hostname='name', client=self.mock_client, mem_limit=1000000000, memswap_limit=2000000000) + self.mock_client.containers.return_value = [] + opts = service._get_container_create_options({'some': 'overrides'}, 1) + self.assertEqual(opts['memswap_limit'], 2000000000) + self.assertEqual(opts['mem_limit'], 1000000000) + + def test_log_opt(self): + log_opt = {'address': 'tcp://192.168.0.42:123'} + service = Service(name='foo', image='foo', hostname='name', client=self.mock_client, log_driver='syslog', log_opt=log_opt) + self.mock_client.containers.return_value = [] + opts = service._get_container_create_options({'some': 'overrides'}, 1) + + self.assertIsInstance(opts['host_config']['LogConfig'], LogConfig) + self.assertEqual(opts['host_config']['LogConfig'].type, 'syslog') + self.assertEqual(opts['host_config']['LogConfig'].config, log_opt) + def test_split_domainname_fqdn(self): service = Service( 'foo', @@ -166,11 +185,10 @@ def test_get_container(self, mock_container_class): @mock.patch('compose.service.log', autospec=True) def test_pull_image(self, mock_log): service = Service('foo', client=self.mock_client, image='someimage:sometag') - service.pull(insecure_registry=True) + service.pull() self.mock_client.pull.assert_called_once_with( 'someimage', tag='sometag', - insecure_registry=True, stream=True) mock_log.info.assert_called_once_with('Pulling foo (someimage:sometag)...') @@ -180,26 +198,8 @@ def test_pull_image_no_tag(self): self.mock_client.pull.assert_called_once_with( 'ababab', tag='latest', - insecure_registry=False, stream=True) - def test_create_container_from_insecure_registry(self): - service = Service('foo', client=self.mock_client, image='someimage:sometag') - images = [] - - def pull(repo, tag=None, insecure_registry=False, **kwargs): - self.assertEqual('someimage', repo) - self.assertEqual('sometag', tag) - self.assertTrue(insecure_registry) - images.append({'Id': 'abc123'}) - return [] - - service.image = lambda: images[0] if images else None - self.mock_client.pull = pull - - service.create_container(insecure_registry=True) - self.assertEqual(1, len(images)) - @mock.patch('compose.service.Container', autospec=True) def test_recreate_container(self, _): mock_container = mock.create_autospec(Container) @@ -243,7 +243,7 @@ def pull(repo, tag=None, **kwargs): images.append({'Id': 'abc123'}) return [] - service.image = lambda: images[0] if images else None + service.image = lambda *args, **kwargs: mock_get_image(images) self.mock_client.pull = pull service.create_container() @@ -253,7 +253,7 @@ def test_create_container_with_build(self): service = Service('foo', client=self.mock_client, build='.') images = [] - service.image = lambda *args, **kwargs: images[0] if images else None + service.image = lambda *args, **kwargs: mock_get_image(images) service.build = lambda: images.append({'Id': 'abc123'}) service.create_container(do_build=True) @@ -268,7 +268,7 @@ def test_create_container_no_build(self): def test_create_container_no_build_but_needs_build(self): service = Service('foo', client=self.mock_client, build='.') - service.image = lambda: None + service.image = lambda *args, **kwargs: mock_get_image([]) with self.assertRaises(NeedsBuildError): service.create_container(do_build=False) @@ -285,6 +285,13 @@ def test_build_does_not_pull(self): self.assertFalse(self.mock_client.build.call_args[1]['pull']) +def mock_get_image(images): + if images: + return images[0] + else: + raise NoSuchImageError() + + class ServiceVolumesTest(unittest.TestCase): def setUp(self): @@ -302,14 +309,13 @@ def test_parse_volume_spec_with_mode(self): spec = parse_volume_spec('external:interval:ro') self.assertEqual(spec, ('external', 'interval', 'ro')) + spec = parse_volume_spec('external:interval:z') + self.assertEqual(spec, ('external', 'interval', 'z')) + def test_parse_volume_spec_too_many_parts(self): with self.assertRaises(ConfigError): parse_volume_spec('one:two:three:four') - def test_parse_volume_bad_mode(self): - with self.assertRaises(ConfigError): - parse_volume_spec('one:two:notrw') - def test_build_volume_binding(self): binding = build_volume_binding(parse_volume_spec('/outside:/inside')) self.assertEqual(binding, ('/inside', '/outside:/inside:rw')) @@ -438,3 +444,26 @@ def test_different_host_path_in_container_json(self): create_options['host_config']['Binds'], ['/mnt/sda1/host/path:/data:rw'], ) + + def test_create_with_special_volume_mode(self): + self.mock_client.inspect_image.return_value = {'Id': 'imageid'} + + create_calls = [] + + def create_container(*args, **kwargs): + create_calls.append((args, kwargs)) + return {'Id': 'containerid'} + + self.mock_client.create_container = create_container + + volumes = ['/tmp:/foo:z'] + + Service( + 'web', + client=self.mock_client, + image='busybox', + volumes=volumes, + ).create_container() + + self.assertEqual(len(create_calls), 1) + self.assertEqual(create_calls[0][1]['host_config']['Binds'], volumes) From 557cbb616c887d35c0c79e2ed2a79c0344f58de4 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Tue, 28 Jul 2015 17:26:32 -0400 Subject: [PATCH 3/4] ports documentation Signed-off-by: Yuval Kohavi --- docs/yml.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/yml.md b/docs/yml.md index 18551bf22fa..17dbc59ad28 100644 --- a/docs/yml.md +++ b/docs/yml.md @@ -106,7 +106,7 @@ An entry with the ip address and hostname will be created in `/etc/hosts` inside ### ports Expose ports. Either specify both ports (`HOST:CONTAINER`), or just the container -port (a random host port will be chosen). +port (a random host port will be chosen). You can specify a port range instead of a single port (`START-END`). If you use a range for the container ports, you may specify a range for the host ports as well. both ranges must be of equal size. > **Note:** When mapping ports in the `HOST:CONTAINER` format, you may experience > erroneous results when using a container port lower than 60, because YAML will @@ -115,9 +115,12 @@ port (a random host port will be chosen). ports: - "3000" + - "3000-3005" - "8000:8000" + - "9090-9091:8080-8081" - "49100:22" - "127.0.0.1:8001:8001" + - "127.0.0.1:5000-5010:5000-5010" ### expose @@ -410,3 +413,4 @@ dollar sign (`$$`). - [Command line reference](cli.md) - [Compose environment variables](env.md) - [Compose command line completion](completion.md) + From d1455acb6469f1a36376f5f765ed5188336673ee Mon Sep 17 00:00:00 2001 From: Mazz Mosley Date: Fri, 7 Aug 2015 16:30:00 +0100 Subject: [PATCH 4/4] Update docs inline with feedback Signed-off-by: Mazz Mosley --- docs/yml.md | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/docs/yml.md b/docs/yml.md index 17dbc59ad28..1055004254a 100644 --- a/docs/yml.md +++ b/docs/yml.md @@ -105,13 +105,25 @@ An entry with the ip address and hostname will be created in `/etc/hosts` inside ### ports -Expose ports. Either specify both ports (`HOST:CONTAINER`), or just the container -port (a random host port will be chosen). You can specify a port range instead of a single port (`START-END`). If you use a range for the container ports, you may specify a range for the host ports as well. both ranges must be of equal size. +Makes an exposed port accessible on a host and the port is available to +any client that can reach that host. Docker binds the exposed port to a random +port on the host within an *ephemeral port range* defined by +`/proc/sys/net/ipv4/ip_local_port_range`. You can also map to a specific port or range of ports. -> **Note:** When mapping ports in the `HOST:CONTAINER` format, you may experience -> erroneous results when using a container port lower than 60, because YAML will -> parse numbers in the format `xx:yy` as sexagesimal (base 60). For this reason, -> we recommend always explicitly specifying your port mappings as strings. +Acceptable formats for the `ports` value are: + +* `containerPort` +* `ip:hostPort:containerPort` +* `ip::containerPort` +* `hostPort:containerPort` + +You can specify a range for both the `hostPort` and the `containerPort` values. +When specifying ranges, the container port values in the range must match the +number of host port values in the range, for example, +`1234-1236:1234-1236/tcp`. Once a host is running, use the 'docker-compose port' command +to see the actual mapping. + +The following configuration shows examples of the port formats in use: ports: - "3000" @@ -122,6 +134,13 @@ port (a random host port will be chosen). You can specify a port range instead o - "127.0.0.1:8001:8001" - "127.0.0.1:5000-5010:5000-5010" + +When mapping ports, in the `hostPort:containerPort` format, you may +experience erroneous results when using a container port lower than 60. This +happens because YAML parses numbers in the format `xx:yy` as sexagesimal (base +60). To avoid this problem, always explicitly specify your port +mappings as strings. + ### expose Expose ports without publishing them to the host machine - they'll only be