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

dcnm_vrf: Fix issue #352 #364

Open
wants to merge 60 commits into
base: develop
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
40d93d6
Tentative fix for Issue #351
allenrobel Dec 3, 2024
0b17715
dcnm_vrf: to_bool() fix to return correct value, or call fail_json()
allenrobel Dec 3, 2024
6e32f42
dcnm_image_policy: fix for issue #347 (#348)
allenrobel Dec 3, 2024
244929b
dcnm_fabric: hardening (#349)
allenrobel Dec 3, 2024
5f67187
dcnm_vrf: remove bool() casts, more...
allenrobel Dec 4, 2024
9508840
dcnm_vrf: More refactoring and simplifying
allenrobel Dec 4, 2024
098d17f
Rename var for readability
allenrobel Dec 4, 2024
be41882
Rename var for readability
allenrobel Dec 4, 2024
13e076e
dcnm_vrf: Avoid code duplication
allenrobel Dec 5, 2024
6a6de04
Remove TODO comment
allenrobel Dec 5, 2024
12f9e53
dcnm_vrf: leverage get_vrf_lite_objects() everywhere
allenrobel Dec 5, 2024
bbbf285
Appease pylint f-string complaints, more...
allenrobel Dec 5, 2024
da54fa1
test_log_v2.py: Temporarily disable unit tests
allenrobel Dec 5, 2024
c8c578b
Appease pylint
allenrobel Dec 5, 2024
14da7d7
Fix pep8 too-many-blank-lines
allenrobel Dec 5, 2024
1e028a2
Remove python 3.9 incompatible type hint
allenrobel Dec 5, 2024
ee179b5
Re-enable test_log_v2.py unit tests and "fix" UT errors
allenrobel Dec 5, 2024
0c980b3
Appease linters
allenrobel Dec 5, 2024
5b8c1c3
Merge branch 'develop' into dcnm-vrf-fix-issue-351
allenrobel Dec 6, 2024
b401ab8
Update another conditional
allenrobel Dec 6, 2024
978b0d2
dcnm_vrf: dict_values_differ() use generic names
allenrobel Dec 6, 2024
75a24d5
dcnm_vrf: Address mwiebe review comments
allenrobel Dec 6, 2024
f747767
Address mwiebe coments part 2
allenrobel Dec 6, 2024
dbfe057
dcnm_vrf: Protect dictionary access
allenrobel Dec 6, 2024
cc804e0
dcnm_vrf: Refactor push_to_remote, validate_input
allenrobel Dec 7, 2024
78f9e0d
appease linters
allenrobel Dec 7, 2024
e861626
Appease linters
allenrobel Dec 7, 2024
3a1a514
More refactoring
allenrobel Dec 7, 2024
fed3a70
Fix typo
allenrobel Dec 7, 2024
930dbec
dcnm_vrf: fix IT files, minor cleanup
allenrobel Dec 8, 2024
d83d4fc
Fix UT failures
allenrobel Dec 8, 2024
832a3e8
Appease pylint
allenrobel Dec 8, 2024
cf27b33
Cleanup IT deleted, query, merged
allenrobel Dec 8, 2024
72bfc10
Update playbooks/roles
allenrobel Dec 8, 2024
5749eda
Move dynamic_inventory.py into playbooks/files
allenrobel Dec 8, 2024
acb3890
Appease ansible sanity
allenrobel Dec 8, 2024
0bc26be
Appease linters
allenrobel Dec 8, 2024
b78e996
dcnm_vrf: Updates to integration tests
allenrobel Dec 10, 2024
ca7bea8
Appease linters
allenrobel Dec 10, 2024
8fb0907
Fix unprotected dictionary access
allenrobel Dec 10, 2024
509c335
Update integration tests
allenrobel Dec 10, 2024
b61fd0b
dcnm_vrf/dcnm_tests.yaml - Include all vars
allenrobel Dec 11, 2024
a565a46
Update Usage section and assign additional fabric_* vars
allenrobel Dec 11, 2024
001931e
IT: interface var naming change
allenrobel Dec 11, 2024
ecd1849
IT: Update scale.yaml
allenrobel Dec 12, 2024
a0f44c3
dcnm_vrf: IT dynamic_inventory.py small modifications
allenrobel Dec 12, 2024
41a8fd7
dcnm_vrf: fix for #356, and for an undeploy case, simplify, more...
allenrobel Dec 15, 2024
c30cfd4
dcnm_vrf: Fix for issue #357
allenrobel Dec 16, 2024
d754e3e
dcnm_vrf IT: Update task titles, print results
allenrobel Dec 16, 2024
75334e0
Merge branch 'develop' into dcnm-vrf-fix-issue-351
allenrobel Dec 16, 2024
e92c1e2
IT: Update comment for workaround.
allenrobel Dec 17, 2024
a9d9c9e
Uncomment dcnm_get_ip_addr_info, more...
allenrobel Dec 18, 2024
2a19271
Safe get in dict_values_differ() method
mikewiebe Dec 18, 2024
0ecfe58
Change conf_changed to module scope
allenrobel Dec 18, 2024
a3ccb52
UT: Update unit tests to accommodate issue #357
allenrobel Dec 20, 2024
1ced9a1
dcnm_vrf: diff_for_create() consistent return statements
allenrobel Dec 20, 2024
baa756b
dcnm_vrf: fix for improper update of lanAttachList
allenrobel Dec 20, 2024
7e41d5d
Merge branch 'develop' into dcnm-vrf-fix-issue-351
allenrobel Dec 20, 2024
e1c4d1e
Initial commit
allenrobel Jan 10, 2025
4a4c398
Merge branch 'develop' into dcnm-vrf-fix-issue-352
allenrobel Jan 16, 2025
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
Prev Previous commit
Next Next commit
Appease pylint f-string complaints, more...
1. Appease pylint f-string complaints

2. optimize a couple conditionals

3. Change an "== True" to the preferred "is True"

4. Add a few TODO comments
  • Loading branch information
allenrobel committed Dec 5, 2024
commit bbbf28510652d6810d6d7196c849d3add900f174
77 changes: 36 additions & 41 deletions plugins/modules/dcnm_vrf.py
Original file line number Diff line number Diff line change
@@ -647,7 +647,7 @@ def __init__(self, module):
else:
self.paths = self.dcnm_vrf_paths[self.dcnm_version]

self.result = dict(changed=False, diff=[], response=[])
self.result = {"changed": False, "diff": [], "response": []}

self.failed_to_rollback = False
self.WAIT_TIME_FOR_DELETE_LOOP = 5 # in seconds
@@ -1878,14 +1878,11 @@ def get_diff_merge(self, replace=False):
)

