-
Notifications
You must be signed in to change notification settings - Fork 4
Bug fix/sda fabric devices pcg #409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
a4fa392
ffe0f2b
19412ee
ae333bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -290,8 +290,9 @@ def validate_component_specific_filters(self, component_specific_filters): | |
| comp for comp in components_list if comp not in network_elements | ||
| ] | ||
| if invalid_components: | ||
| valid_components = list(network_elements.keys()) + ["component_list"] | ||
| self.msg = "Invalid network components provided for module '{0}': {1}. Valid components are: {2}".format( | ||
| self.module_name, invalid_components, list(network_elements.keys()) | ||
| self.module_name, invalid_components, valid_components | ||
| ) | ||
| self.fail_and_exit(self.msg) | ||
|
|
||
|
|
@@ -361,6 +362,15 @@ def validate_component_specific_filters(self, component_specific_filters): | |
| ) | ||
| continue | ||
|
|
||
| # Check for missing required filters in this entry | ||
| for req_filter_name, req_filter_spec in valid_filters_for_component.items(): | ||
| if req_filter_spec.get("required", False) and req_filter_name not in component_filter: | ||
| invalid_filters.append( | ||
| "Component '{0}' filter entry {1}/{2} is missing required filter '{3}'".format( | ||
| component_name, index, len(component_filters), req_filter_name | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it correct to say index, len(component_filters) ? if len(component_filters) is 5, and index starts with 0.. then |
||
| ) | ||
| ) | ||
|
|
||
| for filter_name, filter_value in component_filter.items(): | ||
| self.log( | ||
| "Processing filter '{0}' in entry {1}/{2} for component '{3}': value={4}".format( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,7 +87,9 @@ | |
| - Filters specific to fabric device configuration retrieval. | ||
| - Used to narrow down which fabric sites and devices should be included in the generated YAML file. | ||
| - If no filters are provided, all fabric devices from all fabric sites in Cisco Catalyst Center will be retrieved. | ||
| type: dict | ||
| - Each list entry targets a specific fabric site and optionally narrows down by device IP or roles. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add the AND and OR logic? you can read and update if required.. |
||
| type: list | ||
| elements: dict | ||
| suboptions: | ||
| fabric_name: | ||
| description: | ||
|
|
@@ -233,7 +235,7 @@ | |
| component_specific_filters: | ||
| components_list: ["fabric_devices"] | ||
| fabric_devices: | ||
| fabric_name: "Global/USA/SAN-JOSE" | ||
| - fabric_name: "Global/USA/SAN-JOSE" | ||
|
|
||
| # Example 4: Generate configuration for devices with specific roles in a fabric site | ||
| - name: Generate configuration for border and control plane devices | ||
|
|
@@ -263,8 +265,8 @@ | |
| component_specific_filters: | ||
| components_list: ["fabric_devices"] | ||
| fabric_devices: | ||
| fabric_name: "Global/USA/SAN-JOSE" | ||
| device_roles: ["BORDER_NODE", "CONTROL_PLANE_NODE"] | ||
| - fabric_name: "Global/USA/SAN-JOSE" | ||
| device_roles: ["BORDER_NODE", "CONTROL_PLANE_NODE"] | ||
|
|
||
| # Example 5: Generate configuration for a specific device in a fabric site | ||
| - name: Generate configuration for a specific fabric device | ||
|
|
@@ -294,8 +296,8 @@ | |
| component_specific_filters: | ||
| components_list: ["fabric_devices"] | ||
| fabric_devices: | ||
| fabric_name: "Global/USA/SAN-JOSE" | ||
| device_ip: "10.0.0.1" | ||
| - fabric_name: "Global/USA/SAN-JOSE" | ||
| device_ip: "10.0.0.1" | ||
|
|
||
| # Example 6: Auto-populate components_list from component filters | ||
| - name: Generate configuration with auto-populated components_list | ||
|
|
@@ -326,7 +328,7 @@ | |
| # No components_list specified, but fabric_devices filters are provided | ||
| # The 'fabric_devices' component will be automatically added to components_list | ||
| fabric_devices: | ||
| fabric_name: "Global/USA/SAN-JOSE" | ||
| - fabric_name: "Global/USA/SAN-JOSE" | ||
|
|
||
| # Example 7: Generate configuration with append mode | ||
| - name: Generate and append SDA fabric device configuration | ||
|
|
@@ -356,8 +358,8 @@ | |
| component_specific_filters: | ||
| components_list: ["fabric_devices"] | ||
| fabric_devices: | ||
| fabric_name: "Global/India/Bangalore" | ||
| device_roles: ["BORDER_NODE"] | ||
| - fabric_name: "Global/India/Bangalore" | ||
| device_roles: ["BORDER_NODE"] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see an example for multiple fabric sites.. Can we add it? |
||
| """ | ||
|
|
||
| RETURN = r""" | ||
|
|
@@ -1120,15 +1122,25 @@ def retrieve_all_fabric_devices_from_api( | |
|
|
||
| def get_fabric_devices_configuration(self, network_element, filters=None): | ||
| """ | ||
| Retrieve and transform fabric devices configuration. | ||
| Retrieve and transform fabric devices configuration into playbook-ready format. | ||
|
|
||
| Parameters: | ||
| network_element (dict): Network element schema with API and transform details. | ||
| filters (dict, optional): Dictionary containing 'component_specific_filters'. | ||
| - component_specific_filters (list/dict): Filters for fabric_name, device_ip, device_roles. | ||
| network_element (dict): Network element schema containing: | ||
| - api_family (str): API family to use (e.g. 'sda'). | ||
| - api_function (str): API function name (e.g. 'get_fabric_devices'). | ||
| - reverse_mapping_function (callable): Returns the temp_spec OrderedDict for transformation. | ||
| filters (dict, optional): Dictionary containing: | ||
| - component_specific_filters (list of dict): Each entry may include: | ||
| - fabric_name (str): Name of the fabric site to filter by. | ||
| - device_ip (str): IP address of a specific device to filter by. | ||
| - device_roles (list of str): Roles to filter by (e.g. 'BORDER_NODE'). | ||
| If omitted or None, all fabric sites and their devices are retrieved. | ||
|
|
||
| Returns: | ||
| dict: Dictionary with 'fabric_devices' key containing transformed device configs. | ||
| dict: Dictionary with key 'fabric_devices' mapping to a list of transformed fabric | ||
| site entries, each containing fabric_name and device_config list. | ||
| None: If no valid query parameters could be built from the provided filters, or if | ||
| no fabric devices are found matching the filters. | ||
|
|
||
| Description: | ||
| Main function to fetch fabric devices and transform them to playbook format. | ||
|
|
@@ -1154,74 +1166,79 @@ def get_fabric_devices_configuration(self, network_element, filters=None): | |
|
|
||
| if component_specific_filters: | ||
| self.log( | ||
| "Processing component-specific filters", | ||
| f"Processing {len(component_specific_filters)} component-specific filter(s)", | ||
| "DEBUG", | ||
| ) | ||
| params_for_query = {} | ||
| for filter_idx, filter_entry in enumerate(component_specific_filters, 1): | ||
| self.log( | ||
| f"Processing filter entry {filter_idx}/{len(component_specific_filters)}: {self.pprint(filter_entry)}", | ||
| "DEBUG", | ||
| ) | ||
| params_for_query = {} | ||
|
|
||
| fabric_name = filter_entry.get("fabric_name") | ||
| if fabric_name: | ||
| self.log(f"Applying fabric_name filter: '{fabric_name}'", "DEBUG") | ||
| fabric_site_id = self.fabric_site_name_to_id_dict.get(fabric_name) | ||
|
|
||
| fabric_name = component_specific_filters.get("fabric_name") | ||
| if fabric_name: | ||
| self.log(f"Applying fabric_name filter: '{fabric_name}'", "DEBUG") | ||
| fabric_site_id = self.fabric_site_name_to_id_dict.get(fabric_name) | ||
| if not fabric_site_id: | ||
| self.log( | ||
| f"Fabric site '{fabric_name}' not found in Cisco Catalyst Center. Skipping filter entry {filter_idx}.", | ||
| "WARNING", | ||
| ) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When a filter entry is skipped (fabric not found, device not found), the code logs a WARNING and continues. But the user never sees these warnings in the module result. Can we collect the skipped entries and including them in the final result['response'] message or result['warnings'] so the user knows which entries were silently dropped... Is it a good idea or overhead? Can you check with @DNACENSolutions / team and update? I see that in many places we log with "WARNING" in log messages but not captured in final result.. |
||
| continue | ||
|
|
||
| if not fabric_site_id: | ||
| self.log( | ||
| f"Fabric site '{fabric_name}' not found in Cisco Catalyst Center.", | ||
| "WARNING", | ||
| f"Fabric site '{fabric_name}' found with fabric_id '{fabric_site_id}'", | ||
| "DEBUG", | ||
| ) | ||
| return {"fabric_devices": []} | ||
| params_for_query["fabric_id"] = fabric_site_id | ||
|
|
||
| self.log( | ||
| f"Fabric site '{fabric_name}' found with fabric_id '{fabric_site_id}'", | ||
| "DEBUG", | ||
| ) | ||
| params_for_query["fabric_id"] = fabric_site_id | ||
| device_ip = filter_entry.get("device_ip") | ||
| if device_ip: | ||
| self.log( | ||
| f"Applying device_ip filter: '{device_ip}'", | ||
| "DEBUG", | ||
| ) | ||
| device_list_params = self.get_device_list_params( | ||
| ip_address_list=device_ip | ||
| ) | ||
| device_info_map = self.get_device_list(device_list_params) | ||
| if not device_info_map or device_ip not in device_info_map: | ||
| self.log( | ||
| f"Device with IP '{device_ip}' not found in Cisco Catalyst Center. Skipping filter entry {filter_idx}.", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above.. |
||
| "WARNING", | ||
| ) | ||
| continue | ||
|
|
||
| network_device_id = device_info_map[device_ip].get("device_id") | ||
| self.log( | ||
| f"Device with IP '{device_ip}' found with network_device_id '{network_device_id}'", | ||
| "DEBUG", | ||
| ) | ||
| self.log(f"Adding device_id filter: {network_device_id}", "DEBUG") | ||
| params_for_query["networkDeviceId"] = network_device_id | ||
|
|
||
| device_ip = component_specific_filters.get("device_ip") | ||
| if device_ip: | ||
| self.log( | ||
| f"Applying device_ip filter: '{device_ip}'", | ||
| "DEBUG", | ||
| ) | ||
| device_list_params = self.get_device_list_params( | ||
| ip_address_list=device_ip | ||
| ) | ||
| device_info_map = self.get_device_list(device_list_params) | ||
| if not device_info_map or device_ip not in device_info_map: | ||
| device_roles = filter_entry.get("device_roles") | ||
| if device_roles: | ||
| self.log( | ||
| f"Device with IP '{device_ip}' not found in Cisco Catalyst Center.", | ||
| "WARNING", | ||
| f"Applying device_roles filter: {device_roles}", | ||
| "DEBUG", | ||
| ) | ||
| return {"fabric_devices": []} | ||
| params_for_query["deviceRoles"] = device_roles | ||
|
|
||
| network_device_id = device_info_map[device_ip].get("device_id") | ||
| self.log( | ||
| f"Device with IP '{device_ip}' found with network_device_id '{network_device_id}'", | ||
| "DEBUG", | ||
| ) | ||
| self.log(f"Adding device_id filter: {network_device_id}", "DEBUG") | ||
| params_for_query["networkDeviceId"] = network_device_id | ||
| if not params_for_query: | ||
| self.log( | ||
| f"No valid filters provided for filter entry {filter_idx}, skipping.", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above |
||
| "WARNING", | ||
| ) | ||
| continue | ||
|
|
||
| device_roles = component_specific_filters.get("device_roles") | ||
| if device_roles: | ||
| self.log( | ||
| f"Applying device_roles filter: {device_roles}", | ||
| f"Adding query parameters to list: {params_for_query}", | ||
| "DEBUG", | ||
| ) | ||
| params_for_query["deviceRoles"] = device_roles | ||
|
|
||
| if not params_for_query: | ||
| self.log( | ||
| "No valid filters provided after processing component-specific filters.", | ||
| "WARNING", | ||
| ) | ||
| return {"fabric_devices": []} | ||
|
|
||
| self.log( | ||
| f"Adding query parameters to list: {params_for_query}", | ||
| "DEBUG", | ||
| ) | ||
| fabric_devices_params_list_to_query.append(params_for_query) | ||
| fabric_devices_params_list_to_query.append(params_for_query) | ||
| else: | ||
| self.log( | ||
| "No component-specific filters provided. Retrieving all fabric devices from all fabric sites.", | ||
|
|
@@ -1234,6 +1251,12 @@ def get_fabric_devices_configuration(self, network_element, filters=None): | |
| ) | ||
| fabric_devices_params_list_to_query.append({"fabric_id": fabric_id}) | ||
|
|
||
| if not fabric_devices_params_list_to_query: | ||
| self.log( | ||
| "No fabric devices parameters to query, Returning None" | ||
| ) | ||
| return None | ||
|
|
||
| self.log( | ||
| f"Total fabric device queries to execute: {len(fabric_devices_params_list_to_query)}", | ||
| "INFO", | ||
|
|
@@ -1258,10 +1281,10 @@ def get_fabric_devices_configuration(self, network_element, filters=None): | |
|
|
||
| if not all_fabric_devices: | ||
| self.log( | ||
| "No fabric devices found matching the provided filters", | ||
| "No fabric devices found matching the provided filters, Returning None", | ||
| "WARNING", | ||
| ) | ||
| return {"fabric_devices": []} | ||
| return None | ||
|
|
||
| self.log( | ||
| f"Successfully retrieved {len(all_fabric_devices)} fabric device(s) for the provided filters", | ||
|
|
@@ -1350,6 +1373,11 @@ def get_fabric_devices_configuration(self, network_element, filters=None): | |
| transformed_fabric_devices_list = self.modify_parameters( | ||
| temp_spec, fabric_entries_for_transformation | ||
| ) | ||
| if not transformed_fabric_devices_list: | ||
| self.log( | ||
| "No fabric devices were transformed successfully, returning None", | ||
| ) | ||
| return None | ||
|
|
||
| self.log( | ||
| f"Transformation complete. Generated {len(transformed_fabric_devices_list)} fabric site(s) with devices", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it "component_list" or "components_list" ..
In line number 290, we are using components_list.. Can you check and update?