From 42eaa0037135cd5f502beae283c1330849720c99 Mon Sep 17 00:00:00 2001 From: Will Thames Date: Wed, 11 Jul 2018 16:32:03 +1000 Subject: [PATCH] Provide kubernetes definition diffs in check_mode (#41471) Move dict_merge from azure_rm_resource module to module_utils.common.dict_transformations and add tests. Use dict_merge to provide a fairly realistic, reliable diff output when k8s-based modules are run in check_mode. Rename unit tests so that they actually run and reflect the module_utils they're based on. --- .../common/dict_transformations.py | 16 +++++++ lib/ansible/module_utils/k8s/raw.py | 23 +++++---- .../modules/cloud/azure/azure_rm_resource.py | 17 +------ .../test_dict_transformations.py} | 47 ++++++++++++++++++- 4 files changed, 78 insertions(+), 25 deletions(-) rename test/units/module_utils/{ec2/test_camel_to_snake.py => common/test_dict_transformations.py} (59%) diff --git a/lib/ansible/module_utils/common/dict_transformations.py b/lib/ansible/module_utils/common/dict_transformations.py index b02fe0372d9..c6d7c3b42d6 100644 --- a/lib/ansible/module_utils/common/dict_transformations.py +++ b/lib/ansible/module_utils/common/dict_transformations.py @@ -8,6 +8,7 @@ __metaclass__ = type import re +from copy import deepcopy def camel_dict_to_snake_dict(camel_dict, reversible=False, ignore_list=()): @@ -105,3 +106,18 @@ def _camel_to_snake(name, reversible=False): all_cap_pattern = r'([a-z0-9])([A-Z]+)' s2 = re.sub(first_cap_pattern, r'\1_\2', s1) return re.sub(all_cap_pattern, r'\1_\2', s2).lower() + + +def dict_merge(a, b): + '''recursively merges dicts. not just simple a['key'] = b['key'], if + both a and b have a key whose value is a dict then dict_merge is called + on both values and the result stored in the returned dictionary.''' + if not isinstance(b, dict): + return b + result = deepcopy(a) + for k, v in b.items(): + if k in result and isinstance(result[k], dict): + result[k] = dict_merge(result[k], v) + else: + result[k] = deepcopy(v) + return result diff --git a/lib/ansible/module_utils/k8s/raw.py b/lib/ansible/module_utils/k8s/raw.py index a67dd846d6d..b993332bd4a 100644 --- a/lib/ansible/module_utils/k8s/raw.py +++ b/lib/ansible/module_utils/k8s/raw.py @@ -21,6 +21,7 @@ from __future__ import absolute_import, division, print_function from ansible.module_utils.six import string_types from ansible.module_utils.k8s.common import KubernetesAnsibleModule +from ansible.module_utils.common.dict_transformations import dict_merge try: @@ -143,7 +144,9 @@ class KubernetesRawModule(KubernetesAnsibleModule): return result else: if not existing: - if not self.check_mode: + if self.check_mode: + k8s_obj = definition + else: try: k8s_obj = resource.create(definition, namespace=namespace) except ConflictError: @@ -153,7 +156,7 @@ class KubernetesRawModule(KubernetesAnsibleModule): self.warn("{0} was not found, but creating it returned a 409 Conflict error. This can happen \ if the resource you are creating does not directly create a resource of the same kind.".format(name)) return result - result['result'] = k8s_obj.to_dict() + result['result'] = k8s_obj.to_dict() result['changed'] = True result['method'] = 'create' return result @@ -162,28 +165,32 @@ class KubernetesRawModule(KubernetesAnsibleModule): diffs = [] if existing and force: - if not self.check_mode: + if self.check_mode: + k8s_obj = definition + else: try: k8s_obj = resource.replace(definition, name=name, namespace=namespace).to_dict() - match, diffs = self.diff_objects(existing.to_dict(), k8s_obj) - result['result'] = k8s_obj except DynamicApiError as exc: self.fail_json(msg="Failed to replace object: {0}".format(exc.body), error=exc.status, status=exc.status, reason=exc.reason) + match, diffs = self.diff_objects(existing.to_dict(), k8s_obj) + result['result'] = k8s_obj result['changed'] = not match result['method'] = 'replace' result['diff'] = diffs return result # Differences exist between the existing obj and requested params - if not self.check_mode: + if self.check_mode: + k8s_obj = dict_merge(existing.to_dict(), definition) + else: try: k8s_obj = resource.patch(definition, name=name, namespace=namespace).to_dict() - match, diffs = self.diff_objects(existing.to_dict(), k8s_obj) - result['result'] = k8s_obj except DynamicApiError as exc: self.fail_json(msg="Failed to patch object: {0}".format(exc.body), error=exc.status, status=exc.status, reason=exc.reason) + match, diffs = self.diff_objects(existing.to_dict(), k8s_obj) + result['result'] = k8s_obj result['changed'] = not match result['method'] = 'patch' result['diff'] = diffs diff --git a/lib/ansible/modules/cloud/azure/azure_rm_resource.py b/lib/ansible/modules/cloud/azure/azure_rm_resource.py index 5bb015a3559..202af8cd527 100644 --- a/lib/ansible/modules/cloud/azure/azure_rm_resource.py +++ b/lib/ansible/modules/cloud/azure/azure_rm_resource.py @@ -114,7 +114,7 @@ response: from ansible.module_utils.azure_rm_common import AzureRMModuleBase from ansible.module_utils.azure_rm_common_rest import GenericRestClient -from copy import deepcopy +from ansible.module_utils.common.dict_transformations import dict_merge try: from msrestazure.azure_exceptions import CloudError @@ -265,21 +265,6 @@ class AzureRMResource(AzureRMModuleBase): return self.results -def dict_merge(a, b): - '''recursively merges dict's. not just simple a['key'] = b['key'], if - both a and bhave a key who's value is a dict then dict_merge is called - on both values and the result stored in the returned dictionary.''' - if not isinstance(b, dict): - return b - result = deepcopy(a) - for k, v in b.items(): - if k in result and isinstance(result[k], dict): - result[k] = dict_merge(result[k], v) - else: - result[k] = deepcopy(v) - return result - - def main(): AzureRMResource() diff --git a/test/units/module_utils/ec2/test_camel_to_snake.py b/test/units/module_utils/common/test_dict_transformations.py similarity index 59% rename from test/units/module_utils/ec2/test_camel_to_snake.py rename to test/units/module_utils/common/test_dict_transformations.py index 7dc1e25cffe..6756a2b54f9 100644 --- a/test/units/module_utils/ec2/test_camel_to_snake.py +++ b/test/units/module_utils/common/test_dict_transformations.py @@ -17,7 +17,7 @@ # along with Ansible. If not, see . from ansible.compat.tests import unittest -from ansible.module_utils.ec2 import _camel_to_snake, _snake_to_camel, camel_dict_to_snake_dict +from ansible.module_utils.common.dict_transformations import _camel_to_snake, _snake_to_camel, camel_dict_to_snake_dict, dict_merge EXPECTED_SNAKIFICATION = { 'alllower': 'alllower', @@ -69,3 +69,48 @@ class CamelDictToSnakeDictTestCase(unittest.TestCase): snake_dict = camel_dict_to_snake_dict(camel_dict, ignore_list='World') self.assertEqual(snake_dict['hello'], dict(one='one', two='two')) self.assertEqual(snake_dict['world'], dict(Three='three', Four='four')) + + +class DictMergeTestCase(unittest.TestCase): + def test_dict_merge(self): + base = dict(obj2=dict(), b1=True, b2=False, b3=False, + one=1, two=2, three=3, obj1=dict(key1=1, key2=2), + l1=[1, 3], l2=[1, 2, 3], l4=[4], + nested=dict(n1=dict(n2=2))) + + other = dict(b1=True, b2=False, b3=True, b4=True, + one=1, three=4, four=4, obj1=dict(key1=2), + l1=[2, 1], l2=[3, 2, 1], l3=[1], + nested=dict(n1=dict(n2=2, n3=3))) + + result = dict_merge(base, other) + + # string assertions + self.assertTrue('one' in result) + self.assertTrue('two' in result) + self.assertEqual(result['three'], 4) + self.assertEqual(result['four'], 4) + + # dict assertions + self.assertTrue('obj1' in result) + self.assertTrue('key1' in result['obj1']) + self.assertTrue('key2' in result['obj1']) + + # list assertions + # this line differs from the network_utils/common test of the function of the + # same name as this method does not merge lists + self.assertEqual(result['l1'], [2, 1]) + self.assertTrue('l2' in result) + self.assertEqual(result['l3'], [1]) + self.assertTrue('l4' in result) + + # nested assertions + self.assertTrue('obj1' in result) + self.assertEqual(result['obj1']['key1'], 2) + self.assertTrue('key2' in result['obj1']) + + # bool assertions + self.assertTrue('b1' in result) + self.assertTrue('b2' in result) + self.assertTrue(result['b3']) + self.assertTrue(result['b4'])