if missing_fabric or not_ok:
msg1 = "Fabric {0} not present on DCNM".format(self.fabric)
msg2 = (
"Unable to generate vrfId for vrf: {0} "
"under fabric: {1}".format(
want_c["vrfName"], self.fabric
)
)

# arobel: TODO: Not covered by UT
msg0 = f"{self.class_name}.{method_name}: "
msg1 = f"{msg0} Fabric {self.fabric} not present on the controller"
msg2 = f"{msg0} Unable to generate vrfId for vrf "
msg2 += f"{want_c['vrfName']} under fabric {self.fabric}"
self.module.fail_json(msg=msg1 if missing_fabric else msg2)

if not vrf_id_obj["DATA"]:
@@ -1896,8 +1893,10 @@ def get_diff_merge(self, replace=False):
elif self.dcnm_version >= 12:
vrf_id = vrf_id_obj["DATA"].get("l3vni")
else:
msg = "Unsupported DCNM version: version {0}".format(
self.dcnm_version
# arobel: TODO: Not covered by UT
msg = f"{self.class_name}.{method_name}: "
msg += (
f"Unsupported controller version: {self.dcnm_version}"
)
self.module.fail_json(msg)

@@ -1996,7 +1995,7 @@ def get_diff_merge(self, replace=False):
break

if not vrf_id:
# arobel: TODO: This is not covered in UT
# arobel: TODO: Not covered by UT
msg = f"{self.class_name}.{method_name}: "
msg += f"Unable to generate vrfId for vrf: {want_c['vrfName']} "
msg += f"under fabric: {self.fabric}"
@@ -2009,6 +2008,7 @@ def get_diff_merge(self, replace=False):
if self.module.check_mode:
continue

# arobel: TODO: The below is not covered by UT
resp = dcnm_send(
self.module, method, create_path, json.dumps(want_c)
)
@@ -2295,8 +2295,10 @@ def get_diff_query(self):
return

if missing_fabric or not_ok:
msg1 = "Fabric {0} not present on DCNM".format(self.fabric)
msg2 = "Unable to find VRFs under fabric: {0}".format(self.fabric)
# arobel: TODO: Not covered by UT
msg0 = f"{self.class_name}.{method_name}:"
msg1 = f"{msg0} Fabric {self.fabric} not present on the controller"
msg2 = f"{msg0} Unable to find VRFs under fabric: {self.fabric}"
self.module.fail_json(msg=msg1 if missing_fabric else msg2)

if not vrf_objects["DATA"]:
@@ -2326,16 +2328,14 @@ def get_diff_query(self):
)

