fix nxos_bgp_neighbor_af issues (#36472)

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

@@ -111,9 +111,9 @@ options:
     default: null
   allowas_in_max:
     description:
-      - Optional max-occurrences value for allowas_in. Valid values are
-        an integer value or 'default'. Can be used independently or in
-        conjunction with allowas_in.
+      - Max-occurrences value for allowas_in. Valid values are
+        an integer value or 'default'. This is mutually exclusive with
+        allowas_in.
     required: false
     default: null
   as_override:
@@ -130,10 +130,10 @@ options:
     default: null
   default_originate_route_map:
     description:
-      - Optional route-map for the default_originate property. Can be
-        used independently or in conjunction with C(default_originate).
+      - Route-map for the default_originate property.
         Valid values are a string defining a route-map name,
-        or 'default'.
+        or 'default'. This is mutually exclusive with
+        default_originate.
     required: false
     default: null
   disable_peer_as_check:
@@ -352,6 +352,7 @@ def get_value(arg, config, module):
         'max_prefix_interval',
         'max_prefix_threshold',
         'max_prefix_warning',
+        'send_community',
         'soft_reconfiguration_in'
     ]
     command = PARAM_TO_COMMAND_KEYMAP[arg]
@@ -384,12 +385,6 @@ def get_value(arg, config, module):
         if has_cmd_direction_val:
             value = has_cmd_direction_val.group('value')
 
-    elif arg == 'send_community':
-        if has_command:
-            value = 'none'
-            if has_command_val:
-                value = has_command_val.group('value')
-
     elif has_command_val:
         value = has_command_val.group('value')
 
@@ -439,6 +434,20 @@ def get_custom_value(arg, config, module):
                 else:
                     value = 'enable'
 
+    elif arg == 'send_community':
+        value = 'none'
+        for line in splitted_config:
+            if command in line:
+                if 'extended' in line:
+                    if value == 'standard':
+                        value = 'both'
+                    else:
+                        value = 'extended'
+                elif 'both' in line:
+                    value = 'both'
+                else:
+                    value = 'standard'
+
     return value
 
 
@@ -488,19 +497,16 @@ def apply_key_map(key_map, table):
 
 def get_default_command(key, value, existing_commands):
     command = ''
-    if key == 'send-community' and existing_commands.get(key) == 'none':
-        command = 'no {0}'.format(key)
-
-    elif existing_commands.get(key):
+    if existing_commands.get(key):
         existing_value = existing_commands.get(key)
         if value == 'inherit':
             if existing_value != 'inherit':
                 command = 'no {0}'.format(key)
         else:
-            if key == 'advertise-map exist':
+            if key == 'advertise-map exist-map':
                 command = 'no advertise-map {0} exist-map {1}'.format(
                     existing_value[0], existing_value[1])
-            elif key == 'advertise-map non-exist':
+            elif key == 'advertise-map non-exist-map':
                 command = 'no advertise-map {0} non-exist-map {1}'.format(
                     existing_value[0], existing_value[1])
             elif key == 'filter-list in':
@@ -528,7 +534,7 @@ def get_default_command(key, value, existing_commands):
     return command
 
 
-def fix_proposed(module, proposed):
+def fix_proposed(module, existing, proposed):
     allowas_in = proposed.get('allowas_in')
     allowas_in_max = proposed.get('allowas_in_max')
 
@@ -537,12 +543,14 @@ def fix_proposed(module, proposed):
     elif allowas_in and allowas_in_max:
         proposed.pop('allowas_in')
 
+    if existing.get('send_community') == 'none' and proposed.get('send_community') == 'default':
+        proposed.pop('send_community')
     return proposed
 
 
 def state_present(module, existing, proposed, candidate):
     commands = list()
-    proposed = fix_proposed(module, proposed)
+    proposed = fix_proposed(module, existing, proposed)
 
     proposed_commands = apply_key_map(PARAM_TO_COMMAND_KEYMAP, proposed)
     existing_commands = apply_key_map(PARAM_TO_COMMAND_KEYMAP, existing)
@@ -559,14 +567,15 @@ def state_present(module, existing, proposed, candidate):
                         commands.append(cmd)
 
         elif key.startswith('maximum-prefix'):
