Addressing review comments

Author: prabahal

plugins/module_utils/msd/query_child_fab.py

@@ -20,7 +20,7 @@
 import logging
 
 from ..common.results import Results
-from ..msd.Fabric_associations import FabricAssociations
+from ..msd.fabric_associations import FabricAssociations
 
 
 class childFabricQuery():

plugins/modules/dcnm_child_fabric.py renamed to plugins/modules/dcnm_fabric_member.py

@@ -20,7 +20,7 @@
 
 DOCUMENTATION = """
 ---
-module: dcnm_child_fabric
+module: dcnm_fabric_member
 short_description: Manage addition and deletion of NDFC fabrics to MSD.
 version_added: "3.5.0"
 author: Prabahal (@prabahal)
@@ -63,7 +63,7 @@
 EXAMPLES = """
 
 - name: add child fabrics to MSD
- cisco.dcnm.dcnm_child_fabric:
+ cisco.dcnm.dcnm_fabric_member:
  state: merged
  config:
  - FABRIC_NAME: MSD_Parent1
@@ -79,7 +79,7 @@
 # Query the child fabrics of a MSD Fabric.
 
 - name: Query the child fabrics of MSD fabrics.
- cisco.dcnm.dcnm_child_fabric:
+ cisco.dcnm.dcnm_fabric_member:
  state: query
  config:
  - FABRIC_NAME: MSD_Fabric1
@@ -123,9 +123,9 @@
 from ..module_utils.msd.query_child_fab import childFabricQuery
 from ..module_utils.msd.delete_child_fab import childFabricDelete
 from ..module_utils.msd.add_child_fab import childFabricAdd
-from ..module_utils.msd.Fabric_associations import FabricAssociations
+from ..module_utils.msd.fabric_associations import FabricAssociations
 from ..module_utils.fabric.verify_playbook_params import VerifyPlaybookParams
-
+from ..module_utils.network.dcnm.dcnm import validate_list_of_dicts
 
 @Properties.add_rest_send
 class childCommon():
@@ -151,7 +151,6 @@ def __init__(self, params):
  self.results = Results()
  self.results.state = self.state
  self.results.check_mode = self.check_mode
- self._verify_playbook_params = VerifyPlaybookParams()
  self.conversion = ConversionUtils()
  self.payloads = []
  self.want = []
@@ -218,36 +217,54 @@ def verify_child_fabric_is_already_member(self, item) -> bool:
  if (self.data[fabric]['fabricParent'] == item["destFabric"]):
  return True
  return False
-
- def get_want(self):
- method_name = inspect.stack()[0][3]
+ 
+ def validate_input(self):
+ if self.state != "query":
+ fab_member_spec = dict(
+ FABRIC_NAME=dict(required=True, type="str"),
+ CHILD_FABRIC_NAME=dict(required=True, type="str"),
+ DEPLOY=dict(type="bool", default=False),
+ )
+ else:
+ fab_member_spec = dict(
+ FABRIC_NAME=dict(type="str"),
+ ) 
+ fab_mem_info, invalid_params = validate_list_of_dicts(self.config, fab_member_spec, None)
+ if invalid_params:
+ mesg = "Invalid parameters in playbook: {0}".format(
+ "while processing config "
+ + "\n".join(invalid_params)
+ )
+ raise ValueError(mesg)
  for config in self.config:
- msg = f"payload: {config}"
- self.log.debug(msg)
-
  if not isinstance(config, dict):
  msg = f"{self.class_name}.{method_name}: "
- msg += "Playbook configuration for fabrics must be a dict. "
+ msg += "Playbook configuration for fabric_member must be a dict. "
  msg += f"Got type {type(config).__name__}, "
  msg += f"value {config}."
  raise ValueError(msg)
+ msd_fabric = config.get("FABRIC_NAME", None)
+ child_fabric = config.get("CHILD_FABRIC_NAME", None)
  try:
