Add diff mode change reporting to 3 modules

Add diff mode change reporting to:

- nsxt_policy_segment
- nsxt_transport_zones
- nsxt_uplink_profiles

This will allow them to report diffs when using with `--diff`.

This commit factors out the `check_for_update` function from
`nsxt_base_resource.py` into `common_utils` and adds functionality so it
can be used by multiple modules.

Related Issue: #387

Signed-off-by: Jake <[email protected]>

plugins/module_utils/common_utils.py

@@ -164,3 +164,91 @@ 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 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 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 compares items ignoring order.
+
+    Returns True if the params differ
+    '''
+    if not existing_params and not resource_params:
+        return False
+    if not existing_params:
+        return False
+    if not resource_params:
+        return True
+
+    for k, v in resource_params.items():
+        if k not in existing_params:
+            return True
+        elif type(v) == dict:
+            if check_for_update(existing_params[k], v):
+                return True
+        elif type(v) == list:
+            try:
+                set1 = set(v)
+                set2 = set(existing_params[k])
+                if set1 != set2:
+                    return True
+            except Exception:
+                return True
+        elif v != existing_params[k]:
+            return True
+    return False
+
+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": False,
                     "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

@@ -133,6 +133,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.module_utils._text import to_native
 
@@ -160,29 +161,6 @@ def get_tz_from_display_name(module, manager_url, mgr_username, mgr_password, va
             return transport_zone
     return None
 
-def check_for_update(module, manager_url, mgr_username, mgr_password, validate_certs, transport_zone_params):
-    existing_transport_zone = get_tz_from_display_name(module, manager_url, mgr_username, mgr_password, validate_certs, transport_zone_params['display_name'])
-    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()
@@ -212,34 +190,45 @@ def main():
 
   zone_dict = get_tz_from_display_name (module, manager_url, mgr_username, mgr_password, validate_certs, display_name)
   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)
 
     if not updated:
       # add the node
       if module.check_mode:
-          module.exit_json(changed=False, 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-zones', data=request_data, headers=headers, method='POST',
                                 url_username=mgr_username, url_password=mgr_password, validate_certs=validate_certs, ignore_errors=True)
       except Exception as err:
           module.fail_json(msg="Failed to add transport zone. Request body [%s]. Error[%s]." % (request_data, to_native(err)))
       #dict_resp = json.loads(resp)
       time.sleep(5)
-      module.exit_json(changed=True, id=resp["id"], body= str(resp), message="Transport zone with display name %s created. " % (module.params['display_name']))
+      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)
@@ -251,23 +240,23 @@ def main():
           module.fail_json(msg="Failed to update transport zone with id %s. Request body [%s]. Error[%s]." % (id, request_data, to_native(err)))
 
       time.sleep(5)
-      module.exit_json(changed=True, id=resp["id"], body= str(resp), message="Transport zone with zone id %s updated." % id)
+      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-zones/%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__':