-            command = 'maximum-prefix {0}'.format(module.params['max_prefix_limit'])
-            if module.params['max_prefix_threshold']:
-                command += ' {0}'.format(module.params['max_prefix_threshold'])
-            if module.params['max_prefix_interval']:
-                command += ' restart {0}'.format(module.params['max_prefix_interval'])
-            elif module.params['max_prefix_warning']:
-                command += ' warning-only'
-            commands.append(command)
+            if module.params['max_prefix_limit'] != 'default':
+                command = 'maximum-prefix {0}'.format(module.params['max_prefix_limit'])
+                if module.params['max_prefix_threshold']:
+                    command += ' {0}'.format(module.params['max_prefix_threshold'])
+                if module.params['max_prefix_interval']:
+                    command += ' restart {0}'.format(module.params['max_prefix_interval'])
+                elif module.params['max_prefix_warning']:
+                    command += ' warning-only'
+                commands.append(command)
 
         elif value is True:
             commands.append(key)
@@ -594,14 +603,14 @@ def state_present(module, existing, proposed, candidate):
             commands.append(command)
         elif key == 'send-community':
             command = key
-            if value != 'none':
-                command += ' {0}'.format(value)
+            if value in ['standard', 'extended']:
+                commands.append('no ' + key + ' both')
+            command += ' {0}'.format(value)
             commands.append(command)
         else:
             command = '{0} {1}'.format(key, value)
             commands.append(command)
 
-    commands = set(commands)
     if commands:
         parents = ['router bgp {0}'.format(module.params['asn'])]
         if module.params['vrf'] != 'default':
@@ -672,7 +681,9 @@ def main():
     module = AnsibleModule(
         argument_spec=argument_spec,
         mutually_exclusive=[['advertise_map_exist', 'advertise_map_non_exist'],
-                            ['max_prefix_interval', 'max_prefix_warning']],
+                            ['max_prefix_interval', 'max_prefix_warning'],
+                            ['default_originate', 'default_originate_route_map'],
+                            ['allowas_in', 'allowas_in_max']],
         supports_check_mode=True,
     )
 
@@ -717,6 +728,8 @@ def main():
                         value = False
                     else:
                         value = 'default'
+                elif key == 'send_community' and str(value).lower() == 'none':
+                    value = 'default'
             if existing.get(key) != value:
                 proposed[key] = value
 

test/integration/targets/nxos_bgp_neighbor_af/tests/common/sanity.yaml

@@ -3,6 +3,9 @@
 - debug: msg="Using provider={{ connection.transport }}"
   when: ansible_connection == "local"
 
+- set_fact: soft_reconfiguration_ina="always"
+  when: imagetag and (imagetag is version_compare('D1', 'ne'))
+
 - name: "Disable feature BGP"
   nxos_feature: &disable_bgp
     feature: bgp
@@ -18,48 +21,32 @@
   ignore_errors: yes
 
 - block:
-  - name: "Configure BGP neighbor address-family defaults"
-    nxos_bgp_neighbor_af: &configure_default
+  - name: "Configure BGP neighbor address-family"
+    nxos_bgp_neighbor_af: &configure1
       asn: 65535
       neighbor: '3.3.3.3'
       afi: ipv4
       safi: unicast
-      additional_paths_receive: 'inherit'
-      additional_paths_send: 'inherit'
-      advertise_map_exist: 'default'
-      # -----------------------------------------------
-      # advertise_map_exist and advertise_map_non_exist
-      # are mutually exclusive
-      #advertise_map_non_exist: 'default'
-      # -----------------------------------------------
-      allowas_in: 'false'
-      allowas_in_max: 'default'
-      as_override: 'false'
-      default_originate: 'false'
-      default_originate_route_map: 'default'
-      filter_list_in: 'default'
-      filter_list_out: 'default'
-      # ------------------------------------------
-      # max_prefix* properities are off by default
-      max_prefix_limit: 'default'
-      # For the default case, only max_prefix_limit
-      # is set.  The rest should be left unset.
-      #max_prefix_interval: 'default'
-      #max_prefix_threshold: 'default'
-      #max_prefix_warning: 'false'
-      # ------------------------------------------
-      next_hop_self: 'false'
-      next_hop_third_party: 'true'
-      prefix_list_in: 'default'
-      prefix_list_out: 'default'
-      route_map_in: 'default'
-      route_map_out: 'default'
-      route_reflector_client: 'false'
-      send_community: 'default'
-      soft_reconfiguration_in: 'inherit'
-      suppress_inactive: 'false'
-      unsuppress_map: 'default'
-      weight: 'default'
+      additional_paths_receive: 'enable'
+      additional_paths_send: 'enable'
+      advertise_map_exist: ['ansible_rm', 'my_exist_map']
+      allowas_in: True
+      default_originate: True
+      disable_peer_as_check: True
+      filter_list_in: 'my_filter_list_in'
+      filter_list_out: 'my_filter_list_out'
+      max_prefix_limit: 100
+      max_prefix_threshold: 50
+      max_prefix_warning: 'true'
+      next_hop_self: True
+      next_hop_third_party: False
+      prefix_list_in: 'pfx_in'
+      prefix_list_out: 'pfx_out'
+      send_community: 'both'
+      soft_reconfiguration_in: 'enable'
+      suppress_inactive: True
+      unsuppress_map: 'unsup_map'
+      weight: '30'
       state: present
       provider: "{{ connection }}"
     register: result
