wbrawner/ansible
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

docker_* modules: simplify idempotency comparisons (#47709)

Browse source 
* More generic comparison code from docker_container to docker_common.

* More flexibility if a is None and method is allow_to_present.

Note that this odes not affect docker_container, as there a is never None.

* Update docker_secret and docker_config: simplify labels comparison.

* Added unit tests.

* Use proper subsequence test for allow_more_present for lists.

Note that this does not affect existing code in docker_container, since lists
don't use allow_more_present. Using allow_more_present will only be possible
in Ansible 2.8.

* pep8
This commit is contained in:
Felix Fontein 2018-10-30 09:50:34 +01:00 committed by John R Barker
parent e217ba6e19
commit 73533d3fc2
5 changed files with 577 additions and 103 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

105
lib/ansible/module_utils/docker_common.py
View file

@ -541,3 +541,108 @@ class AnsibleDockerClient(Client):
new_tag = self.find_image(name, tag)
return new_tag, old_tag == new_tag
def compare_dict_allow_more_present(av, bv):
'''
Compare two dictionaries for whether every entry of the first is in the second.
'''
for key, value in av.items():
if key not in bv:
return False
if bv[key] != value:
return False
return True
def compare_generic(a, b, method, type):
'''
Compare values a and b as described by method and type.
Returns ``True`` if the values compare equal, and ``False`` if not.
``a`` is usually the module's parameter, while ``b`` is a property
of the current object. ``a`` must not be ``None`` (except for
``type == 'value'``).
Valid values for ``method`` are:
- ``ignore`` (always compare as equal);
- ``strict`` (only compare if really equal)
- ``allow_more_present`` (allow b to have elements which a does not have).
Valid values for ``type`` are:
- ``value``: for simple values (strings, numbers, ...);
- ``list``: for ``list``s or ``tuple``s where order matters;
- ``set``: for ``list``s, ``tuple``s or ``set``s where order does not
matter;
- ``set(dict)``: for ``list``s, ``tuple``s or ``sets`` where order does
not matter and which contain ``dict``s; ``allow_more_present`` is used
for the ``dict``s, and these are assumed to be dictionaries of values;
- ``dict``: for dictionaries of values.
'''
if method == 'ignore':
return True
# If a or b is None:
if a is None or b is None:
# If both are None: equality
if a == b:
return True
# Otherwise, not equal for values, and equal
# if the other is empty for set/list/dict
if type == 'value':
return False
# For allow_more_present, allow a to be None
if method == 'allow_more_present' and a is None:
return True
# Otherwise, the iterable object which is not None must have length 0
return len(b if a is None else a) == 0
# Do proper comparison (both objects not None)
if type == 'value':
return a == b
elif type == 'list':
if method == 'strict':
return a == b
else:
i = 0
for v in a:
while i < len(b) and b[i] != v:
i += 1
if i == len(b):
return False
i += 1
return True
elif type == 'dict':
if method == 'strict':
return a == b
else:
return compare_dict_allow_more_present(a, b)
elif type == 'set':
set_a = set(a)
set_b = set(b)
if method == 'strict':
return set_a == set_b
else:
return set_b >= set_a
elif type == 'set(dict)':
for av in a:
found = False
for bv in b:
if compare_dict_allow_more_present(av, bv):
found = True
break
if not found:
return False
if method == 'strict':
# If we would know that both a and b do not contain duplicates,
# we could simply compare len(a) to len(b) to finish this test.
# We can assume that b has no duplicates (as it is returned by
# docker), but we don't know for a.
for bv in b:
found = False
for av in a:
if compare_dict_allow_more_present(av, bv):
found = True
break
if not found:
return False
return True

20
lib/ansible/modules/cloud/docker/docker_config.py
View file

@ -152,7 +152,7 @@ except ImportError:
# missing docker-py handled in ansible.module_utils.docker
pass
from ansible.module_utils.docker_common import AnsibleDockerClient, DockerBaseClass
from ansible.module_utils.docker_common import AnsibleDockerClient, DockerBaseClass, compare_generic
from ansible.module_utils._text import to_native, to_bytes
@ -224,22 +224,8 @@ class ConfigManager(DockerBaseClass):
if attrs.get('Labels', {}).get('ansible_key'):
if attrs['Labels']['ansible_key'] != self.data_key:
data_changed = True
labels_changed = False
if self.labels and attrs.get('Labels'):
# check if user requested a label change
for label in attrs['Labels']:
if self.labels.get(label) and self.labels[label] != attrs['Labels'][label]:
labels_changed = True
# check if user added a label
labels_added = False
if self.labels:
if attrs.get('Labels'):
for label in self.labels:
if label not in attrs['Labels']:
labels_added = True
else:
labels_added = True
if data_changed or labels_added or labels_changed or self.force:
labels_changed = not compare_generic(self.labels, attrs.get('Labels'), 'allow_more_present', 'dict')
if data_changed or labels_changed or self.force:
# if something changed or force, delete and re-create the config
self.absent()
config_id = self.create_config()

71
lib/ansible/modules/cloud/docker/docker_container.py
View file

@ -753,6 +753,7 @@ from ansible.module_utils.basic import human_to_bytes
from ansible.module_utils.docker_common import (
HAS_DOCKER_PY_2, HAS_DOCKER_PY_3, AnsibleDockerClient,
DockerBaseClass, sanitize_result, is_image_name_id,
compare_generic,
)
from ansible.module_utils.six import string_types
@ -1537,79 +1538,11 @@ class Container(DockerBaseClass):
return True
return False
def _compare_dict_allow_more_present(self, av, bv):
'''
Compare two dictionaries for whether every entry of the first is in the second.
'''
for key, value in av.items():
if key not in bv:
return False
if bv[key] != value:
return False
return True
def _compare(self, a, b, compare):
'''
Compare values a and b as described in compare.
'''
method = compare['comparison']
if method == 'ignore':
return True
# If a or b is None:
if a is None or b is None:
# If both are None: equality
if a == b:
return True
# Otherwise, not equal for values, and equal
# if the other is empty for set/list/dict
if compare['type'] == 'value':
return False
return len(b if a is None else a) == 0
# Do proper comparison (both objects not None)
if compare['type'] == 'value':
return a == b
elif compare['type'] == 'list':
if method == 'strict':
return a == b
else:
set_a = set(a)
set_b = set(b)
return set_b >= set_a
elif compare['type'] == 'dict':
if method == 'strict':
return a == b
else:
return self._compare_dict_allow_more_present(a, b)
elif compare['type'] == 'set':
set_a = set(a)
set_b = set(b)
if method == 'strict':
return set_a == set_b
else:
return set_b >= set_a
elif compare['type'] == 'set(dict)':
for av in a:
found = False
for bv in b:
if self._compare_dict_allow_more_present(av, bv):
found = True
break
if not found:
return False
if method == 'strict':
# If we would know that both a and b do not contain duplicates,
# we could simply compare len(a) to len(b) to finish this test.
# We can assume that b has no duplicates (as it is returned by
# docker), but we don't know for a.
for bv in b:
found = False
for av in a:
if self._compare_dict_allow_more_present(av, bv):
found = True
break
if not found:
return False
return True
return compare_generic(a, b, compare['comparison'], compare['type'])
def has_different_configuration(self, image):
'''

20
lib/ansible/modules/cloud/docker/docker_secret.py
View file

@ -147,7 +147,7 @@ except ImportError:
# missing docker-py handled in ansible.module_utils.docker_common
pass
from ansible.module_utils.docker_common import AnsibleDockerClient, DockerBaseClass
from ansible.module_utils.docker_common import AnsibleDockerClient, DockerBaseClass, compare_generic
from ansible.module_utils._text import to_native, to_bytes
@ -219,22 +219,8 @@ class SecretManager(DockerBaseClass):
if attrs.get('Labels', {}).get('ansible_key'):
if attrs['Labels']['ansible_key'] != self.data_key:
data_changed = True
labels_changed = False
if self.labels and attrs.get('Labels'):
# check if user requested a label change
for label in attrs['Labels']:
if self.labels.get(label) and self.labels[label] != attrs['Labels'][label]:
labels_changed = True
# check if user added a label
labels_added = False
if self.labels:
if attrs.get('Labels'):
for label in self.labels:
if label not in attrs['Labels']:
labels_added = True
else:
labels_added = True
if data_changed or labels_added or labels_changed or self.force:
labels_changed = not compare_generic(self.labels, attrs.get('Labels'), 'allow_more_present', 'dict')
if data_changed or labels_changed or self.force:
# if something changed or force, delete and re-create the secret
self.absent()
secret_id = self.create_secret()

464
test/units/module_utils/test_docker_common.py Normal file
View file

@ -0,0 +1,464 @@
import pytest
from ansible.module_utils.docker_common import (
compare_dict_allow_more_present,
compare_generic,
)
DICT_ALLOW_MORE_PRESENT = (
{
'av': {},
'bv': {'a': 1},
'result': True
},
{
'av': {'a': 1},
'bv': {'a': 1, 'b': 2},
'result': True
},
{
'av': {'a': 1},
'bv': {'b': 2},
'result': False
},
{
'av': {'a': 1},
'bv': {'a': None, 'b': 1},
'result': False
},
{
'av': {'a': None},
'bv': {'b': 1},
'result': False
},
)
COMPARE_GENERIC = [
########################################################################################
# value
{
'a': 1,
'b': 2,
'method': 'strict',
'type': 'value',
'result': False
},
{
'a': 'hello',
'b': 'hello',
'method': 'strict',
'type': 'value',
'result': True
},
{
'a': None,
'b': 'hello',
'method': 'strict',
'type': 'value',
'result': False
},
{
'a': None,
'b': None,
'method': 'strict',
'type': 'value',
'result': True
},
{
'a': 1,
'b': 2,
'method': 'ignore',
'type': 'value',
'result': True
},
{
'a': None,
'b': 2,
'method': 'ignore',
'type': 'value',
'result': True
},
########################################################################################
# list
{
'a': [
'x',
],
'b': [
'y',
],
'method': 'strict',
'type': 'list',
'result': False
},
{
'a': [
'x',
],
'b': [
'x',
'x',
],
'method': 'strict',
'type': 'list',
'result': False
},
{
'a': [
'x',
'y',
],
'b': [
'x',
'y',
],
'method': 'strict',
'type': 'list',
'result': True
},
{
'a': [
'x',
'y',
],
'b': [
'y',
'x',
],
'method': 'strict',
'type': 'list',
'result': False
},
{
'a': [
'x',
'y',
],
'b': [
'x',
],
'method': 'allow_more_present',
'type': 'list',
'result': False
},
{
'a': [
'x',
],
'b': [
'x',
'y',
],
'method': 'allow_more_present',
'type': 'list',
'result': True
},
{
'a': [
'x',
'x',
'y',
],
'b': [
'x',
'y',
],
'method': 'allow_more_present',
'type': 'list',
'result': False
},
{
'a': [
'x',
'z',
],
'b': [
'x',
'y',
'x',
'z',
],
'method': 'allow_more_present',
'type': 'list',
'result': True
},
{
'a': [
'x',
'y',
],
'b': [
'y',
'x',
],
'method': 'ignore',
'type': 'list',
'result': True
},
########################################################################################
# set
{
'a': [
'x',
],
'b': [
'y',
],
'method': 'strict',
'type': 'set',
'result': False
},
{
'a': [
'x',
],
'b': [
'x',
'x',
],
'method': 'strict',
'type': 'set',
'result': True
},
{
'a': [
'x',
'y',
],
'b': [
'x',
'y',
],
'method': 'strict',
'type': 'set',
'result': True
},
{
'a': [
'x',
'y',
],
'b': [
'y',
'x',
],
'method': 'strict',
'type': 'set',
'result': True
},
{
'a': [
'x',
'y',
],
'b': [
'x',
],
'method': 'allow_more_present',
'type': 'set',
'result': False
},
{
'a': [
'x',
],
'b': [
'x',
'y',
],
'method': 'allow_more_present',
'type': 'set',
'result': True
},
{
'a': [
'x',
'x',
'y',
],
'b': [
'x',
'y',
],
'method': 'allow_more_present',
'type': 'set',
'result': True
},
{
'a': [
'x',
'z',
],
'b': [
'x',
'y',
'x',
'z',
],
'method': 'allow_more_present',
'type': 'set',
'result': True
},
{
'a': [
'x',
'a',
],
'b': [
'y',
'z',
],
'method': 'ignore',
'type': 'set',
'result': True
},
########################################################################################
# set(dict)
{
'a': [
{'x': 1},
],
'b': [
{'y': 1},
],
'method': 'strict',
'type': 'set(dict)',
'result': False
},
{
'a': [
{'x': 1},
],
'b': [
{'x': 1},
],
'method': 'strict',
'type': 'set(dict)',
'result': True
},
{
'a': [
{'x': 1},
],
'b': [
{'x': 1, 'y': 2},
],
'method': 'strict',
'type': 'set(dict)',
'result': True
},
{
'a': [
{'x': 1},
{'x': 2, 'y': 3},
],
'b': [
{'x': 1},
{'x': 2, 'y': 3},
],
'method': 'strict',
'type': 'set(dict)',
'result': True
},
{
'a': [
{'x': 1},
],
'b': [
{'x': 1, 'z': 2},
{'x': 2, 'y': 3},
],
'method': 'allow_more_present',
'type': 'set(dict)',
'result': True
},
{
'a': [
{'x': 1, 'y': 2},
],
'b': [
{'x': 1},
{'x': 2, 'y': 3},
],
'method': 'allow_more_present',
'type': 'set(dict)',
'result': False
},
{
'a': [
{'x': 1, 'y': 3},
],
'b': [
{'x': 1},
{'x': 1, 'y': 3, 'z': 4},
],
'method': 'allow_more_present',
'type': 'set(dict)',
'result': True
},
{
'a': [
{'x': 1},
{'x': 2, 'y': 3},
],
'b': [
{'x': 1},
],
'method': 'ignore',
'type': 'set(dict)',
'result': True
},
########################################################################################
# dict
{
'a': {'x': 1},
'b': {'y': 1},
'method': 'strict',
'type': 'dict',
'result': False
},
{
'a': {'x': 1},
'b': {'x': 1, 'y': 2},
'method': 'strict',
'type': 'dict',
'result': False
},
{
'a': {'x': 1},
'b': {'x': 1},
'method': 'strict',
'type': 'dict',
'result': True
},
{
'a': {'x': 1, 'z': 2},
'b': {'x': 1, 'y': 2},
'method': 'strict',
'type': 'dict',
'result': False
},
{
'a': {'x': 1, 'z': 2},
'b': {'x': 1, 'y': 2},
'method': 'ignore',
'type': 'dict',
'result': True
},
] + [{
'a': entry['av'],
'b': entry['bv'],
'method': 'allow_more_present',
'type': 'dict',
'result': entry['result']
} for entry in DICT_ALLOW_MORE_PRESENT]
@pytest.mark.parametrize("entry", DICT_ALLOW_MORE_PRESENT)
def test_dict_allow_more_present(entry):
assert compare_dict_allow_more_present(entry['av'], entry['bv']) == entry['result']
@pytest.mark.parametrize("entry", COMPARE_GENERIC)
def test_compare_generic(entry):
assert compare_generic(entry['a'], entry['b'], entry['method'], entry['type']) == entry['result']