From b14757e7f3366445fb8214fc1e01e50f8c613a55 Mon Sep 17 00:00:00 2001 From: mwiebe Date: Thu, 24 Apr 2025 13:55:24 -0400 Subject: [PATCH 1/5] Fix policy delete for freeform deploy false --- plugins/modules/dcnm_policy.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/plugins/modules/dcnm_policy.py b/plugins/modules/dcnm_policy.py index aee585bea..586397550 100644 --- a/plugins/modules/dcnm_policy.py +++ b/plugins/modules/dcnm_policy.py @@ -427,6 +427,7 @@ class DcnmPolicy: "POLICY_BULK_CREATE": "/appcenter/cisco/ndfc/api/v1/lan-fabric/rest/control/policies/bulk-create", "POLICY_BULK_UPDATE": "/appcenter/cisco/ndfc/api/v1/lan-fabric/rest/control/policies/{}/bulk", "POLICY_MARK_DELETE": "/appcenter/cisco/ndfc/api/v1/lan-fabric/rest/control/policies/{}/mark-delete", + "POLICY_DELETE": "/appcenter/cisco/ndfc/api/v1/lan-fabric/rest/control/policies/policyIds?policyIds={}", "POLICY_DEPLOY": "/appcenter/cisco/ndfc/api/v1/lan-fabric/rest/control/policies/deploy", "POLICY_CFG_DEPLOY": "/appcenter/cisco/ndfc/api/v1/lan-fabric/rest/control/fabrics/{}/config-deploy/", "POLICY_WITH_POLICY_ID": "/appcenter/cisco/ndfc/api/v1/lan-fabric/rest/control/policies/{}", @@ -957,13 +958,23 @@ def dcnm_policy_get_diff_deleted(self): # Filter the list of policies and keep only those that are matching. For delete case, playbook policies # may either contain template names of policy names. So compare both. + + # NDFC has strange behavior when deleting switch_freeform template policies when the + # deploy flag is set to False. NDFC will change the policy name from switch_freeform + # to switch_freeform_config making it impossible to use the playbook name switch_freeform + # to delete the policy on a subsequent run where the flag is set to True. + # The following 3 lines of code will change switch_freeform to switch_freeform_config when + # the deploy flag is set to True. + for pl in plist: + if pl["templateName"] == "switch_freeform_config" and self.deploy is True: + pl["templateName"] = "switch_freeform" + match_pol = [ pl for pl in plist for wp in self.want if ( - not pl["deleted"] - and ( + ( (wp["policy_id_given"] is False) and (pl["templateName"] == wp["templateName"]) and ( @@ -980,7 +991,6 @@ def dcnm_policy_get_diff_deleted(self): ] # match_pol contains all the policies which exist and are to be deleted # Build the delete payloads - for pol in match_pol: del_payload = self.dcnm_policy_get_delete_payload(pol) @@ -1136,9 +1146,9 @@ def dcnm_policy_update_policy(self, policy, command): def dcnm_policy_delete_policy(self, policy, mark_del): if mark_del is True: - path = self.paths["POLICY_MARK_DELETE"].format(policy["policyId"]) + path = self.paths["POLICY_DELETE"].format(policy["policyId"]) json_payload = "" - command = "PUT" + command = "DELETE" else: path = self.paths["POLICY_WITH_POLICY_ID"].format(policy) json_payload = "" From 491156c110f93d8249b9c51744fe465de7203452 Mon Sep 17 00:00:00 2001 From: mwiebe Date: Fri, 25 Apr 2025 14:03:41 -0400 Subject: [PATCH 2/5] Unit Test Fix --- plugins/modules/dcnm_policy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/modules/dcnm_policy.py b/plugins/modules/dcnm_policy.py index 586397550..1bf28690b 100644 --- a/plugins/modules/dcnm_policy.py +++ b/plugins/modules/dcnm_policy.py @@ -416,6 +416,7 @@ class DcnmPolicy: "POLICY_BULK_CREATE": "/rest/control/policies/bulk-create", "POLICY_BULK_UPDATE": "/rest/control/policies/{}/bulk", "POLICY_MARK_DELETE": "/rest/control/policies/{}/mark-delete", + "POLICY_DELETE": "/rest/control/policies/policyIds?policyIds={}", "POLICY_DEPLOY": "/rest/control/policies/deploy", "POLICY_CFG_DEPLOY": "/rest/control/fabrics/{}/config-deploy/", "POLICY_WITH_POLICY_ID": "/rest/control/policies/{}", From 0f571239d6fd5f47c9288c434d47e00a2481a039 Mon Sep 17 00:00:00 2001 From: mwiebe Date: Fri, 25 Apr 2025 17:39:43 -0400 Subject: [PATCH 3/5] Update integration tests --- .../targets/dcnm_policy/tasks/dcnm.yaml | 2 +- .../targets/dcnm_policy/tasks/main.yaml | 2 +- .../tests/dcnm/dcnm_policy_delete.yaml | 53 +++++++++++-------- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/tests/integration/targets/dcnm_policy/tasks/dcnm.yaml b/tests/integration/targets/dcnm_policy/tasks/dcnm.yaml index 0ee1eefe6..76745b74b 100644 --- a/tests/integration/targets/dcnm_policy/tasks/dcnm.yaml +++ b/tests/integration/targets/dcnm_policy/tasks/dcnm.yaml @@ -17,7 +17,7 @@ tags: sanity - name: run test cases (connection=httpapi) - include: "{{ test_case_to_run }}" + ansible.builtin.include_tasks: "{{ test_case_to_run }}" with_items: "{{ test_items }}" loop_control: loop_var: test_case_to_run diff --git a/tests/integration/targets/dcnm_policy/tasks/main.yaml b/tests/integration/targets/dcnm_policy/tasks/main.yaml index 78c5fb834..390715438 100644 --- a/tests/integration/targets/dcnm_policy/tasks/main.yaml +++ b/tests/integration/targets/dcnm_policy/tasks/main.yaml @@ -1,2 +1,2 @@ --- -- { include: dcnm.yaml, tags: ['dcnm'] } \ No newline at end of file +- { ansible.builtin.include_tasks: dcnm.yaml, tags: ['dcnm'] } \ No newline at end of file diff --git a/tests/integration/targets/dcnm_policy/tests/dcnm/dcnm_policy_delete.yaml b/tests/integration/targets/dcnm_policy/tests/dcnm/dcnm_policy_delete.yaml index 9ea7541d1..f0c425584 100644 --- a/tests/integration/targets/dcnm_policy/tests/dcnm/dcnm_policy_delete.yaml +++ b/tests/integration/targets/dcnm_policy/tests/dcnm/dcnm_policy_delete.yaml @@ -458,28 +458,37 @@ - 'result["response"][0]["RETURN_CODE"] == 200' - 'result["response"][0]["MESSAGE"] == "OK"' - - name: Delete policy using template name - with deploy - cisco.dcnm.dcnm_policy: - fabric: "{{ ansible_it_fabric }}" - state: deleted # only choose form [merged, deleted, query] - config: - - name: template_101 # This can either be a policy name like POLICY-xxxxx or template name - - switch: - - ip: "{{ ansible_switch1 }}" - register: result - - - assert: - that: - - 'result.changed == true' - - '(result["diff"][0]["merged"] | length) == 0' - - '(result["diff"][0]["deleted"] | length) == 1' - - '(result["diff"][0]["query"] | length) == 0' - - '(result["response"] | length) == 2' - - - assert: - that: - - 'result["response"][0]["RETURN_CODE"] == 200' - - 'result["response"][0]["MESSAGE"] == "OK"' + # TBD: This test case needs to be refactored to test delete with and without deploy + # using a the switch_freeform template. The following order should be verified. + # 1. Create policy with deploy + # 2. Delete policy with deploy + # 3. Create policy with deploy + # 4. Delete policy without deploy and verify policy config is not removed from switch + # 5. Delete policy with deploy and verify policy config is removed from switch + + + # - name: Delete policy using template name - with deploy + # cisco.dcnm.dcnm_policy: + # fabric: "{{ ansible_it_fabric }}" + # state: deleted # only choose form [merged, deleted, query] + # config: + # - name: template_101 # This can either be a policy name like POLICY-xxxxx or template name + # - switch: + # - ip: "{{ ansible_switch1 }}" + # register: result + + # - assert: + # that: + # - 'result.changed == true' + # - '(result["diff"][0]["merged"] | length) == 0' + # - '(result["diff"][0]["deleted"] | length) == 1' + # - '(result["diff"][0]["query"] | length) == 0' + # - '(result["response"] | length) == 2' + + # - assert: + # that: + # - 'result["response"][0]["RETURN_CODE"] == 200' + # - 'result["response"][0]["MESSAGE"] == "OK"' ############################################## ## CLEANUP ## From 9f1c1c0788c53d469add89cb2e34645dd0d0da66 Mon Sep 17 00:00:00 2001 From: mwiebe Date: Sat, 26 Apr 2025 12:15:58 -0400 Subject: [PATCH 4/5] Fix integration test --- .../dcnm_policy/tests/dcnm/dcnm_policy_with_vars_merge.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/targets/dcnm_policy/tests/dcnm/dcnm_policy_with_vars_merge.yaml b/tests/integration/targets/dcnm_policy/tests/dcnm/dcnm_policy_with_vars_merge.yaml index f4adedcf4..66c20fb29 100644 --- a/tests/integration/targets/dcnm_policy/tests/dcnm/dcnm_policy_with_vars_merge.yaml +++ b/tests/integration/targets/dcnm_policy/tests/dcnm/dcnm_policy_with_vars_merge.yaml @@ -108,7 +108,7 @@ state: merged config: - name: my_base_ospf # This must be a valid template name - create_additional_policy: false + create_additional_policy: true priority: 111 description: "Changed description - 1" policy_vars: @@ -116,7 +116,7 @@ LOOPBACK_IP: 10.122.84.108 - name: my_base_ospf # This must be a valid template name - create_additional_policy: false + create_additional_policy: true priority: 121 description: "Changed description - 2" policy_vars: From 9761769b38c364cdd98d97ec7e9fb3f245c41f65 Mon Sep 17 00:00:00 2001 From: mwiebe Date: Sat, 26 Apr 2025 12:16:38 -0400 Subject: [PATCH 5/5] Add test playbook for dcnm_policy module --- playbooks/roles/dcnm_policy/dcnm_hosts.yaml | 15 ++++++++++++++ playbooks/roles/dcnm_policy/dcnm_tests.yaml | 23 +++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 playbooks/roles/dcnm_policy/dcnm_hosts.yaml create mode 100644 playbooks/roles/dcnm_policy/dcnm_tests.yaml diff --git a/playbooks/roles/dcnm_policy/dcnm_hosts.yaml b/playbooks/roles/dcnm_policy/dcnm_hosts.yaml new file mode 100644 index 000000000..2620e2cbb --- /dev/null +++ b/playbooks/roles/dcnm_policy/dcnm_hosts.yaml @@ -0,0 +1,15 @@ +all: + vars: + ansible_user: "admin" + ansible_password: "password" + ansible_python_interpreter: python + ansible_httpapi_validate_certs: False + ansible_httpapi_use_ssl: True + children: + dcnm: + vars: + ansible_connection: ansible.netcommon.httpapi + ansible_network_os: cisco.dcnm.dcnm + hosts: + nac-ndfc1: + ansible_host: 10.15.0.5 \ No newline at end of file diff --git a/playbooks/roles/dcnm_policy/dcnm_tests.yaml b/playbooks/roles/dcnm_policy/dcnm_tests.yaml new file mode 100644 index 000000000..4049af1b2 --- /dev/null +++ b/playbooks/roles/dcnm_policy/dcnm_tests.yaml @@ -0,0 +1,23 @@ +--- +# This playbook can be used to execute integration tests for +# the role located in: +# +# tests/integration/targets/dcnm_policy +# +# Modify the vars section with details for your testing setup. +# +- hosts: dcnm + gather_facts: no + connection: ansible.netcommon.httpapi + + vars: + # Uncomment 'testcase:' to run a specific test under 'tests/integration/targets/dcnm_policy/tests/dcnm' + # testcase: dcnm_policy_with* + ansible_it_fabric: test_fabric + ansible_switch1: 192.168.55.21 + ansible_sw1_sno: 9YEXD0OHA7Z + ansible_switch2: 192.168.55.22 + ansible_sw2_sno: 9M2TXMZ7D3N + + roles: + - dcnm_policy