mirror of
https://github.com/valitydev/salt.git
synced 2024-11-07 00:55:19 +00:00
Merge pull request #45941 from terminalmage/issue45679
Fix incorrect translation of docker port_bindings -> ports
This commit is contained in:
Nicole Thomas
GitHub
commit
bc13ff8b48
@ -630,7 +630,6 @@ def _client_wrapper(attr, *args, **kwargs):
|
||||
)
|
||||
ret = func(*args, **kwargs)
|
||||
except docker.errors.APIError as exc:
|
||||
log.exception('Encountered error running API function %s', attr)
|
||||
if catch_api_errors:
|
||||
# Generic handling of Docker API errors
|
||||
raise CommandExecutionError(
|
||||
|
||||
@ -114,28 +114,27 @@ def _post_processing(kwargs, skip_translate, invalid):
|
||||
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'))
|
||||
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)
|
||||
ports_to_bind = list(kwargs['port_bindings'])
|
||||
if ports_to_bind:
|
||||
ports_to_open = set(kwargs.get('ports', []))
|
||||
ports_to_open.update([helpers.get_port_def(x) for x in ports_to_bind])
|
||||
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
|
||||
|
||||
|
||||
# Functions below must match names of docker-py arguments
|
||||
@ -552,13 +551,7 @@ def ports(val, **kwargs): # pylint: disable=unused-argument
|
||||
raise SaltInvocationError(exc.__str__())
|
||||
new_ports.update([helpers.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
|
||||
|
||||
@ -444,6 +444,44 @@ class DockerContainerTestCase(ModuleCase, SaltReturnAssertsMixin):
|
||||
image_info['Config']['Cmd']
|
||||
)
|
||||
|
||||
@container_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.
|
||||
'''
|
||||
# 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']
|
||||
)
|
||||
|
||||
@container_name
|
||||
def test_absent_with_stopped_container(self, name):
|
||||
'''
|
||||
|
||||
@ -798,6 +798,29 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
'''
|
||||
translator = salt.utils.docker.translate.container
|
||||
|
||||
@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:
|
||||
tcp_ports = []
|
||||
udp_ports = []
|
||||
for item in ret['ports']:
|
||||
if isinstance(item, six.integer_types):
|
||||
tcp_ports.append(item)
|
||||
else:
|
||||
udp_ports.append(item)
|
||||
ret['ports'] = sorted(tcp_ports) + sorted(udp_ports)
|
||||
return ret
|
||||
|
||||
@assert_bool(salt.utils.docker.translate.container)
|
||||
def test_auto_remove(self):
|
||||
'''
|
||||
@ -1288,9 +1311,11 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
)
|
||||
for val in (bindings, bindings.split(',')):
|
||||
self.assertEqual(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
port_bindings=val,
|
||||
self.normalize_ports(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
port_bindings=val,
|
||||
)
|
||||
),
|
||||
{'port_bindings': {80: [('10.1.2.3', 8080),
|
||||
('10.1.2.3', 8888)],
|
||||
@ -1302,8 +1327,9 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
'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')]}
|
||||
)
|
||||
|
||||
# ip::containerPort - Bind a specific IP and an ephemeral port to a
|
||||
@ -1315,9 +1341,11 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
)
|
||||
for val in (bindings, bindings.split(',')):
|
||||
self.assertEqual(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
port_bindings=val,
|
||||
self.normalize_ports(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
port_bindings=val,
|
||||
)
|
||||
),
|
||||
{'port_bindings': {80: [('10.1.2.3',), ('10.1.2.3',)],
|
||||
3333: ('10.4.5.6',),
|
||||
@ -1327,8 +1355,9 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
'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')]}
|
||||
)
|
||||
|
||||
# hostPort:containerPort - Bind a specific port on all of the host's
|
||||
@ -1339,9 +1368,11 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
)
|
||||
for val in (bindings, bindings.split(',')):
|
||||
self.assertEqual(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
port_bindings=val,
|
||||
self.normalize_ports(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
port_bindings=val,
|
||||
)
|
||||
),
|
||||
{'port_bindings': {80: [8080, 8888],
|
||||
3333: 3333,
|
||||
@ -1351,8 +1382,9 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
'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')]}
|
||||
)
|
||||
|
||||
# containerPort - Bind an ephemeral port on all of the host's
|
||||
@ -1360,9 +1392,11 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
bindings = '80,3333,4505-4506,81/udp,3334/udp,5505-5506/udp'
|
||||
for val in (bindings, bindings.split(',')):
|
||||
self.assertEqual(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
port_bindings=val,
|
||||
self.normalize_ports(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
port_bindings=val,
|
||||
)
|
||||
),
|
||||
{'port_bindings': {80: None,
|
||||
3333: None,
|
||||
@ -1372,8 +1406,9 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
'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')]}
|
||||
)
|
||||
|
||||
# Test a mixture of different types of input
|
||||
@ -1384,9 +1419,11 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
)
|
||||
for val in (bindings, bindings.split(',')):
|
||||
self.assertEqual(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
port_bindings=val,
|
||||
self.normalize_ports(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
port_bindings=val,
|
||||
)
|
||||
),
|
||||
{'port_bindings': {80: ('10.1.2.3', 8080),
|
||||
3333: ('10.4.5.6',),
|
||||
@ -1402,10 +1439,10 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
'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')]}
|
||||
)
|
||||
|
||||
# Error case: too many items (max 3)
|
||||
@ -1506,11 +1543,13 @@ class TranslateContainerInputTestCase(TranslateBase):
|
||||
[1111, '2222/tcp', '3333/udp', '4505-4506'],
|
||||
['1111', '2222/tcp', '3333/udp', '4505-4506']):
|
||||
self.assertEqual(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
ports=val,
|
||||
self.normalize_ports(
|
||||
salt.utils.docker.translate_input(
|
||||
self.translator,
|
||||
ports=val,
|
||||
)
|
||||
),
|
||||
{'ports': [1111, 2222, (3333, 'udp'), 4505, 4506]}
|
||||
{'ports': [1111, 2222, 4505, 4506, (3333, 'udp')]}
|
||||
)
|
||||
|
||||
# Error case: non-integer and non/string value
|
||||
|
||||
Loading…
Reference in New Issue
Block a user