From ce3f91f130b23a443ac55fde540b3b9e9a59e48d Mon Sep 17 00:00:00 2001 From: Sabari Jaganathan <93724860+sajagana@users.noreply.github.com> Date: Thu, 24 Oct 2024 18:38:04 +0530 Subject: [PATCH] [ignore] Fixed the update logic to clear bfd config on ndo_l3out_node_group_policy module --- plugins/module_utils/template.py | 8 +- .../modules/ndo_l3out_node_group_policy.py | 75 ++++++++++------ .../tasks/main.yml | 90 +++++++++++++++++-- 3 files changed, 133 insertions(+), 40 deletions(-) diff --git a/plugins/module_utils/template.py b/plugins/module_utils/template.py index 8a93e480..bfbd8f54 100644 --- a/plugins/module_utils/template.py +++ b/plugins/module_utils/template.py @@ -227,9 +227,9 @@ def get_l3out_object(self, uuid=None, name=None, fail_module=False): def get_l3out_node_group(self, name, l3out_object, fail_module=False): """ - Get the L3Out Node/Interface Group Policy by name. - :param name: Name of the L3Out Node/Interface Group Policy to search for -> Str - :param l3out_object: L3Out object to search Node/Interface Group Policy -> Dict + Get the L3Out Node Group Policy by name. + :param name: Name of the L3Out Node Group Policy to search for -> Str + :param l3out_object: L3Out object to search Node Group Policy -> Dict :param fail_module: When match is not found fail the ansible module -> Bool :return: Dict | None | List[Dict] | List[]: The processed result which could be: When the UUID | Name is existing in the search list -> Dict @@ -239,5 +239,5 @@ def get_l3out_node_group(self, name, l3out_object, fail_module=False): """ existing_l3out_node_groups = l3out_object.get("nodeGroups", []) if name: # Query a specific object - return self.get_object_by_key_value_pairs("L3Out Node/Interface Group Policy", existing_l3out_node_groups, [KVPair("name", name)], fail_module) + return self.get_object_by_key_value_pairs("L3Out Node Group Policy", existing_l3out_node_groups, [KVPair("name", name)], fail_module) return existing_l3out_node_groups # Query all objects diff --git a/plugins/modules/ndo_l3out_node_group_policy.py b/plugins/modules/ndo_l3out_node_group_policy.py index 419e70c0..7afec1ee 100644 --- a/plugins/modules/ndo_l3out_node_group_policy.py +++ b/plugins/modules/ndo_l3out_node_group_policy.py @@ -46,16 +46,19 @@ node_routing_policy: description: - The name of the L3Out Node Routing Policy. + - Providing an empty string will remove the O(node_routing_policy="") from L3Out Node Group Policy. type: str bfd_multi_hop_authentication: description: - The Bidirectional Forwarding Detection (BFD) multi-hop authentication of the L3Out Node Group Policy. - To enable the O(bfd_multi_hop_authentication) BGP routing protocol must be configured on the L3Out. + - To clear the BFD auth configuration, provide the values O(bfd_multi_hop_authentication=""), O(bfd_multi_hop_key=""), and O(bfd_multi_hop_key_id=0). type: str - choices: [ enabled, disabled ] + choices: [ enabled, disabled, "" ] bfd_multi_hop_key_id: description: - The BFD multi-hop key ID of the L3Out Node Group Policy. + - Providing 0 will remove the O(bfd_multi_hop_key_id=0) from L3Out Node Group Policy. type: int bfd_multi_hop_key: description: @@ -176,9 +179,9 @@ def main(): name=dict(type="str", aliases=["l3out_node_group_policy"]), description=dict(type="str"), node_routing_policy=dict(type="str"), - bfd_multi_hop_authentication=dict(type="str", choices=["enabled", "disabled"]), + bfd_multi_hop_authentication=dict(type="str", choices=["enabled", "disabled", ""]), bfd_multi_hop_key_id=dict(type="int"), - bfd_multi_hop_key=dict(type="str", no_log=False), + bfd_multi_hop_key=dict(type="str", no_log=True), target_dscp=dict(type="str", choices=list(TARGET_DSCP_MAP)), state=dict(type="str", default="query", choices=["absent", "query", "present"]), ) @@ -235,6 +238,9 @@ def main(): if mso.existing: proposed_payload = copy.deepcopy(mso.existing) + if proposed_payload.get("bfdMultiHop", {}).get("key", {}).get("ref"): + proposed_payload["bfdMultiHop"]["key"].pop("ref", None) + mso.existing["bfdMultiHop"]["key"].pop("ref", None) if description is not None and mso.existing.get("description") != description: ops.append(dict(op="replace", path=node_group_policy_path + "/description", value=description)) @@ -245,41 +251,50 @@ def main(): dict(op="replace", path=node_group_policy_path + "/nodeRoutingPolicyRef", value=l3out_node_routing_policy_object.details.get("uuid")) ) proposed_payload["nodeRoutingPolicyRef"] = l3out_node_routing_policy_object.details.get("uuid") - elif node_routing_policy == "" and mso.existing.get("nodeRoutingPolicyRef"): - ops.append(dict(op="remove", path=node_group_policy_path + "/nodeRoutingPolicyRef")) - proposed_payload.pop("nodeRoutingPolicyRef", None) - if (bfd_multi_hop_authentication or bfd_multi_hop_key or bfd_multi_hop_key_id) and not mso.existing.get("bfdMultiHop"): - ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop", value=dict())) - proposed_payload["bfdMultiHop"] = dict() - - if bfd_multi_hop_authentication is not None and mso.existing.get("bfdMultiHop", {}).get("authEnabled") is not ( - True if bfd_multi_hop_authentication == "enabled" else False - ): - ops.append( - dict( - op="replace", - path=node_group_policy_path + "/bfdMultiHop/authEnabled", - value=True if bfd_multi_hop_authentication == "enabled" else False, + elif node_routing_policy == "" and mso.existing.get( + "nodeRoutingPolicyRef" + ): # Clear the node routing policy when node_routing_policy is empty string + ops.append(dict(op="replace", path=node_group_policy_path + "/nodeRoutingPolicyRef", value=node_routing_policy)) + proposed_payload["nodeRoutingPolicyRef"] = node_routing_policy + + if bfd_multi_hop_authentication or bfd_multi_hop_key or bfd_multi_hop_key_id: + if not mso.existing.get("bfdMultiHop"): + ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop", value=dict())) + proposed_payload["bfdMultiHop"] = dict() + + # Ignore when bfd_multi_hop_authentication is None or empty string + if bfd_multi_hop_authentication and mso.existing.get("bfdMultiHop", {}).get("authEnabled") is not ( + True if bfd_multi_hop_authentication == "enabled" else False + ): + ops.append( + dict( + op="replace", + path=node_group_policy_path + "/bfdMultiHop/authEnabled", + value=True if bfd_multi_hop_authentication == "enabled" else False, + ) ) - ) - proposed_payload["bfdMultiHop"]["authEnabled"] = True if bfd_multi_hop_authentication == "enabled" else False + proposed_payload["bfdMultiHop"]["authEnabled"] = True if bfd_multi_hop_authentication == "enabled" else False - if bfd_multi_hop_key_id is not None and mso.existing.get("bfdMultiHop", {}).get("keyID") != bfd_multi_hop_key_id: - ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/keyID", value=bfd_multi_hop_key_id)) - proposed_payload["bfdMultiHop"]["keyID"] = bfd_multi_hop_key_id + if bfd_multi_hop_key_id is not None and mso.existing.get("bfdMultiHop", {}).get("keyID") != bfd_multi_hop_key_id: + ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/keyID", value=bfd_multi_hop_key_id)) + proposed_payload["bfdMultiHop"]["keyID"] = bfd_multi_hop_key_id - if bfd_multi_hop_key is not None: - ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/key", value=dict())) - ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/key/value", value=bfd_multi_hop_key)) - proposed_payload["bfdMultiHop"]["key"] = dict(value=bfd_multi_hop_key) + if bfd_multi_hop_key is not None: + ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/key", value=dict())) + ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/key/value", value=bfd_multi_hop_key)) + proposed_payload["bfdMultiHop"]["key"] = dict(value=bfd_multi_hop_key) + + # Clear the complete BFD multi-hop configuration when all parameters are empty + elif bfd_multi_hop_authentication == "" and bfd_multi_hop_key == "" and bfd_multi_hop_key_id == 0 and mso.existing.get("bfdMultiHop"): + ops.append(dict(op="remove", path=node_group_policy_path + "/bfdMultiHop")) + proposed_payload.pop("bfdMultiHop") if target_dscp is not None and mso.existing.get("targetDscp") != target_dscp: ops.append(dict(op="replace", path=node_group_policy_path + "/targetDscp", value=target_dscp)) proposed_payload["targetDscp"] = target_dscp mso.sanitize(proposed_payload, collate=True) - else: payload = dict(name=name) @@ -291,7 +306,7 @@ def main(): bfd_multi_hop = dict() - if bfd_multi_hop_authentication is not None: + if bfd_multi_hop_authentication: bfd_multi_hop["authEnabled"] = True if bfd_multi_hop_authentication == "enabled" else False if bfd_multi_hop_key_id: @@ -321,6 +336,8 @@ def main(): l3out_node_group = mso_template.get_l3out_node_group(name, l3out_object.details) if l3out_node_group: mso.existing = l3out_node_group.details # When the state is present + if mso.existing.get("bfdMultiHop", {}).get("key", {}).get("ref"): + mso.existing["bfdMultiHop"]["key"].pop("ref") else: mso.existing = {} # When the state is absent elif module.check_mode and state != "query": # When the state is present/absent with check mode diff --git a/tests/integration/targets/ndo_l3out_node_group_policy/tasks/main.yml b/tests/integration/targets/ndo_l3out_node_group_policy/tasks/main.yml index eae463ab..fdecf7cd 100644 --- a/tests/integration/targets/ndo_l3out_node_group_policy/tasks/main.yml +++ b/tests/integration/targets/ndo_l3out_node_group_policy/tasks/main.yml @@ -270,7 +270,7 @@ that: - cm_update_node_group_policy_1 is changed - cm_update_node_group_policy_1.current.bfdMultiHop.keyID == 1 - - cm_update_node_group_policy_1.current.bfdMultiHop.key.value == "TestKey" + - cm_update_node_group_policy_1.current.bfdMultiHop.key.value is defined - cm_update_node_group_policy_1.current.description == "Test description" - cm_update_node_group_policy_1.current.name == "node_group_policy_1" - cm_update_node_group_policy_1.current.nodeRoutingPolicyRef != "" @@ -298,7 +298,7 @@ - nm_update_node_group_policy_1.previous.nodeRoutingPolicyRef == "" - nm_update_node_group_policy_1.previous.targetDscp == "unspecified" - - name: Update L3Out node group policy object with normal mode again - task status should be changed because of the auth key ref change + - name: Update L3Out node group policy object with normal mode again cisco.mso.ndo_l3out_node_group_policy: <<: *nm_update_node_group_policy_1 register: nm_update_node_group_policy_1_again @@ -306,10 +306,9 @@ - name: Assertion check for update L3Out node group policy object with normal mode again ansible.builtin.assert: that: - - nm_update_node_group_policy_1_again is changed + - nm_update_node_group_policy_1_again is not changed - nm_update_node_group_policy_1_again.current.bfdMultiHop.authEnabled == true - nm_update_node_group_policy_1_again.current.bfdMultiHop.keyID == 1 - - nm_update_node_group_policy_1_again.current.bfdMultiHop.key.ref != nm_update_node_group_policy_1_again.previous.bfdMultiHop.key.ref - nm_update_node_group_policy_1_again.current.description == "Test description" - nm_update_node_group_policy_1_again.current.name == "node_group_policy_1" - nm_update_node_group_policy_1_again.current.nodeRoutingPolicyRef != "" @@ -473,7 +472,6 @@ that: - add_ngp_3_auth_disabled is changed - add_ngp_3_auth_disabled.current.bfdMultiHop.authEnabled == true - - add_ngp_3_auth_disabled.current.bfdMultiHop.key.ref is defined - add_ngp_3_auth_disabled.current.bfdMultiHop.keyID == 2 - add_ngp_3_auth_disabled.current.name == "node_group_policy_30" - add_ngp_3_auth_disabled.current.nodeRoutingPolicyRef != "" @@ -500,7 +498,6 @@ - add_ngp_3_auth_disabled is changed - add_ngp_3_auth_disabled.current.bfdMultiHop.authEnabled == false - add_ngp_3_auth_disabled.current.bfdMultiHop.keyID == 2 - - add_ngp_3_auth_disabled.current.bfdMultiHop.key.ref is not defined - add_ngp_3_auth_disabled.current.name == "node_group_policy_3" - add_ngp_3_auth_disabled.current.nodeRoutingPolicyRef != "" - add_ngp_3_auth_disabled.current.targetDscp == "af12" @@ -578,6 +575,61 @@ - add_ngp_4_auth_disabled.current.targetDscp == "af12" - add_ngp_4_auth_disabled.previous == {} + - name: Add bfd_multi_hop_key_id to node_group_policy_4 when bfd_multi_hop_authentication is disabled + cisco.mso.ndo_l3out_node_group_policy: + <<: *mso_info + template: '{{ ansible_l3out_template | default("ansible_test") }}' + l3out: "l3out_2" + name: "node_group_policy_4" + bfd_multi_hop_key_id: 10 + state: present + register: add_key_id_ngp_4_auth_disabled + + - name: Assertion check for add bfd_multi_hop_key_id to node_group_policy_4 when bfd_multi_hop_authentication is disabled + ansible.builtin.assert: + that: + - add_key_id_ngp_4_auth_disabled is changed + - add_key_id_ngp_4_auth_disabled.current.bfdMultiHop.authEnabled == false + - add_key_id_ngp_4_auth_disabled.current.bfdMultiHop.keyID == 10 + - add_key_id_ngp_4_auth_disabled.current.name == "node_group_policy_4" + - add_key_id_ngp_4_auth_disabled.current.nodeRoutingPolicyRef != "" + - add_key_id_ngp_4_auth_disabled.current.targetDscp == "af12" + - add_key_id_ngp_4_auth_disabled.previous.bfdMultiHop.authEnabled == false + - add_key_id_ngp_4_auth_disabled.previous.name == "node_group_policy_4" + - add_key_id_ngp_4_auth_disabled.previous.nodeRoutingPolicyRef != "" + - add_key_id_ngp_4_auth_disabled.previous.bfdMultiHop.keyID is not defined + - add_key_id_ngp_4_auth_disabled.previous.targetDscp == "af12" + + - name: Remove bfd_multi_hop_key_id from node_group_policy_4 when bfd_multi_hop_authentication is disabled + cisco.mso.ndo_l3out_node_group_policy: + <<: *mso_info + template: '{{ ansible_l3out_template | default("ansible_test") }}' + l3out: "l3out_2" + name: "node_group_policy_4" + bfd_multi_hop_authentication: disabled + bfd_multi_hop_key_id: 0 + state: present + output_level: debug + register: clear_key_id_ngp_4_auth_disabled + + - name: Assertion check for remove bfd_multi_hop_key_id from node_group_policy_4 when bfd_multi_hop_authentication is disabled + ansible.builtin.assert: + that: + - clear_key_id_ngp_4_auth_disabled is changed + - clear_key_id_ngp_4_auth_disabled.current.bfdMultiHop.keyID is not defined + - clear_key_id_ngp_4_auth_disabled.current.bfdMultiHop.authEnabled == false + - clear_key_id_ngp_4_auth_disabled.current.name == "node_group_policy_4" + - clear_key_id_ngp_4_auth_disabled.current.nodeRoutingPolicyRef != "" + - clear_key_id_ngp_4_auth_disabled.current.targetDscp == "af12" + - clear_key_id_ngp_4_auth_disabled.previous.bfdMultiHop.authEnabled == false + - clear_key_id_ngp_4_auth_disabled.previous.bfdMultiHop.keyID == 10 + - clear_key_id_ngp_4_auth_disabled.previous.name == "node_group_policy_4" + - clear_key_id_ngp_4_auth_disabled.previous.nodeRoutingPolicyRef != "" + - clear_key_id_ngp_4_auth_disabled.previous.targetDscp == "af12" + - clear_key_id_ngp_4_auth_disabled.proposed.bfdMultiHop.authEnabled == false + - clear_key_id_ngp_4_auth_disabled.proposed.bfdMultiHop.keyID == 0 + - clear_key_id_ngp_4_auth_disabled.proposed.name == "node_group_policy_4" + - name: Create node_group_policy_5 without bfd config cisco.mso.ndo_l3out_node_group_policy: <<: *mso_info @@ -657,7 +709,6 @@ that: - update_ngp_5_bfd_config is changed - update_ngp_5_bfd_config.current.bfdMultiHop.authEnabled == true - - update_ngp_5_bfd_config.current.bfdMultiHop.key.ref is defined - update_ngp_5_bfd_config.current.bfdMultiHop.keyID == 3 - update_ngp_5_bfd_config.current.name == "node_group_policy_5" - update_ngp_5_bfd_config.current.nodeRoutingPolicyRef != "" @@ -668,6 +719,31 @@ - update_ngp_5_bfd_config.previous.nodeRoutingPolicyRef != "" - update_ngp_5_bfd_config.previous.targetDscp == "unspecified" + - name: Clear complete bfd and node_routing_policy config from node_group_policy_5 + cisco.mso.ndo_l3out_node_group_policy: + <<: *mso_info + template: '{{ ansible_l3out_template | default("ansible_test") }}' + l3out: "l3out_2" + name: "node_group_policy_5" + node_routing_policy: "" + bfd_multi_hop_authentication: "" + bfd_multi_hop_key_id: 0 + bfd_multi_hop_key: "" + state: present + register: clear_ngp_5_bfd_config + + - name: Assertion check for clear complete bfd and node_routing_policy config from node_group_policy_5 + ansible.builtin.assert: + that: + - clear_ngp_5_bfd_config is changed + - clear_ngp_5_bfd_config.current.name == "node_group_policy_5" + - clear_ngp_5_bfd_config.current.nodeRoutingPolicyRef == "" + - clear_ngp_5_bfd_config.current.targetDscp == "unspecified" + - clear_ngp_5_bfd_config.previous.name == "node_group_policy_5" + - clear_ngp_5_bfd_config.previous.nodeRoutingPolicyRef != "" + - clear_ngp_5_bfd_config.previous.targetDscp == "unspecified" + - clear_ngp_5_bfd_config.previous.bfdMultiHop is defined + - name: Remove node_group_policy_2 with check mode cisco.mso.ndo_l3out_node_group_policy: &cm_rm_node_group_policy_2 <<: *add_node_group_policy_2