nxos_vlan tweaks (#23194)

* Remove commented code

Remove a lot of unnecessary output

* Tests

* Sort vlan ids _after_ running set operations, so order is not lost

lib/ansible/modules/network/nxos/nxos_vlan.py

@@ -16,9 +16,11 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-ANSIBLE_METADATA = {'metadata_version': '1.0',
-                    'status': ['preview'],
-                    'supported_by': 'community'}
+ANSIBLE_METADATA = {
+    'metadata_version': '1.0',
+    'status': ['preview'],
+    'supported_by': 'community',
+}
 
 
 DOCUMENTATION = '''
@@ -28,145 +30,87 @@ extends_documentation_fragment: nxos
 version_added: "2.1"
 short_description: Manages VLAN resources and attributes.
 description:
-    - Manages VLAN configurations on NX-OS switches.
+  - Manages VLAN configurations on NX-OS switches.
 author: Jason Edelman (@jedelman8)
 options:
-    vlan_id:
-        description:
-            - Single VLAN ID.
-        required: false
-        default: null
-    vlan_range:
-        description:
-            - Range of VLANs such as 2-10 or 2,5,10-15, etc.
-        required: false
-        default: null
-    name:
-        description:
-            - Name of VLAN.
-        required: false
-        default: null
-    vlan_state:
-        description:
-            - Manage the vlan operational state of the VLAN
-              (equivalent to state {active | suspend} command.
-        required: false
-        default: active
-        choices: ['active','suspend']
-    admin_state:
-        description:
-            - Manage the VLAN administrative state of the VLAN equivalent
-              to shut/no shut in VLAN config mode.
-        required: false
-        default: up
-        choices: ['up','down']
-    mapped_vni:
-        description:
-            - The Virtual Network Identifier (VNI) ID that is mapped to the
-              VLAN. Valid values are integer and keyword 'default'.
-        required: false
-        default: null
-        version_added: "2.2"
-    state:
-        description:
-            - Manage the state of the resource.
-        required: false
-        default: present
-        choices: ['present','absent']
+  vlan_id:
+    description:
+      - Single VLAN ID.
+    required: false
+    default: null
+  vlan_range:
+    description:
+      - Range of VLANs such as 2-10 or 2,5,10-15, etc.
+    required: false
+    default: null
+  name:
+    description:
+      - Name of VLAN.
+    required: false
+    default: null
+  vlan_state:
+    description:
+      - Manage the vlan operational state of the VLAN
+        (equivalent to state {active | suspend} command.
+    required: false
+    default: active
+    choices: ['active','suspend']
+  admin_state:
+    description:
+      - Manage the VLAN administrative state of the VLAN equivalent
+        to shut/no shut in VLAN config mode.
+    required: false
+    default: up
+    choices: ['up','down']
+  mapped_vni:
+    description:
+      - The Virtual Network Identifier (VNI) ID that is mapped to the
+        VLAN. Valid values are integer and keyword 'default'.
+    required: false
+    default: null
+    version_added: "2.2"
+  state:
+    description:
+      - Manage the state of the resource.
+    required: false
+    default: present
+    choices: ['present','absent']
 
 '''
 EXAMPLES = '''
 - name: Ensure a range of VLANs are not present on the switch
   nxos_vlan:
     vlan_range: "2-10,20,50,55-60,100-150"
-    host: 68.170.147.165
-    username: cisco
-    password: cisco
     state: absent
     transport: nxapi
 
 - name: Ensure VLAN 50 exists with the name WEB and is in the shutdown state
   nxos_vlan:
     vlan_id: 50
-    host: 68.170.147.165
     admin_state: down
     name: WEB
     transport: nxapi
-    username: cisco
-    password: cisco
 
 - name: Ensure VLAN is NOT on the device
   nxos_vlan:
     vlan_id: 50
-    host: 68.170.147.165
     state: absent
     transport: nxapi
-    username: cisco
-    password: cisco
 '''
 
 RETURN = '''
-
-proposed_vlans_list:
-    description: list of VLANs being proposed
-    returned: when debug enabled
-    type: list
-    sample: ["100"]
-existing_vlans_list:
-    description: list of existing VLANs on the switch prior to making changes
-    returned: when debug enabled
-    type: list
-    sample: ["1", "2", "3", "4", "5", "20"]
-end_state_vlans_list:
-    description: list of VLANs after the module is executed
-    returned: when debug enabled
-    type: list
-    sample:  ["1", "2", "3", "4", "5", "20", "100"]
-proposed:
-    description: k/v pairs of parameters passed into module (does not include
-                 vlan_id or vlan_range)
-    returned: when debug enabled
-    type: dict
-    sample: {"admin_state": "down", "name": "app_vlan",
-            "vlan_state": "suspend", "mapped_vni": "5000"}
-existing:
-    description: k/v pairs of existing vlan or null when using vlan_range
-    returned: when debug enabled
-    type: dict
-    sample: {"admin_state": "down", "name": "app_vlan",
-             "vlan_id": "20", "vlan_state": "suspend", "mapped_vni": ""}
-end_state:
-    description: k/v pairs of the VLAN after executing module or null
-                 when using vlan_range
-    returned: when debug enabled
-    type: dict
-    sample: {"admin_state": "down", "name": "app_vlan", "vlan_id": "20",
-             "vlan_state": "suspend", "mapped_vni": "5000"}
-updates:
-    description: command string sent to the device
-    returned: always
-    type: list
-    sample: ["vlan 20", "vlan 55", "vn-segment 5000"]
 commands:
-    description: command string sent to the device
+    description: Set of command strings to send to the remote device
     returned: always
     type: list
     sample: ["vlan 20", "vlan 55", "vn-segment 5000"]
-changed:
-    description: check to see if a change was made on the device
-    returned: always
-    type: boolean
-    sample: true
 '''
+import re
+
 from ansible.module_utils.nxos import get_config, load_config, run_commands
 from ansible.module_utils.nxos import nxos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
 
-import re
-
-from ansible.module_utils.nxos import nxos_argument_spec, check_args
-from ansible.module_utils.nxos import run_commands, load_config, get_config
-from ansible.module_utils.basic import AnsibleModule
 
 def vlan_range_to_list(vlans):
     result = []
@@ -175,25 +119,23 @@ def vlan_range_to_list(vlans):
             if part == 'none':
                 break
             if '-' in part:
-                a, b = part.split('-')
-                a, b = int(a), int(b)
-                result.extend(range(a, b + 1))
+                start, end = part.split('-')
+                start, end = int(start), int(end)
+                result.extend([str(i) for i in range(start, end + 1)])
             else:
-                a = int(part)
-                result.append(a)
-        return numerical_sort(result)
+                result.append(part)
     return result
 
 
-def numerical_sort(string_int_list):
+def numerical_sort(iterable):
     """Sort list of strings (VLAN IDs) that are digits in numerical order.
     """
-
     as_int_list = []
-    as_str_list = []
-    for vlan in string_int_list:
+    for vlan in iterable:
         as_int_list.append(int(vlan))
     as_int_list.sort()
+
+    as_str_list = []
     for vlan in as_int_list:
         as_str_list.append(str(vlan))
     return as_str_list
@@ -269,8 +211,7 @@ def get_list_of_vlans(module):
 def get_vni(vlanid, module):
     flags = str('all | section vlan.{0}'.format(vlanid)).split(' ')
     body = get_config(module, flags=flags)
-    #command = 'show run all | section vlan.{0}'.format(vlanid)
-    #body = execute_show_command(command, module, command_type='cli_show_ascii')[0]
+
     value = ''
     if body:
         REGEX = re.compile(r'(?:vn-segment\s)(?P<value>.*)$', re.M)
@@ -285,9 +226,6 @@ def get_vlan(vlanid, module):
     command = 'show vlan id %s | json' % vlanid
     body = run_commands(module, [command])
 
-    #command = 'show vlan id ' + vlanid
-    #body = execute_show_command(command, module)
-
     try:
         vlan_table = body[0]['TABLE_vlanbriefid']['ROW_vlanbriefid']
     except (TypeError, IndexError):
@@ -328,6 +266,7 @@ def apply_value_map(value_map, resource):
         resource[key] = value[resource.get(key)]
     return resource
 
+
 def main():
     argument_spec = dict(
         vlan_id=dict(required=False, type='str'),
@@ -335,17 +274,15 @@ def main():
         name=dict(required=False),
         vlan_state=dict(choices=['active', 'suspend'], required=False),
         mapped_vni=dict(required=False, type='str'),
-        state=dict(choices=['present', 'absent'], default='present',
-                       required=False),
+        state=dict(choices=['present', 'absent'], default='present', required=False),
         admin_state=dict(choices=['up', 'down'], required=False),
+
+        # Deprecated in Ansible 2.4
         include_defaults=dict(default=False),
         config=dict(),
         save=dict(type='bool', default=False)
     )
 
-    argument_spec.update(nxos_argument_spec)
-
-
     argument_spec.update(nxos_argument_spec)
 
     module = AnsibleModule(argument_spec=argument_spec,
@@ -356,10 +293,6 @@ def main():
     warnings = list()
     check_args(module, warnings)
 
-
-    warnings = list()
-    check_args(module, warnings)
-
     vlan_range = module.params['vlan_range']
     vlan_id = module.params['vlan_id']
     name = module.params['name']
@@ -379,9 +312,8 @@ def main():
 
     proposed = dict((k, v) for k, v in args.items() if v is not None)
 
-    proposed_vlans_list = numerical_sort(vlan_range_to_list(
-        vlan_id or vlan_range))
-    existing_vlans_list = numerical_sort(get_list_of_vlans(module))
+    proposed_vlans_list = vlan_range_to_list(vlan_id or vlan_range)
+    existing_vlans_list = get_list_of_vlans(module)
     commands = []
     existing = {}
 
@@ -389,20 +321,19 @@ def main():
         if state == 'present':
             # These are all of the VLANs being proposed that don't
             # already exist on the switch
-            vlans_delta = list(
+            vlans_delta = numerical_sort(
                 set(proposed_vlans_list).difference(existing_vlans_list))
             commands = build_commands(vlans_delta, state)
         elif state == 'absent':
             # VLANs that are common between what is being proposed and
             # what is on the switch
-            vlans_common = list(
+            vlans_common = numerical_sort(
                 set(proposed_vlans_list).intersection(existing_vlans_list))
             commands = build_commands(vlans_common, state)
     else:
         existing = get_vlan(vlan_id, module)
-        if state == 'absent':
-            if existing:
-                commands = ['no vlan ' + vlan_id]
+        if state == 'absent' and existing:
+            commands = ['no vlan ' + vlan_id]
         elif state == 'present':
             if (existing.get('mapped_vni') == '0' and
                     proposed.get('mapped_vni') == 'default'):
@@ -411,11 +342,8 @@ def main():
             if delta or not existing:
                 commands = get_vlan_config_commands(delta, vlan_id)
 
-    end_state = existing
-    end_state_vlans_list = existing_vlans_list
-
     if commands:
-        if existing.get('mapped_vni') and state != 'absent':
+        if existing.get('mapped_vni'):
             if (existing.get('mapped_vni') != proposed.get('mapped_vni') and
                     existing.get('mapped_vni') != '0' and proposed.get('mapped_vni') != 'default'):
                 commands.insert(1, 'no vn-segment')
@@ -425,32 +353,15 @@ def main():
         else:
             load_config(module, commands)
             changed = True
-            end_state_vlans_list = numerical_sort(get_list_of_vlans(module))
-            if 'configure' in commands:
-                commands.pop(0)
-            if vlan_id:
-                end_state = get_vlan(vlan_id, module)
 
     results = {
         'commands': commands,
-        'updates': commands,
         'changed': changed,
         'warnings': warnings
     }
 
-    if module._debug:
-        results.update({
-            'proposed_vlans_list': proposed_vlans_list,
-            'existing_vlans_list': existing_vlans_list,
-            'proposed': proposed,
-            'existing': existing,
-            'end_state': end_state,
-            'end_state_vlans_list': end_state_vlans_list
-        })
-
     module.exit_json(**results)
 
 
 if __name__ == '__main__':
     main()
-

test/sanity/pep8/legacy-files.txt

@@ -601,7 +601,6 @@ lib/ansible/modules/network/nxos/nxos_system.py
 lib/ansible/modules/network/nxos/nxos_udld.py
 lib/ansible/modules/network/nxos/nxos_udld_interface.py
 lib/ansible/modules/network/nxos/nxos_user.py
-lib/ansible/modules/network/nxos/nxos_vlan.py
 lib/ansible/modules/network/nxos/nxos_vpc.py
 lib/ansible/modules/network/nxos/nxos_vpc_interface.py
 lib/ansible/modules/network/nxos/nxos_vrf.py

test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan.txt

@@ -0,0 +1,18 @@
+{
+  "TABLE_vlanbrief": {
+    "ROW_vlanbrief": {
+      "vlanshowbr-vlanid": 16777216, 
+      "vlanshowbr-vlanid-utf": 1, 
+      "vlanshowbr-vlanname": "default", 
+      "vlanshowbr-vlanstate": "active", 
+      "vlanshowbr-shutstate": "noshutdown"
+    }
+  }, 
+  "TABLE_mtuinfo": {
+    "ROW_mtuinfo": {
+      "vlanshowinfo-vlanid": 1, 
+      "vlanshowinfo-media-type": "enet", 
+      "vlanshowinfo-vlanmode": "ce-vlan"
+    }
+  }
+}

test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan_id_1.txt

@@ -0,0 +1,20 @@
+{
+  "TABLE_vlanbriefid": {
+    "ROW_vlanbriefid": {
+      "vlanshowbr-vlanid": 16777216, 
+      "vlanshowbr-vlanid-utf": 1, 
+      "vlanshowbr-vlanname": "default", 
+      "vlanshowbr-vlanstate": "active", 
+      "vlanshowbr-shutstate": "noshutdown"
+    }
+  }, 
+  "TABLE_mtuinfoid": {
+    "ROW_mtuinfoid": {
+      "vlanshowinfo-vlanid": 1, 
+      "vlanshowinfo-media-type": "enet", 
+      "vlanshowinfo-vlanmode": "ce-vlan"
+    }
+  }, 
+  "vlanshowrspan-vlantype": "notrspan", 
+  "is-vtp-manageable": "enabled"
+}

test/units/modules/network/nxos/test_nxos_vlan.py

@@ -0,0 +1,99 @@
+# (c) 2016 Red Hat Inc.
+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import json
+
+from ansible.compat.tests.mock import patch
+from ansible.modules.network.nxos import nxos_vlan
+from .nxos_module import TestNxosModule, load_fixture, set_module_args
+
+
+class TestNxosVlanModule(TestNxosModule):
+
+    module = nxos_vlan
+
+    def setUp(self):
+        self.mock_run_commands = patch('ansible.modules.network.nxos.nxos_vlan.run_commands')
+        self.run_commands = self.mock_run_commands.start()
+
+        self.mock_load_config = patch('ansible.modules.network.nxos.nxos_vlan.load_config')
+        self.load_config = self.mock_load_config.start()
+
+        self.mock_get_config = patch('ansible.modules.network.nxos.nxos_vlan.get_config')
+        self.get_config = self.mock_get_config.start()
+
+    def tearDown(self):
+        self.mock_run_commands.stop()
+        self.mock_load_config.stop()
+
+    def load_fixtures(self, commands=None):
+        def load_from_file(*args, **kwargs):
+            module, commands = args
+            output = list()
+
+            for item in commands:
+                try:
+                    obj = json.loads(item)
+                    command = obj['command']
+                except ValueError:
+                    command = item
+                filename = str(command).split(' | ')[0].replace(' ', '_')
+                filename = 'nxos_vlan/%s.txt' % filename
+                output.append(load_fixture(filename))
+            return output
+
+        self.run_commands.side_effect = load_from_file
+        self.load_config.return_value = None
+
+    def test_nxos_vlan_range(self):
+        set_module_args(dict(vlan_range='6-10'))
+        result = self.execute_module(changed=True)
+        self.assertEqual(result['commands'], ['vlan 6', 'vlan 7', 'vlan 8', 'vlan 9', 'vlan 10'])
+
+    def test_nxos_vlan_range_absent(self):
+        set_module_args(dict(vlan_range='1-5', state='absent'))
+        result = self.execute_module(changed=True)
+        self.assertEqual(result['commands'], ['no vlan 1'])
+
+    def test_nxos_vlan_id(self):
+        set_module_args(dict(vlan_id='15', state='present'))
+        result = self.execute_module(changed=True)
+        self.assertEqual(result['commands'], ['vlan 15', 'exit'])
+
+    def test_nxos_vlan_id_absent(self):
+        set_module_args(dict(vlan_id='1', state='absent'))
+        result = self.execute_module(changed=True)
+        self.assertEqual(result['commands'], ['no vlan 1'])
+
+    def test_nxos_vlan_named_vlan(self):
+        set_module_args(dict(vlan_id='15', name='WEB'))
+        result = self.execute_module(changed=True)
+        self.assertEqual(result['commands'], ['vlan 15', 'name WEB', 'exit'])
+
+    def test_nxos_vlan_shut_down(self):
+        set_module_args(dict(vlan_id='1', admin_state='down'))
+        result = self.execute_module(changed=True)
+        self.assertEqual(result['commands'], ['vlan 1', 'shutdown', 'exit'])
+
+    def test_nxos_vlan_no_change(self):
+        set_module_args(dict(vlan_id='1', name='default', vlan_state='active', admin_state='up'))
+        result = self.execute_module(changed=False)
+        self.assertEqual(result['commands'], [])