saltstack/salt
1
0
Fork 0
mirror of https://github.com/saltstack/salt.git synced 2025-04-17 10:10:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Add detection of removed settings.

Browse source 
When removing settings from dockerng.running state definition,
changes where ignored as `_compare()` was iterating only over user defined
parameters.

By looping over all available options we are now sure to not miss
removed entries.

Extend definition of expected default values:

 - Some default values are predictable and hardcoded in the VALID_CREATE_OPTS dict.
 - Some default needs to be read from images (`image_path`)
 - Some defaults values are unpredictable (hostname, mac_address)
 we are comparing desired value against value reported by the container
 (`get_default_from_container`).
This commit is contained in:
Nicolas Delaby 2016-02-18 20:23:31 +01:00
parent c58f654bc3
commit 7bedd86ebe
3 changed files with 388 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
salt/modules/dockerng.py
View file

@ -360,14 +360,17 @@ argument name:
VALID_CREATE_OPTS = {
'command': {
'path': 'Config:Cmd',
'image_path': 'Config:Cmd',
},
'hostname': {
'validator': 'string',
'path': 'Config:Hostname',
'get_default_from_container': True,
},
'domainname': {
'validator': 'string',
'path': 'Config:Domainname',
'default': '',
},
'interactive': {
'api_name': 'stdin_open',
@ -382,25 +385,27 @@ VALID_CREATE_OPTS = {
},
'user': {
'path': 'Config:User',
'default': '',
},
'detach': {
'validator': 'bool',
'path': ('Config:AttachStdout', 'Config:AttachStderr'),
'default': True,
'default': False,
},
'memory': {
'api_name': 'mem_limit',
'path': 'Config:Memory',
'path': 'HostConfig:Memory',
'default': 0,
},
'memory_swap': {
'api_name': 'memswap_limit',
'path': 'Config:MemorySwap',
'path': 'HostConfig:MemorySwap',
'default': 0,
},
'mac_address': {
'validator': 'string',
'path': 'Config:MacAddress',
'path': 'NetworkSettings:MacAddress',
'get_default_from_container': True,
},
'network_disabled': {
'validator': 'bool',
@ -409,38 +414,49 @@ VALID_CREATE_OPTS = {
},
'ports': {
'path': 'Config:ExposedPorts',
'image_path': 'Config:ExposedPorts',
},
'working_dir': {
'path': 'Config:WorkingDir',
'image_path': 'Config:WorkingDir',
},
'entrypoint': {
'path': 'Config:Entrypoint',
'image_path': 'Config:Entrypoint',
},
'environment': {
'path': 'Config:Env',
'default': [],
},
'volumes': {
'path': 'Config:Volumes',
'image_path': 'Config:Volumes',
},
'cpu_shares': {
'validator': 'number',
'path': 'Config:CpuShares',
'path': 'HostConfig:CpuShares',
'default': 0,
},
'cpuset': {
'path': 'Config:Cpuset',
'path': 'HostConfig:CpusetCpus',
'default': '',
},
'labels': {
'path': 'Config:Labels',
'default': {},
},
'binds': {
'path': 'HostConfig:Binds',
'default': None,
},
'port_bindings': {
'path': 'HostConfig:PortBindings',
'default': None,
},
'lxc_conf': {
'validator': 'dict',
'path': 'HostConfig:LxcConf',
'default': None,
},
'publish_all_ports': {
'validator': 'bool',
@ -449,6 +465,7 @@ VALID_CREATE_OPTS = {
},
'links': {
'path': 'HostConfig:Links',
'default': None,
},
'privileged': {
'validator': 'bool',
@ -457,39 +474,47 @@ VALID_CREATE_OPTS = {
},
'dns': {
'path': 'HostConfig:Dns',
'default': [],
},
'dns_search': {
'validator': 'stringlist',
'path': 'HostConfig:DnsSearch',
'default': [],
},
'volumes_from': {
'path': 'HostConfig:VolumesFrom',
'default': None,
},
'network_mode': {
'path': 'HostConfig:NetworkMode',
'default': 'bridge',
'default': 'default',
},
'restart_policy': {
'path': 'HostConfig:RestartPolicy',
'min_docker': (1, 2, 0)
'min_docker': (1, 2, 0),
'default': {'MaximumRetryCount': 0, 'Name': ''},
},
'cap_add': {
'validator': 'stringlist',
'path': 'HostConfig:CapAdd',
'min_docker': (1, 2, 0)
'min_docker': (1, 2, 0),
'default': None,
},
'cap_drop': {
'validator': 'stringlist',
'path': 'HostConfig:CapDrop',
'min_docker': (1, 2, 0)
'min_docker': (1, 2, 0),
'default': None,
},
'extra_hosts': {
'path': 'HostConfig:ExtraHosts',
'min_docker': (1, 3, 0)
'min_docker': (1, 3, 0),
'default': None,
},
'pid_mode': {
'path': 'HostConfig:PidMode',
'min_docker': (1, 5, 0)
'min_docker': (1, 5, 0),
'default': '',
},
}

93
salt/states/dockerng.py
View file

@ -121,7 +121,7 @@ def _prep_input(kwargs):
raise SaltInvocationError(err)
def _compare(actual, create_kwargs):
def _compare(actual, create_kwargs, defaults_from_image):
'''
Compare the desired configuration against the actual configuration returned
by dockerng.inspect_container
@ -129,16 +129,28 @@ def _compare(actual, create_kwargs):
_get = lambda path: (
salt.utils.traverse_dict(actual, path, NOTSET, delimiter=':')
)
_image_get = lambda path: (
salt.utils.traverse_dict(defaults_from_image, path, NOTSET,
delimiter=':')
)
ret = {}
for item, data, in six.iteritems(create_kwargs):
if item not in VALID_CREATE_OPTS:
log.error(
'Trying to compare \'{0}\', but it is not a valid '
'parameter. Skipping.'.format(item)
)
continue
for item, config in six.iteritems(VALID_CREATE_OPTS):
try:
data = create_kwargs[item]
except KeyError:
try:
data = _image_get(config['image_path'])
except KeyError:
if config.get('get_default_from_container'):
data = _get(config['path'])
else:
data = config.get('default')
else:
if data is NOTSET:
_api_mismatch(item)
log.trace('dockerng.running: comparing ' + item)
conf_path = VALID_CREATE_OPTS[item]['path']
conf_path = config['path']
if isinstance(conf_path, tuple):
actual_data = [_get(x) for x in conf_path]
for val in actual_data:
@ -147,17 +159,18 @@ def _compare(actual, create_kwargs):
else:
actual_data = _get(conf_path)
if actual_data is NOTSET:
_api_mismatch(item)
if item == 'network_disabled':
# XXX hack ! docker daemon doesn't
# return Config:NetworkDisabled from inspect command.
# therefore the path is correct.
actual_data = config.get('default')
else:
_api_mismatch(item)
log.trace('dockerng.running ({0}): desired value: {1}'
.format(item, data))
log.trace('dockerng.running ({0}): actual value: {1}'
.format(item, actual_data))
if actual_data is None and data is not None \
or actual_data is not None and data is None:
ret.update({item: {'old': actual_data, 'new': data}})
continue
# 'create' comparison params
if item == 'detach':
# Something unique here. Two fields to check, if both are False
@ -206,8 +219,10 @@ def _compare(actual, create_kwargs):
for port_def in data:
if isinstance(port_def, tuple):
desired_ports.append('{0}/{1}'.format(*port_def))
else:
elif '/' not in port_def:
desired_ports.append('{0}/tcp'.format(port_def))
else:
desired_ports.append(port_def)
desired_ports.sort()
log.trace('dockerng.running ({0}): munged actual value: {1}'
.format(item, actual_ports))
@ -219,6 +234,10 @@ def _compare(actual, create_kwargs):
continue
elif item == 'binds':
if actual_data is None:
actual_data = {}
if data is None:
data = {}
actual_binds = []
for bind in actual_data:
bind_parts = bind.split(':')
@ -239,10 +258,14 @@ def _compare(actual, create_kwargs):
desired_binds.sort()
if actual_binds != desired_binds:
ret.update({item: {'old': actual_binds,
'new': desired_binds}})
'new': desired_binds}})
continue
elif item == 'port_bindings':
if actual_data is None:
actual_data = {}
if data is None:
data = {}
actual_binds = []
for container_port, bind_list in six.iteritems(actual_data):
if container_port.endswith('/tcp'):
@ -312,6 +335,10 @@ def _compare(actual, create_kwargs):
continue
elif item == 'links':
if actual_data is None:
actual_data = []
if data is None:
data = []
actual_links = []
for link in actual_data:
try:
@ -338,6 +365,10 @@ def _compare(actual, create_kwargs):
continue
elif item == 'extra_hosts':
if actual_data is None:
actual_data = {}
if data is None:
data = {}
actual_hosts = sorted(actual_data)
desired_hosts = sorted(
['{0}:{1}'.format(x, y) for x, y in six.iteritems(data)]
@ -347,6 +378,18 @@ def _compare(actual, create_kwargs):
'new': desired_hosts}})
continue
elif item == 'dns':
# Sometimes docker daemon returns `None` and
# sometimes `[]`. We have to deal with it.
if bool(actual_data) != bool(data):
ret.update({item: {'old': actual_data, 'new': data}})
elif item == 'dns_search':
# Sometimes docker daemon returns `None` and
# sometimes `[]`. We have to deal with it.
if bool(actual_data) != bool(data):
ret.update({item: {'old': actual_data, 'new': data}})
elif isinstance(data, list):
# Compare two sorted lists of items. Won't work for "command"
# or "entrypoint" because those are both shell commands and the
@ -374,6 +417,10 @@ def _compare(actual, create_kwargs):
return ret
def _get_defaults_from_image(image_id):
return __salt__['dockerng.inspect_image'](image_id)
def image_present(name,
build=None,
load=None,
@ -1518,8 +1565,10 @@ def running(name,
else:
# Container is the correct image, let's check the container
# config and see if we need to replace the container
defaults_from_image = _get_defaults_from_image(image_id)
try:
changes_needed = _compare(pre_config, create_kwargs)
changes_needed = _compare(pre_config, create_kwargs,
defaults_from_image)
if changes_needed:
log.debug(
'dockerng.running: Analysis of container \'{0}\' '
@ -1571,8 +1620,8 @@ def running(name,
# Container exists, stop if necessary, then remove and recreate
if pre_state != 'stopped':
result = __salt__['dockerng.stop'](name,
timeout=stop_timeout,
unpause=True)['result']
timeout=stop_timeout,
unpause=True)['result']
if result is not True:
comments.append(
'Container was slated to be replaced, but the '
@ -1652,7 +1701,9 @@ def running(name,
if changes_needed:
try:
post_config = __salt__['dockerng.inspect_container'](name)
changes_still_needed = _compare(post_config, create_kwargs)
defaults_from_image = _get_defaults_from_image(image_id)
changes_still_needed = _compare(post_config, create_kwargs,
defaults_from_image)
if changes_still_needed:
log.debug(
'dockerng.running: Analysis of container \'{0}\' after '

287
tests/unit/states/dockerng_test.py
View file

@ -318,12 +318,64 @@ class DockerngTestCase(TestCase):
dockerng_start = Mock()
dockerng_list_containers = Mock(return_value=['cont'])
dockerng_inspect_container = Mock(
return_value={'Config': {'Image': 'image:latest'},
'Image': image_id})
return_value={
'Config': {
'Image': 'image:latest',
'Tty': False,
'Labels': {},
'Domainname': '',
'User': '',
'AttachStderr': True,
'AttachStdout': True,
'Hostname': 'saltstack-container',
'Env': [],
'WorkingDir': '/',
'Cmd': ['bash'],
'Volumes': {},
'Entrypoint': None,
'ExposedPorts': {},
'OpenStdin': False,
},
'HostConfig': {
'PublishAllPorts': False,
'Dns': [],
'Links': None,
'CpusetCpus': '',
'RestartPolicy': {'MaximumRetryCount': 0, 'Name': ''},
'CapAdd': None,
'NetworkMode': 'default',
'PidMode': '',
'MemorySwap': 0,
'ExtraHosts': None,
'PortBindings': None,
'LxcConf': None,
'DnsSearch': [],
'Privileged': False,
'Binds': None,
'Memory': 0,
'VolumesFrom': None,
'CpuShares': 0,
'CapDrop': None,
},
'NetworkSettings': {
'MacAddress': '00:00:00:00:00:01',
},
'Image': image_id})
dockerng_inspect_image = MagicMock(
return_value={
'Id': image_id,
'Config': {
'Hostname': 'saltstack-container',
'WorkingDir': '/',
'Cmd': ['bash'],
'Volumes': {},
'Entrypoint': None,
'ExposedPorts': {},
},
})
__salt__ = {'dockerng.list_containers': dockerng_list_containers,
'dockerng.inspect_container': dockerng_inspect_container,
'dockerng.inspect_image': MagicMock(
return_value={'Id': image_id}),
'dockerng.inspect_image': dockerng_inspect_image,
'dockerng.list_tags': MagicMock(),
'dockerng.pull': MagicMock(),
'dockerng.state': MagicMock(side_effect=['stopped',
@ -355,12 +407,64 @@ class DockerngTestCase(TestCase):
dockerng_start = Mock()
dockerng_list_containers = Mock(return_value=['cont'])
dockerng_inspect_container = Mock(
return_value={'Config': {'Image': 'image:latest'},
'Image': image_id})
return_value={
'Config': {
'Image': 'image:latest',
'Tty': False,
'Labels': {},
'Domainname': '',
'User': '',
'AttachStderr': True,
'AttachStdout': True,
'Hostname': 'saltstack-container',
'Env': [],
'WorkingDir': '/',
'Cmd': ['bash'],
'Volumes': {},
'Entrypoint': None,
'ExposedPorts': {},
'OpenStdin': False,
},
'HostConfig': {
'PublishAllPorts': False,
'Dns': [],
'Links': None,
'CpusetCpus': '',
'RestartPolicy': {'MaximumRetryCount': 0, 'Name': ''},
'CapAdd': None,
'NetworkMode': 'default',
'PidMode': '',
'MemorySwap': 0,
'ExtraHosts': None,
'PortBindings': None,
'LxcConf': None,
'DnsSearch': [],
'Privileged': False,
'Binds': None,
'Memory': 0,
'VolumesFrom': None,
'CpuShares': 0,
'CapDrop': None,
},
'NetworkSettings': {
'MacAddress': '00:00:00:00:00:01',
},
'Image': image_id})
dockerng_inspect_image = MagicMock(
return_value={
'Id': image_id,
'Config': {
'Hostname': 'saltstack-container',
'WorkingDir': '/',
'Cmd': ['bash'],
'Volumes': {},
'Entrypoint': None,
'ExposedPorts': {},
},
})
__salt__ = {'dockerng.list_containers': dockerng_list_containers,
'dockerng.inspect_container': dockerng_inspect_container,
'dockerng.inspect_image': MagicMock(
return_value={'Id': image_id}),
'dockerng.inspect_image': dockerng_inspect_image,
'dockerng.list_tags': MagicMock(),
'dockerng.pull': MagicMock(),
'dockerng.state': MagicMock(side_effect=['stopped',
@ -659,6 +763,173 @@ class DockerngTestCase(TestCase):
'changes': {'removed': 'removed'},
'result': True})
def test_removal_of_parameter_is_detected(self):
'''
Test dockerng.running with deleted parameter.
1. define your sls
.. code-block:: yaml
container:
dockerng.running:
- name: super-container
- binds:
- /path:/path:ro
2. run state.highstate
3. modify your sls by removing `- binds:`
.. code-block:: yaml
container:
dockerng.running:
- name: super-container
4. enjoy your new created container without mounted volumes.
'''
image_id = 'abcdefg'
dockerng_create = Mock(return_value=True)
dockerng_start = Mock()
dockerng_list_containers = Mock(return_value=['cont'])
dockerng_inspect_container = Mock(
side_effect=[{
'Config': {
'Image': 'image:latest',
'Tty': False,
'Labels': {},
'Domainname': '',
'User': '',
'AttachStderr': True,
'AttachStdout': True,
'Hostname': 'saltstack-container',
'Env': [],
'WorkingDir': '/',
'Cmd': ['bash'],
'Volumes': {'/path': {}},
'Entrypoint': None,
'ExposedPorts': {},
'OpenStdin': False,
},
'HostConfig': {
'PublishAllPorts': False,
'Dns': [],
'Links': None,
'CpusetCpus': '',
'RestartPolicy': {'MaximumRetryCount': 0, 'Name': ''},
'CapAdd': None,
'NetworkMode': 'default',
'PidMode': '',
'MemorySwap': 0,
'ExtraHosts': None,
'PortBindings': None,
'LxcConf': None,
'DnsSearch': [],
'Privileged': False,
'Binds': ['/path:/path:ro'],
'Memory': 0,
'VolumesFrom': None,
'CpuShares': 0,
'CapDrop': None,
},
'NetworkSettings': {
'MacAddress': '00:00:00:00:00:01',
},
'Image': image_id},
{'Config': {
'Image': 'image:latest',
'Tty': False,
'Labels': {},
'Domainname': '',
'User': '',
'AttachStderr': True,
'AttachStdout': True,
'Hostname': 'saltstack-container',
'Env': [],
'WorkingDir': '/',
'Cmd': ['bash'],
'Volumes': {'/path': {}},
'Entrypoint': None,
'ExposedPorts': {},
'OpenStdin': False,
},
'HostConfig': {
'PublishAllPorts': False,
'Dns': [],
'Links': None,
'CpusetCpus': '',
'RestartPolicy': {'MaximumRetryCount': 0, 'Name': ''},
'CapAdd': None,
'NetworkMode': 'default',
'PidMode': '',
'MemorySwap': 0,
'ExtraHosts': None,
'PortBindings': None,
'LxcConf': None,
'DnsSearch': [],
'Privileged': False,
'Binds': None,
'Memory': 0,
'VolumesFrom': None,
'CpuShares': 0,
'CapDrop': None,
},
'NetworkSettings': {
'MacAddress': '00:00:00:00:00:01',
},
'Image': image_id}]
)
dockerng_inspect_image = MagicMock(
return_value={
'Id': image_id,
'Config': {
'Hostname': 'saltstack-container',
'WorkingDir': '/',
'Cmd': ['bash'],
'Volumes': {'/path': {}},
'Entrypoint': None,
'ExposedPorts': {},
},
})
__salt__ = {'dockerng.list_containers': dockerng_list_containers,
'dockerng.inspect_container': dockerng_inspect_container,
'dockerng.inspect_image': dockerng_inspect_image,
'dockerng.list_tags': MagicMock(),
'dockerng.pull': MagicMock(return_value=True),
'dockerng.state': MagicMock(side_effect=['stopped',
'running']),
'dockerng.rm': MagicMock(return_value='cont'),
'dockerng.create': dockerng_create,
'dockerng.start': dockerng_start,
}
with patch.dict(dockerng_state.__dict__,
{'__salt__': __salt__}):
ret = dockerng_state.running(
'cont',
image='image:latest',
)
self.assertEqual(ret, {'name': 'cont',
'comment': "Container 'cont' changed state.."
" Container 'cont' was replaced.",
'changes': {
'diff': {'binds':
{'new': [],
'old': ['/path:/path:ro']}},
'image': True,
'removed': 'cont',
'state': {'new': 'running',
'old': 'stopped'},
'added': True,
},
'result': True,
})
dockerng_create.assert_called_with('image:latest',
validate_ip_addrs=False,
validate_input=False,
name='cont',
client_timeout=60)
if __name__ == '__main__':
from integration import run_tests