if missing_fabric or not_ok:
msg1 = "Fabric {0} not present on DCNM".format(self.fabric)
msg2 = (
"Unable to find attachments for "
"vrfs: {0} under fabric: {1}".format(
vrf["vrfName"], self.fabric
)
# arobel: TODO: Not covered by UT
msg0 = f"{self.class_name}.{method_name}:"
msg1 = f"{msg0} Fabric {self.fabric} not present on the controller"
msg2 = f"{msg0} Unable to find attachments for "
msg2 += (
f"vrfs: {vrf['vrfName']} under fabric: {self.fabric}"
)

self.module.fail_json(msg=msg1 if missing_fabric else msg2)
return

if not vrf_attach_objects["DATA"]:
return
@@ -2378,13 +2378,9 @@ def get_diff_query(self):
missing_fabric, not_ok = self.handle_response(vrf_objects, "query_dcnm")

if missing_fabric or not_ok:
msg1 = "Fabric {0} not present on DCNM".format(self.fabric)
msg2 = (
"Unable to find attachments for "
"vrfs: {0} under fabric: {1}".format(
vrf["vrfName"], self.fabric
)
)
msg1 = f"Fabric {self.fabric} not present on DCNM"
msg2 = "Unable to find attachments for "
msg2 += f"vrfs: {vrf['vrfName']} under fabric: {self.fabric}"

self.module.fail_json(msg=msg1 if missing_fabric else msg2)
return
@@ -2422,7 +2418,7 @@ def push_to_remote(self, is_rollback=False):
method = "PUT"
if self.diff_create_update:
for vrf in self.diff_create_update:
update_path = path + "/{0}".format(vrf["vrfName"])
update_path = f"{path}/{vrf['vrfName']}"
resp = dcnm_send(self.module, method, update_path, json.dumps(vrf))
self.result["response"].append(resp)
fail, self.result["changed"] = self.handle_response(resp, "create")
@@ -2497,9 +2493,9 @@ def push_to_remote(self, is_rollback=False):
self.failure(resp)

if del_failure:
self.result["response"].append(
"Deletion of vrfs {0} has failed".format(del_failure[:-1])
)
msg = f"{self.class_name}.{method_name}: "
msg += f"Deletion of vrfs {del_failure[:-1]} has failed"
self.result["response"].append(msg)
self.module.fail_json(msg=self.result)

method = "POST"
@@ -2514,11 +2510,10 @@ def push_to_remote(self, is_rollback=False):
vlan_data = dcnm_send(self.module, "GET", vlan_path)

if vlan_data["RETURN_CODE"] != 200:
self.module.fail_json(
msg="Failure getting autogenerated vlan_id {0}".format(
vlan_data
)
)
msg = f"{self.class_name}.{method_name}: "
msg += f"Failure getting autogenerated vlan_id {vlan_data}"
self.module.fail_json(msg=msg)

vlanId = vlan_data["DATA"]

t_conf = {
@@ -2784,7 +2779,7 @@ def wait_for_vrf_del_ready(self):
break
if (
atch["lanAttachState"] == "DEPLOYED"
and atch["isLanAttached"] == True
and atch["isLanAttached"] is True
):
vrf_name = atch.get("vrfName", "unknown")
fabric_name = atch.get("fabricName", "unknown")
@@ -2819,7 +2814,7 @@ def validate_input(self):

state = self.params["state"]

if state == "merged" or state == "overridden" or state == "replaced":
if state in ("merged", "overridden", "replaced"):

vrf_spec = dict(
vrf_name=dict(required=True, type="str", length_max=32),
@@ -2907,7 +2902,7 @@ def validate_input(self):
if "ip_address" not in attach:
msg = "ip_address is mandatory under attach parameters"
else:
if state == "merged" or state == "replaced":
if state in ("merged", "replaced"):
msg = f"{self.class_name}.{method_name}: "
msg += f"config element is mandatory for {state} state"

5 changes: 2 additions & 3 deletions tests/unit/modules/dcnm/test_dcnm_vrf.py
Original file line number Diff line number Diff line change
@@ -1146,9 +1146,8 @@ def test_dcnm_vrf_delete_failure(self):
dict(state="deleted", fabric="test_fabric", config=self.playbook_config)
)
result = self.execute_module(changed=False, failed=True)
self.assertEqual(
result["msg"]["response"][2], "Deletion of vrfs test_vrf_1 has failed"
)
msg = "DcnmVrf.push_to_remote: Deletion of vrfs test_vrf_1 has failed"
self.assertEqual(result["msg"]["response"][2], msg)

def test_dcnm_vrf_query(self):
set_module_args(
Loading