Removing dnac_log_level from code along with fixing is… #127
Conversation
…sues related to discovery
…sues related to discovery
abimishr
changed the title
plugins/modules/discovery_intent.py
Outdated
| timeout: | ||
| description: Time to wait for device response in seconds | ||
| type: int | ||
| cli_cred_len: | ||
| description: Total CLI credentials that needs to be used. This value can vary from 1 to 5. |
There was a problem hiding this comment.
description: Specifies the total number of CLI credentials to be used, ranging from 1 to 5.
plugins/modules/discovery_intent.py
Outdated
| return devices_list | ||
| ip_address_list = self.validated_config[0].get('ip_address_list') | ||
| self.result.update(dict(devices_info=ip_address_list)) | ||
| self.log("Devices list info passed: {0}".format(str(ip_address_list)), "INFO") |
There was a problem hiding this comment.
can we. say
self.log("Details of the device list passed: {0}".format(str(ip_address_list)), "INFO") (OR)
self.log("Information about the list of devices passed: {0}".format(str(ip_address_list)), "INFO")
plugins/modules/discovery_intent.py
Outdated
| """ | ||
| Preprocess the devices' information. Extract the IP addresses from | ||
| the list of devices and perform additional processing based on the | ||
| 'discovery_type' in the validated configuration. | ||
|
|
||
| Parameters: | ||
| - devices_list: The list of devices to preprocess. If not | ||
| - ip_address_list: The list of devices to preprocess. If not |
There was a problem hiding this comment.
do we meant to say
" the list of devices' IP addresses intended for preprocessing" ?
ip_address_list: The list of devices' IP addresses intended for preprocessing. If not provided, an empty list will be used.
There was a problem hiding this comment.
yeah
It is resolved now
plugins/modules/discovery_intent.py
Outdated
|
|
||
| self.log("Discovery type passed for the discovery is {0}".format(self.validated_config[0].get('discovery_type')), "INFO") | ||
| if self.validated_config[0].get('discovery_type') in ["SINGLE", "CDP", "LLDP"]: | ||
| if len(ip_address_list) == 1: | ||
| ip_address_list = ip_address_list[0] | ||
| else: | ||
| self.log("Device list's length is longer than 1", "ERROR") | ||
| self.module.fail_json(msg="Device list's length is longer than 1", response=[]) | ||
| self.log("IP Address list's length is longer than 1", "ERROR") |
There was a problem hiding this comment.
self.log("The length of the IP address list is greater than 1", "ERROR")
greater or longer than 1
plugins/modules/discovery_intent.py
Outdated
| elif self.validated_config[0].get('discovery_type') == "CIDR": | ||
| if len(ip_address_list) == 1 and self.validated_config[0].get('prefix_length'): | ||
| ip_address_list = ip_address_list[0] | ||
| ip_address_list = str(ip_address_list) + "/" + str(self.validated_config[0].get('prefix_length')) | ||
| else: | ||
| self.log("Device list's length is longer than 1", "ERROR") | ||
| self.module.fail_json(msg="Device list's length is longer than 1", response=[]) | ||
| self.log("IP Address list's length is longer than 1", "ERROR") |
There was a problem hiding this comment.
self.log("The length of the IP address list is greater than 1", "ERROR")
plugins/modules/discovery_intent.py
Outdated
|
|
||
| ip_address_list = [device['ip'] for device in devices_list] | ||
| if ip_address_list is None: | ||
| ip_address_list = [] | ||
|
|
There was a problem hiding this comment.
we can get and store discovery_type as we use many places.. self.validated_config[0].get('discovery_type')
discovery_type = self.validated_config[0].get('discovery_type')
self.log(f"Discovery type: {discovery_type}", "INFO")
plugins/modules/discovery_intent.py
Outdated
|
|
||
| def preprocessing_devices_info(self, devices_list=None): | ||
| def preprocessing_devices_info(self, ip_address_list=None): |
There was a problem hiding this comment.
Can we name the function like preprocess_device_discovery?
| ip_address_list = ip_address_list[0] | ||
| else: | ||
| ip_address_list = "{0}-{1}".format(ip_address_list[0], ip_address_list[0]) | ||
| else: |
There was a problem hiding this comment.
All these error messages are repeated for more than 3 times. Can we write an API and simplify it?
def preprocess_device_discovery_handle_error(self):
self.log("IP Address list's length is longer than 1", "ERROR")
self.module.fail_json(msg="IP Address list's length is longer than 1", response=[])
| ip_address_list = "{0}-{1}".format(ip_address_list[0], ip_address_list[0]) | ||
| else: | ||
| self.log("IP Address list's length is longer than 1", "ERROR") | ||
| self.module.fail_json(msg="IP Address list's length is longer than 1", response=[]) | ||
| else: |
There was a problem hiding this comment.
As many lines are repeated in this API:
if discovery_type in ["SINGLE", "CDP", "LLDP"]:
if len(ip_address_list) == 1:
processed_ip_addresses = ip_address_list[0]
else:
self.handle_discovery_error()
elif discovery_type == "CIDR":
processed_ip_addresses = self.process_discovery_cidr(ip_address_list)
elif discovery_type == "RANGE":
processed_ip_addresses = self.process_discovery_range(ip_address_list)
else:
processed_ip_addresses = self.process_discovery_other(ip_address_list)
self.log("Collected IP address/addresses are {0}".format(str(processed_ip_addresses)), "INFO")
And followed by the functions?
def handle_discovery_error(self):
self.log("The length of the IP address list is longer than 1", "ERROR")
self.module.fail_json(msg="he length of the IP address list is longer than 1", response=[])
def process_cidr(self, ip_address_list):
if len(ip_address_list) != 1:
self.handle_discovery_error()
return
prefix_length = self.validated_config[0].get('prefix_length')
#check prefix length if required...
return "{0}/{1}".format(ip_address_list[0], prefix_length))
Likewise you can write other APIs...
| elif self.validated_config[0].get('discovery_type') == "RANGE": | ||
| if len(ip_address_list) == 1: | ||
| if len(str(ip_address_list[0]).split("-")) == 2: | ||
| ip_address_list = ip_address_list[0] |
There was a problem hiding this comment.
are we getting a list from a list?
or
ip_address = ip_address_list[0]
We can also simply write like:
ip = ip_list[0]
There was a problem hiding this comment.
We are actually fetching the data for performing the API call
Problem description: Discovery can't take more than 5 CLI credentials. Along with that, we haven't covered range based discovery.
Fix: Now the customer has the option to add the number of CLI credentials to be added. We have changed the IP address list input method.
Test cases: Tested for CDP, LLDP, MULTI RANGE and SINGLE RANGE
Limitation: Deletion is not tested