nxos_bgp_neighbor cleanup (#24446)

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

@@ -16,9 +16,11 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-ANSIBLE_METADATA = {'metadata_version': '1.0',
+ANSIBLE_METADATA = {
+    'metadata_version': '1.0',
     'status': ['preview'],
-                    'supported_by': 'community'}
+    'supported_by': 'community',
+}
 
 
 DOCUMENTATION = '''
@@ -198,62 +200,26 @@ EXAMPLES = '''
     update_source: Ethernet1/3
     shutdown: default
     state: present
-    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: {"asn": "65535", "description": "just a description",
-            "local_as": "20", "neighbor": "3.3.3.3",
-            "remote_as": "30", "shutdown": "default",
-            "update_source": "Ethernet1/3", "vrf": "default"}
-existing:
-    description: k/v pairs of existing BGP neighbor configuration
-    returned: verbose mode
-    type: dict
-    sample: {}
-end_state:
-    description: k/v pairs of BGP neighbor configuration after module execution
-    returned: verbose mode
-    type: dict
-    sample: {"asn": "65535", "capability_negotiation": false,
-            "connected_check": false, "description": "just a description",
-            "dynamic_capability": true, "ebgp_multihop": "",
-            "local_as": "20", "log_neighbor_changes": "",
-            "low_memory_exempt": false, "maximum_peers": "",
-            "neighbor": "3.3.3.3", "pwd": "",
-            "pwd_type": "", "remote_as": "30",
-            "remove_private_as": "disable", "shutdown": false,
-            "suppress_4_byte_as": false, "timers_holdtime": "180",
-            "timers_keepalive": "60", "transport_passive_only": false,
-            "update_source": "Ethernet1/3", "vrf": "default"}
-updates:
   description: commands sent to the device
   returned: always
   type: list
   sample: ["router bgp 65535", "neighbor 3.3.3.3",
            "remote-as 30", "update-source Ethernet1/3",
            "description just a description", "local-as 20"]
-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 get_config, load_config
 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
 
 
-WARNINGS = []
 BOOL_PARAMS = [
     'capability_negotiation',
     'shutdown',
@@ -276,13 +242,13 @@ PARAM_TO_COMMAND_KEYMAP = {
     'maximum_peers': 'maximum-peers',
     'neighbor': 'neighbor',
     'pwd': 'password',
-    'pwd_type': 'password-type',
+    'pwd_type': 'password',
     'remote_as': 'remote-as',
     'remove_private_as': 'remove-private-as',
     'shutdown': 'shutdown',
     'suppress_4_byte_as': 'capability suppress 4-byte-as',
-    'timers_keepalive': 'timers-keepalive',
+    'timers_keepalive': 'timers',
-    'timers_holdtime': 'timers-holdtime',
+    'timers_holdtime': 'timers',
     'transport_passive_only': 'transport connection-mode passive',
     'update_source': 'update-source',
     'vrf': 'vrf'
@@ -294,136 +260,89 @@ PARAM_TO_DEFAULT_KEYMAP = {
     'timers_holdtime': 180
 }
 
-def invoke(name, *args, **kwargs):
-    func = globals().get(name)
-    if func:
-        return func(*args, **kwargs)
 
+def get_value(arg, config):
+    command = PARAM_TO_COMMAND_KEYMAP[arg]
+    has_command = re.search(r'\s+{0}\s*$'.format(command), config, re.M)
+    has_command_value = re.search(r'(?:{0}\s)(?P<value>.*)$'.format(command), config, re.M)
 
-def get_value(arg, config, module):
     if arg in BOOL_PARAMS:
-        REGEX = re.compile(r'\s+{0}\s*$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M)
         value = False
         try:
-            if REGEX.search(config):
+            if has_command:
                 value = True
         except TypeError:
             value = False
-
+    elif arg == 'log_neighbor_changes':
-    else:
-        REGEX = re.compile(r'(?:{0}\s)(?P<value>.*)$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M)
-        value = ''
-        if PARAM_TO_COMMAND_KEYMAP[arg] in config:
-            value = REGEX.search(config).group('value')
-    return value
-
-
-def get_custom_value(arg, config, module):
         value = ''
-    splitted_config = config.splitlines()
+        if has_command:
-
+            if has_command_value:
-    if arg == 'log_neighbor_changes':
-        for line in splitted_config:
-            if 'log-neighbor-changes' in line:
-                if 'disable' in line:
                 value = 'disable'
             else:
                 value = 'enable'
 
-    elif arg == 'pwd':
-        for line in splitted_config:
-            if 'password' in line:
-                splitted_line = line.split()
-                value = splitted_line[2]
-
-    elif arg == 'pwd_type':
-        for line in splitted_config:
-            if 'password' in line:
-                splitted_line = line.split()
-                value = splitted_line[1]
-
     elif arg == 'remove_private_as':
         value = 'disable'
-        for line in splitted_config:
+        if has_command:
-            if 'remove-private-as' in line:
+            if has_command_value:
-                splitted_line = line.split()
+                value = has_command_value.group('value')
-                if len(splitted_line) == 1:
+            else:
                 value = 'enable'
-                elif len(splitted_line) == 2:
+    else:
-                    value = splitted_line[1]
-
-    elif arg == 'timers_keepalive':
-        REGEX = re.compile(r'(?:timers\s)(?P<value>.*)$', re.M)
         value = ''
-        if 'timers' in config:
-            parsed = REGEX.search(config).group('value').split()
-            value = parsed[0]
 
-    elif arg == 'timers_holdtime':
+        if has_command_value:
-        REGEX = re.compile(r'(?:timers\s)(?P<value>.*)$', re.M)
+            value = has_command_value.group('value')
+
+            if command in ['timers', 'password']:
+                split_value = value.split()
                 value = ''
-        if 'timers' in config:
+
-            parsed = REGEX.search(config).group('value').split()
+                if arg in ['timers_keepalive', 'pwd_type']:
-            if len(parsed) == 2:
+                    value = split_value[0]
-                value = parsed[1]
+                elif arg in ['timers_holdtime', 'pwd'] and len(split_value) == 2:
+                    value = split_value[1]
 
     return value
 
 
-def get_existing(module, args):
+def get_existing(module, args, warnings):
     existing = {}
     netcfg = CustomNetworkConfig(indent=2, contents=get_config(module))
-    custom = [
+
-        'log_neighbor_changes',
+    asn_regex = re.compile(r'.*router\sbgp\s(?P<existing_asn>\d+).*', re.S)
-        'pwd',
+    match_asn = asn_regex.match(str(netcfg))
-        'pwd_type',
+
-        'remove_private_as',
+    if match_asn:
-        'timers_holdtime',
+        existing_asn = match_asn.group('existing_asn')
-        'timers_keepalive'
-    ]
-    try:
-        asn_regex = '.*router\sbgp\s(?P<existing_asn>\d+).*'
-        match_asn = re.match(asn_regex, str(netcfg), re.DOTALL)
-        existing_asn_group = match_asn.groupdict()
-        existing_asn = existing_asn_group['existing_asn']
-    except AttributeError:
-        existing_asn = ''
-
-    if existing_asn:
         parents = ["router bgp {0}".format(existing_asn)]
+
         if module.params['vrf'] != 'default':
             parents.append('vrf {0}'.format(module.params['vrf']))
 
         parents.append('neighbor {0}'.format(module.params['neighbor']))
         config = netcfg.get_section(parents)
-
         if config:
             for arg in args:
                 if arg not in ['asn', 'vrf', 'neighbor']:
-                    if arg in custom:
+                    existing[arg] = get_value(arg, config)
-                        existing[arg] = get_custom_value(arg, config, module)
-                    else:
-                        existing[arg] = get_value(arg, config, module)
 
             existing['asn'] = existing_asn
             existing['neighbor'] = module.params['neighbor']
             existing['vrf'] = module.params['vrf']
     else:
-        WARNINGS.append("The BGP process didn't exist but the task"
+        warnings.append("The BGP process didn't exist but the task"
                         " just created it.")
     return existing
 
 
 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
 
 
@@ -435,10 +354,8 @@ def state_present(module, existing, proposed, candidate):
     for key, value in proposed_commands.items():
         if value is True:
             commands.append(key)
-
         elif value is False:
             commands.append('no {0}'.format(key))
-
         elif value == 'default':
             if existing_commands.get(key):
                 existing_value = existing_commands.get(key)
@@ -483,7 +400,7 @@ def state_present(module, existing, proposed, candidate):
                 commands.append(command)
 
     if commands:
-        parents = ["router bgp {0}".format(module.params['asn'])]
+        parents = ['router bgp {0}'.format(module.params['asn'])]
         if module.params['vrf'] != 'default':
             parents.append('vrf {0}'.format(module.params['vrf']))
 
@@ -532,97 +449,60 @@ def main():
         transport_passive_only=dict(required=False, type='bool'),
         update_source=dict(required=False, type='str'),
         m_facts=dict(required=False, default=False, type='bool'),
-        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)
     )
-
     argument_spec.update(nxos_argument_spec)
 
-    module = AnsibleModule(argument_spec=argument_spec,
+    module = AnsibleModule(
-                                required_together=[['timer_bgp_hold',
+        argument_spec=argument_spec,
-                                            'timer_bgp_keepalive']],
+        required_together=[['timers_holdtime', 'timers_keepalive']],
-                                supports_check_mode=True)
+        supports_check_mode=True,
+    )
 
     warnings = list()
     check_args(module, warnings)
-
+    result = dict(changed=False, warnings=warnings)
 
     state = module.params['state']
+
     if module.params['pwd_type'] == 'default':
         module.params['pwd_type'] = '0'
 
-    args =  [
+    args = PARAM_TO_COMMAND_KEYMAP.keys()
-        'asn',
+    existing = get_existing(module, args, warnings)
-        'capability_negotiation',
-        'connected_check',
-        'description',
-        'dynamic_capability',
-        'ebgp_multihop',
-        'local_as',
-        'log_neighbor_changes',
-        'low_memory_exempt',
-        'maximum_peers',
-        'neighbor',
-        'pwd',
-        'pwd_type',
-        'remote_as',
-        'remove_private_as',
-        'shutdown',
-        'suppress_4_byte_as',
-        'timers_keepalive',
-        'timers_holdtime',
-        'transport_passive_only',
-        'update_source',
-        'vrf'
-    ]
 
-    existing = invoke('get_existing', module, args)
+    if existing.get('asn') and state == 'present':
-    if existing.get('asn'):
+        if existing['asn'] != module.params['asn']:
-        if (existing.get('asn') != module.params['asn'] and
-                state == 'present'):
             module.fail_json(msg='Another BGP ASN already exists.',
                              proposed_asn=module.params['asn'],
                              existing_asn=existing.get('asn'))
 
-    end_state = existing
     proposed_args = dict((k, v) for k, v in module.params.items()
                          if v is not None and k in args)
-
     proposed = {}
     for key, value in proposed_args.items():
         if key not in ['asn', 'vrf', 'neighbor', 'pwd_type']:
             if str(value).lower() == 'default':
-                value = PARAM_TO_DEFAULT_KEYMAP.get(key)
+                value = PARAM_TO_DEFAULT_KEYMAP.get(key, 'default')
-                if value is None:
+            if existing.get(key) != value:
-                    value = 'default'
-            if existing.get(key) or (not existing.get(key) and value):
                 proposed[key] = value
 
-    result = {}
-    if state == 'present' or (state == 'absent' and existing):
     candidate = CustomNetworkConfig(indent=3)
-        invoke('state_%s' % state, module, existing, proposed, candidate)
+    if state == 'present':
-        response = load_config(module, candidate)
+        state_present(module, existing, proposed, candidate)
-        result.update(response)
+    elif state == 'absent' and existing:
-
+        state_absent(module, existing, proposed, candidate)
+
+    if candidate:
+        candidate = candidate.items_text()
+        load_config(module, candidate)
+        result['changed'] = True
+        result['commands'] = candidate
     else:
-        result['updates'] = []
+        result['commands'] = []
-
-    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

@@ -487,7 +487,6 @@ lib/ansible/modules/network/nxos/_nxos_mtu.py
 lib/ansible/modules/network/nxos/_nxos_template.py
 lib/ansible/modules/network/nxos/nxos_aaa_server.py
 lib/ansible/modules/network/nxos/nxos_aaa_server_host.py
-lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py
 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

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

@@ -0,0 +1,51 @@
+# (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_bgp_neighbor
+from .nxos_module import TestNxosModule, load_fixture, set_module_args
+
+
+class TestNxosBgpNeighborModule(TestNxosModule):
+
+    module = nxos_bgp_neighbor
+
+    def setUp(self):
+        self.mock_load_config = patch('ansible.modules.network.nxos.nxos_bgp_neighbor.load_config')
+        self.load_config = self.mock_load_config.start()
+
+        self.mock_get_config = patch('ansible.modules.network.nxos.nxos_bgp_neighbor.get_config')
+        self.get_config = self.mock_get_config.start()
+
+    def tearDown(self):
+        self.mock_load_config.stop()
+        self.mock_get_config.stop()
+
+    def load_fixtures(self, commands=None):
+        self.get_config.return_value = load_fixture('nxos_bgp_config.cfg')
+        self.load_config.return_value = None
+
+    def test_nxos_bgp_neighbor(self):
+        set_module_args(dict(asn=65535, neighbor='3.3.3.3', description='some words'))
+        result = self.execute_module(changed=True)
+        self.assertEqual(result['commands'], ['router bgp 65535', 'neighbor 3.3.3.3', 'description some words'])