nxos_bgp_neighbor_af fix problem with advertise-map properties (#28413)

* Fix problem with advertise-map properties

* Change limit_to back to run all tests

* Fix unit tests

* Move advertise_map out of get_custom_value

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

@@ -304,8 +304,8 @@ PARAM_TO_COMMAND_KEYMAP = {
     'neighbor': 'neighbor',
     'additional_paths_receive': 'capability additional-paths receive',
     'additional_paths_send': 'capability additional-paths send',
-    'advertise_map_exist': 'advertise-map exist',
+    'advertise_map_exist': 'advertise-map exist-map',
-    'advertise_map_non_exist': 'advertise-map non-exist',
+    'advertise_map_non_exist': 'advertise-map non-exist-map',
     'allowas_in': 'allowas-in',
     'allowas_in_max': 'allowas-in',
     'as_override': 'as-override',
@@ -339,8 +339,6 @@ def get_value(arg, config, module):
     custom = [
         'additional_paths_send',
         'additional_paths_receive',
-        'advertise_map_exist',
-        'advertise_map_non_exist',
         'max_prefix_limit',
         'max_prefix_interval',
         'max_prefix_threshold',
@@ -366,6 +364,12 @@ def get_value(arg, config, module):
         if has_command:
             value = True
 
+    elif command.startswith('advertise-map'):
+        value = []
+        has_adv_map = re.search(r'{0}\s(?P<value1>.*)\s{1}\s(?P<value2>.*)$'.format(*command.split()), config, re.M)
+        if has_adv_map:
+            value = list(has_adv_map.groups())
+
     elif command.split()[0] in ['filter-list', 'prefix-list', 'route-map']:
         has_cmd_direction_val = re.search(r'{0}\s(?P<value>.*)\s{1}$'.format(*command.split()), config, re.M)
         if has_cmd_direction_val:
@@ -401,22 +405,6 @@ def get_custom_value(arg, config, module):
                     value = 'disable'
                 else:
                     value = 'enable'
-    elif arg == 'advertise_map_exist':
-        value = []
-        for line in splitted_config:
-            if 'advertise-map' in line and 'exist-map' in line:
-                splitted_line = line.split()
-                value = [splitted_line[1], splitted_line[3]]
-    elif command == 'advertise-map':
-        value = []
-        exclude = 'non_exist' in arg
-        for line in splitted_config:
-            if 'advertise-map' in line and (
-                (exclude and 'non-exist-map' in line) or
-                (not exclude and 'exist-map' in line)
-            ):
-                splitted_line = line.split()
-                value = [splitted_line[1], splitted_line[3]]
     elif arg.startswith('max_prefix'):
         for line in splitted_config:
             if 'maximum-prefix' in line:

test/integration/nxos.yaml

@@ -321,6 +321,12 @@
       rescue:
         - set_fact: test_failed=true
 
+    - block:
+      - include_role:
+          name: nxos_bgp_neighbor_af
+        when: "limit_to in ['*', 'nxos_bgp_neighbor_af']"
+      rescue:
+        - set_fact: test_failed=true
 ###########
     - debug: var=failed_modules
       when: test_failed

test/integration/targets/nxos_bgp_neighbor_af/defaults/main.yaml

@@ -0,0 +1,2 @@
+---
+testcase: "*"

test/integration/targets/nxos_bgp_neighbor_af/meta/main.yml

@@ -0,0 +1,2 @@
+dependencies:
+  - prepare_nxos_tests

test/integration/targets/nxos_bgp_neighbor_af/tasks/cli.yaml

@@ -0,0 +1,15 @@
+---
+- name: collect all cli test cases
+  find:
+    paths: "{{ role_path }}/tests/cli"
+    patterns: "{{ testcase }}.yaml"
+  register: test_cases
+
+- name: set test_items
+  set_fact: test_items="{{ test_cases.files | map(attribute='path') | list }}"
+
+- name: run test case
+  include: "{{ test_case_to_run }}"
+  with_items: "{{ test_items }}"
+  loop_control:
+    loop_var: test_case_to_run

test/integration/targets/nxos_bgp_neighbor_af/tasks/main.yaml

@@ -0,0 +1,7 @@
+---
+# Use block to ensure that both cli and nxapi tests
+# will run even if there are failures or errors.
+- block:
+  - { include: cli.yaml, tags: ['cli'] }
+  always:
+  - { include: nxapi.yaml, tags: ['nxapi'] }

test/integration/targets/nxos_bgp_neighbor_af/tasks/nxapi.yaml

@@ -0,0 +1,28 @@
+---
+- name: collect all nxapi test cases
+  find:
+    paths: "{{ role_path }}/tests/nxapi"
+    patterns: "{{ testcase }}.yaml"
+  register: test_cases
+
+- name: set test_items
+  set_fact: test_items="{{ test_cases.files | map(attribute='path') | list }}"
+
+- name: enable nxapi
+  nxos_config:
+    lines:
+      - feature nxapi
+      - nxapi http port 80
+    provider: "{{ cli }}"
+
+- name: run test case
+  include: "{{ test_case_to_run }}"
+  with_items: "{{ test_items }}"
+  loop_control:
+    loop_var: test_case_to_run
+
+- name: disable nxapi
+  nxos_config:
+    lines:
+      - no feature nxapi
+    provider: "{{ cli }}"

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

@@ -0,0 +1,4 @@
+---
+- set_fact: connection="{{ cli }}"
+
+- import_tasks: "{{ role_path }}/tests/common/sanity.yaml"

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

@@ -0,0 +1,204 @@
+---
+- debug: msg="START TRANSPORT:{{ connection.transport }} nxos_bgp_neighbor_af sanity test"
+
+- name: "Disable feature BGP"
+  nxos_feature: &disable_bgp
+    feature: bgp
+    state: disabled
+    provider: "{{ connection }}"
+  ignore_errors: yes
+
+- name: "Enable feature BGP"
+  nxos_feature:
+    feature: bgp
+    state: enabled
+    provider: "{{ connection }}"
+  ignore_errors: yes
+
+- block:
+  - name: "Configure BGP neighbor address-family defaults"
+    nxos_bgp_neighbor_af: &configure_default
+      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'
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: &true
+      that:
+        - "result.changed == true"
+
+  - name: "Check Idempotence"
+    nxos_bgp_neighbor_af: *configure_default
+    register: result
+
+  - assert: &false
+      that:
+        - "result.changed == false"
+
+  - name: "Setup: Remove BGP config"
+    nxos_bgp: &remove
+      asn: 65535
+      state: absent
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_bgp: *remove
+    register: result
+
+  - assert: *false
+
+  - name: "Configure eBGP setting remote_as 2 prior to non-defaults test"
+    nxos_bgp_neighbor:
+      asn: 65535
+      vrf: 'blue'
+      neighbor: '3.3.3.3'
+      remote_as: 2
+      provider: "{{ connection }}"
+
+  - name: "Configure BGP neighbor non-defaults"
+    nxos_bgp_neighbor_af: &configure_non_default
+      asn: 65535
+      vrf: 'blue'
+      neighbor: '3.3.3.3'
+      afi: ipv4
+      safi: unicast
+      additional_paths_receive: 'enable'
+      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'
+      soo: '3:3'
+      suppress_inactive: 'true'
+      unsuppress_map: 'unsup_map'
+      weight: '30'
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_bgp_neighbor_af: *configure_non_default
+    register: result
+
+  - assert: *false
+
+  - name: "Remove BGP"
+    nxos_bgp: *remove
+    register: result
+
+  - assert: *true
+
+  - name: "Configure BGP neighbor af route_reflector_client"
+    nxos_bgp_neighbor:
+      asn: 65535
+      neighbor: '2.2.2.2'
+      remote_as: 65535
+      provider: "{{ connection }}"
+
+  - name: "Configure BGP neighbor af route_reflector_client"
+    nxos_bgp_neighbor_af: &configure_rr_client
+      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
+    register: result
+
+  - assert: *false
+
+  always:
+  - name: "Cleanup BGP"
+    nxos_bgp: *remove
+    ignore_errors: yes
+
+  - name: "Disable feature bgp"
+    nxos_feature: *disable_bgp
+
+  - debug: msg="END TRANSPORT:{{ connection.transport }} nxos_bgp_neighbor_af sanity test"

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

@@ -0,0 +1,4 @@
+---
+- set_fact: connection="{{ nxapi }}"
+
+- import_tasks: "{{ role_path }}/tests/common/sanity.yaml"

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

@@ -70,15 +70,15 @@ class TestNxosBgpNeighborAfModule(TestNxosModule):
                              advertise_map_exist=['my_advertise_map', 'my_exist_map']))
         self.execute_module(
             changed=True, sort=False,
-            commands=['router bgp 65535', 'neighbor 3.3.3.5', 'address-family ipv4 unicast', 'advertise-map my_advertise_map exist my_exist_map']
+            commands=['router bgp 65535', 'neighbor 3.3.3.5', 'address-family ipv4 unicast', 'advertise-map my_advertise_map exist-map my_exist_map']
         )
 
     def test_nxos_bgp_neighbor_af_advertise_map_non_exist(self):
         set_module_args(dict(asn=65535, neighbor='3.3.3.5', afi='ipv4', safi='unicast',
-                             advertise_map_non_exist=['my_advertise_map', 'my_exist_map']))
+                             advertise_map_non_exist=['my_advertise_map', 'my_non_exist_map']))
         self.execute_module(
             changed=True, sort=False,
-            commands=['router bgp 65535', 'neighbor 3.3.3.5', 'address-family ipv4 unicast', 'advertise-map my_advertise_map non-exist my_exist_map']
+            commands=['router bgp 65535', 'neighbor 3.3.3.5', 'address-family ipv4 unicast', 'advertise-map my_advertise_map non-exist-map my_non_exist_map']
         )
 
     def test_nxos_bgp_neighbor_af_max_prefix_limit_default(self):