From 297941e460bbd1556e4fa5b81ee5816f3f4b0773 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Mon, 6 Jul 2015 12:15:50 -0400 Subject: [PATCH] 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 2e0490a508..07f268c267 100644 --- a/compose/service.py +++ b/compose/service.py @@ -10,6 +10,7 @@ from operator import attrgetter 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 @@ class Service(object): 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 9496ee0826..c213068def 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 0e86c2792f..e844fa2a3b 100644 --- a/tests/integration/cli_test.py +++ b/tests/integration/cli_test.py @@ -334,6 +334,7 @@ class CLITestCase(DockerClientTestCase): # 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 @@ class CLITestCase(DockerClientTestCase): 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 @@ class CLITestCase(DockerClientTestCase): 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 bc6b9e485e..151fcee94b 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -5,7 +5,6 @@ from .. import unittest 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.const import LABEL_SERVICE, LABEL_PROJECT, LABEL_ONE_OFF 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 @@ class ServiceTest(unittest.TestCase): 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 @@ class ServiceTest(unittest.TestCase): 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 @@ class ServiceTest(unittest.TestCase): @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 @@ class ServiceTest(unittest.TestCase): 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 @@ class ServiceTest(unittest.TestCase): 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 @@ class ServiceTest(unittest.TestCase): 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 @@ class ServiceTest(unittest.TestCase): 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 @@ class ServiceTest(unittest.TestCase): 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 @@ class ServiceVolumesTest(unittest.TestCase): 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 @@ class ServiceVolumesTest(unittest.TestCase): 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)