Skip to content
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

Issues #251 and #264 in ansible solutions. #361

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions plugins/module_utils/common/conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import inspect
import re
import ipaddress


class ConversionUtils:
Expand Down Expand Up @@ -171,3 +172,13 @@ def validate_fabric_name(self, value):
msg += "Fabric name must start with a letter A-Z or a-z and "
msg += "contain only the characters in: [A-Z,a-z,0-9,-,_]."
raise ValueError(msg)

@staticmethod
def make_ipv4_or_ipv6_address(ip_addr):
try:
if (isinstance(ipaddress.ip_address(ip_addr), ipaddress.IPv6Address)
or isinstance(ipaddress.ip_address(ip_addr), ipaddress.IPv4Address)):
sw_ip = str(ipaddress.ip_address(ip_addr))
except ValueError:
sw_ip = ip_addr
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Help me understand the intent here. Why are we just setting sw_ip to the value that was passed in if we encounter an error?

return sw_ip
24 changes: 16 additions & 8 deletions plugins/modules/dcnm_inventory.py
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@
import re
from ansible.module_utils.six.moves.urllib.parse import urlencode
from ansible.module_utils.basic import AnsibleModule
from ansible_collections.cisco.dcnm.plugins.module_utils.common.conversion import ConversionUtils
from ansible_collections.cisco.dcnm.plugins.module_utils.network.dcnm.dcnm import (
dcnm_send,
validate_list_of_dicts,
Expand Down Expand Up @@ -509,6 +510,7 @@ def __init__(self, module):
self.node_migration = False
self.nd_prefix = "/appcenter/cisco/ndfc/api/v1/lan-fabric"
self.switch_snos = []
self.conversion = ConversionUtils()

self.result = dict(changed=False, diff=[], response=[])

Expand All @@ -525,17 +527,18 @@ def discover_poap_params(self, poap_upd, poap):

for have_c in self.have_create:
# Idempotence - Bootstrap but already in inventory
sw_ip = self.conversion.make_ipv4_or_ipv6_address(have_c["switches"][0]["ipaddr"])
if poap_upd["serialNumber"]:
if (
poap_upd["ipAddress"] == have_c["switches"][0]["ipaddr"]
poap_upd["ipAddress"] == sw_ip
and poap_upd["serialNumber"] == have_c["switches"][0]["serialNumber"]
):
return {}

# Idempotence - Preprovision but already in inventory
if poap_upd["preprovisionSerial"] and not poap_upd["serialNumber"]:
if (
poap_upd["ipAddress"] == have_c["switches"][0]["ipaddr"]
poap_upd["ipAddress"] == sw_ip
):
return {}

Expand Down Expand Up @@ -694,7 +697,8 @@ def update_discover_params(self, inv):
if "DATA" in response:
switch = []
for sw in response["DATA"]:
if inv["seedIP"] == sw["ipaddr"]:
sw_ip = self.conversion.make_ipv4_or_ipv6_address(sw["ipaddr"])
if inv["seedIP"] == sw_ip:
switch.append(sw)
return switch
return 0
Expand Down Expand Up @@ -1411,7 +1415,8 @@ def lancred_all_switches(self):
self.fabric
)
self.module.fail_json(msg=msg)
if lan["ipAddress"] == create["switches"][0]["ipaddr"]:
lan_ip = self.conversion.make_ipv4_or_ipv6_address(lan["ipAddress"])
if lan_ip == create["switches"][0]["ipaddr"]:
set_lan = {
"switchIds": lan["switchDbID"],
"userName": create["username"],
Expand Down Expand Up @@ -1446,7 +1451,8 @@ def assign_role(self):
self.fabric
)
self.module.fail_json(msg=msg)
if role["ipAddress"] == create["switches"][0]["ipaddr"]:
role_ip = self.conversion.make_ipv4_or_ipv6_address(role["ipAddress"])
if role_ip == create["switches"][0]["ipaddr"]:
method = "PUT"
path = "/fm/fmrest/topology/role/{0}?newRole={1}".format(
role["switchDbID"], create["role"].replace("_", "%20")
Expand All @@ -1468,7 +1474,8 @@ def assign_role(self):
self.fabric
)
self.module.fail_json(msg=msg)
if role["ipAddress"] == create["ipAddress"]:
role_ip = self.conversion.make_ipv4_or_ipv6_address(role["ipAddress"])
if role_ip == create["ipAddress"]:
method = "PUT"
path = "/fm/fmrest/topology/role/{0}?newRole={1}".format(
role["switchDbID"], create["role"].replace("_", "%20")
Expand Down Expand Up @@ -1614,16 +1621,17 @@ def get_diff_query(self):
if self.config and inv_objects["DATA"]:
for want_c in self.want_create:
for inv in inv_objects["DATA"]:
inv_ip = self.conversion.make_ipv4_or_ipv6_address(inv["ipAddress"])
if want_c["role"] == "None" and want_c["seedIP"] != "None":
if want_c["seedIP"] == inv["ipAddress"]:
if want_c["seedIP"] == inv_ip:
query.append(inv)
continue
elif want_c["role"] != "None" and want_c["seedIP"] == "None":
if want_c["role"] == inv["switchRole"].replace(" ", "_"):
query.append(inv)
continue
else:
if want_c["seedIP"] == inv["ipAddress"] and want_c[
if want_c["seedIP"] == inv_ip and want_c[
"role"
] == inv["switchRole"].replace(" ", "_"):
query.append(inv)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/targets/dcnm_inventory/tasks/dcnm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
tags: sanity

- name: run test cases (connection=httpapi)
include: "{{ test_case_to_run }}"
include_tasks: "{{ test_case_to_run }}"
with_items: "{{ test_items }}"
loop_control:
loop_var: test_case_to_run
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/targets/dcnm_inventory/tasks/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@
- 'controller_version != "Unable to determine controller version"'
tags: sanity

- { include: dcnm.yaml, tags: ['dcnm'] }
- { include_tasks: dcnm.yaml, tags: ['dcnm'] }
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@
- assert:
that:
- 'result.changed == false'
- '"Invalid choice provided" in result.msg'
- '"Invalid choice" in result.msg'

- name: MERGED - Merge a Switch with invalid auth choice
cisco.dcnm.dcnm_inventory:
Expand All @@ -286,7 +286,7 @@
- assert:
that:
- 'result.changed == false'
- '"Invalid choice provided" in result.msg'
- '"Invalid choice" in result.msg'


- name: MERGED - Merge a Switch without a config
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/targets/dcnm_vrf/tasks/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,4 @@

- name: Import Role Tasks
ansible.builtin.import_tasks: dcnm.yaml
tags: ['dcnm']
tags: ['dcnm']
Loading