Assorted nxos_bgp_* fixes (#25080)

* Simplify apply_key_map

* Fix nxapi

* Clean up get_value

* Fix missing non-values

* Add test for existing bgp_af case

* Fix small issues with bgp_neighbor_af

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

@@ -334,85 +334,52 @@ DAMPENING_PARAMS = [
 ]
 
 
-def get_custom_list_value(config, arg, module):
+def get_value(arg, config, module):
-    value_list = []
+    command = PARAM_TO_COMMAND_KEYMAP[arg]
-    splitted_config = config.splitlines()
+    command_val_re = re.compile(r'(?:{0}\s)(?P<value>.*)$'.format(command), re.M)
-    if arg == 'inject_map':
+    has_command_val = command_val_re.search(config)
-        REGEX_INJECT = r'.*inject-map\s(?P<inject_map>\S+)\sexist-map\s(?P<exist_map>\S+)-*'
 
-        for line in splitted_config:
+    if arg == 'inject_map':
-            value = []
+        inject_re = r'.*inject-map\s(?P<inject_map>\S+)\sexist-map\s(?P<exist_map>\S+)-*'
-            inject_group = {}
+
-            try:
+        value = []
-                match_inject = re.match(REGEX_INJECT, line, re.DOTALL)
+        match_inject = re.match(inject_re, config, re.DOTALL)
-                inject_group = match_inject.groupdict()
+        if match_inject:
-                inject_map = inject_group['inject_map']
+            inject_group = match_inject.groupdict()
-                exist_map = inject_group['exist_map']
+            inject_map = inject_group['inject_map']
-                value.append(inject_map)
+            exist_map = inject_group['exist_map']
-                value.append(exist_map)
+            value.append(inject_map)
-            except AttributeError:
+            value.append(exist_map)
-                value = []
+
+            inject_map_command = ('inject-map {0} exist-map {1} '
+                                  'copy-attributes'.format(
+                                      inject_group['inject_map'],
+                                      inject_group['exist_map']))
+
+            inject_re = re.compile(r'\s+{0}\s*$'.format(inject_map_command), re.M)
+            if inject_re.search(config):
+                value.append('copy_attributes')
+
+    elif arg in ['networks', 'redistribute']:
+        value = []
+        if has_command_val:
+            value = has_command_val.group('value').split()
 
             if value:
-                copy_attributes = False
+                if len(value) == 3:
-                inject_map_command = ('inject-map {0} exist-map {1} '
+                    value.pop(1)
-                                      'copy-attributes'.format(
+                elif arg == 'redistribute' and len(value) == 4:
-                                          inject_group['inject_map'],
+                    value = ['{0} {1}'.format(
-                                          inject_group['exist_map']))
+                        value[0], value[1]), value[3]]
 
-                REGEX = re.compile(r'\s+{0}\s*$'.format(inject_map_command), re.M)
+    elif command == 'distance':
-                try:
+        distance_re = r'.*distance\s(?P<d_ebgp>\w+)\s(?P<d_ibgp>\w+)\s(?P<d_local>\w+)'
-                    if REGEX.search(config):
+        match_distance = re.match(distance_re, config, re.DOTALL)
-                        copy_attributes = True
-                except TypeError:
-                    copy_attributes = False
 
-                if copy_attributes:
+        value = ''
-                    value.append('copy_attributes')
+        if match_distance:
-                value_list.append(value)
-
-    elif arg == 'networks':
-        REGEX_NETWORK = re.compile(r'(?:network\s)(?P<value>.*)$')
-
-        for line in splitted_config:
-            value = []
-            if 'network' in line:
-                value = REGEX_NETWORK.search(line).group('value').split()
-
-                if value:
-                    if len(value) == 3:
-                        value.pop(1)
-                    value_list.append(value)
-
-    elif arg == 'redistribute':
-        RED_REGEX = re.compile(r'(?:{0}\s)(?P<value>.*)$'.format(
-            PARAM_TO_COMMAND_KEYMAP[arg]), re.M)
-        for line in splitted_config:
-            value = []
-            if 'redistribute' in line:
-                value = RED_REGEX.search(line).group('value').split()
-                if value:
-                    if len(value) == 3:
-                        value.pop(1)
-                    elif len(value) == 4:
-                        value = ['{0} {1}'.format(
-                            value[0], value[1]), value[3]]
-                    value_list.append(value)
-    return value_list
-
-
-def get_custom_string_value(config, arg, module):
-    value = ''
-    if arg.startswith('distance'):
-        REGEX_DISTANCE = ('.*distance\s(?P<d_ebgp>\w+)\s(?P<d_ibgp>\w+)'
-                          '\s(?P<d_local>\w+)')
-        try:
-            match_distance = re.match(REGEX_DISTANCE, config, re.DOTALL)
             distance_group = match_distance.groupdict()
-        except AttributeError:
-            distance_group = {}
 
-        if distance_group:
             if arg == 'distance_ebgp':
                 value = distance_group['d_ebgp']
             elif arg == 'distance_ibgp':
@@ -420,22 +387,17 @@ def get_custom_string_value(config, arg, module):
             elif arg == 'distance_local':
                 value = distance_group['d_local']
 
-    elif arg.startswith('dampening'):
+    elif command.split()[0] == 'dampening':
-        REGEX = re.compile(r'(?:{0}\s)(?P<value>.*)$'.format(
+        value = ''
-            PARAM_TO_COMMAND_KEYMAP[arg]), re.M)
         if arg == 'dampen_igp_metric' or arg == 'dampening_routemap':
-            value = ''
+            if command in config:
-            if PARAM_TO_COMMAND_KEYMAP[arg] in config:
+                value = has_command_val.group('value')
-                value = REGEX.search(config).group('value')
         else:
-            REGEX_DAMPENING = r'.*dampening\s(?P<half>\w+)\s(?P<reuse>\w+)\s(?P<suppress>\w+)\s(?P<max_suppress>\w+)'
+            dampening_re = r'.*dampening\s(?P<half>\w+)\s(?P<reuse>\w+)\s(?P<suppress>\w+)\s(?P<max_suppress>\w+)'
-            try:
+            match_dampening = re.match(dampening_re, config, re.DOTALL)
-                match_dampening = re.match(REGEX_DAMPENING, config, re.DOTALL)
+            if match_dampening:
                 dampening_group = match_dampening.groupdict()
-            except AttributeError:
-                dampening_group = {}
 
-            if dampening_group:
                 if arg == 'dampening_half_time':
                     value = dampening_group['half']
                 elif arg == 'dampening_reuse_time':
@@ -446,34 +408,17 @@ def get_custom_string_value(config, arg, module):
                     value = dampening_group['max_suppress']
 
     elif arg == 'table_map_filter':
-        TMF_REGEX = re.compile(r'\s+table-map.*filter$', re.M)
+        tmf_regex = re.compile(r'\s+table-map.*filter$', re.M)
         value = False
-        try:
+        if tmf_regex.search(config):
-            if TMF_REGEX.search(config):
+            value = True
-                value = True
+
-        except TypeError:
-            value = False
     elif arg == 'table_map':
-        TM_REGEX = re.compile(r'(?:table-map\s)(?P<value>\S+)(\sfilter)?$', re.M)
+        tm_regex = re.compile(r'(?:table-map\s)(?P<value>\S+)(\sfilter)?$', re.M)
+        has_tablemap = tm_regex.search(config)
         value = ''
-        if PARAM_TO_COMMAND_KEYMAP[arg] in config:
+        if has_tablemap:
-            value = TM_REGEX.search(config).group('value')
+            value = has_tablemap.group('value')
-    return value
-
-
-def get_value(arg, config, module):
-    custom = [
-        'inject_map',
-        'networks',
-        'redistribute'
-    ]
-
-    if arg in custom:
-        value = get_custom_list_value(config, arg, module)
-
-    elif (arg.startswith('distance') or arg.startswith('dampening') or
-          arg.startswith('table_map')):
-        value = get_custom_string_value(config, arg, module)
 
     elif arg in BOOL_PARAMS:
         command_re = re.compile(r'\s+{0}\s*'.format(command), re.M)
@@ -483,12 +428,10 @@ def get_value(arg, config, module):
             value = True
 
     else:
-        command_val_re = re.compile(r'(?:{0}\s)(?P<value>.*)$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M)
         value = ''
 
-        has_command = command_val_re.search(config)
+        if has_command_val:
-        if has_command:
+            value = has_command_val.group('value')
-            value = has_command.group('value')
 
     return value
 
@@ -526,10 +469,10 @@ def get_existing(module, args, warnings):
 
 def apply_key_map(key_map, table):
     new_dict = {}
-    for key in table:
+    for key, value in table.items():
         new_key = key_map.get(key)
         if new_key:
-            new_dict[new_key] = table.get(key)
+            new_dict[new_key] = value
 
     return new_dict
 
@@ -741,7 +684,7 @@ def state_present(module, existing, proposed, candidate):
             candidate.add(commands, parents=parents)
 
 
-def state_absent(module, existing, proposed, candidate):
+def state_absent(module, candidate):
     commands = []
     parents = ["router bgp {0}".format(module.params['asn'])]
     if module.params['vrf'] != 'default':
@@ -830,12 +773,10 @@ def main():
     proposed_args = dict((k, v) for k, v in module.params.items()
                          if v is not None and k in args)
 
-    if proposed_args.get('networks'):
+    for arg in ['networks', 'inject_map']:
-        if proposed_args['networks'][0] == 'default':
+        if proposed_args.get(arg):
-            proposed_args['networks'] = 'default'
+            if proposed_args[arg][0] == 'default':
-    if proposed_args.get('inject_map'):
+                proposed_args[arg] = 'default'
-        if proposed_args['inject_map'][0] == 'default':
-            proposed_args['inject_map'] = 'default'
 
     proposed = {}
     for key, value in proposed_args.items():
@@ -849,12 +790,13 @@ def main():
     if state == 'present':
         state_present(module, existing, proposed, candidate)
     elif state == 'absent' and existing:
-        state_absent(module, existing, proposed, candidate)
+        state_absent(module, candidate)
 
     if candidate:
+        candidate = candidate.items_text()
         load_config(module, candidate)
         result['changed'] = True
-        result['commands'] = candidate.items_text()
+        result['commands'] = candidate
     else:
         result['commands'] = []
 

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

@@ -352,7 +352,8 @@ def get_value(arg, config, module):
     ]
     command = PARAM_TO_COMMAND_KEYMAP[arg]
     has_command = re.search(r'\s+{0}\s*'.format(command), config, re.M)
-    has_command_val = command_val_re.search(r'(?:{0}\s)(?P<value>.*)$'.format(command), config, re.M)
+    has_command_val = re.search(r'(?:{0}\s)(?P<value>.*)$'.format(command), config, re.M)
+    value = ''
 
     if arg in custom:
         value = get_custom_value(arg, config, module)
@@ -363,7 +364,6 @@ def get_value(arg, config, module):
             value = True
 
     elif command.split()[0] in ['filter-list', 'prefix-list', 'route-map']:
-        value = ''
         direction = arg.rsplit('_', 1)[1]
         if has_command_val:
             params = has_command_val.group('value').split()
@@ -376,11 +376,8 @@ def get_value(arg, config, module):
             if has_command_val:
                 value = has_command_val.group('value')
 
-    else:
+    elif has_command_val:
-        value = ''
+        value = has_command_val.group('value')
-
-        if has_command_val:
-            value = has_command_val.group('value')
 
     return value
 
@@ -747,7 +744,7 @@ def main():
     if state == 'present':
         state_present(module, existing, proposed, candidate)
     elif state == 'absent' and existing:
-        state_absent(module, existing, proposed, candidate)
+        state_absent(module, existing, candidate)
 
     if candidate:
         candidate = candidate.items_text()

test/units/modules/network/nxos/fixtures/nxos_bgp_config.cfg

@@ -5,4 +5,7 @@ router bgp 65535
   event-history cli size medium
   event-history detail
   vrf test2
+    address-family ipv4 unicast
     timers bgp 1 10
+  neighbor 3.3.3.5
+    address-family ipv4 unicast

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

@@ -48,17 +48,21 @@ class TestNxosBgpAfModule(TestNxosModule):
     def test_nxos_bgp_af(self):
         set_module_args(dict(asn=65535, afi='ipv4', safi='unicast'))
         self.execute_module(
-            changed=True,
+            changed=True, sort=False,
             commands=['router bgp 65535', 'address-family ipv4 unicast']
         )
 
     def test_nxos_bgp_af_vrf(self):
         set_module_args(dict(asn=65535, vrf='test', afi='ipv4', safi='unicast'))
         self.execute_module(
-            changed=True,
+            changed=True, sort=False,
             commands=['router bgp 65535', 'vrf test', 'address-family ipv4 unicast']
         )
 
+    def test_nxos_bgp_af_vrf_exists(self):
+        set_module_args(dict(asn=65535, vrf='test2', afi='ipv4', safi='unicast'))
+        self.execute_module(changed=False, commands=[])
+
     def test_nxos_bgp_af_dampening_routemap(self):
         set_module_args(dict(asn=65535, afi='ipv4', safi='unicast',
                              dampening_routemap='route-map-a'))

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

@@ -53,3 +53,14 @@ class TestNxosBgpNeighborAfModule(TestNxosModule):
             'router bgp 65535', 'neighbor 3.3.3.3', 'address-family ipv4 unicast',
             'route-reflector-client'
         ])
+
+    def test_nxos_bgp_neighbor_af_exists(self):
+        set_module_args(dict(asn=65535, neighbor='3.3.3.5', afi='ipv4', safi='unicast'))
+        self.execute_module(changed=False, commands=[])
+
+    def test_nxos_bgp_neighbor_af_absent(self):
+        set_module_args(dict(asn=65535, neighbor='3.3.3.5', afi='ipv4', safi='unicast', state='absent'))
+        self.execute_module(
+            changed=True, sort=False,
+            commands=['router bgp 65535', 'neighbor 3.3.3.5', 'no address-family ipv4 unicast']
+        )