diff --git a/salt/utils/docker/__init__.py b/salt/utils/docker/__init__.py index e186d639d7..8ea7002d73 100644 --- a/salt/utils/docker/__init__.py +++ b/salt/utils/docker/__init__.py @@ -287,27 +287,31 @@ def translate_input(**kwargs): actual_volumes.sort() if kwargs.get('port_bindings') is not None \ - and (skip_translate is True or - all(x not in skip_translate - for x in ('port_bindings', 'expose', 'ports'))): + and all(x not in skip_translate + for x in ('port_bindings', 'expose', 'ports')): # Make sure that all ports defined in "port_bindings" are included in # the "ports" param. - auto_ports = list(kwargs['port_bindings']) - if auto_ports: - actual_ports = [] - # Sort list to make unit tests more reliable - for port in auto_ports: - if port in actual_ports: - continue - if isinstance(port, six.integer_types): - actual_ports.append((port, 'tcp')) + ports_to_bind = list(kwargs['port_bindings']) + if ports_to_bind: + ports_to_open = set(kwargs.get('ports', [])) + for port_def in ports_to_bind: + if isinstance(port_def, six.integer_types): + ports_to_open.add(port_def) else: - port, proto = port.split('/') - actual_ports.append((int(port), proto)) - actual_ports.sort() - actual_ports = [ - port if proto == 'tcp' else '{}/{}'.format(port, proto) for (port, proto) in actual_ports - ] - kwargs.setdefault('ports', actual_ports) + port_num, proto = port_def.split('/') + ports_to_open.add((int(port_num), proto)) + kwargs['ports'] = list(ports_to_open) + + if 'ports' in kwargs \ + and all(x not in skip_translate for x in ('expose', 'ports')): + # TCP ports should only be passed as the port number. Normalize the + # input so a port definition of 80/tcp becomes just 80 instead of + # (80, 'tcp'). + for index, _ in enumerate(kwargs['ports']): + try: + if kwargs['ports'][index][1] == 'tcp': + kwargs['ports'][index] = ports_to_open[index][0] + except TypeError: + continue return kwargs, invalid, sorted(collisions) diff --git a/salt/utils/docker/translate.py b/salt/utils/docker/translate.py index 372596a759..2c3504e9ef 100644 --- a/salt/utils/docker/translate.py +++ b/salt/utils/docker/translate.py @@ -705,13 +705,7 @@ def ports(val, **kwargs): # pylint: disable=unused-argument raise SaltInvocationError(exc.__str__()) new_ports.update([_get_port_def(x, proto) for x in range(range_start, range_end + 1)]) - ordered_new_ports = [ - port if proto == 'tcp' else (port, proto) for (port, proto) in sorted( - [(new_port, 'tcp') if isinstance(new_port, six.integer_types) else new_port - for new_port in new_ports] - ) - ] - return ordered_new_ports + return list(new_ports) def privileged(val, **kwargs): # pylint: disable=unused-argument diff --git a/tests/integration/states/test_docker_container.py b/tests/integration/states/test_docker_container.py index 061bcbb060..9cc14b0fbd 100644 --- a/tests/integration/states/test_docker_container.py +++ b/tests/integration/states/test_docker_container.py @@ -448,6 +448,50 @@ class DockerContainerTestCase(ModuleCase, SaltReturnAssertsMixin): if name in self.run_function('docker.list_containers', all=True): self.run_function('docker.rm', [name], force=True) + @with_random_name + def test_running_with_port_bindings(self, name): + ''' + This tests that the ports which are being bound are also exposed, even + when not explicitly configured. This test will create a container with + only some of the ports exposed, including some which aren't even bound. + The resulting containers exposed ports should contain all of the ports + defined in the "ports" argument, as well as each of the ports which are + being bound. + ''' + try: + # Create the container + ret = self.run_state( + 'docker_container.running', + name=name, + image=self.image, + command='sleep 600', + shutdown_timeout=1, + port_bindings=[1234, '1235-1236', '2234/udp', '2235-2236/udp'], + ports=[1235, '2235/udp', 9999], + ) + self.assertSaltTrueReturn(ret) + + # Check the created container's port bindings and exposed ports. + # The port bindings should only contain the ports defined in the + # port_bindings argument, while the exposed ports should also + # contain the extra port (9999/tcp) which was included in the ports + # argument. + + cinfo = self.run_function('docker.inspect_container', [name]) + ports = ['1234/tcp', '1235/tcp', '1236/tcp', + '2234/udp', '2235/udp', '2236/udp'] + self.assertEqual( + sorted(cinfo['HostConfig']['PortBindings']), + ports + ) + self.assertEqual( + sorted(cinfo['Config']['ExposedPorts']), + ports + ['9999/tcp'] + ) + finally: + if name in self.run_function('docker.list_containers', all=True): + self.run_function('docker.rm', [name], force=True) + @with_random_name def test_absent_with_stopped_container(self, name): ''' diff --git a/tests/unit/utils/test_docker.py b/tests/unit/utils/test_docker.py index 5ee6edc236..9ba14e2cec 100644 --- a/tests/unit/utils/test_docker.py +++ b/tests/unit/utils/test_docker.py @@ -520,6 +520,29 @@ class TranslateInputTestCase(TestCase): ''' maxDiff = None + @staticmethod + def normalize_ports(ret): + ''' + When we translate exposed ports, we can end up with a mixture of ints + (representing TCP ports) and tuples (representing UDP ports). Python 2 + will sort an iterable containing these mixed types, but Python 3 will + not. This helper is used to munge the ports in the return data so that + the resulting list is sorted in a way that can reliably be compared to + the expected results in the test. + + This helper should only be needed for port_bindings and ports. + ''' + if 'ports' in ret[0]: + tcp_ports = [] + udp_ports = [] + for item in ret[0]['ports']: + if isinstance(item, six.integer_types): + tcp_ports.append(item) + else: + udp_ports.append(item) + ret[0]['ports'] = sorted(tcp_ports) + sorted(udp_ports) + return ret + def tearDown(self): ''' Test skip_translate kwarg @@ -1108,28 +1131,32 @@ class TranslateInputTestCase(TestCase): '3334/udp': ('10.4.5.6', 3334), '5505/udp': ('10.7.8.9', 15505), '5506/udp': ('10.7.8.9', 15506)}, - 'ports': [80, '81/udp', 3333, '3334/udp', 4505, 4506, '5505/udp', '5506/udp'], + 'ports': [80, 3333, 4505, 4506, + (81, 'udp'), (3334, 'udp'), + (5505, 'udp'), (5506, 'udp')] }, {}, [] ) - translated_input = docker_utils.translate_input( + translated_input = self.normalize_ports(docker_utils.translate_input( port_bindings='10.1.2.3:8080:80,10.1.2.3:8888:80,10.4.5.6:3333:3333,' '10.7.8.9:14505-14506:4505-4506,10.1.2.3:8080:81/udp,' '10.1.2.3:8888:81/udp,10.4.5.6:3334:3334/udp,' '10.7.8.9:15505-15506:5505-5506/udp', - ) + )) self.assertEqual(translated_input, expected) self.assertEqual( - docker_utils.translate_input( - port_bindings=[ - '10.1.2.3:8080:80', - '10.1.2.3:8888:80', - '10.4.5.6:3333:3333', - '10.7.8.9:14505-14506:4505-4506', - '10.1.2.3:8080:81/udp', - '10.1.2.3:8888:81/udp', - '10.4.5.6:3334:3334/udp', - '10.7.8.9:15505-15506:5505-5506/udp'] + self.normalize_ports( + docker_utils.translate_input( + port_bindings=[ + '10.1.2.3:8080:80', + '10.1.2.3:8888:80', + '10.4.5.6:3333:3333', + '10.7.8.9:14505-14506:4505-4506', + '10.1.2.3:8080:81/udp', + '10.1.2.3:8888:81/udp', + '10.4.5.6:3334:3334/udp', + '10.7.8.9:15505-15506:5505-5506/udp'] + ) ), expected ) @@ -1146,27 +1173,33 @@ class TranslateInputTestCase(TestCase): '3334/udp': ('10.4.5.6',), '5505/udp': ('10.7.8.9',), '5506/udp': ('10.7.8.9',)}, - 'ports': [80, '81/udp', 3333, '3334/udp', 4505, 4506, '5505/udp', '5506/udp'], + 'ports': [80, 3333, 4505, 4506, + (81, 'udp'), (3334, 'udp'), + (5505, 'udp'), (5506, 'udp')] }, {}, [] ) self.assertEqual( - docker_utils.translate_input( - port_bindings='10.1.2.3::80,10.1.2.3::80,10.4.5.6::3333,10.7.8.9::4505-4506,10.1.2.3::81/udp,10.1.2.3::81/udp,10.4.5.6::3334/udp,10.7.8.9::5505-5506/udp', + self.normalize_ports( + docker_utils.translate_input( + port_bindings='10.1.2.3::80,10.1.2.3::80,10.4.5.6::3333,10.7.8.9::4505-4506,10.1.2.3::81/udp,10.1.2.3::81/udp,10.4.5.6::3334/udp,10.7.8.9::5505-5506/udp', + ) ), expected ) self.assertEqual( - docker_utils.translate_input( - port_bindings=[ - '10.1.2.3::80', - '10.1.2.3::80', - '10.4.5.6::3333', - '10.7.8.9::4505-4506', - '10.1.2.3::81/udp', - '10.1.2.3::81/udp', - '10.4.5.6::3334/udp', - '10.7.8.9::5505-5506/udp'] + self.normalize_ports( + docker_utils.translate_input( + port_bindings=[ + '10.1.2.3::80', + '10.1.2.3::80', + '10.4.5.6::3333', + '10.7.8.9::4505-4506', + '10.1.2.3::81/udp', + '10.1.2.3::81/udp', + '10.4.5.6::3334/udp', + '10.7.8.9::5505-5506/udp'] + ) ), expected ) @@ -1182,26 +1215,32 @@ class TranslateInputTestCase(TestCase): '3334/udp': 3334, '5505/udp': 15505, '5506/udp': 15506}, - 'ports': [80, '81/udp', 3333, '3334/udp', 4505, 4506, '5505/udp', '5506/udp'], + 'ports': [80, 3333, 4505, 4506, + (81, 'udp'), (3334, 'udp'), + (5505, 'udp'), (5506, 'udp')] }, {}, [] ) self.assertEqual( - docker_utils.translate_input( - port_bindings='8080:80,8888:80,3333:3333,14505-14506:4505-4506,8080:81/udp,8888:81/udp,3334:3334/udp,15505-15506:5505-5506/udp', + self.normalize_ports( + docker_utils.translate_input( + port_bindings='8080:80,8888:80,3333:3333,14505-14506:4505-4506,8080:81/udp,8888:81/udp,3334:3334/udp,15505-15506:5505-5506/udp', + ) ), expected ) self.assertEqual( - docker_utils.translate_input( - port_bindings=['8080:80', - '8888:80', - '3333:3333', - '14505-14506:4505-4506', - '8080:81/udp', - '8888:81/udp', - '3334:3334/udp', - '15505-15506:5505-5506/udp'] + self.normalize_ports( + docker_utils.translate_input( + port_bindings=['8080:80', + '8888:80', + '3333:3333', + '14505-14506:4505-4506', + '8080:81/udp', + '8888:81/udp', + '3334:3334/udp', + '15505-15506:5505-5506/udp'] + ) ), expected ) @@ -1217,20 +1256,26 @@ class TranslateInputTestCase(TestCase): '3334/udp': None, '5505/udp': None, '5506/udp': None}, - 'ports': [80, '81/udp', 3333, '3334/udp', 4505, 4506, '5505/udp', '5506/udp'], + 'ports': [80, 3333, 4505, 4506, + (81, 'udp'), (3334, 'udp'), + (5505, 'udp'), (5506, 'udp')] }, {}, [] ) self.assertEqual( - docker_utils.translate_input( - port_bindings='80,3333,4505-4506,81/udp,3334/udp,5505-5506/udp', + self.normalize_ports( + docker_utils.translate_input( + port_bindings='80,3333,4505-4506,81/udp,3334/udp,5505-5506/udp', + ) ), expected ) self.assertEqual( - docker_utils.translate_input( - port_bindings=['80', '3333', '4505-4506', - '81/udp', '3334/udp', '5505-5506/udp'] + self.normalize_ports( + docker_utils.translate_input( + port_bindings=['80', '3333', '4505-4506', + '81/udp', '3334/udp', '5505-5506/udp'] + ) ), expected ) @@ -1251,28 +1296,35 @@ class TranslateInputTestCase(TestCase): '19999/udp': None, '20000/udp': None, '20001/udp': None}, - 'ports': [80, '81/udp', 3333, '3334/udp', 4505, 4506, '5505/udp', '5506/udp', - 9999, 10000, 10001, '19999/udp', '20000/udp', '20001/udp'] + 'ports': [80, 3333, 4505, 4506, 9999, 10000, 10001, + (81, 'udp'), (3334, 'udp'), (5505, 'udp'), + (5506, 'udp'), (19999, 'udp'), + (20000, 'udp'), (20001, 'udp')] + }, {}, [] ) self.assertEqual( - docker_utils.translate_input( - port_bindings='10.1.2.3:8080:80,10.4.5.6::3333,14505-14506:4505-4506,9999-10001,10.1.2.3:8080:81/udp,10.4.5.6::3334/udp,15505-15506:5505-5506/udp,19999-20001/udp', + self.normalize_ports( + docker_utils.translate_input( + port_bindings='10.1.2.3:8080:80,10.4.5.6::3333,14505-14506:4505-4506,9999-10001,10.1.2.3:8080:81/udp,10.4.5.6::3334/udp,15505-15506:5505-5506/udp,19999-20001/udp', + ) ), expected ) self.assertEqual( - docker_utils.translate_input( - port_bindings=[ - '10.1.2.3:8080:80', - '10.4.5.6::3333', - '14505-14506:4505-4506', - '9999-10001', - '10.1.2.3:8080:81/udp', - '10.4.5.6::3334/udp', - '15505-15506:5505-5506/udp', - '19999-20001/udp'] + self.normalize_ports( + docker_utils.translate_input( + port_bindings=[ + '10.1.2.3:8080:80', + '10.4.5.6::3333', + '14505-14506:4505-4506', + '9999-10001', + '10.1.2.3:8080:81/udp', + '10.4.5.6::3334/udp', + '15505-15506:5505-5506/udp', + '19999-20001/udp'] + ) ), expected ) @@ -1439,24 +1491,32 @@ class TranslateInputTestCase(TestCase): the port numbers must end up as integers. None of the decorators will suffice so this one must be tested specially. ''' - expected = ({'ports': [1111, 2222, (3333, 'udp'), 4505, 4506]}, {}, []) + expected = ({'ports': [1111, 2222, 4505, 4506, (3333, 'udp')]}, {}, []) # Comma-separated list self.assertEqual( - docker_utils.translate_input(ports='1111,2222/tcp,3333/udp,4505-4506'), + self.normalize_ports( + docker_utils.translate_input( + ports='1111,2222/tcp,3333/udp,4505-4506' + ) + ), expected ) # Python list self.assertEqual( - docker_utils.translate_input( - ports=[1111, '2222/tcp', '3333/udp', '4505-4506'] + self.normalize_ports( + docker_utils.translate_input( + ports=[1111, '2222/tcp', '3333/udp', '4505-4506'] + ) ), expected ) # Same as above but with the first port as a string (it should be # converted to an integer). self.assertEqual( - docker_utils.translate_input( - ports=['1111', '2222/tcp', '3333/udp', '4505-4506'] + self.normalize_ports( + docker_utils.translate_input( + ports=['1111', '2222/tcp', '3333/udp', '4505-4506'] + ) ), expected )