mirror of
https://github.com/saltstack/salt.git
synced 2025-04-17 10:10:20 +00:00
Merge pull request #31711 from ticosax/fix-port-and-volume-discovery
[dockerng] Port and Volume comparison should consider Dockerfile
This commit is contained in:
Mike Place
commit
24568b1a5d
2 changed files with 75 additions and 50 deletions
|
|
@ -116,8 +116,8 @@ def _compare(actual, create_kwargs, defaults_from_image):
|
|||
def _get(path, default=None):
|
||||
return salt.utils.traverse_dict(actual, path, default, delimiter=':')
|
||||
|
||||
def _image_get(path):
|
||||
return salt.utils.traverse_dict(defaults_from_image, path, None,
|
||||
def _image_get(path, default=None):
|
||||
return salt.utils.traverse_dict(defaults_from_image, path, default,
|
||||
delimiter=':')
|
||||
ret = {}
|
||||
for item, config in six.iteritems(VALID_CREATE_OPTS):
|
||||
|
|
@ -203,15 +203,33 @@ def _compare(actual, create_kwargs, defaults_from_image):
|
|||
desired_ports.append('{0}/tcp'.format(port_def))
|
||||
else:
|
||||
desired_ports.append(port_def)
|
||||
# Ports declared in docker file should be part of desired_ports.
|
||||
desired_ports.extend([
|
||||
k for k in _image_get(config['image_path']) or [] if
|
||||
k not in desired_ports])
|
||||
desired_ports.sort()
|
||||
log.trace('dockerng.running ({0}): munged actual value: {1}'
|
||||
.format(item, actual_ports))
|
||||
.format(item, actual_ports))
|
||||
log.trace('dockerng.running ({0}): munged desired value: {1}'
|
||||
.format(item, desired_ports))
|
||||
.format(item, desired_ports))
|
||||
if actual_ports != desired_ports:
|
||||
ret.update({item: {'old': actual_ports,
|
||||
'new': desired_ports}})
|
||||
'new': desired_ports}})
|
||||
continue
|
||||
elif item == 'volumes':
|
||||
if actual_data is None:
|
||||
actual_data = []
|
||||
if data is None:
|
||||
data = []
|
||||
actual_volumes = sorted(actual_data)
|
||||
# Volumes declared in docker file should be part of desired_volumes.
|
||||
desired_volumes = sorted(list(data) + [
|
||||
k for k in _image_get(config['image_path']) or [] if
|
||||
k not in data])
|
||||
|
||||
if actual_volumes != desired_volumes:
|
||||
ret.update({item: {'old': actual_volumes,
|
||||
'new': desired_volumes}})
|
||||
|
||||
elif item == 'binds':
|
||||
if actual_data is None:
|
||||
|
|
@ -1472,11 +1490,6 @@ def running(name,
|
|||
'container \'{0}\': {1}'.format(name, exc))
|
||||
return ret
|
||||
|
||||
# If a container is using binds, don't let it also define data-only volumes
|
||||
if kwargs.get('volumes') is not None and kwargs.get('binds') is not None:
|
||||
ret['comment'] = 'Cannot mix data-only volumes and bind mounts'
|
||||
return ret
|
||||
|
||||
# Don't allow conflicting options to be set
|
||||
if kwargs.get('publish_all_ports') \
|
||||
and kwargs.get('port_bindings') is not None:
|
||||
|
|
@ -1510,30 +1523,21 @@ def running(name,
|
|||
_validate_input(create_kwargs,
|
||||
validate_ip_addrs=validate_ip_addrs)
|
||||
|
||||
defaults_from_image = _get_defaults_from_image(image_id)
|
||||
if create_kwargs.get('binds') is not None:
|
||||
if 'volumes' not in create_kwargs:
|
||||
# Check if there are preconfigured volumes in the image
|
||||
for step in __salt__['dockerng.history'](image, quiet=True):
|
||||
if step.lstrip().startswith('VOLUME'):
|
||||
break
|
||||
else:
|
||||
# No preconfigured volumes, we need to make our own. Use
|
||||
# the ones from the "binds" configuration.
|
||||
create_kwargs['volumes'] = [
|
||||
x['bind']
|
||||
for x in six.itervalues(create_kwargs['binds'])
|
||||
]
|
||||
# Be smart and try to provide `volumes` argument derived from the
|
||||
# "binds" configuration.
|
||||
auto_volumes = [x['bind'] for x in six.itervalues(create_kwargs['binds'])]
|
||||
actual_volumes = create_kwargs.setdefault('volumes', [])
|
||||
actual_volumes.extend([v for v in auto_volumes if
|
||||
v not in actual_volumes])
|
||||
if create_kwargs.get('port_bindings') is not None:
|
||||
if 'ports' not in create_kwargs:
|
||||
# Check if there are preconfigured ports in the image
|
||||
for step in __salt__['dockerng.history'](image, quiet=True):
|
||||
if step.lstrip().startswith('EXPOSE'):
|
||||
break
|
||||
else:
|
||||
# No preconfigured ports, we need to make our own. Use
|
||||
# the ones from the "port_bindings" configuration.
|
||||
create_kwargs['ports'] = list(
|
||||
create_kwargs['port_bindings'])
|
||||
# Be smart and try to provide `ports` argument derived from
|
||||
# the "port_bindings" configuration.
|
||||
auto_ports = list(create_kwargs['port_bindings'])
|
||||
actual_ports = create_kwargs.setdefault('ports', [])
|
||||
actual_ports.extend([p for p in auto_ports if
|
||||
p not in actual_ports])
|
||||
|
||||
except SaltInvocationError as exc:
|
||||
ret['comment'] = '{0}'.format(exc)
|
||||
|
|
|
|||
|
|
@ -46,7 +46,6 @@ class DockerngTestCase(TestCase):
|
|||
'''
|
||||
dockerng_create = Mock()
|
||||
dockerng_start = Mock()
|
||||
dockerng_history = MagicMock(return_value=[])
|
||||
__salt__ = {'dockerng.list_containers': MagicMock(),
|
||||
'dockerng.list_tags': MagicMock(),
|
||||
'dockerng.pull': MagicMock(),
|
||||
|
|
@ -54,7 +53,6 @@ class DockerngTestCase(TestCase):
|
|||
'dockerng.inspect_image': MagicMock(),
|
||||
'dockerng.create': dockerng_create,
|
||||
'dockerng.start': dockerng_start,
|
||||
'dockerng.history': dockerng_history,
|
||||
}
|
||||
with patch.dict(dockerng_state.__dict__,
|
||||
{'__salt__': __salt__}):
|
||||
|
|
@ -77,20 +75,22 @@ class DockerngTestCase(TestCase):
|
|||
Test dockerng.running function with an image
|
||||
that already have VOLUME defined.
|
||||
|
||||
The ``binds`` argument, shouldn't have side effects on
|
||||
container creation.
|
||||
The ``binds`` argument, should create a container
|
||||
with ``volumes`` extracted from ``binds``.
|
||||
'''
|
||||
dockerng_create = Mock()
|
||||
dockerng_start = Mock()
|
||||
dockerng_history = MagicMock(return_value=['VOLUME /container-0'])
|
||||
dockerng_inspect_image = Mock(return_value={
|
||||
'Id': 'abcd',
|
||||
'Config': {'Config': {'Volumes': ['/host-1']}},
|
||||
})
|
||||
__salt__ = {'dockerng.list_containers': MagicMock(),
|
||||
'dockerng.list_tags': MagicMock(),
|
||||
'dockerng.pull': MagicMock(),
|
||||
'dockerng.state': MagicMock(),
|
||||
'dockerng.inspect_image': MagicMock(),
|
||||
'dockerng.inspect_image': dockerng_inspect_image,
|
||||
'dockerng.create': dockerng_create,
|
||||
'dockerng.start': dockerng_start,
|
||||
'dockerng.history': dockerng_history,
|
||||
}
|
||||
with patch.dict(dockerng_state.__dict__,
|
||||
{'__salt__': __salt__}):
|
||||
|
|
@ -102,6 +102,7 @@ class DockerngTestCase(TestCase):
|
|||
'image:latest',
|
||||
validate_input=False,
|
||||
binds={'/host-0': {'bind': '/container-0', 'ro': True}},
|
||||
volumes=['/container-0'],
|
||||
validate_ip_addrs=False,
|
||||
name='cont',
|
||||
client_timeout=60)
|
||||
|
|
@ -113,19 +114,21 @@ class DockerngTestCase(TestCase):
|
|||
that doens't have EXPOSE defined.
|
||||
|
||||
The ``port_bindings`` argument, should create a container
|
||||
with respective ``ports`` extracted from ``port_bindings``.
|
||||
with ``ports`` extracted from ``port_bindings``.
|
||||
'''
|
||||
dockerng_create = Mock()
|
||||
dockerng_start = Mock()
|
||||
dockerng_history = MagicMock(return_value=[])
|
||||
dockerng_inspect_image = Mock(return_value={
|
||||
'Id': 'abcd',
|
||||
'Config': {'Config': {'ExposedPorts': {}}},
|
||||
})
|
||||
__salt__ = {'dockerng.list_containers': MagicMock(),
|
||||
'dockerng.list_tags': MagicMock(),
|
||||
'dockerng.pull': MagicMock(),
|
||||
'dockerng.state': MagicMock(),
|
||||
'dockerng.inspect_image': MagicMock(),
|
||||
'dockerng.inspect_image': dockerng_inspect_image,
|
||||
'dockerng.create': dockerng_create,
|
||||
'dockerng.start': dockerng_start,
|
||||
'dockerng.history': dockerng_history,
|
||||
}
|
||||
with patch.dict(dockerng_state.__dict__,
|
||||
{'__salt__': __salt__}):
|
||||
|
|
@ -146,22 +149,40 @@ class DockerngTestCase(TestCase):
|
|||
def test_running_with_predifined_ports(self):
|
||||
'''
|
||||
Test dockerng.running function with an image
|
||||
that contains EXPOSE statements.
|
||||
that expose ports (via Dockerfile EXPOSE statement).
|
||||
|
||||
Check that `ports` contains ports defined on Image and by
|
||||
`port_bindings` argument.
|
||||
|
||||
Inside Dockerfile:
|
||||
|
||||
.. code-block::
|
||||
|
||||
EXPOSE 9898
|
||||
|
||||
In sls:
|
||||
|
||||
.. code-block:: yaml
|
||||
|
||||
container:
|
||||
dockerng.running:
|
||||
- port_bindings:
|
||||
- '9090:9797/tcp'
|
||||
|
||||
The ``port_bindings`` argument, shouldn't have side effect on container
|
||||
creation.
|
||||
'''
|
||||
dockerng_create = Mock()
|
||||
dockerng_start = Mock()
|
||||
dockerng_history = MagicMock(return_value=['EXPOSE 9797/tcp'])
|
||||
dockerng_inspect_image = Mock(return_value={
|
||||
'Id': 'abcd',
|
||||
'Config': {'ExposedPorts': {'9898/tcp': {}}}
|
||||
})
|
||||
__salt__ = {'dockerng.list_containers': MagicMock(),
|
||||
'dockerng.list_tags': MagicMock(),
|
||||
'dockerng.pull': MagicMock(),
|
||||
'dockerng.state': MagicMock(),
|
||||
'dockerng.inspect_image': MagicMock(),
|
||||
'dockerng.inspect_image': dockerng_inspect_image,
|
||||
'dockerng.create': dockerng_create,
|
||||
'dockerng.start': dockerng_start,
|
||||
'dockerng.history': dockerng_history,
|
||||
}
|
||||
with patch.dict(dockerng_state.__dict__,
|
||||
{'__salt__': __salt__}):
|
||||
|
|
@ -173,6 +194,7 @@ class DockerngTestCase(TestCase):
|
|||
'image:latest',
|
||||
validate_input=False,
|
||||
name='cont',
|
||||
ports=[9797],
|
||||
port_bindings={9797: [9090]},
|
||||
validate_ip_addrs=False,
|
||||
client_timeout=60)
|
||||
|
|
@ -517,7 +539,6 @@ class DockerngTestCase(TestCase):
|
|||
'dockerng.state': MagicMock(),
|
||||
'dockerng.create': MagicMock(),
|
||||
'dockerng.start': MagicMock(),
|
||||
'dockerng.history': MagicMock(),
|
||||
}
|
||||
with patch.dict(dockerng_state.__dict__,
|
||||
{'__salt__': __salt__}):
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue