From b3e53ae9acce817660bb2449bfdf67b00fc97283 Mon Sep 17 00:00:00 2001 From: Mike Wiebe Date: Tue, 29 Aug 2023 14:43:56 -0400 Subject: [PATCH] Review comments --- plugins/modules/dcnm_interface.py | 26 +++-- .../tests/dcnm/dcnm_lo_mpls.yaml | 107 +++++++++++++++--- 2 files changed, 111 insertions(+), 22 deletions(-) diff --git a/plugins/modules/dcnm_interface.py b/plugins/modules/dcnm_interface.py index 4597fc613..881281558 100644 --- a/plugins/modules/dcnm_interface.py +++ b/plugins/modules/dcnm_interface.py @@ -1749,6 +1749,8 @@ def __init__(self, module): "PEER2_PO_CONF": "peer2_cmds", "PEER1_ACCESS_VLAN": "peer1_access_vlan", "PEER2_ACCESS_VLAN": "peer2_access_vlan", + "DCI_ROUTING_PROTO": "dci_routing_proto", + "DCI_ROUTING_TAG": "dci_routing_tag", } # New Interfaces @@ -2804,6 +2806,16 @@ def dcnm_intf_get_loopback_payload(self, delem, intf, profile): "route_tag" ] + # Properties for mode 'mpls' Loopback Interfaces + if delem[profile]["mode"] == "mpls": + + # These properties are read_only properties and are not exposed as + # properties that can be modified. They will be updated from the + # self.have dictionary to reflect the actual values later in the + # code workflow that walks the want values and compares to have values. + intf["interfaces"][0]["nvPairs"]["DCI_ROUTING_PROTO"] = "PLACE_HOLDER" + intf["interfaces"][0]["nvPairs"]["DCI_ROUTING_TAG"] = "PLACE_HOLDER" + def dcnm_intf_get_eth_payload(self, delem, intf, profile): # Extract port id from the given name, which is of the form 'po300' @@ -3418,6 +3430,11 @@ def dcnm_intf_compare_elements( if t_e1 != t_e2: if (state == "replaced") or (state == "overridden"): + # Special handling is required for mode 'mpls' loopback interfaces. + # They will contain either of the following two read_only properties. + if k == 'DCI_ROUTING_PROTO' or k == 'DCI_ROUTING_TAG': + return "copy_and_add" + return "add" elif state == "merged": # If the key is included in config, then use the value from want. @@ -4619,15 +4636,6 @@ def dcnm_intf_send_message_to_dcnm(self): path = self.paths["INTERFACE"] for payload in self.diff_replace: - if payload.get('policy') == 'int_mpls_loopback': - # Fabric mpls interfaces have two read_only properties that - # must be added to the payload from the self.have dictionary - # before attempting to modify the interface - drp = self.have[0]["interfaces"][0]["nvPairs"]["DCI_ROUTING_PROTO"] - drt = self.have[0]["interfaces"][0]["nvPairs"]["DCI_ROUTING_TAG"] - payload['interfaces'][0]["nvPairs"]["DCI_ROUTING_PROTO"] = drp - payload['interfaces'][0]["nvPairs"]["DCI_ROUTING_TAG"] = drt - json_payload = json.dumps(payload) resp = dcnm_send(self.module, "PUT", path, json_payload) diff --git a/tests/integration/targets/dcnm_interface/tests/dcnm/dcnm_lo_mpls.yaml b/tests/integration/targets/dcnm_interface/tests/dcnm/dcnm_lo_mpls.yaml index 367351e39..bff211e6b 100644 --- a/tests/integration/targets/dcnm_interface/tests/dcnm/dcnm_lo_mpls.yaml +++ b/tests/integration/targets/dcnm_interface/tests/dcnm/dcnm_lo_mpls.yaml @@ -51,7 +51,7 @@ ## MERGE ## ############################################## - - name: Modify mpls fabric loopback interface + - name: Modify mpls fabric loopback interface using merge cisco.dcnm.dcnm_interface: &lo_merge check_deploy: True fabric: "{{ ansible_it_fabric }}" @@ -71,10 +71,6 @@ description: "Fabric Lpk101 Configured By Ansible" register: result - - name: Pause for 30 seconds - ansible.builtin.pause: - seconds: 30 - - assert: that: - 'result.changed == true' @@ -89,7 +85,23 @@ - 'item["RETURN_CODE"] == 200' loop: '{{ result.response }}' - - name: Modify mpls fabric loopback interfaces - Idempotence + - name: Verify interface properties are modified using merge + cisco.dcnm.dcnm_interface: + check_deploy: True + fabric: "{{ ansible_it_fabric }}" + state: query + config: + - name: lo101 + switch: + - "{{ ansible_switch1 }}" + register: lpk101_data_new + until: + - 'lpk101_data_new.response[0]["interfaces"][0]["nvPairs"]["IP"] == "192.168.55.2"' + - 'lpk101_data_new.response[0]["interfaces"][0]["nvPairs"]["DESC"] == "Fabric Lpk101 Configured By Ansible"' + retries: 5 + delay: 2 + + - name: Modify mpls fabric loopback interface using merge - Idempotence cisco.dcnm.dcnm_interface: *lo_merge register: result @@ -107,21 +119,73 @@ - 'item["RETURN_CODE"] == 200' loop: '{{ result.response }}' - - name: Query and save new data for loopback101 - cisco.dcnm.dcnm_interface: + - name: Modify mpls fabric loopback interface using replace + cisco.dcnm.dcnm_interface: &lo_replace check_deploy: True fabric: "{{ ansible_it_fabric }}" - state: query + state: replaced config: - name: lo101 + type: lo switch: - "{{ ansible_switch1 }}" - register: lpk101_data_new + deploy: true + profile: + admin_state: true + mode: mpls # This mode is needed to manage mpls fabric lpbk interfaces + ipv4_addr: 192.168.88.55 # ipv4 address for the loopback interface + cmds: # Freeform config + - no shutdown + description: "Fabric Lpk101 Replaced By Ansible" + register: result + + - assert: + that: + - 'result.changed == true' + - '(result["diff"][0]["merged"] | length) == 0' + - '(result["diff"][0]["deleted"] | length) == 0' + - '(result["diff"][0]["replaced"] | length) == 1' + - '(result["diff"][0]["overridden"] | length) == 0' + - '(result["diff"][0]["deploy"] | length) == 1' + + - assert: + that: + - 'item["RETURN_CODE"] == 200' + loop: '{{ result.response }}' + + - name: Modify mpls fabric loopback interface using replace - Idempotence + cisco.dcnm.dcnm_interface: *lo_replace + register: result - assert: that: - - 'lpk101_data_new.response[0]["interfaces"][0]["nvPairs"]["IP"] == "192.168.55.2"' - - 'lpk101_data_new.response[0]["interfaces"][0]["nvPairs"]["DESC"] == "Fabric Lpk101 Configured By Ansible"' + - 'result.changed == false' + - '(result["diff"][0]["merged"] | length) == 0' + - '(result["diff"][0]["deleted"] | length) == 0' + - '(result["diff"][0]["replaced"] | length) == 0' + - '(result["diff"][0]["overridden"] | length) == 0' + - '(result["diff"][0]["deploy"] | length) == 0' + + - assert: + that: + - 'item["RETURN_CODE"] == 200' + loop: '{{ result.response }}' + + - name: Verify interface properties are modified using replace + cisco.dcnm.dcnm_interface: + check_deploy: True + fabric: "{{ ansible_it_fabric }}" + state: query + config: + - name: lo101 + switch: + - "{{ ansible_switch1 }}" + register: lpk101_data_new + until: + - 'lpk101_data_new.response[0]["interfaces"][0]["nvPairs"]["IP"] == "192.168.88.55"' + - 'lpk101_data_new.response[0]["interfaces"][0]["nvPairs"]["DESC"] == "Fabric Lpk101 Replaced By Ansible"' + retries: 5 + delay: 2 # Only execute this test if loopback101 is a fabric mpls loopback when: run_test @@ -136,7 +200,7 @@ cisco.dcnm.dcnm_interface: check_deploy: True fabric: "{{ ansible_it_fabric }}" - state: merged + state: overridden config: - name: lo101 type: lo @@ -148,4 +212,21 @@ mode: mpls ipv4_addr: "{{ lpk101_data.response[0]['interfaces'][0]['nvPairs']['IP'] }}" description: "{{ lpk101_data.response[0]['interfaces'][0]['nvPairs']['DESC'] }}" + when: run_test + + - name: Verify interface properties are restored to original values using overridden + cisco.dcnm.dcnm_interface: + check_deploy: True + fabric: "{{ ansible_it_fabric }}" + state: query + config: + - name: lo101 + switch: + - "{{ ansible_switch1 }}" + register: lpk101_data_new + until: + - 'lpk101_data_new.response[0]["interfaces"][0]["nvPairs"]["IP"] == lpk101_data.response[0]["interfaces"][0]["nvPairs"]["IP"]' + - 'lpk101_data_new.response[0]["interfaces"][0]["nvPairs"]["DESC"] == lpk101_data.response[0]["interfaces"][0]["nvPairs"]["DESC"]' + retries: 5 + delay: 2 when: run_test \ No newline at end of file