@@ -69,16 +56,57 @@
         - "result.changed == true"
 
   - name: "Check Idempotence"
-    nxos_bgp_neighbor_af: *configure_default
+    nxos_bgp_neighbor_af: *configure1
     register: result
 
   - assert: &false
       that:
         - "result.changed == false"
 
+  - name: "Configure BGP neighbor address-family def1"
+    nxos_bgp_neighbor_af: &configure_def1
+      asn: 65535
+      neighbor: '3.3.3.3'
+      afi: ipv4
+      safi: unicast
+      additional_paths_receive: 'inherit'
+      additional_paths_send: 'inherit'
+      advertise_map_exist: default
+      allowas_in: False
+      default_originate: False
+      disable_peer_as_check: False
+      filter_list_in: default
+      filter_list_out: default
+      max_prefix_limit: default
+      max_prefix_threshold: default
+      max_prefix_warning: False
+      next_hop_self: False
+      next_hop_third_party: True
+      prefix_list_in: default
+      prefix_list_out: default
+      send_community: 'none'
+      soft_reconfiguration_in: 'inherit'
+      suppress_inactive: False
+      unsuppress_map: default
+      weight: default
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_bgp_neighbor_af: *configure_def1
+    register: result
+
+  - assert: *false
+
   - name: "Setup: Remove BGP config"
-    nxos_bgp: &remove
+    nxos_bgp_neighbor_af: &removenaf
       asn: 65535
+      neighbor: '3.3.3.3'
+      afi: ipv4
+      safi: unicast
       state: absent
       provider: "{{ connection }}"
     register: result
@@ -86,12 +114,76 @@
   - assert: *true
 
   - name: "Check Idempotence"
-    nxos_bgp: *remove
+    nxos_bgp_neighbor_af: *removenaf
+    register: result
+
+  - assert: *false
+
+  - name: "Configure BGP neighbor address-family"
+    nxos_bgp_neighbor_af: &configure2
+      asn: 65535
+      neighbor: '3.3.3.3'
+      afi: ipv4
+      safi: unicast
+      allowas_in_max: '5'
+      advertise_map_non_exist: ['ansible_rm', 'my_non_exist_map']
+      default_originate_route_map: 'my_route_map'
+      max_prefix_limit: 100
+      max_prefix_interval: 30
+      max_prefix_threshold: 50
+      route_map_in: 'rm_in'
+      route_map_out: 'rm_out'
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_bgp_neighbor_af: *configure2
+    register: result
+
+  - assert: *false
+
+  - name: "Configure BGP neighbor address-family def2"
+    nxos_bgp_neighbor_af: &configure_def2
+      asn: 65535
+      neighbor: '3.3.3.3'
+      afi: ipv4
+      safi: unicast
+      allowas_in_max: default
+      advertise_map_non_exist: default
+      default_originate_route_map: default
+      max_prefix_limit: default
+      max_prefix_interval: default
+      max_prefix_threshold: default
+      route_map_in: default
+      route_map_out: default
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_bgp_neighbor_af: *configure_def2
     register: result
 
   - assert: *false
 
-  - name: "Configure eBGP setting remote_as 2 prior to non-defaults test"
+  - name: "Setup: Remove BGP config"
+    nxos_bgp_neighbor_af: *removenaf
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_bgp_neighbor_af: *removenaf
+    register: result
+
+  - assert: *false
+
+  - name: "Configure eBGP"
     nxos_bgp_neighbor:
       asn: 65535
       vrf: 'blue'
@@ -99,46 +191,19 @@
       remote_as: 2
       provider: "{{ connection }}"
 
-  - name: "Configure BGP neighbor non-defaults"
-    nxos_bgp_neighbor_af: &configure_non_default
+  - name: "Configure BGP neighbor 3"
+    nxos_bgp_neighbor_af: &configure3
       asn: 65535
       vrf: 'blue'
       neighbor: '3.3.3.3'
       afi: ipv4
       safi: unicast
