diff --git a/lib/ansible/module_utils/netcfg.py b/lib/ansible/module_utils/netcfg.py index fd31cb644b4..5d2a4a91d2d 100644 --- a/lib/ansible/module_utils/netcfg.py +++ b/lib/ansible/module_utils/netcfg.py @@ -155,6 +155,9 @@ class NetworkConfig(object): def __str__(self): return '\n'.join([c.raw for c in self.items]) + def __len__(self): + return len(self._items) + def load(self, s): self._items = self.parse(s) @@ -368,6 +371,9 @@ class NetworkConfig(object): class CustomNetworkConfig(NetworkConfig): + def items_text(self): + return [item.text for item in self.items] + def expand_section(self, configobj, S=None): if S is None: S = list() diff --git a/lib/ansible/modules/network/nxos/nxos_bgp.py b/lib/ansible/modules/network/nxos/nxos_bgp.py index ea71edcdbbf..8f74b621ba7 100644 --- a/lib/ansible/modules/network/nxos/nxos_bgp.py +++ b/lib/ansible/modules/network/nxos/nxos_bgp.py @@ -308,6 +308,7 @@ commands: ''' import re + 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 @@ -406,88 +407,54 @@ PARAM_TO_COMMAND_KEYMAP = { } -def invoke(name, *args, **kwargs): - func = globals().get(name) - if func: - return func(*args, **kwargs) - - -def get_custom_value(config, arg): - if arg.startswith('event_history'): - REGEX_SIZE = re.compile(r'(?:{0} size\s)(?P.*)$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M) - REGEX = re.compile(r'\s+{0}\s*$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M) - value = False - - if 'no {0}'.format(PARAM_TO_COMMAND_KEYMAP[arg]) in config: - pass - elif PARAM_TO_COMMAND_KEYMAP[arg] in config: - try: - value = REGEX_SIZE.search(config).group('value') - except AttributeError: - if REGEX.search(config): - value = True - - elif arg == 'enforce_first_as' or arg == 'fast_external_fallover': - REGEX = re.compile(r'no\s+{0}\s*$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M) - value = True - try: - if REGEX.search(config): - value = False - except TypeError: - value = True - - elif arg == 'confederation_peers': - REGEX = re.compile(r'(?:confederation peers\s)(?P.*)$', re.M) - value = '' - if 'confederation peers' in config: - value = REGEX.search(config).group('value').split() - - elif arg == 'timer_bgp_keepalive': - REGEX = re.compile(r'(?:timers bgp\s)(?P.*)$', re.M) - value = '' - if 'timers bgp' in config: - parsed = REGEX.search(config).group('value').split() - value = parsed[0] - - elif arg == 'timer_bgp_hold': - REGEX = re.compile(r'(?:timers bgp\s)(?P.*)$', re.M) - value = '' - if 'timers bgp' in config: - parsed = REGEX.search(config).group('value').split() - if len(parsed) == 2: - value = parsed[1] - - return value - - def get_value(arg, config): - custom = [ - 'event_history_cli', - 'event_history_events', - 'event_history_periodic', - 'event_history_detail', - 'confederation_peers', - 'timer_bgp_hold', - 'timer_bgp_keepalive', - 'enforce_first_as', - 'fast_external_fallover' - ] + command = PARAM_TO_COMMAND_KEYMAP.get(arg) - if arg in custom: - value = get_custom_value(config, arg) - elif arg in BOOL_PARAMS: - REGEX = re.compile(r'\s+{0}\s*$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M) + if command.split()[0] == 'event-history': + command_re = re.compile(r'\s+{0}\s*'.format(command), re.M) + + size_re = re.compile(r'(?:{0} size\s)(?P.*)'.format(command), re.M) value = False - try: - if REGEX.search(config): + + if command_re.search(config): + search = size_re.search(config) + if search: + value = search.group('value') + else: value = True - except TypeError: + + elif arg in ['enforce_first_as', 'fast_external_fallover']: + no_command_re = re.compile(r'no\s+{0}\s*'.format(command), re.M) + value = True + + if no_command_re.search(config): value = False + + elif arg in BOOL_PARAMS: + command_re = re.compile(r'\s+{0}\s*'.format(command), re.M) + value = False + + if command_re.search(config): + value = True else: - REGEX = re.compile(r'(?:{0}\s)(?P.*)$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M) + command_val_re = re.compile(r'(?:{0}\s)(?P.*)'.format(command), re.M) value = '' - if PARAM_TO_COMMAND_KEYMAP[arg] in config: - value = REGEX.search(config).group('value') + + has_command = command_val_re.search(config) + if has_command: + found_value = has_command.group('value') + + if arg == 'confederation_peers': + value = found_value.split() + elif arg == 'timer_bgp_keepalive': + value = found_value.split()[0] + elif arg == 'timer_bgp_hold': + split_values = found_value.split() + if len(split_values) == 2: + value = split_values[1] + elif found_value: + value = found_value + return value @@ -495,16 +462,13 @@ def get_existing(module, args, warnings): existing = {} netcfg = CustomNetworkConfig(indent=2, contents=get_config(module)) - try: - asn_regex = r'.*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 = '' + asn_re = re.compile(r'.*router\sbgp\s(?P\d+).*', re.S) + asn_match = asn_re.match(str(netcfg)) - if existing_asn: + if asn_match: + existing_asn = asn_match.group('existing_asn') bgp_parent = 'router bgp {0}'.format(existing_asn) + if module.params['vrf'] != 'default': parents = [bgp_parent, 'vrf {0}'.format(module.params['vrf'])] else: @@ -513,40 +477,28 @@ def get_existing(module, args, warnings): config = netcfg.get_section(parents) if config: for arg in args: - if arg != 'asn': - if module.params['vrf'] != 'default': - if arg not in GLOBAL_PARAMS: - existing[arg] = get_value(arg, config) - else: - existing[arg] = get_value(arg, config) + if arg != 'asn' and (module.params['vrf'] == 'default' or + arg not in GLOBAL_PARAMS): + existing[arg] = get_value(arg, config) existing['asn'] = existing_asn if module.params['vrf'] == 'default': existing['vrf'] = 'default' - else: - if (module.params['state'] == 'present' and - module.params['vrf'] != 'default'): - msg = ("VRF {0} doesn't exist. ".format(module.params['vrf'])) - warnings.append(msg) - else: - if (module.params['state'] == 'present' and - module.params['vrf'] != 'default'): - msg = ("VRF {0} doesn't exist. ".format(module.params['vrf'])) - warnings.append(msg) + + if not existing and module.params['vrf'] != 'default' and module.params['state'] == 'present': + msg = ("VRF {0} doesn't exist.".format(module.params['vrf'])) + warnings.append(msg) 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 @@ -561,67 +513,56 @@ def state_present(module, existing, proposed, candidate): elif value is False: commands.append('no {0}'.format(key)) elif value == 'default': - if key in PARAM_TO_DEFAULT_KEYMAP: - commands.append('{0} {1}'.format(key, PARAM_TO_DEFAULT_KEYMAP[key])) - elif existing_commands.get(key): - existing_value = existing_commands.get(key) + default_value = PARAM_TO_DEFAULT_KEYMAP.get(key) + existing_value = existing_commands.get(key) + + if default_value: + commands.append('{0} {1}'.format(key, default_value)) + elif existing_value: if key == 'confederation peers': - commands.append('no {0} {1}'.format(key, ' '.join(existing_value))) - else: - commands.append('no {0} {1}'.format(key, existing_value)) - else: - if key == 'confederation peers': - existing_confederation_peers = existing.get('confederation_peers') - - if existing_confederation_peers: - if not isinstance(existing_confederation_peers, list): - existing_confederation_peers = [existing_confederation_peers] - else: - existing_confederation_peers = [] - - values = value.split() - for each_value in values: - if each_value not in existing_confederation_peers: - existing_confederation_peers.append(each_value) - peer_string = ' '.join(existing_confederation_peers) - commands.append('{0} {1}'.format(key, peer_string)) - elif key.startswith('timers bgp'): - command = 'timers bgp {0} {1}'.format( - proposed['timer_bgp_keepalive'], - proposed['timer_bgp_hold']) - if command not in commands: - commands.append(command) - else: - if value.startswith('size'): - value = value.replace('_', ' ') - command = '{0} {1}'.format(key, value) + existing_value = ' '.join(existing_value) + commands.append('no {0} {1}'.format(key, existing_value)) + elif key == 'confederation peers': + existing_confederation_peers = set(existing.get('confederation_peers', [])) + new_values = set(value.split()) + peer_string = ' '.join(existing_confederation_peers | new_values) + commands.append('{0} {1}'.format(key, peer_string)) + elif key.startswith('timers bgp'): + command = 'timers bgp {0} {1}'.format( + proposed['timer_bgp_keepalive'], + proposed['timer_bgp_hold']) + if command not in commands: commands.append(command) + else: + if value.startswith('size'): + value = value.replace('_', ' ') + command = '{0} {1}'.format(key, value) + commands.append(command) + parents = [] if commands: commands = fix_commands(commands) parents = ['router bgp {0}'.format(module.params['asn'])] if module.params['vrf'] != 'default': parents.append('vrf {0}'.format(module.params['vrf'])) - candidate.add(commands, parents=parents) elif proposed: if module.params['vrf'] != 'default': commands.append('vrf {0}'.format(module.params['vrf'])) parents = ['router bgp {0}'.format(module.params['asn'])] else: commands.append('router bgp {0}'.format(module.params['asn'])) - parents = [] - candidate.add(commands, parents=parents) + + candidate.add(commands, parents=parents) -def state_absent(module, existing, proposed, candidate): +def state_absent(module, existing, candidate): commands = [] parents = [] if module.params['vrf'] == 'default': commands.append('no router bgp {0}'.format(module.params['asn'])) - else: - if existing.get('vrf') == module.params['vrf']: - commands.append('no vrf {0}'.format(module.params['vrf'])) - parents = ['router bgp {0}'.format(module.params['asn'])] + elif existing.get('vrf') == module.params['vrf']: + commands.append('no vrf {0}'.format(module.params['vrf'])) + parents = ['router bgp {0}'.format(module.params['asn'])] candidate.add(commands, parents=parents) @@ -696,11 +637,7 @@ def main(): timer_bgp_hold=dict(required=False, type='str'), timer_bgp_keepalive=dict(required=False, type='str'), state=dict(choices=['present', 'absent'], default='present', 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, @@ -709,63 +646,22 @@ def main(): warnings = list() check_args(module, warnings) + result = dict(changed=False, warnings=warnings) state = module.params['state'] - args = [ - "asn", - "bestpath_always_compare_med", - "bestpath_aspath_multipath_relax", - "bestpath_compare_neighborid", - "bestpath_compare_routerid", - "bestpath_cost_community_ignore", - "bestpath_med_confed", - "bestpath_med_missing_as_worst", - "bestpath_med_non_deterministic", - "cluster_id", - "confederation_id", - "confederation_peers", - "disable_policy_batching", - "disable_policy_batching_ipv4_prefix_list", - "disable_policy_batching_ipv6_prefix_list", - "enforce_first_as", - "event_history_cli", - "event_history_detail", - "event_history_events", - "event_history_periodic", - "fast_external_fallover", - "flush_routes", - "graceful_restart", - "graceful_restart_helper", - "graceful_restart_timers_restart", - "graceful_restart_timers_stalepath_time", - "isolate", - "local_as", - "log_neighbor_changes", - "maxas_limit", - "neighbor_down_fib_accelerate", - "reconnect_interval", - "router_id", - "shutdown", - "suppress_fib_pending", - "timer_bestpath_limit", - "timer_bgp_hold", - "timer_bgp_keepalive", - "vrf" - ] if module.params['vrf'] != 'default': - for param, inserted_value in module.params.items(): - if param in GLOBAL_PARAMS and inserted_value: - module.fail_json(msg='Global params can be modified only' - ' under "default" VRF.', + for param in GLOBAL_PARAMS: + if module.params[param]: + module.fail_json(msg='Global params can be modified only under "default" VRF.', vrf=module.params['vrf'], global_param=param) + args = PARAM_TO_COMMAND_KEYMAP.keys() existing = get_existing(module, args, warnings) - if existing.get('asn'): - if (existing.get('asn') != module.params['asn'] and - state == 'present'): + if existing.get('asn') and state == 'present': + if existing.get('asn') != module.params['asn']: module.fail_json(msg='Another BGP ASN already exists.', proposed_asn=module.params['asn'], existing_asn=existing.get('asn')) @@ -774,24 +670,23 @@ def main(): if v is not None and k in args) proposed = {} for key, value in proposed_args.items(): - if key != 'asn' and key != 'vrf': + if key not in ['asn', 'vrf']: 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 = dict(changed=False, warnings=warnings) + candidate = CustomNetworkConfig(indent=3) + if state == 'present': + state_present(module, existing, proposed, candidate) + elif existing.get('asn') == module.params['asn']: + state_absent(module, existing, candidate) - if state == 'present' or existing.get('asn') == module.params['asn']: - candidate = CustomNetworkConfig(indent=3) - invoke('state_%s' % state, module, existing, proposed, candidate) - - if (candidate): - load_config(module, candidate) - result['changed'] = True - result['commands'] = [item.text for item in candidate.items] + if candidate: + candidate = candidate.items_text() + load_config(module, candidate) + result['changed'] = True + result['commands'] = candidate else: result['commands'] = [] diff --git a/test/units/modules/network/nxos/fixtures/nxos_bgp_config.cfg b/test/units/modules/network/nxos/fixtures/nxos_bgp_config.cfg new file mode 100644 index 00000000000..5545c3d3c7b --- /dev/null +++ b/test/units/modules/network/nxos/fixtures/nxos_bgp_config.cfg @@ -0,0 +1,8 @@ +feature bgp + +router bgp 65535 + router-id 192.168.1.1 + event-history cli size medium + event-history detail + vrf test2 + timers bgp 1 10 diff --git a/test/units/modules/network/nxos/test_nxos_bgp.py b/test/units/modules/network/nxos/test_nxos_bgp.py index ce8d3d7f165..fbdfcfaf778 100644 --- a/test/units/modules/network/nxos/test_nxos_bgp.py +++ b/test/units/modules/network/nxos/test_nxos_bgp.py @@ -42,9 +42,56 @@ class TestNxosBgpModule(TestNxosModule): 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(self): - set_module_args(dict(asn=65535, vrf='test', router_id='1.1.1.1')) + set_module_args(dict(asn=65535, router_id='1.1.1.1')) result = self.execute_module(changed=True) - self.assertEqual(result['commands'], ['router bgp 65535', 'vrf test', 'router-id 1.1.1.1']) + self.assertEqual(result['commands'], ['router bgp 65535', 'router-id 1.1.1.1']) + + def test_nxos_bgp_change_nothing(self): + set_module_args(dict(asn=65535, router_id='192.168.1.1')) + self.execute_module(changed=False) + + def test_nxos_bgp_wrong_asn(self): + set_module_args(dict(asn=10, router_id='192.168.1.1')) + result = self.execute_module(failed=True) + self.assertEqual(result['msg'], 'Another BGP ASN already exists.') + + def test_nxos_bgp_remove(self): + set_module_args(dict(asn=65535, state='absent')) + self.execute_module(changed=True, commands=['no router bgp 65535']) + + def test_nxos_bgp_remove_vrf(self): + set_module_args(dict(asn=65535, vrf='test2', state='absent')) + self.execute_module(changed=True, commands=['router bgp 65535', 'no vrf test2']) + + def test_nxos_bgp_remove_nonexistant_vrf(self): + set_module_args(dict(asn=65535, vrf='foo', state='absent')) + self.execute_module(changed=False) + + def test_nxos_bgp_remove_wrong_asn(self): + set_module_args(dict(asn=10, state='absent')) + self.execute_module(changed=False) + + def test_nxos_bgp_vrf(self): + set_module_args(dict(asn=65535, vrf='test', router_id='1.1.1.1')) + result = self.execute_module(changed=True, commands=['router bgp 65535', 'vrf test', 'router-id 1.1.1.1']) + self.assertEqual(result['warnings'], ["VRF test doesn't exist."]) + + def test_nxos_bgp_global_param(self): + set_module_args(dict(asn=65535, shutdown=True)) + self.execute_module(changed=True, commands=['router bgp 65535', 'shutdown']) + + def test_nxos_bgp_global_param_outside_default(self): + set_module_args(dict(asn=65535, vrf='test', shutdown=True)) + result = self.execute_module(failed=True) + self.assertEqual(result['msg'], 'Global params can be modified only under "default" VRF.') + + def test_nxos_bgp_default_value(self): + set_module_args(dict(asn=65535, graceful_restart_timers_restart='default')) + self.execute_module( + changed=True, + commands=['router bgp 65535', 'graceful-restart restart-time 120'] + )