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

@@ -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']))

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"

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

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"
+      }
+    ]
+  }
+}

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'])