From f240ba6b60f3dbd4a25b04a046ba53381747c000 Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Tue, 16 May 2017 10:14:10 -0400 Subject: [PATCH] nxos_bgp_neighbor cleanup (#24446) --- .../modules/network/nxos/nxos_bgp_neighbor.py | 602 +++++++----------- test/sanity/pep8/legacy-files.txt | 1 - .../network/nxos/test_nxos_bgp_neighbor.py | 51 ++ 3 files changed, 292 insertions(+), 362 deletions(-) create mode 100644 test/units/modules/network/nxos/test_nxos_bgp_neighbor.py diff --git a/lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py b/lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py index a56e53deae6..bd25378bfd5 100644 --- a/lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py +++ b/lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py @@ -16,9 +16,11 @@ # along with Ansible. If not, see . # -ANSIBLE_METADATA = {'metadata_version': '1.0', - 'status': ['preview'], - 'supported_by': 'community'} +ANSIBLE_METADATA = { + 'metadata_version': '1.0', + 'status': ['preview'], + 'supported_by': 'community', +} DOCUMENTATION = ''' @@ -28,164 +30,164 @@ extends_documentation_fragment: nxos version_added: "2.2" short_description: Manages BGP neighbors configurations. description: - - Manages BGP neighbors configurations on NX-OS switches. + - Manages BGP neighbors configurations on NX-OS switches. author: Gabriele Gerbino (@GGabriele) notes: - - C(state=absent) removes the whole BGP neighbor configuration. - - Default, where supported, restores params default value. + - C(state=absent) removes the whole BGP neighbor configuration. + - Default, where supported, restores params default value. options: - asn: - description: - - BGP autonomous system number. Valid values are string, - Integer in ASPLAIN or ASDOT notation. - required: true - vrf: - description: - - Name of the VRF. The name 'default' is a valid VRF representing - the global bgp. - required: false - default: default - neighbor: - description: - - Neighbor Identifier. Valid values are string. Neighbors may use - IPv4 or IPv6 notation, with or without prefix length. - required: true + asn: + description: + - BGP autonomous system number. Valid values are string, + Integer in ASPLAIN or ASDOT notation. + required: true + vrf: + description: + - Name of the VRF. The name 'default' is a valid VRF representing + the global bgp. + required: false + default: default + neighbor: + description: + - Neighbor Identifier. Valid values are string. Neighbors may use + IPv4 or IPv6 notation, with or without prefix length. + required: true + description: + description: + - Description of the neighbor. + required: false + default: null + connected_check: + description: + - Configure whether or not to check for directly connected peer. + required: false + choices: ['true', 'false'] + default: null + capability_negotiation: + description: + - Configure whether or not to negotiate capability with + this neighbor. + required: false + choices: ['true', 'false'] + default: null + dynamic_capability: + description: + - Configure whether or not to enable dynamic capability. + required: false + choices: ['true', 'false'] + default: null + ebgp_multihop: + description: + - Specify multihop TTL for a remote peer. Valid values are + integers between 2 and 255, or keyword 'default' to disable + this property. + required: false + default: null + local_as: + description: + - Specify the local-as number for the eBGP neighbor. + Valid values are String or Integer in ASPLAIN or ASDOT notation, + or 'default', which means not to configure it. + required: false + default: null + log_neighbor_changes: + description: + - Specify whether or not to enable log messages for neighbor + up/down event. + required: false + choices: ['enable', 'disable', 'inherit'] + default: null + low_memory_exempt: + description: + - Specify whether or not to shut down this neighbor under + memory pressure. + required: false + choices: ['true', 'false'] + default: null + maximum_peers: + description: + - Specify Maximum number of peers for this neighbor prefix + Valid values are between 1 and 1000, or 'default', which does + not impose the limit. + required: false + default: null + pwd: + description: + - Specify the password for neighbor. Valid value is string. + required: false + default: null + pwd_type: + description: + - Specify the encryption type the password will use. Valid values + are '3des' or 'cisco_type_7' encryption. + required: false + choices: ['3des', 'cisco_type_7'] + default: null + remote_as: description: - description: - - Description of the neighbor. - required: false - default: null - connected_check: - description: - - Configure whether or not to check for directly connected peer. - required: false - choices: ['true', 'false'] - default: null - capability_negotiation: - description: - - Configure whether or not to negotiate capability with - this neighbor. - required: false - choices: ['true', 'false'] - default: null - dynamic_capability: - description: - - Configure whether or not to enable dynamic capability. - required: false - choices: ['true', 'false'] - default: null - ebgp_multihop: - description: - - Specify multihop TTL for a remote peer. Valid values are - integers between 2 and 255, or keyword 'default' to disable - this property. - required: false - default: null - local_as: - description: - - Specify the local-as number for the eBGP neighbor. - Valid values are String or Integer in ASPLAIN or ASDOT notation, - or 'default', which means not to configure it. - required: false - default: null - log_neighbor_changes: - description: - - Specify whether or not to enable log messages for neighbor - up/down event. - required: false - choices: ['enable', 'disable', 'inherit'] - default: null - low_memory_exempt: - description: - - Specify whether or not to shut down this neighbor under - memory pressure. - required: false - choices: ['true', 'false'] - default: null - maximum_peers: - description: - - Specify Maximum number of peers for this neighbor prefix - Valid values are between 1 and 1000, or 'default', which does - not impose the limit. - required: false - default: null - pwd: - description: - - Specify the password for neighbor. Valid value is string. - required: false - default: null - pwd_type: - description: - - Specify the encryption type the password will use. Valid values - are '3des' or 'cisco_type_7' encryption. - required: false - choices: ['3des', 'cisco_type_7'] - default: null - remote_as: - description: - - Specify Autonomous System Number of the neighbor. - Valid values are String or Integer in ASPLAIN or ASDOT notation, - or 'default', which means not to configure it. - required: false - default: null - remove_private_as: - description: - - Specify the config to remove private AS number from outbound - updates. Valid values are 'enable' to enable this config, - 'disable' to disable this config, 'all' to remove all - private AS number, or 'replace-as', to replace the private - AS number. - required: false - choices: ['enable', 'disable', 'all', 'replace-as'] - default: null - shutdown: - description: - - Configure to administratively shutdown this neighbor. - required: false - choices: ['true','false'] - default: null - suppress_4_byte_as: - description: - - Configure to suppress 4-byte AS Capability. - required: false - choices: ['true','false'] - default: null - timers_keepalive: - description: - - Specify keepalive timer value. Valid values are integers - between 0 and 3600 in terms of seconds, or 'default', - which is 60. - required: false - default: null - timers_holdtime: - description: - - Specify holdtime timer value. Valid values are integers between - 0 and 3600 in terms of seconds, or 'default', which is 180. - required: false - default: null - transport_passive_only: - description: - - Specify whether or not to only allow passive connection setup. - Valid values are 'true', 'false', and 'default', which defaults - to 'false'. This property can only be configured when the - neighbor is in 'ip' address format without prefix length. - This property and the transport_passive_mode property are - mutually exclusive. - required: false - choices: ['true','false'] - default: null - update_source: - description: - - Specify source interface of BGP session and updates. - required: false - default: null - state: - description: - - Determines whether the config should be present or not - on the device. - required: false - default: present - choices: ['present','absent'] + - Specify Autonomous System Number of the neighbor. + Valid values are String or Integer in ASPLAIN or ASDOT notation, + or 'default', which means not to configure it. + required: false + default: null + remove_private_as: + description: + - Specify the config to remove private AS number from outbound + updates. Valid values are 'enable' to enable this config, + 'disable' to disable this config, 'all' to remove all + private AS number, or 'replace-as', to replace the private + AS number. + required: false + choices: ['enable', 'disable', 'all', 'replace-as'] + default: null + shutdown: + description: + - Configure to administratively shutdown this neighbor. + required: false + choices: ['true','false'] + default: null + suppress_4_byte_as: + description: + - Configure to suppress 4-byte AS Capability. + required: false + choices: ['true','false'] + default: null + timers_keepalive: + description: + - Specify keepalive timer value. Valid values are integers + between 0 and 3600 in terms of seconds, or 'default', + which is 60. + required: false + default: null + timers_holdtime: + description: + - Specify holdtime timer value. Valid values are integers between + 0 and 3600 in terms of seconds, or 'default', which is 180. + required: false + default: null + transport_passive_only: + description: + - Specify whether or not to only allow passive connection setup. + Valid values are 'true', 'false', and 'default', which defaults + to 'false'. This property can only be configured when the + neighbor is in 'ip' address format without prefix length. + This property and the transport_passive_mode property are + mutually exclusive. + required: false + choices: ['true','false'] + default: null + update_source: + description: + - Specify source interface of BGP session and updates. + required: false + default: null + state: + description: + - Determines whether the config should be present or not + on the device. + required: false + default: present + choices: ['present','absent'] ''' EXAMPLES = ''' # create a new neighbor @@ -198,62 +200,26 @@ EXAMPLES = ''' update_source: Ethernet1/3 shutdown: default state: present - username: "{{ un }}" - password: "{{ pwd }}" - host: "{{ inventory_hostname }}" ''' RETURN = ''' -proposed: - 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 +commands: + 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"] ''' 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_holdtime': 'timers-holdtime', + 'timers_keepalive': 'timers', + '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.*)$'.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 - - else: - REGEX = re.compile(r'(?:{0}\s)(?P.*)$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M) + elif arg == 'log_neighbor_changes': 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 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] + if has_command: + if has_command_value: + value = 'disable' + else: + value = 'enable' elif arg == 'remove_private_as': value = 'disable' - for line in splitted_config: - if 'remove-private-as' in line: - splitted_line = line.split() - if len(splitted_line) == 1: - value = 'enable' - elif len(splitted_line) == 2: - value = splitted_line[1] - - elif arg == 'timers_keepalive': - REGEX = re.compile(r'(?:timers\s)(?P.*)$', re.M) + if has_command: + if has_command_value: + value = has_command_value.group('value') + else: + value = 'enable' + else: value = '' - if 'timers' in config: - parsed = REGEX.search(config).group('value').split() - value = parsed[0] - elif arg == 'timers_holdtime': - REGEX = re.compile(r'(?:timers\s)(?P.*)$', re.M) - value = '' - if 'timers' in config: - parsed = REGEX.search(config).group('value').split() - if len(parsed) == 2: - value = parsed[1] + if has_command_value: + value = has_command_value.group('value') + + if command in ['timers', 'password']: + split_value = value.split() + value = '' + + if arg in ['timers_keepalive', 'pwd_type']: + value = split_value[0] + 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', - 'pwd', - 'pwd_type', - 'remove_private_as', - 'timers_holdtime', - 'timers_keepalive' - ] - try: - asn_regex = '.*router\sbgp\s(?P\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: + + asn_regex = re.compile(r'.*router\sbgp\s(?P\d+).*', re.S) + match_asn = asn_regex.match(str(netcfg)) + + if match_asn: + existing_asn = match_asn.group('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_custom_value(arg, config, module) - else: - existing[arg] = get_value(arg, config, module) + existing[arg] = get_value(arg, config) 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) - if value: - new_dict[new_key] = value - else: - new_dict[new_key] = value + new_dict[new_key] = table.get(key) + 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', - required=False), - include_defaults=dict(default=True), - config=dict(), - save=dict(type='bool', default=False) + state=dict(choices=['present', 'absent'], default='present', required=False), ) - argument_spec.update(nxos_argument_spec) - module = AnsibleModule(argument_spec=argument_spec, - required_together=[['timer_bgp_hold', - 'timer_bgp_keepalive']], - supports_check_mode=True) + module = AnsibleModule( + argument_spec=argument_spec, + required_together=[['timers_holdtime', 'timers_keepalive']], + 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 = [ - 'asn', - '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'): - if (existing.get('asn') != module.params['asn'] and - state == 'present'): + args = PARAM_TO_COMMAND_KEYMAP.keys() + existing = get_existing(module, args, warnings) + + if existing.get('asn') and state == 'present': + if existing['asn'] != module.params['asn']: 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) - + 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) - if value is None: - value = 'default' - if existing.get(key) or (not existing.get(key) and value): + value = PARAM_TO_DEFAULT_KEYMAP.get(key, 'default') + if existing.get(key) != 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) - response = load_config(module, candidate) - result.update(response) - + candidate = CustomNetworkConfig(indent=3) + if state == 'present': + state_present(module, existing, proposed, candidate) + 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'] = [] - - 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 + result['commands'] = [] module.exit_json(**result) if __name__ == '__main__': main() - diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index 68e36fd6138..3fd535f7fcd 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/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 diff --git a/test/units/modules/network/nxos/test_nxos_bgp_neighbor.py b/test/units/modules/network/nxos/test_nxos_bgp_neighbor.py new file mode 100644 index 00000000000..6be7a30d4ef --- /dev/null +++ b/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 . + +# 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'])