fixes nxos_evpn_vni and unittest (#24372)

* fixes nxos_evpn_vni

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

* fixes pep8 issue and syntax error

* ansibot tole me to do this

* Unit test

lib/ansible/modules/network/nxos/nxos_evpn_vni.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 = '''
@@ -28,146 +30,116 @@ extends_documentation_fragment: nxos
 version_added: "2.2"
 short_description: Manages Cisco EVPN VXLAN Network Identifier (VNI).
 description:
-    - Manages Cisco Ethernet Virtual Private Network (EVPN) VXLAN Network
+  - Manages Cisco Ethernet Virtual Private Network (EVPN) VXLAN Network
-      Identifier (VNI) configurations of a Nexus device.
+    Identifier (VNI) configurations of a Nexus device.
 author: Gabriele Gerbino (@GGabriele)
 notes:
-    - default, where supported, restores params default value.
+  - default, where supported, restores params default value.
-    - RD override is not permitted. You should set it to the default values
+  - RD override is not permitted. You should set it to the default values
-      first and then reconfigure it.
+    first and then reconfigure it.
-    - C(route_target_both), C(route_target_import) and
+  - C(route_target_both), C(route_target_import) and
-      C(route_target_export valid) values are a list of extended communities,
+    C(route_target_export valid) values are a list of extended communities,
-      (i.e. ['1.2.3.4:5', '33:55']) or the keywords 'auto' or 'default'.
+    (i.e. ['1.2.3.4:5', '33:55']) or the keywords 'auto' or 'default'.
-    - The C(route_target_both) property is discouraged due to the inconsistent
+  - The C(route_target_both) property is discouraged due to the inconsistent
-      behavior of the property across Nexus platforms and image versions.
+    behavior of the property across Nexus platforms and image versions.
-      For this reason it is recommended to use explicit C(route_target_export)
+    For this reason it is recommended to use explicit C(route_target_export)
-      and C(route_target_import) properties instead of C(route_target_both).
+    and C(route_target_import) properties instead of C(route_target_both).
-    - RD valid values are a string in one of the route-distinguisher formats,
+  - RD valid values are a string in one of the route-distinguisher formats,
-      the keyword 'auto', or the keyword 'default'.
+    the keyword 'auto', or the keyword 'default'.
 options:
-    vni:
+  vni:
-        description:
+    description:
-            - The EVPN VXLAN Network Identifier.
+      - The EVPN VXLAN Network Identifier.
-        required: true
+    required: true
-        default: null
+    default: null
-    route_distinguisher:
+  route_distinguisher:
-        description:
+    description:
-            - The VPN Route Distinguisher (RD). The RD is combined with
+      - The VPN Route Distinguisher (RD). The RD is combined with
-              the IPv4 or IPv6 prefix learned by the PE router to create a
+        the IPv4 or IPv6 prefix learned by the PE router to create a
-              globally unique address.
+        globally unique address.
-        required: true
+    required: true
-        default: null
+    default: null
-    route_target_both:
+  route_target_both:
-        description:
+    description:
-            - Enables/Disables route-target settings for both import and
+      - Enables/Disables route-target settings for both import and
-              export target communities using a single property.
+        export target communities using a single property.
-        required: false
+    required: false
-        default: null
+    default: null
-    route_target_import:
+  route_target_import:
-        description:
+    description:
-            - Sets the route-target 'import' extended communities.
+      - Sets the route-target 'import' extended communities.
-        required: false
+    required: false
-        default: null
+    default: null
-    route_target_export:
+  route_target_export:
-        description:
+    description:
-            - Sets the route-target 'import' extended communities.
+      - Sets the route-target 'import' extended communities.
-        required: false
+    required: false
-        default: null
+    default: null
-    state:
+  state:
-        description:
+    description:
-            - Determines whether the config should be present or not
+      - Determines whether the config should be present or not
-              on the device.
+        on the device.
-        required: false
+    required: false
-        default: present
+    default: present
-        choices: ['present','absent']
+    choices: ['present','absent']
 '''
+
 EXAMPLES = '''
-- nxos_evpn_vni:
+- name: vni configuration
+  nxos_evpn_vni:
     vni: 6000
     route_distinguisher: "60:10"
     route_target_import:
-        - "5000:10"
+      - "5000:10"
-        - "4100:100"
+      - "4100:100"
     route_target_export: auto
     route_target_both: default
-    username: "{{ un }}"
-    password: "{{ pwd }}"
-    host: "{{ inventory_hostname }}"
 '''
 
 RETURN = '''
-proposed:
+commands:
-    description: k/v pairs of parameters passed into module
-    returned: verbose mode
-    type: dict
-    sample: {"route_target_import": ["5000:10", "4100:100",
-             "5001:10"],"vni": "6000"}
-existing:
-    description: k/v pairs of existing EVPN VNI configuration
-    returned: verbose mode
-    type: dict
-    sample: {"route_distinguisher": "70:10", "route_target_both": [],
-            "route_target_export": [], "route_target_import": [
-            "4100:100", "5000:10"], "vni": "6000"}
-end_state:
-    description: k/v pairs of EVPN VNI configuration after module execution
-    returned: verbose mode
-    type: dict
-    sample: {"route_distinguisher": "70:10", "route_target_both": [],
-             "route_target_export": [], "route_target_import": [
-             "4100:100", "5000:10", "5001:10"], "vni": "6000"}
-updates:
     description: commands sent to the device
     returned: always
     type: list
     sample: ["evpn", "vni 6000 l2", "route-target import 5001:10"]
-changed:
-    description: check to see if a change was made on the device
-    returned: always
-    type: boolean
-    sample: true
 '''
 
 import re
+import time
 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
 from ansible.module_utils.netcfg import CustomNetworkConfig
 
+
 PARAM_TO_COMMAND_KEYMAP = {
     'vni': 'vni',
+    'route_distinguisher': 'rd',
     'route_target_both': 'route-target both',
     'route_target_import': 'route-target import',
-    'route_target_export': 'route-target export',
+    'route_target_export': 'route-target export'
-    'route_distinguisher': 'rd'
 }
-WARNINGS = []
-
-import time
-
-def invoke(name, *args, **kwargs):
-    func = globals().get(name)
-    if func:
-        return func(*args, **kwargs)
 
 
 def get_value(arg, config, module):
-    REGEX = re.compile(r'(?:{0}\s)(?P<value>.*)$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M)
+    command = PARAM_TO_COMMAND_KEYMAP.get(arg)
+    command_re = re.compile(r'(?:{0}\s)(?P<value>.*)$'.format(command), re.M)
     value = ''
-    if PARAM_TO_COMMAND_KEYMAP[arg] in config:
+    if command in config:
-        value = REGEX.search(config).group('value')
+        value = command_re.search(config).group('value')
     return value
 
 
 def get_route_target_value(arg, config, module):
     splitted_config = config.splitlines()
     value_list = []
-    REGEX = re.compile(r'(?:{0}\s)(?P<value>.*)$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M)
+    command = PARAM_TO_COMMAND_KEYMAP.get(arg)
+    command_re = re.compile(r'(?:{0}\s)(?P<value>.*)$'.format(command), re.M)
 
     for line in splitted_config:
         value = ''
-        if PARAM_TO_COMMAND_KEYMAP[arg] in line.strip():
+        if command in line.strip():
-            value = REGEX.search(line).group('value')
+            value = command_re.search(line).group('value')
             value_list.append(value)
     return value_list
 
@@ -197,14 +169,10 @@ def get_existing(module, args):
 
 def apply_key_map(key_map, table):
     new_dict = {}
-    for key, value in table.items():
+    for key in table:
         new_key = key_map.get(key)
         if new_key:
-            value = table.get(key)
+            new_dict[new_key] = table.get(key)
-            if value:
-                new_dict[new_key] = value
-            else:
-                new_dict[new_key] = value
     return new_dict
 
 
@@ -236,26 +204,25 @@ def state_present(module, existing, proposed):
                 if existing_value:
                     for target in existing_value:
                         commands.append('no {0} {1}'.format(key, target))
-            else:
+            elif not isinstance(value, list):
-                if not isinstance(value, list):
+                value = [value]
-                    value = [value]
+            for target in value:
-                for target in value:
+                if existing:
-                    if existing:
+                    if target not in existing.get(key.replace('-', '_').replace(' ', '_')):
-                        if target not in existing.get(key.replace('-', '_').replace(' ', '_')):
-                            commands.append('{0} {1}'.format(key, target))
-                    else:
                         commands.append('{0} {1}'.format(key, target))
+                else:
+                    commands.append('{0} {1}'.format(key, target))
+        elif value == 'default':
+            existing_value = existing_commands.get(key)
+            if existing_value:
+                commands.append('no {0} {1}'.format(key, existing_value))
         else:
-            if value == 'default':
+            command = '{0} {1}'.format(key, value)
-                existing_value = existing_commands.get(key)
+            commands.append(command)
-                if existing_value:
-                    commands.append('no {0} {1}'.format(key, existing_value))
-            else:
-                command = '{0} {1}'.format(key, value)
-                commands.append(command)
 
-    if commands:
+    else:
-        parents = ['evpn', 'vni {0} l2'.format(module.params['vni'])]
+        commands = ['vni {0} l2'.format(module.params['vni'])]
+        parents = ['evpn']
 
     return commands, parents
 
@@ -266,13 +233,6 @@ def state_absent(module, existing, proposed):
     return commands, parents
 
 
-def execute_config(module, candidate):
-    result = {}
-    response = load_config(module, candidate)
-    result.update(response)
-    return result
-
-
 def main():
     argument_spec = dict(
         vni=dict(required=True, type='str'),
@@ -280,8 +240,7 @@ def main():
         route_target_both=dict(required=False, type='list'),
         route_target_import=dict(required=False, type='list'),
         route_target_export=dict(required=False, type='list'),
-        state=dict(choices=['present', 'absent'], default='present',
+        state=dict(choices=['present', 'absent'], default='present', required=False),
-                       required=False),
         include_defaults=dict(default=True),
         config=dict(),
         save=dict(type='bool', default=False)
@@ -289,26 +248,19 @@ def main():
 
     argument_spec.update(nxos_argument_spec)
 
-    module = AnsibleModule(argument_spec=argument_spec,
+    module = AnsibleModule(argument_spec=argument_spec, supports_check_mode=True)
-                        supports_check_mode=True)
 
     warnings = list()
     check_args(module, warnings)
-
+    results = dict(changed=False, warnings=warnings)
 
     state = module.params['state']
-    args =  [
+    args = PARAM_TO_COMMAND_KEYMAP.keys()
-        'vni',
+    existing = get_existing(module, args)
-        'route_distinguisher',
-        'route_target_both',
-        'route_target_import',
-        'route_target_export'
-    ]
-
-    existing = invoke('get_existing', module, args)
-    end_state = existing
     proposed_args = dict((k, v) for k, v in module.params.items()
-                    if v is not None and k in args)
+                         if v is not None and k in args)
+    commands = []
+    parents = []
 
     proposed = {}
     for key, value in proposed_args.items():
@@ -317,48 +269,42 @@ def main():
                 value = True
             elif value == 'false':
                 value = False
-            if existing.get(key) or (not existing.get(key) and value):
+            if existing.get(key) != value:
                 proposed[key] = value
-    result = {}
-    if state == 'present' or (state == 'absent' and existing):
-        candidate = CustomNetworkConfig(indent=3)
-        commands, parents = invoke('state_%s' % state, module, existing,
-                                                proposed)
-        if commands:
-            if (existing.get('route_distinguisher') and
-                    proposed.get('route_distinguisher')):
-                if (existing['route_distinguisher'] != proposed['route_distinguisher'] and
-                        proposed['route_distinguisher'] != 'default'):
-                    WARNINGS.append('EVPN RD {0} was automatically removed. '
-                                         'It is highly recommended to use a task '
-                                         '(with default as value) to explicitly '
-                                         'unconfigure it.'.format(
-                                             existing['route_distinguisher']))
-                    remove_commands = ['no rd {0}'.format(
-                        existing['route_distinguisher'])]
 
-                    candidate.add(remove_commands, parents=parents)
+    if state == 'present':
-                    result = execute_config(module, candidate)
+        commands, parents = state_present(module, existing, proposed)
-                    time.sleep(30)
+    elif state == 'absent' and existing:
+        commands, parents = state_absent(module, existing, proposed)
 
+    if commands:
+        if (existing.get('route_distinguisher') and
+                proposed.get('route_distinguisher')):
+            if (existing['route_distinguisher'] != proposed['route_distinguisher'] and
+                    proposed['route_distinguisher'] != 'default'):
+                warnings.append('EVPN RD {0} was automatically removed. '
+                                'It is highly recommended to use a task '
+                                '(with default as value) to explicitly '
+                                'unconfigure it.'.format(existing['route_distinguisher']))
+                remove_commands = ['no rd {0}'.format(existing['route_distinguisher'])]
+
+                candidate = CustomNetworkConfig(indent=3)
+                candidate.add(remove_commands, parents=parents)
+                load_config(module, candidate)
+                results['changed'] = True
+                results['commands'] = candidate.items_text()
+                time.sleep(30)
+
+        else:
             candidate = CustomNetworkConfig(indent=3)
             candidate.add(commands, parents=parents)
-            result = execute_config(module, candidate)
+            load_config(module, candidate)
+            results['changed'] = True
+            results['commands'] = candidate.items_text()
     else:
-        result['updates'] = []
+        results['commands'] = []
-
+    module.exit_json(**results)
-    if module._verbosity > 0:
-        end_state = invoke('get_existing', module, args)
-        result['end_state'] = end_state
-        result['existing'] = existing
-        result['proposed'] = proposed_args
-
-    if WARNINGS:
-        result['warnings'] = WARNINGS
-
-    module.exit_json(**result)
 
 
 if __name__ == '__main__':
     main()
-

test/sanity/pep8/legacy-files.txt

@@ -494,7 +494,6 @@ lib/ansible/modules/network/nxos/nxos_bgp_neighbor_af.py
 lib/ansible/modules/network/nxos/nxos_command.py
 lib/ansible/modules/network/nxos/nxos_config.py
 lib/ansible/modules/network/nxos/nxos_evpn_global.py
-lib/ansible/modules/network/nxos/nxos_evpn_vni.py
 lib/ansible/modules/network/nxos/nxos_facts.py
 lib/ansible/modules/network/nxos/nxos_feature.py
 lib/ansible/modules/network/nxos/nxos_gir.py

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

@@ -0,0 +1,59 @@
+# (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_evpn_vni
+from .nxos_module import TestNxosModule, load_fixture, set_module_args
+
+
+class TestNxosEvpnVniModule(TestNxosModule):
+
+    module = nxos_evpn_vni
+
+    def setUp(self):
+        self.mock_run_commands = patch('ansible.modules.network.nxos.nxos_evpn_vni.run_commands')
+        self.run_commands = self.mock_run_commands.start()
+
+        self.mock_load_config = patch('ansible.modules.network.nxos.nxos_evpn_vni.load_config')
+        self.load_config = self.mock_load_config.start()
+
+        self.mock_get_config = patch('ansible.modules.network.nxos.nxos_evpn_vni.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_evpn_vni_absent(self):
+        set_module_args(dict(vni='6000', state='absent'))
+        result = self.execute_module(changed=False)
+        self.assertEqual(result['commands'], [])
+
+    def test_nxos_evpn_vni_present(self):
+        set_module_args(dict(vni='6000', state='present'))
+        result = self.execute_module(changed=True)
+        self.assertEqual(result['commands'], ['evpn', 'vni 6000 l2'])