- msd_fabric = config.get("FABRIC_NAME", None)
- child_fabric = config.get("CHILD_FABRIC_NAME", None)
- deploy = config.get("DEPLOY", None)
- try:
- self.conversion.validate_fabric_name(msd_fabric)
- self.conversion.validate_fabric_name(child_fabric)
- except (TypeError, ValueError) as error:
- msg = f"{self.class_name}: "
- msg += "Playbook configuration for FABRIC_NAME or CHILD_FABRIC_NAME "
- msg += "contains an invalid FABRIC_NAME. "
- # error below already contains a period "." at the end
- msg += f"Error detail: {error} "
- msg += f"Bad configuration: {config}."
- raise ValueError(msg) from error
- except ValueError as error:
- raise ValueError(f"{error}") from error
+ self.conversion.validate_fabric_name(msd_fabric)
+ self.conversion.validate_fabric_name(child_fabric)
+ except (TypeError, ValueError) as error:
+ msg = f"{self.class_name}: "
+ msg += "Playbook configuration for FABRIC_NAME or CHILD_FABRIC_NAME "
+ msg += "contains an invalid FABRIC_NAME. "
+ # error below already contains a period "." at the end
+ msg += f"Error detail: {error} "
+ msg += f"Bad configuration: {config}."
+ raise ValueError(msg) from error
+
+ def get_want(self):
+ method_name = inspect.stack()[0][3]
+ for config in self.config:
+ msg = f"{method_name} payload: {config}"
+ self.log.debug(msg)
+ msd_fabric = config.get("FABRIC_NAME", None)
+ child_fabric = config.get("CHILD_FABRIC_NAME", None)
+ deploy = config.get("DEPLOY", None)
  config_payload = {'destFabric': msd_fabric, 'sourceFabric': child_fabric, 'DEPLOY': deploy}
  self.payloads.append(copy.deepcopy(config_payload))
 
@@ -315,24 +332,7 @@ def populate_state(self):
  msg += f"Expected one of: {','.join(valid_states)}."
  raise ValueError(msg)
 
- def get_want_query(self) -> None:
- """
- ### Summary
- - Validate the playbook configs.
- - Update self.want with the playbook configs.
-
- ### Raises
- - ``ValueError`` if the playbook configs are invalid.
- """
- merged_configs = []
- for config in self.config:
- merged_configs.append(copy.deepcopy(config))
-
- self.want = []
- for config in merged_configs:
- self.want.append(copy.deepcopy(config))
-
-# Keeping this function to check lower NDFC version support. yet to get data
+# Keeping this function to check lower NDFC version support. yet to get data from Mike
  def get_controller_version(self):
  """
  ### Summary
@@ -389,7 +389,8 @@ def commit(self) -> None:
 
  msg = f"ENTERED: {self.class_name}.{method_name}"
  self.log.debug(msg)
-
+ 
+ self.validate_input()
  self.get_want()
 
  self.fab_association = FabricAssociations()
@@ -480,7 +481,9 @@ def commit(self) -> None:
 
  msg = f"ENTERED: {self.class_name}.{method_name}"
  self.log.debug(msg)
-
+ self.get_controller_version()
+ ### Version validation needs to be added
+ self.validate_input()
  self.get_want()
 
  self.add.results = self.results
@@ -582,14 +585,14 @@ def commit(self) -> None:
  query the fabrics.
  """
  self.verify_payload()
- self.get_want_query()
+ self.get_want()
  fabric_query = childFabricQuery()
  fabric_query.rest_send = self.rest_send
  fabric_query.results = self.results
 
  fabric_names_to_query = []
- for want in self.want:
- fabric_names_to_query.append(want["FABRIC_NAME"])
+ for item in self.payloads:
+ fabric_names_to_query.append(item["destFabric"])
  try:
  fabric_query.fabric_names = copy.copy(fabric_names_to_query)
  except ValueError as error:

tests/integration/targets/dcnm_child_fabric/defaults/main.yaml renamed to tests/integration/targets/dcnm_fabric_member/defaults/main.yaml

@@ -69,7 +69,7 @@
 # Example vars:
 #
 # vars:
-# testcase: dcnm_child_fabric_merged
+# testcase: dcnm_fabric_member_merged
 # msd_fabric_name_1: MSD_1
 # MSD_fabric_type: VXLAN_EVPN_MSD
 # child_fabric_name_11: child_11
@@ -562,7 +562,7 @@
 
 ################################################################################
 - name: MERGED - TEST - Merge child fabrics into MSD Fabric
- cisco.dcnm.dcnm_child_fabric:
+ cisco.dcnm.dcnm_fabric_member:
  state: merged
  config:
  - FABRIC_NAME: "{{ msd_fabric_name_1 }}"
@@ -685,7 +685,7 @@
 #}
 #######################################################################################
 - name: MERGED - TEST - Merge additional fabric child_fabric_name_13 into MSD Fabric
- cisco.dcnm.dcnm_child_fabric: &merge_child_fabric
+ cisco.dcnm.dcnm_fabric_member: &merge_child_fabric
  state: merged
  config:
  - FABRIC_NAME: "{{ msd_fabric_name_1 }}"
