From 6bb13bbb84cc3a1ecda61bb6c2ec3b7553a6faae Mon Sep 17 00:00:00 2001 From: Chris Van Heuveln Date: Mon, 3 Jun 2019 23:44:09 -0400 Subject: [PATCH] nxos_vlan: fix broken purge behavior (issue #57101) (#57229) * nxos_vlan: fix broken purge behavior (issue #57101) Symptoms/Analysis: - `nxos_vlan` `purge: true` would fail when `purge` was trying to delete all unspecified vlans, including vlan 1. - `nxos` devices do not allow removing vlan 1 and raise a cli exception error - Previous fix #55144 caused a side effect when `purge` was used: vlan changes specified by `aggregate` were ignored; e.g. - vlan 4 is not present; playbook specifies `aggregate: { vlan: 4 }, purge: true` - results in proper purging but vlan 4 is not created Solutions: - ignore vlan 1 when purging - remove the `not purge` check from state present logic Added additional unit tests and integration tests. Tested against all regression platforms. * PEP fixes * Add agg_show_vlan_brief.txt fixture * Add warning for removing vlan 1 * change method name check --- lib/ansible/modules/network/nxos/nxos_vlan.py | 6 +- .../targets/nxos_vlan/tests/common/agg.yaml | 33 +++++++- .../nxos_vlan/tests/common/sanity.yaml | 1 - .../nxos_vlan/agg_show_vlan_brief.txt | 27 +++++++ .../modules/network/nxos/test_nxos_vlan.py | 76 ++++++++++++++++++- 5 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 test/units/modules/network/nxos/fixtures/nxos_vlan/agg_show_vlan_brief.txt diff --git a/lib/ansible/modules/network/nxos/nxos_vlan.py b/lib/ansible/modules/network/nxos/nxos_vlan.py index eb5335bb253..c9888ced9b4 100644 --- a/lib/ansible/modules/network/nxos/nxos_vlan.py +++ b/lib/ansible/modules/network/nxos/nxos_vlan.py @@ -84,6 +84,7 @@ options: description: - Purge VLANs not defined in the I(aggregate) parameter. This parameter can be used without aggregate as well. + - Removal of Vlan 1 is not allowed and will be ignored by purge. type: bool default: 'no' delay: @@ -220,7 +221,7 @@ def map_obj_to_commands(updates, module): if obj_in_have: commands.append('no vlan {0}'.format(vlan_id)) - elif state == 'present' and not purge: + elif state == 'present': if not obj_in_have: commands.append('vlan {0}'.format(vlan_id)) @@ -318,6 +319,9 @@ def map_obj_to_commands(updates, module): if purge: for h in have: + if h['vlan_id'] == '1': + module.warn("Deletion of vlan 1 is not allowed; purge will ignore vlan 1") + continue obj_in_want = search_obj_in_list(h['vlan_id'], want) if not obj_in_want: commands.append('no vlan {0}'.format(h['vlan_id'])) diff --git a/test/integration/targets/nxos_vlan/tests/common/agg.yaml b/test/integration/targets/nxos_vlan/tests/common/agg.yaml index 07d95de6b5e..0ce4ba707b6 100644 --- a/test/integration/targets/nxos_vlan/tests/common/agg.yaml +++ b/test/integration/targets/nxos_vlan/tests/common/agg.yaml @@ -8,6 +8,7 @@ lines: - no vlan 102 - no vlan 103 + - no vlan 104 provider: "{{ connection }}" ignore_errors: yes @@ -84,8 +85,38 @@ that: - 'result.changed == false' +- name: "setup for purge test with aggregate add" + nxos_vlan: + vlan_id: 104 + purge: true + provider: "{{ connection }}" + +- name: purge 104 with aggregate add 102-103 + nxos_vlan: &purge_add + aggregate: + - { vlan_id: 102 } + - { vlan_id: 103 } + purge: true + provider: "{{ connection }}" + register: result + +- assert: + that: + - 'result.changed == true' + - '"vlan 102" in result.commands' + - '"vlan 103" in result.commands' + - '"no vlan 104" in result.commands' + +- name: purge_add - Idempotence + nxos_vlan: *purge_add + register: result + +- assert: + that: + - 'result.changed == false' + - name: teardown nxos_config: *rm ignore_errors: yes -- debug: msg="END connection={{ ansible_connection }}/agg.yaml" +- debug: msg="END connection={{ ansible_connection }}/agg.yaml" diff --git a/test/integration/targets/nxos_vlan/tests/common/sanity.yaml b/test/integration/targets/nxos_vlan/tests/common/sanity.yaml index a4d134de8fc..cc4e5b74f08 100644 --- a/test/integration/targets/nxos_vlan/tests/common/sanity.yaml +++ b/test/integration/targets/nxos_vlan/tests/common/sanity.yaml @@ -134,7 +134,6 @@ - assert: *false when: platform is search('N3K|N7K') -# Uncomment this once the get_capabilities() work on nxapi as well - name: Change mode nxos_vlan: &mode1 vlan_id: 50 diff --git a/test/units/modules/network/nxos/fixtures/nxos_vlan/agg_show_vlan_brief.txt b/test/units/modules/network/nxos/fixtures/nxos_vlan/agg_show_vlan_brief.txt new file mode 100644 index 00000000000..4ba12553edb --- /dev/null +++ b/test/units/modules/network/nxos/fixtures/nxos_vlan/agg_show_vlan_brief.txt @@ -0,0 +1,27 @@ +{ + "TABLE_vlanbriefxbrief": { + "ROW_vlanbriefxbrief": [ + { + "vlanshowbr-vlanid": 1, + "vlanshowbr-vlanid-utf": 1, + "vlanshowbr-vlanname": "default", + "vlanshowbr-vlanstate": "active", + "vlanshowbr-shutstate": "noshutdown" + }, + { + "vlanshowbr-vlanid": 4, + "vlanshowbr-vlanid-utf": 4, + "vlanshowbr-vlanname": "_4_", + "vlanshowbr-vlanstate": "active", + "vlanshowbr-shutstate": "noshutdown" + }, + { + "vlanshowbr-vlanid": 5, + "vlanshowbr-vlanid-utf": 5, + "vlanshowbr-vlanname": "_5_", + "vlanshowbr-vlanstate": "active", + "vlanshowbr-shutstate": "noshutdown" + } + ] + } +} diff --git a/test/units/modules/network/nxos/test_nxos_vlan.py b/test/units/modules/network/nxos/test_nxos_vlan.py index eee544e8b95..91bcbc5f749 100644 --- a/test/units/modules/network/nxos/test_nxos_vlan.py +++ b/test/units/modules/network/nxos/test_nxos_vlan.py @@ -68,10 +68,84 @@ class TestNxosVlanModule(TestNxosModule): output.append(load_fixture('nxos_vlan', filename)) return output - self.run_commands.side_effect = load_from_file + def agg_load_from_file(*args, **kwargs): + """Load vlan output for aggregate/purge tests""" + return([load_fixture('nxos_vlan', 'agg_show_vlan_brief.txt')]) + + if '_agg_' in self._testMethodName: + self.run_commands.side_effect = agg_load_from_file + else: + self.run_commands.side_effect = load_from_file + self.load_config.return_value = None self.get_config.return_value = load_fixture('nxos_vlan', 'config.cfg') + def test_nxos_vlan_agg_1(self): + # Aggregate: vlan 4/5 exist -> Add 6 + set_module_args(dict(aggregate=[ + {'name': '_5_', 'vlan_id': 5}, + {'name': '_6_', 'vlan_id': 6} + ])) + self.execute_module(changed=True, commands=[ + 'vlan 6', + 'name _6_', + 'state active', + 'no shutdown', + 'exit' + ]) + + def test_nxos_vlan_agg_2(self): + # Aggregate: vlan 4/5 exist -> Add none (idempotence) + set_module_args(dict(aggregate=[ + {'name': '_5_', 'vlan_id': 5}, + {'name': '_4_', 'vlan_id': 4} + ])) + self.execute_module(changed=False) + + def test_nxos_vlan_agg_3(self): + # Aggregate/Purge: vlan 4/5 exist -> Add 6, Purge 4 + set_module_args(dict(aggregate=[ + {'name': '_5_', 'vlan_id': 5}, + {'name': '_6_', 'vlan_id': 6} + ], purge=True)) + self.execute_module(changed=True, commands=[ + 'vlan 6', + 'name _6_', + 'state active', + 'no shutdown', + 'exit', + 'no vlan 4' + ]) + + def test_nxos_vlan_agg_4(self): + # Aggregate/Purge: vlan 4/5 exist -> Purge None (idempotence) + set_module_args(dict(aggregate=[ + {'name': '_5_', 'vlan_id': 5}, + {'name': '_4_', 'vlan_id': 4} + ])) + self.execute_module(changed=False) + + def test_nxos_vlan_agg_5(self): + # Purge with Single Vlan: vlan 4/5 exist -> Add 6, Purge 4/5 + set_module_args(dict(vlan_id=6, name='_6_', purge=True)) + self.execute_module(changed=True, commands=[ + 'vlan 6', + 'name _6_', + 'state active', + 'no shutdown', + 'exit', + 'no vlan 4', + 'no vlan 5' + ]) + + def test_nxos_vlan_agg_6(self): + # Purge All: vlan 4/5 exist -> Purge 4/5 + set_module_args(dict(vlan_id=1, purge=True)) + self.execute_module(changed=True, commands=[ + 'no vlan 4', + 'no vlan 5' + ]) + def test_nxos_vlan_range(self): set_module_args(dict(vlan_range='6-10')) self.execute_module(changed=True, commands=['vlan 6', 'vlan 7', 'vlan 8', 'vlan 9', 'vlan 10'])