dcnm_vrf: UT - fix null fabric name in vrf attachments (#365)

* Initial commit

1. module_utils/network/dcnm.py

Add two new methods which are duplicate functionality to get_ip_sn_fabric_dict()

get_ip_fabric_dict()
get_sn_fabric_dict()

get_ip_sn_fabric_dict() was not touched.

The reason for the new defs is because patch (in test_dcnm_vrf.py setUp() did not like patching to a def that returns more than one value.  But another reason is that it's cleaner for these utility defs to be as simple as possible.

2. test_dcnm_vrf.py

- Add the mock for sn_fab

- Modify load_fixtures() for all test cases that required having sn_fab dict mocked.

- Define two new vars for fabric details (for a future commit)
    -  fabric_details_mfd
    - fabric_details_vxlan

3. dcnm_vrf.json

Add the following objects (fabric_details_* for a future commit)

- mock_sn_fab
- fabric_details_mfd
- fabric_details_vxlan

* Appease linters

1. test_dcnm_vrf.py

Fix black whitespace on blank line.

2. dcnm_vrf.py

Fix pylint unused-import

3. module_utils/network/dcnm/dcnm.py

1. Run through black
2. Fix pep8 expected 2 blank lines, found 1

* dcnm_vrf: Update fixture data

Update fabric_details_*

1. tests/unit/modules/dcnm/fixtures/dcnm_vrf.json

- Remove dcnm_fabric_orig
- Update fabric_details_mfd with default MSD fabric
- Update fabric_details_vxlan with default VXLAN/EVPN fabric

* dcnm_vrf: Add mock_ip_fab to dcnm_vrf.json

Adding for future use.

* Improve docstrings

* Fix docstrings

module_utils/network/dcnm/dcnm.py

Clean up the two recently-added method's docstrings.  Fix typos and descriptions of return values.

* Sanity-check inventory_data

Maybe overly paranoid, but better to fail at earliest possible point, IMHO.

plugins/module_utils/network/dcnm/dcnm.py

@@ -15,6 +15,7 @@
 
 __metaclass__ = type
 
+import copy
 import socket
 import json
 import time
@@ -51,7 +52,7 @@
     "boolean": "bool",
     "enum": "str",
     "ipV4AddressWithSubnet": "ipv4_subnet",
-    "ipV6AddressWithSubnet": "ipv6_subnet"
+    "ipV6AddressWithSubnet": "ipv6_subnet",
 }
 
 
@@ -73,15 +74,11 @@ def validate_ip_address_format(type, item, invalid_params):
                 subnet = item.split("/")[1]
                 if not subnet or int(subnet) > mask_len:
                     invalid_params.append(
-                        "{0} : Invalid {1} gw/subnet syntax".format(
-                            item, addr_type
-                        )
+                        "{0} : Invalid {1} gw/subnet syntax".format(item, addr_type)
                     )
             else:
                 invalid_params.append(
-                    "{0} : Invalid {1} gw/subnet syntax".format(
-                        item, addr_type
-                    )
+                    "{0} : Invalid {1} gw/subnet syntax".format(item, addr_type)
                 )
         try:
             socket.inet_pton(addr_family, address)
@@ -176,9 +173,7 @@ def validate_list_of_dicts(param_list, spec, module=None):
                         module.no_log_values.add(item)
                     else:
                         msg = "\n\n'{0}' is a no_log parameter".format(param)
-                        msg += (
-                            "\nAnsible module object must be passed to this "
-                        )
+                        msg += "\nAnsible module object must be passed to this "
                         msg += "\nfunction to ensure it is not logged\n\n"
                         raise Exception(msg)
 
@@ -284,6 +279,68 @@ def get_ip_sn_fabric_dict(inventory_data):
     return ip_fab, sn_fab
 
 
+def get_ip_fabric_dict(inventory_data):
+    """
+    Maps the switch ip address to the switch's member fabric.
+
+    Parameters:
+        inventory_data: Fabric inventory data
+
+    Raises:
+
+        ValueError, if inventory_data does not contain ipAddress
+        or fabricName.
+
+    Returns:
+        dict: Switch ip address - fabric_name mapping
+    """
+    mapping_dict = {}
+    for device_key in inventory_data.keys():
+        ip_address = inventory_data[device_key].get("ipAddress")
+        fabric_name = inventory_data[device_key].get("fabricName")
+        if ip_address is None:
+            msg = "Cannot parse ipAddress from inventory_data:"
+            msg += f"{json.dumps(inventory_data, indent=4, sort_keys=True)}"
+            raise ValueError(msg)
+        if fabric_name is None:
+            msg = "Cannot parse fabricName from inventory_data:"
+            msg += f"{json.dumps(inventory_data, indent=4, sort_keys=True)}"
+            raise ValueError(msg)
+        mapping_dict.update({ip_address: fabric_name})
+    return copy.deepcopy(mapping_dict)
+
+
+def get_sn_fabric_dict(inventory_data):
+    """
+    Maps the switch serial number to the switch's member fabric.
+
+    Parameters:
+        inventory_data: Fabric inventory data
+
+    Raises:
+
+        ValueError, if inventory_data does not contain serialNumber
+        or fabricName.
+
+    Returns:
+        dict: Switch serial number - fabric_name mapping
+    """
+    mapping_dict = {}
+    for device_key in inventory_data.keys():
+        serial_number = inventory_data[device_key].get("serialNumber")
+        fabric_name = inventory_data[device_key].get("fabricName")
+        if serial_number is None:
+            msg = "Cannot parse serial_number from inventory_data:"
+            msg += f"{json.dumps(inventory_data, indent=4, sort_keys=True)}"
+            raise ValueError(msg)
+        if fabric_name is None:
+            msg = "Cannot parse fabric_name from inventory_data:"
+            msg += f"{json.dumps(inventory_data, indent=4, sort_keys=True)}"
+            raise ValueError(msg)
+        mapping_dict.update({serial_number: fabric_name})
+    return copy.deepcopy(mapping_dict)
+
+
 # sw_elem can be ip_addr, hostname, dns name or serial number. If the given
 # sw_elem is ip_addr, then it is returned as is. If DNS or hostname then a DNS
 # lookup is performed to get the IP address to be returned. If not ip_sn
@@ -293,7 +350,9 @@ def dcnm_get_ip_addr_info(module, sw_elem, ip_sn, hn_sn):
 
     msg_dict = {"Error": ""}
     msg = 'Given switch elem = "{}" is not a valid one for this fabric\n'
-    msg1 = 'Given switch elem = "{}" cannot be validated, provide a valid ip_sn object\n'
+    msg1 = (
+        'Given switch elem = "{}" cannot be validated, provide a valid ip_sn object\n'
+    )
 
     # Check if the given sw_elem is a v4 ip_addr
     try:
@@ -421,9 +480,7 @@ def dcnm_reset_connection(module):
 
     conn = Connection(module._socket_path)
 
-    return conn.login(
-        conn.get_option("remote_user"), conn.get_option("password")
-    )
+    return conn.login(conn.get_option("remote_user"), conn.get_option("password"))
 
 
 def dcnm_version_supported(module):
@@ -526,16 +583,14 @@ def dcnm_get_url(module, fabric, path, items, module_name):
         elif iter != (send_count - 1):
             itemstr = ",".join(
                 itemlist[
-                    (iter * (len(itemlist) // send_count)):(
+                    (iter * (len(itemlist) // send_count)) : (
                         (iter + 1) * (len(itemlist) // send_count)
                     )
                 ]
             )
             url = path.format(fabric, itemstr)
         else:
-            itemstr = ",".join(
-                itemlist[iter * (len(itemlist) // send_count):]
-            )
+            itemstr = ",".join(itemlist[iter * (len(itemlist) // send_count) :])
             url = path.format(fabric, itemstr)
 
         att_objects = dcnm_send(module, method, url)
@@ -582,12 +637,7 @@ def dcnm_get_template_details(module, version, name):
         module, "GET", dcnm_paths[version]["TEMPLATE_WITH_NAME"].format(name)
     )
 
-    if (
-        resp
-        and resp["RETURN_CODE"] == 200
-        and resp["MESSAGE"] == "OK"
-        and resp["DATA"]
-    ):
+    if resp and resp["RETURN_CODE"] == 200 and resp["MESSAGE"] == "OK" and resp["DATA"]:
         if resp["DATA"]["name"] == name:
             return resp["DATA"]
         else:
@@ -617,20 +667,12 @@ def dcnm_update_arg_specs(mspec, arg_specs):
                     # Given key is included in the mspec. So mark this a 'true' in the aspec. Final 'eval'
                     # on the item["required"] will yield the desired bool value.
                     item["required"] = item["required"].replace("true", "True")
-                    item["required"] = item["required"].replace(
-                        "false", "False"
-                    )
-                    item["required"] = eval(
-                        item["required"].replace(key, "True")
-                    )
+                    item["required"] = item["required"].replace("false", "False")
+                    item["required"] = eval(item["required"].replace(key, "True"))
                 else:
                     item["required"] = item["required"].replace("true", "True")
-                    item["required"] = item["required"].replace(
-                        "false", "False"
-                    )
-                    item["required"] = eval(
-                        item["required"].replace(key, "False")
-                    )
+                    item["required"] = item["required"].replace("false", "False")
+                    item["required"] = eval(item["required"].replace(key, "False"))
 
 
 def dcnm_get_template_specs(module, name, version):
@@ -656,12 +698,12 @@ def dcnm_get_template_specs(module, name, version):
             )
 
             if "IsShow" in p["annotations"]:
-                pb_template[name][p["name"]] += ", Mandatory: " + p[
-                    "annotations"
-                ]["IsShow"].replace('"', "")
-                pb_template[name + "_spec"][p["name"]]["required"] = p[
-                    "annotations"
-                ]["IsShow"].replace('"', "")
+                pb_template[name][p["name"]] += ", Mandatory: " + p["annotations"][
+                    "IsShow"
+                ].replace('"', "")
+                pb_template[name + "_spec"][p["name"]]["required"] = p["annotations"][
+                    "IsShow"
+                ].replace('"', "")
             else:
                 # If 'defaultValue' is included, then the object can be marked as optional.
                 if p["metaProperties"].get("defaultValue", None) is not None:
@@ -714,14 +756,14 @@ def dcnm_get_template_specs(module, name, version):
                 pb_template[name][p["name"]] += (
                     ", Type: " + dcnm_template_type_xlations[str(p["parameterType"])]
                 )
-                pb_template[name + "_spec"][p["name"]]["type"] = dcnm_template_type_xlations[
-                    p["parameterType"]
-                ]
+                pb_template[name + "_spec"][p["name"]]["type"] = (
+                    dcnm_template_type_xlations[p["parameterType"]]
+                )
                 if p.get("parameterType") == "string[]":
                     pb_template[name][p["name"]] += ", elements: " + "str"
-                    pb_template[name + "_spec"][p["name"]]["type"] = dcnm_template_type_xlations[
-                        p["parameterType"]
-                    ]
+                    pb_template[name + "_spec"][p["name"]]["type"] = (
+                        dcnm_template_type_xlations[p["parameterType"]]
+                    )
                     pb_template[name + "_spec"][p["name"]]["elements"] = "str"
 
             if p["metaProperties"].get("defaultValue", None) is not None:
@@ -734,9 +776,9 @@ def dcnm_get_template_specs(module, name, version):
                             "metaProperties"
                         ]["defaultValue"].replace('""', "")
                     else:
-                        pb_template[name + "_spec"][p["name"]][
-                            "default"
-                        ] = int(p["metaProperties"]["defaultValue"])
+                        pb_template[name + "_spec"][p["name"]]["default"] = int(
+                            p["metaProperties"]["defaultValue"]
+                        )
                 else:
                     pb_template[name + "_spec"][p["name"]]["default"] = p[
                         "metaProperties"
@@ -769,9 +811,7 @@ def dcnm_get_auth_token(module):
 
 def dcnm_post_request(path, hdrs, verify_flag, upload_files):
 
-    resp = requests.post(
-        path, headers=hdrs, verify=verify_flag, files=upload_files
-    )
+    resp = requests.post(path, headers=hdrs, verify=verify_flag, files=upload_files)
     json_resp = resp.json()
     if json_resp:
         json_resp["RETURN_CODE"] = resp.status_code

plugins/modules/dcnm_vrf.py

@@ -572,7 +572,7 @@
 from ansible_collections.cisco.dcnm.plugins.module_utils.network.dcnm.dcnm import (
     dcnm_get_ip_addr_info, dcnm_get_url, dcnm_send, dcnm_version_supported,
     get_fabric_details, get_fabric_inventory_details, get_ip_sn_dict,
-    get_ip_sn_fabric_dict, validate_list_of_dicts)
+    get_sn_fabric_dict, validate_list_of_dicts)
 
 from ..module_utils.common.log_v2 import Log
 
@@ -661,7 +661,13 @@ def __init__(self, module):
         self.sn_ip = {value: key for (key, value) in self.ip_sn.items()}
         self.fabric_data = get_fabric_details(self.module, self.fabric)
         self.fabric_type = self.fabric_data.get("fabricType")
-        self.ip_fab, self.sn_fab = get_ip_sn_fabric_dict(self.inventory_data)
+
+        try:
+            self.sn_fab = get_sn_fabric_dict(self.inventory_data)
+        except ValueError as error:
+            msg += f"{self.class_name}.__init__(): {error}"
+            module.fail_json(msg=msg)
+
         if self.dcnm_version > 12:
             self.paths = dcnm_vrf_paths[12]
         else:
@@ -3049,6 +3055,80 @@ def send_to_controller(self, action, verb, path, payload, is_rollback=False):
             self.log.debug(msg)
             self.failure(resp)
 
+    def update_vrf_attach_fabric_name(self, vrf_attach: dict) -> dict:
+        """
+        # Summary
+
+        For multisite fabrics, replace `vrf_attach.fabric` with the name of
+        the child fabric returned by `self.sn_fab[vrf_attach.serialNumber]`
+
+        ## params
+
+        - `vrf_attach`
+
+        A `vrf_attach` dictionary containing the following keys:
+
+            - `fabric` : fabric name
+            - `serialNumber` : switch serial number
+        """
+        method_name = inspect.stack()[0][3]
+        caller = inspect.stack()[1][3]
+
+        msg = "ENTERED. "
+        msg += f"caller: {caller}. "
+        self.log.debug(msg)
+
+        msg = "Received vrf_attach: "
+        msg += f"{json.dumps(vrf_attach, indent=4, sort_keys=True)}"
+        self.log.debug(msg)
+
+        if self.fabric_type != "MFD":
+            msg = "Early return. "
+            msg += f"FABRIC_TYPE {self.fabric_type} is not MFD. "
+            msg += "Returning unmodified vrf_attach."
+            self.log.debug(msg)
+            return copy.deepcopy(vrf_attach)
+
+        parent_fabric_name = vrf_attach.get("fabric")
+
+        msg = f"fabric_type: {self.fabric_type}, "
+        msg += "replacing parent_fabric_name "
+        msg += f"({parent_fabric_name}) "
+        msg += "with child fabric name."
+        self.log.debug(msg)
+
+        serial_number = vrf_attach.get("serialNumber")
+
+        if serial_number is None:
+            msg = f"{self.class_name}.{method_name}: "
+            msg += f"caller: {caller}. "
+            msg += "Unable to parse serial_number from vrf_attach. "
+            msg += f"{json.dumps(vrf_attach, indent=4, sort_keys=True)}"
+            self.log.debug(msg)
+            self.module.fail_json(msg)
+
+        child_fabric_name = self.sn_fab[serial_number]
+
+        if child_fabric_name is None:
+            msg = f"{self.class_name}.{method_name}: "
+            msg += f"caller: {caller}. "
+            msg += "Unable to determine child fabric name for serial_number "
+            msg += f"{serial_number}."
+            self.log.debug(msg)
+            self.module.fail_json(msg)
+
+        msg = f"serial_number: {serial_number}, "
+        msg += f"child fabric name: {child_fabric_name}. "
+        self.log.debug(msg)
+
+        vrf_attach["fabric"] = child_fabric_name
+
+        msg += "Updated vrf_attach: "
+        msg += f"{json.dumps(vrf_attach, indent=4, sort_keys=True)}"
+        self.log.debug(msg)
+
+        return copy.deepcopy(vrf_attach)
+
     def push_diff_attach(self, is_rollback=False):
         """
         # Summary
@@ -3086,6 +3166,8 @@ def push_diff_attach(self, is_rollback=False):
                 msg += f"{json.dumps(vrf_attach, indent=4, sort_keys=True)}"
                 self.log.debug(msg)
 
+                vrf_attach = self.update_vrf_attach_fabric_name(vrf_attach)
+
                 if "is_deploy" in vrf_attach:
                     del vrf_attach["is_deploy"]
                 if not vrf_attach.get("vrf_lite"):
@@ -3164,13 +3246,6 @@ def push_diff_attach(self, is_rollback=False):
         path = self.paths["GET_VRF"].format(self.fabric)
         attach_path = path + "/attachments"
 
-        # For multisite fabrics, update the fabric name to the child fabric
-        # containing the switches.
-        if self.fabric_type == "MFD":
-            for elem in new_diff_attach_list:
-                for node in elem["lanAttachList"]:
-                    node["fabric"] = self.sn_fab[node["serialNumber"]]
-
         self.send_to_controller(
             action, verb, attach_path, new_diff_attach_list, is_rollback
         )