-      additional_paths_receive: 'enable'
+      additional_paths_receive: 'disable'
       additional_paths_send: 'disable'
-      advertise_map_non_exist: ['ansible_rm', 'my_non_exist_map']
-      # -----------------------------------------------
-      # advertise_map_exist and advertise_map_non_exist
-      # are mutually exclusive
-      #advertise_map_exist: 'my_map'
-      # -----------------------------------------------
-      allowas_in: 'true'
-      allowas_in_max: '5'
       as_override: 'true'
-      default_originate: 'true'
-      default_originate_route_map: 'my_route_map'
-      filter_list_in: 'my_filter_list_in'
-      filter_list_out: 'my_filter_list_out'
-      max_prefix_limit: 100
-      # max_prefix_interval and max_prefix_warning
-      # are mutually exclusive.
-      max_prefix_interval: 30
-      max_prefix_threshold: 50
-      #max_prefix_warning: 'true'
-      next_hop_self: 'true'
-      next_hop_third_party: 'false'
-      prefix_list_in: 'pfx_in'
-      prefix_list_out: 'pfx_out'
-      route_map_in: 'rm_in'
-      route_map_out: 'rm_out'
-      send_community: 'extended'
-      soft_reconfiguration_in: 'enable'
+      send_community: 'standard'
+      soft_reconfiguration_in: "{{soft_reconfiguration_ina|default(omit)}}"
       soo: '3:3'
-      suppress_inactive: 'true'
-      unsuppress_map: 'unsup_map'
-      weight: '30'
       state: present
       provider: "{{ connection }}"
     register: result
@@ -146,13 +211,40 @@
   - assert: *true
 
   - name: "Check Idempotence"
-    nxos_bgp_neighbor_af: *configure_non_default
+    nxos_bgp_neighbor_af: *configure3
     register: result
 
   - assert: *false
 
-  - name: "Remove BGP"
-    nxos_bgp: *remove
+  - name: "Configure BGP neighbor def3"
+    nxos_bgp_neighbor_af: &configure_def3
+      asn: 65535
+      vrf: 'blue'
+      neighbor: '3.3.3.3'
+      afi: ipv4
+      safi: unicast
+      additional_paths_receive: 'inherit'
+      additional_paths_send: 'inherit'
+      as_override: False
+      send_community: default
+      soo: default
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_bgp_neighbor_af: *configure_def3
+    register: result
+
+  - assert: *false
+
+  - name: "Setup: Remove BGP config"
+    nxos_bgp: &remove
+      asn: 65535
+      state: absent
+      provider: "{{ connection }}"
     register: result
 
   - assert: *true
@@ -164,33 +256,38 @@
       remote_as: 65535
       provider: "{{ connection }}"
 
-  - name: "Configure BGP neighbor af route_reflector_client"
-    nxos_bgp_neighbor_af: &configure_rr_client
+  - name: "Configure BGP neighbor 4"
+    nxos_bgp_neighbor_af: &configure4
       asn: 65535
       neighbor: '2.2.2.2'
       afi: ipv4
       safi: unicast
       route_reflector_client: 'true'
-      #advertise_map_non_exist: ['ansible_rm', 'my_non_exist_map']
-      # -----------------------------------------------
-      # advertise_map_exist and advertise_map_non_exist
-      # are mutually exclusive
-      advertise_map_exist: ['ansible_rm', 'my_exist_map']
-      # -----------------------------------------------
-      # max_prefix_interval and max_prefix_warning
-      # are mutually exclusive. Add testing for
-      # warning since interval is tested above.
-      max_prefix_limit: 100
-      #max_prefix_interval: 30
-      max_prefix_threshold: 50
-      max_prefix_warning: 'true'
       provider: "{{ connection }}"
     register: result
 
   - assert: *true
 
   - name: "Check Idempotence"
-    nxos_bgp_neighbor_af: *configure_rr_client
+    nxos_bgp_neighbor_af: *configure4
+    register: result
+
+  - assert: *false
+
+  - name: "Configure BGP neighbor def4"
+    nxos_bgp_neighbor_af: &configure_def4
+      asn: 65535
+      neighbor: '2.2.2.2'
+      afi: ipv4
+      safi: unicast
+      route_reflector_client: False
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_bgp_neighbor_af: *configure_def4
     register: result
 
   - assert: *false