Refactor nxos_interface and add unit test (#24008)

* [WIP] Refactor nxos_interface

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Modification in refactor
Unittest for nxos_interface

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

@@ -16,9 +16,11 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-ANSIBLE_METADATA = {'metadata_version': '1.0',
+ANSIBLE_METADATA = {
-                    'status': ['preview'],
+    'metadata_version': '1.0',
-                    'supported_by': 'community'}
+    'status': ['preview'],
+    'supported_by': 'community'
+}
 
 
 DOCUMENTATION = '''
@@ -31,63 +33,63 @@ description:
     - Manages physical attributes of interfaces of NX-OS switches.
 author: Jason Edelman (@jedelman8)
 notes:
-    - This module is also used to create logical interfaces such as
+  - This module is also used to create logical interfaces such as
-      svis and loopbacks.
+    svis and loopbacks.
-    - Be cautious of platform specific idiosyncrasies. For example,
+  - Be cautious of platform specific idiosyncrasies. For example,
-      when you default a loopback interface, the admin state toggles
+    when you default a loopback interface, the admin state toggles
-      on certain versions of NX-OS.
+    on certain versions of NX-OS.
-    - The M(nxos_overlay_global) C(anycast_gateway_mac) attribute must be
+  - The M(nxos_overlay_global) C(anycast_gateway_mac) attribute must be
-      set before setting the C(fabric_forwarding_anycast_gateway) property.
+    set before setting the C(fabric_forwarding_anycast_gateway) property.
 options:
-    interface:
+  interface:
-        description:
-            - Full name of interface, i.e. Ethernet1/1, port-channel10.
-        required: true
-        default: null
-    interface_type:
-        description:
-            - Interface type to be unconfigured from the device.
-        required: false
-        default: null
-        choices: ['loopback', 'portchannel', 'svi', 'nve']
-        version_added: "2.2"
-    admin_state:
-        description:
-            - Administrative state of the interface.
-        required: false
-        default: up
-        choices: ['up','down']
     description:
-        description:
+      - Full name of interface, i.e. Ethernet1/1, port-channel10.
-            - Interface description.
+    required: true
-        required: false
+    default: null
-        default: null
+  interface_type:
-    mode:
+    description:
-        description:
+      - Interface type to be unconfigured from the device.
-            - Manage Layer 2 or Layer 3 state of the interface.
+    required: false
-        required: false
+    default: null
-        default: null
+    choices: ['loopback', 'portchannel', 'svi', 'nve']
-        choices: ['layer2','layer3']
+    version_added: "2.2"
-    ip_forward:
+  admin_state:
-        description:
+    description:
-            - Enable/Disable ip forward feature on SVIs.
+      - Administrative state of the interface.
-        required: false
+    required: false
-        default: null
+    default: up
-        choices: ['enable','disable']
+    choices: ['up','down']
-        version_added: "2.2"
+  description:
-    fabric_forwarding_anycast_gateway:
+    description:
-        description:
+      - Interface description.
-            - Associate SVI with anycast gateway under VLAN configuration mode.
+    required: false
-        required: false
+    default: null
-        default: null
+  mode:
-        choices: ['true','false']
+    description:
-        version_added: "2.2"
+      - Manage Layer 2 or Layer 3 state of the interface.
-    state:
+    required: false
-        description:
+    default: null
-            - Specify desired state of the resource.
+    choices: ['layer2','layer3']
-        required: true
+  ip_forward:
-        default: present
+    description:
-        choices: ['present','absent','default']
+      - Enable/Disable ip forward feature on SVIs.
+    required: false
+    default: null
+    choices: ['enable','disable']
+    version_added: "2.2"
+  fabric_forwarding_anycast_gateway:
+    description:
+      - Associate SVI with anycast gateway under VLAN configuration mode.
+    required: false
+    default: null
+    choices: ['true','false']
+    version_added: "2.2"
+  state:
+    description:
+      - Specify desired state of the resource.
+    required: true
+    default: present
+    choices: ['present','absent','default']
 '''
 
 EXAMPLES = '''
@@ -135,35 +137,11 @@ EXAMPLES = '''
 '''
 
 RETURN = '''
-proposed:
+commands:
-    description: k/v pairs of parameters passed into module
-    returned: always
-    type: dict
-    sample: {"admin_state": "down"}
-existing:
-    description: k/v pairs of existing switchport
-    returned: always
-    type: dict
-    sample:  {"admin_state": "up", "description": "None",
-              "interface": "port-channel101", "mode": "layer2",
-              "type": "portchannel", "ip_forward": "enable"}
-end_state:
-    description: k/v pairs of switchport after module execution
-    returned: always
-    type: dict
-    sample:  {"admin_state": "down", "description": "None",
-              "interface": "port-channel101", "mode": "layer2",
-              "type": "portchannel", "ip_forward": "enable"}
-updates:
     description: command list sent to the device
     returned: always
     type: list
     sample: ["interface port-channel101", "shutdown"]
-changed:
-    description: check to see if a change was made on the device
-    returned: always
-    type: boolean
-    sample: true
 '''
 
 from ansible.module_utils.nxos import get_config, load_config, run_commands
@@ -183,11 +161,10 @@ def is_default_interface(interface, module):
         False: if it does not have a default config
         DNE (str): if the interface does not exist - loopbacks, SVIs, etc.
     """
-    command = 'show run interface ' + interface
+    command = 'show run interface {0}'.format(interface)
 
     try:
-        body = execute_show_command(command, module,
+        body = execute_show_command(command, module)[0]
-                                    command_type='cli_show_ascii')[0]
     except IndexError:
         body = ''
 
@@ -242,10 +219,10 @@ def get_manual_interface_attributes(interface, module):
     """
 
     if get_interface_type(interface) == 'svi':
-        command = 'show interface ' + interface
+        command = 'show interface {0}'.format(interface)
         try:
-            body = execute_modified_show_for_cli_text(command, module)[0]
+            body = execute_show_command(command, module)[0]
-        except (IndexError, ShellError):
+        except IndexError:
             return None
 
         command_list = body.split('\n')
@@ -299,7 +276,7 @@ def get_interface(intf, module):
     key_map = {}
     interface = {}
 
-    command = 'show interface ' + intf
+    command = 'show interface {0}'.format(intf)
     try:
         body = execute_show_command(command, module)[0]
     except IndexError:
@@ -328,9 +305,8 @@ def get_interface(intf, module):
                                                           'nxapibug'))
             interface['description'] = str(attributes.get('description',
                                                           'nxapi_bug'))
-            command = 'show run interface ' + intf
+            command = 'show run interface {0}'.format(intf)
-            body = execute_show_command(command, module,
+            body = execute_show_command(command, module)[0]
-                                        command_type='cli_show_ascii')[0]
             if 'ip forward' in body:
                 interface['ip_forward'] = 'enable'
             else:
@@ -413,7 +389,7 @@ def get_interfaces_dict(module):
 
     interface_list = body.get('TABLE_interface')['ROW_interface']
     for index in interface_list:
-        intf = index ['interface']
+        intf = index['interface']
         intf_type = get_interface_type(intf)
 
         interfaces[intf_type].append(intf)
@@ -521,12 +497,13 @@ def get_interface_config_commands(interface, intf, existing):
             commands.append('no fabric forwarding mode anycast-gateway')
 
     if commands:
-        commands.insert(0, 'interface ' + intf)
+        commands.insert(0, 'interface {0}'.format(intf))
 
     return commands
 
 
 def get_admin_state(interface, intf, admin_state):
+    command = ''
     if admin_state == 'up':
         command = 'no shutdown'
     elif admin_state == 'down':
@@ -568,24 +545,11 @@ def smart_existing(module, intf_type, normalized_interface):
     return existing, is_default
 
 
-def execute_show_command(command, module, command_type='cli_show'):
+def execute_show_command(command, module):
     if module.params['transport'] == 'cli':
         command += ' | json'
-        cmds = [command]
-        body = run_commands(module, cmds)
-    elif module.params['transport'] == 'nxapi':
-        cmds = [{'command': command, 'output': 'json'}]
-        body = run_commands(module, cmds)
-    return body
-
-
-def execute_modified_show_for_cli_text(command, module):
     cmds = [command]
-    if module.params['transport'] == 'cli':
+    body = run_commands(module, cmds)
-        body = run_commands(module, cmds)
-    else:
-        body = run_commands(module, cmds)
-    body = response
     return body
 
 
@@ -616,12 +580,10 @@ def main():
         admin_state=dict(default='up', choices=['up', 'down'], required=False),
         description=dict(required=False, default=None),
         mode=dict(choices=['layer2', 'layer3'], required=False),
-        interface_type=dict(required=False,
+        interface_type=dict(required=False, choices=['loopback', 'portchannel', 'svi', 'nve']),
-                            choices=['loopback', 'portchannel', 'svi', 'nve']),
         ip_forward=dict(required=False, choices=['enable', 'disable']),
         fabric_forwarding_anycast_gateway=dict(required=False, type='bool'),
-        state=dict(choices=['absent', 'present', 'default'],
+        state=dict(choices=['absent', 'present', 'default'], default='present', required=False),
-                   default='present', required=False),
         include_defaults=dict(default=True),
         config=dict(),
         save=dict(type='bool', default=False)
@@ -630,12 +592,14 @@ def main():
     argument_spec.update(nxos_argument_spec)
 
     module = AnsibleModule(argument_spec=argument_spec,
-                                mutually_exclusive=[['interface', 'interface_type']],
+                           mutually_exclusive=[['interface', 'interface_type']],
-                                supports_check_mode=True)
+                           supports_check_mode=True)
 
     warnings = list()
     check_args(module, warnings)
 
+    results = dict(changed=False, warnings=warnings)
+
     interface = module.params['interface']
     interface_type = module.params['interface_type']
     admin_state = module.params['admin_state']
@@ -658,8 +622,7 @@ def main():
                 module.fail_json(msg='description and mode params are not '
                                      'supported in this module. Use '
                                      'nxos_vxlan_vtep instead.')
-        if ((ip_forward or fabric_forwarding_anycast_gateway) and
+        if (ip_forward or fabric_forwarding_anycast_gateway) and intf_type != 'svi':
-             intf_type != 'svi'):
             module.fail_json(msg='The ip_forward and '
                                  'fabric_forwarding_anycast_gateway features '
                                  ' are only available for SVIs.')
@@ -668,9 +631,8 @@ def main():
                     fabric_forwarding_anycast_gateway=fabric_forwarding_anycast_gateway)
 
         if intf_type == 'unknown':
-            module.fail_json(
+            module.fail_json(msg='unknown interface type found-1',
-                msg='unknown interface type found-1',
+                             interface=interface)
-                interface=interface)
 
         existing, is_default = smart_existing(module, intf_type, normalized_interface)
         proposed = get_proposed(existing, normalized_interface, args)
@@ -678,7 +640,6 @@ def main():
         intf_type = normalized_interface = interface_type
         proposed = dict(interface_type=interface_type)
 
-    changed = False
     commands = []
     if interface:
         delta = dict()
@@ -694,29 +655,22 @@ def main():
                     commands.append(cmds)
         elif state == 'present':
             if not existing:
-                cmds = get_interface_config_commands(proposed,
+                cmds = get_interface_config_commands(proposed, normalized_interface, existing)
-                                                     normalized_interface,
-                                                     existing)
                 commands.append(cmds)
             else:
-                delta = dict(set(proposed.items()).difference(
+                delta = dict(set(proposed.items()).difference(existing.items()))
-                    existing.items()))
                 if delta:
-                    cmds = get_interface_config_commands(delta,
+                    cmds = get_interface_config_commands(delta, normalized_interface, existing)
-                                                         normalized_interface,
-                                                         existing)
                     commands.append(cmds)
         elif state == 'default':
             if is_default is False:
                 cmds = ['default interface {0}'.format(normalized_interface)]
                 commands.append(cmds)
             elif is_default == 'DNE':
-                module.exit_json(msg='interface you are trying to default does'
+                module.exit_json(msg='interface you are trying to default does not exist')
-                                     ' not exist')
     elif interface_type:
         if state == 'present':
-            module.fail_json(msg='The interface_type param can be used '
+            module.fail_json(msg='The interface_type param can be used only with state absent.')
-                                 'only with state absent.')
 
         existing = get_interfaces_dict(module)[interface_type]
         cmds = get_interface_type_removed_cmds(existing)
@@ -730,7 +684,7 @@ def main():
             module.exit_json(changed=True, commands=cmds)
         else:
             load_config(module, cmds)
-            changed = True
+            results['changed'] = True
             if module.params['interface']:
                 if delta.get('mode'): # or delta.get('admin_state'):
                     # if the mode changes from L2 to L3, the admin state
@@ -748,17 +702,10 @@ def main():
                 end_state = get_interfaces_dict(module)[interface_type]
             cmds = [cmd for cmd in cmds if cmd != 'configure']
 
-    results = {}
+    results['commands'] = cmds
-    results['proposed'] = proposed
-    results['existing'] = existing
-    results['end_state'] = end_state
-    results['updates'] = cmds
-    results['changed'] = changed
-    results['warnings'] = warnings
 
     module.exit_json(**results)
 
 
 if __name__ == '__main__':
     main()
-

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

@@ -0,0 +1,64 @@
+# (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_interface
+from .nxos_module import TestNxosModule, load_fixture, set_module_args
+
+
+class TestNxosInterfaceModule(TestNxosModule):
+
+    module = nxos_interface
+
+    def setUp(self):
+        self.mock_run_commands = patch('ansible.modules.network.nxos.nxos_interface.run_commands')
+        self.run_commands = self.mock_run_commands.start()
+
+        self.mock_load_config = patch('ansible.modules.network.nxos.nxos_interface.load_config')
+        self.load_config = self.mock_load_config.start()
+
+        self.mock_get_config = patch('ansible.modules.network.nxos.nxos_interface.get_config')
+        self.get_config = self.mock_get_config.start()
+
+    def tearDown(self):
+        self.mock_run_commands.stop()
+        self.mock_load_config.stop()
+        self.mock_get_config.stop()
+
+    def load_fixtures(self, commands=None):
+        self.load_config.return_value = None
+
+    def test_nxos_interface_up(self):
+        set_module_args(dict(interface='loopback0'))
+        result = self.execute_module(changed=True)
+        self.assertEqual(result['commands'], ['interface loopback0', 'no shutdown'])
+
+    def test_nxos_interface_down(self):
+        set_module_args(dict(interface='loopback0', admin_state='down'))
+        result = self.execute_module(changed=True)
+        self.assertEqual(result['commands'], ['interface loopback0', 'shutdown'])
+
+    def test_nxos_interface_delete(self):
+        set_module_args(dict(interface='loopback0', state='absent'))
+        result = self.execute_module(changed=False)
+        self.assertEqual(result['commands'], [])