@@ -728,7 +728,7 @@
 #-name QUERY - TEST - Query child fabrics of MSD Fabric
 ###################################################################################################
 - name: QUERY - TEST - Query child fabrics of MSD Fabric
- cisco.dcnm.dcnm_child_fabric: 
+ cisco.dcnm.dcnm_fabric_member: 
  state: query
  config:
  - FABRIC_NAME: "{{ msd_fabric_name_1 }}"
@@ -831,7 +831,7 @@
 #}
 #################################################################################
 - name: MERGED - TEST - Merge additional config into MSD fabric - idempotence
- cisco.dcnm.dcnm_child_fabric: *merge_child_fabric
+ cisco.dcnm.dcnm_fabric_member: *merge_child_fabric
  register: result
 - debug:
  var: result
@@ -945,7 +945,7 @@
 #}
 ################################################################################
 - name: DELETED - SETUP - Delete child fabrics from MSD fabric
- cisco.dcnm.dcnm_child_fabric: &delete_child_fabric
+ cisco.dcnm.dcnm_fabric_member: &delete_child_fabric
  state: deleted
  config:
  - FABRIC_NAME: "{{ msd_fabric_name_1 }}"
@@ -1132,7 +1132,7 @@
 #}
 ###############################################################################################
 - name: MERGED - TEST - Delete child Fabrics into MSD fabric - idempotence
- cisco.dcnm.dcnm_child_fabric: *delete_child_fabric
+ cisco.dcnm.dcnm_fabric_member: *delete_child_fabric
  register: result
 - debug:
  var: result
@@ -1214,7 +1214,7 @@
 #- name: MERGED - TEST - ADD child Fabrics into MSD fabric with Deploy
 ################################################################################################
 - name: Merged - TEST - Merge child fabrics into MSD Fabric with Deploy flag - To save the config
- cisco.dcnm.dcnm_child_fabric:
+ cisco.dcnm.dcnm_fabric_member:
  state: merged
  config:
  - FABRIC_NAME: "{{ msd_fabric_name_1 }}"
@@ -1361,7 +1361,7 @@
 #- name: DELETED - TEST - Delete child Fabrics from MSD fabric with Deploy
 ################################################################################################
 - name: Deleted - TEST - Delete child fabrics from MSD Fabric with Deploy flag - To save the config
- cisco.dcnm.dcnm_child_fabric:
+ cisco.dcnm.dcnm_fabric_member:
  state: deleted
  config:
  - FABRIC_NAME: "{{ msd_fabric_name_1 }}"

tests/integration/targets/dcnm_child_fabric/meta/main.yaml renamed to tests/integration/targets/dcnm_fabric_member/meta/main.yaml

@@ -24,4 +24,4 @@ plugins/httpapi/dcnm.py import-3.10!skip
 plugins/module_utils/common/sender_requests.py import-3.9 # TODO remove this if/when requests is added to the standard library
 plugins/module_utils/common/sender_requests.py import-3.10 # TODO remove this if/when requests is added to the standard library
 plugins/module_utils/common/sender_requests.py import-3.11 # TODO remove this if/when requests is added to the standard library
-plugins/modules/dcnm_child_fabric.py validate-modules:missing-gplv3-license # GPLv3 license header not found in the first 20 lines of the module
+plugins/modules/dcnm_fabric_member.py validate-modules:missing-gplv3-license # GPLv3 license header not found in the first 20 lines of the module

tests/integration/targets/dcnm_child_fabric/tasks/dcnm.yaml renamed to tests/integration/targets/dcnm_fabric_member/tasks/dcnm.yaml

@@ -21,4 +21,4 @@ plugins/modules/dcnm_bootflash.py validate-modules:missing-gplv3-license # GPLv3
 plugins/modules/dcnm_log.py validate-modules:missing-gplv3-license # GPLv3 license header not found in the first 20 lines of the module
 plugins/module_utils/common/sender_requests.py import-3.10 # TODO remove this if/when requests is added to the standard library
 plugins/module_utils/common/sender_requests.py import-3.11 # TODO remove this if/when requests is added to the standard library
-plugins/modules/dcnm_child_fabric.py validate-modules:missing-gplv3-license # GPLv3 license header not found in the first 20 lines of the module
+plugins/modules/dcnm_fabric_member.py validate-modules:missing-gplv3-license # GPLv3 license header not found in the first 20 lines of the module