Add support for diff and check mode for 3 modules

plugins/module_utils/common_utils.py

@@ -169,3 +169,106 @@ def get_upgrade_orchestrator_node(module, mgr_hostname, mgr_username, mgr_passwo
         module.fail_json(changed=True, msg='Error getting ip address of the upgrade'
                         ' orchestrator node. Error: {}'.format(err))
     return resp['service_properties']['enabled_on'];
+
+def deep_same(a, b):
+    '''
+    params:
+    - a: A python literal, list, or dict
+    - b: A python literal, list, or dict
+
+    Compares a and b, including their subcomponents. Lists are compared
+    ignoring order.
+
+    Returns True if a and b are the same, False if they are not.
+    '''
+    if isinstance(a, dict) and isinstance(b, dict):
+        for k, v in a.items():
+            if k not in b:
+                return False
+            if not deep_same(v, b[k]):
+                return False
+        return True
+
+    if isinstance(a, list) and isinstance(b, list):
+        if len(a) != len(b):
+            return False
+        # Compare the two lists as sets. The standard set() type cannot be used
+        # because lists and dicts are not hashable.
+        for subitem_a in a:
+            match_found = False
+            for subitem_b in b:
+                if deep_same(subitem_a, subitem_b):
+                    match_found = True
+                    break
+            if not match_found:
+                return False
+        return True
+
+    return a == b
+
+def check_for_update(existing_params, resource_params):
+    '''
+    params:
+    - existing_params: A dict representing the existing state
+    - resource_params: A dict representing the expected future state
+
+    Compares the existing_params with resource_params and returns
+    True if an update is needed.
+
+    Returns True if the params should trigger an update.
+    '''
+    # A resource exists in reality but not in ansible. No need to update.
+    if not existing_params:
+        return False
+
+    # An update is needed if they are not the same.
+    return not deep_same(existing_params, resource_params)
+
+def format_for_ansible_diff(before, after):
+    '''
+    params:
+    - before: An object representing the existing state
+    - after: An object representing the expected future state
+
+    If the before and after objects implement MutableMapping (e.g. dict), they
+    will be automatically serialized to json by ansible. If not, they should be
+    run through json.dumps() beforehand.
+
+    Returns a dict formatted for ansible diff
+    '''
+    return {'before': before, 'after': after}
+
+def diff_for_update(existing_params, resource_params, strict_keys=[], lazy_keys=[]):
+    '''
+    params:
+    - existing_params: A dict representing the current resource state
+    - resource_params: A dict representing the desired (unapplied) state
+    - strict_keys: Always compare these top-level keys. If strict_keys is
+                   empty, it will default to all keys in new_params.
+    - lazy_keys: Compare these keys only if they are defined in both sets of
+                 params. These may overlap with strict_keys.
+
+
+    Returns a tuple of (is_updated, diff)
+    '''
+    # Generate representative "before" and "after" objects for diff output with
+    # only the relevant keys.
+    old_params = existing_params or {}
+    new_params = resource_params or {}
+    before = {}
+    after = {}
+    keys = strict_keys if strict_keys else list(new_params.keys())
+    for key in set(keys + lazy_keys):
+        if key in lazy_keys and \
+           not (key in old_params and key in new_params):
+            continue
+        before[key] = old_params.get(key)
+        after[key] = new_params.get(key)
+
+    # Compute diff using before and after objects, rather than existing_params
+    # and new_params, so that the diff output matches the is_updated state.
+    # This allows support for lazy keys without showing them in the diff when
+    # they only exist in one set of params.
+    is_updated = check_for_update(before, after) if existing_params else False
+    diff = format_for_ansible_diff(before, after)
+    return (is_updated, diff)

plugins/module_utils/nsxt_base_resource.py

@@ -19,6 +19,7 @@
 # EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
+from ansible_collections.vmware.ansible_for_nsxt.plugins.module_utils.common_utils import diff_for_update
 from ansible_collections.vmware.ansible_for_nsxt.plugins.module_utils.policy_communicator import PolicyCommunicator
 from ansible_collections.vmware.ansible_for_nsxt.plugins.module_utils.policy_communicator import DuplicateRequestError
 
@@ -257,44 +258,6 @@ def update_resource_params(self, nsx_resource_params):
         # Should be overridden in the subclass if needed
         pass
 
-    def check_for_update(self, existing_params, resource_params):
-        """
-            resource_params: dict
-            existing_params: dict
-            Compares the existing_params with resource_params and returns
-            True if they are different. At a base level, it traverses the
-            params and matches one-to-one. If the value to be matched is a
-            - dict, it traverses that also.
-            - list, it merely compares the order.
-            Can be overriden in the subclass for specific custom checking.
-            Returns true if the params differ
-        """
-        if not existing_params:
-            return False
-        for k, v in resource_params.items():
-            if k not in existing_params:
-                return True
-            elif type(v).__name__ == 'dict':
-                if self.check_for_update(existing_params[k], v):
-                    return True
-            elif v != existing_params[k]:
-                def compare_lists(list1, list2):
-                    # Returns True if list1 and list2 differ
-                    try:
-                        # If the lists can be converted into sets, do so and
-                        # compare lists as sets.
-                        set1 = set(list1)
-                        set2 = set(list2)
-                        return set1 != set2
-                    except Exception:
-                        return True
-                if type(v).__name__ == 'list':
-                    if compare_lists(v, existing_params[k]):
-                        return True
-                    continue
-                return True
-        return False
-
     def update_parent_info(self, parent_info):
         # Override this and fill in self._parent_info if that is to be passed
         # to the sub-resource
@@ -562,15 +525,18 @@ def filter_with_spec(spec):
 
     def _achieve_present_state(self, successful_resource_exec_logs):
         self.update_resource_params(self.nsx_resource_params)
-        is_resource_updated = self.check_for_update(
+
+        is_resource_updated, diff = diff_for_update(
             self.existing_resource, self.nsx_resource_params)
+
         if not is_resource_updated:
             # Either the resource does not exist or it exists but was not
             # updated in the YAML.
             if self.module.check_mode:
                 successful_resource_exec_logs.append({
-                    "changed": True,
+                    "changed": False if self.existing_resource else True,
                     "debug_out": self.resource_params,
+                    "diff": diff,
                     "id": '12345',
                     "resource_type": self.get_resource_name()
                 })
@@ -580,6 +546,7 @@ def _achieve_present_state(self, successful_resource_exec_logs):
                     # Resource already exists
                     successful_resource_exec_logs.append({
                         "changed": False,
+                        "diff": diff,
                         "id": self.id,
                         "message": "%s with id %s already exists." %
                         (self.get_resource_name(), self.id),
@@ -595,6 +562,7 @@ def _achieve_present_state(self, successful_resource_exec_logs):
 
                 successful_resource_exec_logs.append({
                     "changed": True,
+                    "diff": diff,
                     "id": self.id,
                     "body": str(resp),
                     "message": "%s with id %s created." %
@@ -616,6 +584,7 @@ def _achieve_present_state(self, successful_resource_exec_logs):
                 successful_resource_exec_logs.append({
                     "changed": True,
                     "debug_out": self.resource_params,
+                    "diff": diff,
                     "id": self.id,
                     "resource_type": self.get_resource_name()
                 })
@@ -633,6 +602,7 @@ def _achieve_present_state(self, successful_resource_exec_logs):
                         '_revision'] != self.existing_resource_revision:
                     successful_resource_exec_logs.append({
                         "changed": True,
+                        "diff": diff,
                         "id": self.id,
                         "body": str(patch_resp),
                         "message": "%s with id %s updated." %
@@ -642,6 +612,7 @@ def _achieve_present_state(self, successful_resource_exec_logs):
                 else:
                     successful_resource_exec_logs.append({
                         "changed": False,
+                        "diff": diff,
                         "id": self.id,
                         "message": "%s with id %s already exists." %
                         (self.get_resource_name(), self.id),
@@ -661,9 +632,13 @@ def _achieve_absent_state(self, successful_resource_exec_logs):
         if self.skip_delete():
             return
 
+        _is_updated, diff = diff_for_update(
+            self.existing_resource, self.nsx_resource_params)
+
         if self.existing_resource is None:
             successful_resource_exec_logs.append({
                 "changed": False,
+                "diff": diff,
                 "msg": 'No %s exist with id %s' %
                 (self.get_resource_name(), self.id),
                 "resource_type": self.get_resource_name()
@@ -672,6 +647,7 @@ def _achieve_absent_state(self, successful_resource_exec_logs):
         if self.module.check_mode:
             successful_resource_exec_logs.append({
                 "changed": True,
+                "diff": diff,
                 "debug_out": self.resource_params,
                 "id": self.id,
                 "resource_type": self.get_resource_name()
@@ -682,6 +658,7 @@ def _achieve_absent_state(self, successful_resource_exec_logs):
             self._wait_till_delete()
             successful_resource_exec_logs.append({
                 "changed": True,
+                "diff": diff,
                 "id": self.id,
                 "message": "%s with id %s deleted." %
                 (self.get_resource_name(), self.id)
@@ -778,6 +755,7 @@ def _achieve_state(self, resource_params,
                     break
             srel = successful_resource_exec_logs
             self.module.exit_json(changed=changed,
+                                  diff=successful_resource_exec_log["diff"],
                                   successfully_updated_resources=srel)
 
     def _get_sub_resources_class_of(self, resource_class):

plugins/modules/nsxt_transport_zones.py

@@ -126,6 +126,7 @@
 
 import json, time
 from ansible.module_utils.basic import AnsibleModule
+from ansible_collections.vmware.ansible_for_nsxt.plugins.module_utils.common_utils import diff_for_update
 from ansible_collections.vmware.ansible_for_nsxt.plugins.module_utils.vmware_nsxt import vmware_argument_spec, request
 from ansible_collections.vmware.ansible_for_nsxt.plugins.module_utils.nsxt_resource_urls import TRANSPORT_ZONE_URL
 from ansible.module_utils._text import to_native
@@ -168,35 +169,6 @@ def get_tz_from_display_name(module, manager_url, mgr_username, mgr_password, va
   return None
 
 
-def check_for_update(module, manager_url, mgr_username, mgr_password, validate_certs, transport_zone_params, transport_zone_base_url):
-  existing_transport_zone = get_tz_from_display_name(module, manager_url, mgr_username, mgr_password, validate_certs,
-                                                     transport_zone_params['display_name'], transport_zone_base_url)
-  if existing_transport_zone is None:
-    return False
-  if existing_transport_zone.__contains__('is_default') and transport_zone_params.__contains__('is_default') and \
-          existing_transport_zone['is_default'] != transport_zone_params['is_default']:
-    return True
-  if not existing_transport_zone.__contains__('description') and transport_zone_params.__contains__('description'):
-    return True
-  if existing_transport_zone.__contains__('description') and not transport_zone_params.__contains__('description'):
-    return True
-  if existing_transport_zone.__contains__('description') and transport_zone_params.__contains__('description') and \
-          existing_transport_zone['description'] != transport_zone_params['description']:
-    return True
-  if not existing_transport_zone.__contains__('uplink_teaming_policy_names') and transport_zone_params.__contains__(
-          'uplink_teaming_policy_names'):
-    return True
-  if existing_transport_zone.__contains__('uplink_teaming_policy_names') and not transport_zone_params.__contains__(
-          'uplink_teaming_policy_names'):
-    return True
-  if existing_transport_zone.__contains__('uplink_teaming_policy_names') and transport_zone_params.__contains__(
-          'uplink_teaming_policy_names') and \
-          existing_transport_zone['uplink_teaming_policy_names'] != transport_zone_params[
-    'uplink_teaming_policy_names']:
-    return True
-  return False
-
-
 def main():
   argument_spec = vmware_argument_spec()
   argument_spec.update(display_name=dict(required=True, type='str'),
@@ -223,26 +195,31 @@ def main():
   zone_dict = get_tz_from_display_name(module, manager_url, mgr_username, mgr_password, validate_certs, display_name,
                                        transport_zone_base_url)
   zone_id, revision = None, None
+  updated, diff = diff_for_update(existing_params=zone_dict,
+                                  resource_params=transport_zone_params,
+                                  strict_keys=['is_default', 'description', 'uplink_teaming_policy_names'],
+                                  lazy_keys=['is_default'])
+
   if zone_dict:
     zone_id = zone_dict['id']
     revision = zone_dict['_revision']
 
   if state == 'present':
     headers = dict(Accept="application/json")
     headers['Content-Type'] = 'application/json'
-    updated = check_for_update(module, manager_url, mgr_username, mgr_password, validate_certs,
-                               transport_zone_params, transport_zone_base_url)
 
     if not updated:
       # add the node
       if module.check_mode:
-        module.exit_json(changed=True, debug_out=str(json.dumps(transport_zone_params)), id='12345')
+        module.exit_json(changed=False, debug_out=str(json.dumps(transport_zone_params)), diff=diff, id='12345')
       request_data = json.dumps(transport_zone_params)
       try:
         if zone_id:
-          module.exit_json(changed=False, id=zone_id,
-                           message="Transport zone with display_name %s already exist." % module.params[
-                             'display_name'])
+          module.exit_json(changed=False,
+                           diff=diff,
+                           id=zone_id,
+                           message="Transport zone with display_name %s already exist." % module.params['display_name'])
+
         (rc, resp) = request(manager_url + transport_zone_base_url + '/%s' % module.params['display_name'],
                              data=request_data, headers=headers, method='PUT',
                              url_username=mgr_username, url_password=mgr_password,
@@ -251,11 +228,14 @@ def main():
         module.fail_json(
           msg="Failed to add transport zone. Request body [%s]. Error[%s]." % (request_data, to_native(err)))
       time.sleep(5)
-      module.exit_json(changed=True, id=resp["id"], body=str(resp),
+      module.exit_json(changed=True,
+                       diff=diff,
+                       id=resp["id"],
+                       body=str(resp),
                        message="Transport zone with display name %s created. " % (module.params['display_name']))
     else:
       if module.check_mode:
-        module.exit_json(changed=True, debug_out=str(json.dumps(transport_zone_params)), id=zone_id)
+        module.exit_json(changed=True, debug_out=str(json.dumps(transport_zone_params)), diff=diff, id=zone_id)
 
       transport_zone_params['_revision'] = revision  # update current revision
       request_data = json.dumps(transport_zone_params)
@@ -270,25 +250,25 @@ def main():
           id, request_data, to_native(err)))
 
       time.sleep(5)
-      module.exit_json(changed=True, id=resp["id"], body=str(resp),
+      module.exit_json(changed=True, diff=diff, id=resp["id"], body=str(resp),
                        message="Transport zone with zone id %s updated." % id)
 
   elif state == 'absent':
     # delete the array
     id = zone_id
     if id is None:
-      module.exit_json(changed=False, msg='No transport zone exist with display name %s' % display_name)
+      module.exit_json(changed=False, diff=diff, msg='No transport zone exist with display name %s' % display_name)
     if module.check_mode:
-      module.exit_json(changed=True, debug_out=str(json.dumps(transport_zone_params)), id=id)
+      module.exit_json(changed=True, debug_out=str(json.dumps(transport_zone_params)), diff=diff, id=id)
     try:
       (rc, resp) = request(manager_url + transport_zone_base_url + "/%s" % id, method='DELETE',
                            url_username=mgr_username, url_password=mgr_password, validate_certs=validate_certs)
     except Exception as err:
       module.fail_json(msg="Failed to delete transport zone with id %s. Error[%s]." % (id, to_native(err)))
 
     time.sleep(5)
-    module.exit_json(changed=True, object_name=id, message="Transport zone with zone id %s deleted." % id)
+    module.exit_json(changed=True, diff=diff, object_name=id, message="Transport zone with zone id %s deleted." % id)
 
 
 if __name__ == '__main__':
-  main()
+  main()