rebasing port range changes

Signed-off-by: Yuval Kohavi <[email protected]>

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
 
 

tests/fixtures/ports-composefile/docker-compose.yml

@@ -5,3 +5,4 @@ simple:
   ports:
     - '3000'
     - '49152:3001'
+    - '49153-49154:3002-3003'

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):
 

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)