From 173c41aefe30f772808713322c309df4acc985b3 Mon Sep 17 00:00:00 2001 From: Mike Wiebe Date: Thu, 14 Sep 2017 16:36:32 -0400 Subject: [PATCH] Rel240/fix nxos pim interface (#29885) * fix nxos_pim_interface * Add integration test coverage and fix unit test * Add clarifying comments * Make ansibot happy --- .../network/nxos/nxos_pim_interface.py | 90 ++++++++++------ test/integration/nxos.yaml | 7 ++ .../nxos_pim_interface/defaults/main.yaml | 2 + .../targets/nxos_pim_interface/meta/main.yml | 2 + .../targets/nxos_pim_interface/tasks/cli.yaml | 15 +++ .../nxos_pim_interface/tasks/main.yaml | 7 ++ .../nxos_pim_interface/tasks/nxapi.yaml | 28 +++++ .../nxos_pim_interface/tests/cli/sanity.yaml | 4 + .../tests/common/sanity.yaml | 102 ++++++++++++++++++ .../tests/nxapi/sanity.yaml | 4 + .../sh_run_interface_eth2_1_all | 0 .../network/nxos/test_nxos_pim_interface.py | 4 +- 12 files changed, 228 insertions(+), 37 deletions(-) create mode 100644 test/integration/targets/nxos_pim_interface/defaults/main.yaml create mode 100644 test/integration/targets/nxos_pim_interface/meta/main.yml create mode 100644 test/integration/targets/nxos_pim_interface/tasks/cli.yaml create mode 100644 test/integration/targets/nxos_pim_interface/tasks/main.yaml create mode 100644 test/integration/targets/nxos_pim_interface/tasks/nxapi.yaml create mode 100644 test/integration/targets/nxos_pim_interface/tests/cli/sanity.yaml create mode 100644 test/integration/targets/nxos_pim_interface/tests/common/sanity.yaml create mode 100644 test/integration/targets/nxos_pim_interface/tests/nxapi/sanity.yaml create mode 100644 test/units/modules/network/nxos/fixtures/nxos_pim_interface/sh_run_interface_eth2_1_all diff --git a/lib/ansible/modules/network/nxos/nxos_pim_interface.py b/lib/ansible/modules/network/nxos/nxos_pim_interface.py index affa0f8bc0e..c89dd9196a4 100644 --- a/lib/ansible/modules/network/nxos/nxos_pim_interface.py +++ b/lib/ansible/modules/network/nxos/nxos_pim_interface.py @@ -147,7 +147,7 @@ commands: "ip pim neighbor-policy test"] ''' - +import re from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.nxos import get_config, load_config, run_commands from ansible.module_utils.nxos import nxos_argument_spec, check_args @@ -246,6 +246,29 @@ def get_interface_mode(interface, intf_type, module): return mode +def get_jp_policy_out(module, interface): + ''' + This is a workaround for an nxos structured output problem. + The 'show ip pim interface' command does not display jp_policy_out + information properly for all supported nxos platforms when using + structured output. This method uses the show running information to + mitigate the problem. + ''' + command = 'sh run interface {0} all | grep \"jp.*out\"'.format(interface) + name = None + try: + body = execute_show_command(command, module, text=True)[0] + except IndexError: + return name + + if body: + mo = re.search(r'ip pim jp-policy\s+(\S+)\s+out', body) + if mo: + name = mo.group(1) + + return name + + def get_pim_interface(module, interface): pim_interface = {} command = 'show ip pim interface {0}'.format(interface) @@ -255,50 +278,47 @@ def get_pim_interface(module, interface): if 'not running' not in body[0]: body = execute_show_command(command, module) + # Some nxos platforms have the TABLE_vrf/ROW_vrf key and some don't try: get_data = body[0]['TABLE_vrf']['ROW_vrf']['TABLE_iod']['ROW_iod'] + except (KeyError, AttributeError, TypeError, IndexError): + try: + get_data = body[0]['TABLE_iod']['ROW_iod'] + except (KeyError, AttributeError, TypeError, IndexError): + return pim_interface - if isinstance(get_data.get('dr-priority'), list): - pim_interface['dr_prio'] = get_data.get('dr-priority')[0] - else: - pim_interface['dr_prio'] = str(get_data.get('dr-priority')) + if isinstance(get_data.get('dr-priority'), list): + pim_interface['dr_prio'] = get_data.get('dr-priority')[0] + else: + pim_interface['dr_prio'] = str(get_data.get('dr-priority')) - hello_interval = get_data.get('hello-interval-sec') - if hello_interval: - hello_interval_msec = int(get_data.get('hello-interval-sec')) * 1000 + hello_interval = get_data.get('hello-interval-sec') + if hello_interval: + hello_interval_msec = int(get_data.get('hello-interval-sec')) * 1000 pim_interface['hello_interval'] = str(hello_interval_msec) - border = get_data.get('is-border') - if border == 'true': - pim_interface['border'] = True - elif border == 'false': - pim_interface['border'] = False + border = get_data.get('is-border') + if border == 'true': + pim_interface['border'] = True + elif border == 'false': + pim_interface['border'] = False - isauth = get_data.get('isauth-config') - if isauth == 'true': - pim_interface['isauth'] = True - elif isauth == 'false': - pim_interface['isauth'] = False + isauth = get_data.get('isauth-config') + if isauth == 'true': + pim_interface['isauth'] = True + elif isauth == 'false': + pim_interface['isauth'] = False - pim_interface['neighbor_policy'] = get_data.get('nbr-policy-name') - if pim_interface['neighbor_policy'] == 'none configured': - pim_interface['neighbor_policy'] = None + pim_interface['neighbor_policy'] = get_data.get('nbr-policy-name') + if pim_interface['neighbor_policy'] == 'none configured': + pim_interface['neighbor_policy'] = None - jp_in_policy = get_data.get('jp-in-policy-name') - pim_interface['jp_policy_in'] = jp_in_policy - if jp_in_policy == 'none configured': - pim_interface['jp_policy_in'] = None + jp_in_policy = get_data.get('jp-in-policy-name') + pim_interface['jp_policy_in'] = jp_in_policy + if jp_in_policy == 'none configured': + pim_interface['jp_policy_in'] = None - if isinstance(get_data.get('jp-out-policy-name'), string_types): - pim_interface['jp_policy_out'] = get_data.get('jp-out-policy-name') - else: - pim_interface['jp_policy_out'] = get_data.get('jp-out-policy-name')[0] - - if pim_interface['jp_policy_out'] == 'none configured': - pim_interface['jp_policy_out'] = None - - except (KeyError, AttributeError, TypeError, IndexError): - return {} + pim_interface['jp_policy_out'] = get_jp_policy_out(module, interface) body = get_config(module, flags=['interface {0}'.format(interface)]) diff --git a/test/integration/nxos.yaml b/test/integration/nxos.yaml index 4b2fbc313c6..fd76f87cdcc 100644 --- a/test/integration/nxos.yaml +++ b/test/integration/nxos.yaml @@ -353,6 +353,13 @@ rescue: - set_fact: test_failed=true + - block: + - include_role: + name: nxos_pim_interface + when: "limit_to in ['*', 'nxos_pim_interface']" + rescue: + - set_fact: test_failed=true + ########### - debug: var=failed_modules when: test_failed diff --git a/test/integration/targets/nxos_pim_interface/defaults/main.yaml b/test/integration/targets/nxos_pim_interface/defaults/main.yaml new file mode 100644 index 00000000000..5f709c5aac1 --- /dev/null +++ b/test/integration/targets/nxos_pim_interface/defaults/main.yaml @@ -0,0 +1,2 @@ +--- +testcase: "*" diff --git a/test/integration/targets/nxos_pim_interface/meta/main.yml b/test/integration/targets/nxos_pim_interface/meta/main.yml new file mode 100644 index 00000000000..ae741cbdc71 --- /dev/null +++ b/test/integration/targets/nxos_pim_interface/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: + - prepare_nxos_tests diff --git a/test/integration/targets/nxos_pim_interface/tasks/cli.yaml b/test/integration/targets/nxos_pim_interface/tasks/cli.yaml new file mode 100644 index 00000000000..d675462dd02 --- /dev/null +++ b/test/integration/targets/nxos_pim_interface/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 diff --git a/test/integration/targets/nxos_pim_interface/tasks/main.yaml b/test/integration/targets/nxos_pim_interface/tasks/main.yaml new file mode 100644 index 00000000000..fea9337c14c --- /dev/null +++ b/test/integration/targets/nxos_pim_interface/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'] } diff --git a/test/integration/targets/nxos_pim_interface/tasks/nxapi.yaml b/test/integration/targets/nxos_pim_interface/tasks/nxapi.yaml new file mode 100644 index 00000000000..ea525379f7f --- /dev/null +++ b/test/integration/targets/nxos_pim_interface/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 }}" diff --git a/test/integration/targets/nxos_pim_interface/tests/cli/sanity.yaml b/test/integration/targets/nxos_pim_interface/tests/cli/sanity.yaml new file mode 100644 index 00000000000..420af7420e9 --- /dev/null +++ b/test/integration/targets/nxos_pim_interface/tests/cli/sanity.yaml @@ -0,0 +1,4 @@ +--- +- set_fact: connection="{{ cli }}" + +- import_tasks: "{{ role_path }}/tests/common/sanity.yaml" diff --git a/test/integration/targets/nxos_pim_interface/tests/common/sanity.yaml b/test/integration/targets/nxos_pim_interface/tests/common/sanity.yaml new file mode 100644 index 00000000000..7f659cc7bab --- /dev/null +++ b/test/integration/targets/nxos_pim_interface/tests/common/sanity.yaml @@ -0,0 +1,102 @@ +--- +- debug: msg="START TRANSPORT:{{ connection.transport }} nxos_pim_interface sanity test" + +- name: "Disable feature PIM" + nxos_feature: &disable_feature + feature: pim + state: disabled + provider: "{{ connection }}" + +- name: "Enable feature PIM" + nxos_feature: + feature: pim + state: enabled + provider: "{{ connection }}" + +- set_fact: testint="{{ nxos_int1 }}" + +- name: "Setup: Put interface {{ testint }} into a default state" + nxos_config: + lines: + - "default interface {{ testint }}" + provider: "{{ connection }}" + ignore_errors: yes + +- name: "Ensure {{testint}} is layer3" + nxos_interface: + interface: "{{ testint }}" + mode: layer3 + description: 'Configured by Ansible - Layer3' + admin_state: 'up' + state: present + provider: "{{ connection }}" + +- name: Configure nxos_pim_interface state absent + nxos_pim_interface: + interface: "{{ testint }}" + state: absent + provider: "{{ connection }}" + +- block: + - name: configure pim interface + nxos_pim_interface: &config + interface: "{{ testint }}" + dr_prio: 10 + hello_interval: 40 + border: 'false' + neighbor_policy: 'ansible_policy' + neighbor_type: 'prefix' + state: present + provider: "{{ connection }}" + register: result + + - assert: &true + that: + - "result.changed == true" + + - name: Check idempotence + nxos_pim_interface: *config + register: result + + - assert: &false + that: + - "result.changed == false" + + - name: configure gp policy and type + nxos_pim_interface: &configjp + interface: "{{ testint }}" + jp_policy_in: JPIN + jp_policy_out: JPOUT + jp_type_in: routemap + jp_type_out: routemap + provider: "{{ connection }}" + register: result + + - assert: *true + + - name: Check idempotence + nxos_pim_interface: *configjp + register: result + + - assert: *false + + - name: configure state default + nxos_pim_interface: &configdefault + interface: "{{ testint }}" + state: default + provider: "{{ connection }}" + register: result + + - assert: *true + + - name: Check idempotence + nxos_pim_interface: *configdefault + register: result + + - assert: *false + + always: + - name: "Disable feature PIM" + nxos_feature: *disable_feature + +- debug: msg="END TRANSPORT:{{ connection.transport }} nxos_pim_interface sanity test" diff --git a/test/integration/targets/nxos_pim_interface/tests/nxapi/sanity.yaml b/test/integration/targets/nxos_pim_interface/tests/nxapi/sanity.yaml new file mode 100644 index 00000000000..e30ea6eaf70 --- /dev/null +++ b/test/integration/targets/nxos_pim_interface/tests/nxapi/sanity.yaml @@ -0,0 +1,4 @@ +--- +- set_fact: connection="{{ nxapi }}" + +- import_tasks: "{{ role_path }}/tests/common/sanity.yaml" diff --git a/test/units/modules/network/nxos/fixtures/nxos_pim_interface/sh_run_interface_eth2_1_all b/test/units/modules/network/nxos/fixtures/nxos_pim_interface/sh_run_interface_eth2_1_all new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/units/modules/network/nxos/test_nxos_pim_interface.py b/test/units/modules/network/nxos/test_nxos_pim_interface.py index 05b0595b257..b75c2703669 100644 --- a/test/units/modules/network/nxos/test_nxos_pim_interface.py +++ b/test/units/modules/network/nxos/test_nxos_pim_interface.py @@ -62,12 +62,12 @@ class TestNxosIPInterfaceModule(TestNxosModule): self.run_commands.side_effect = load_from_file def test_nxos_pim_interface_present(self): - set_module_args(dict(interface='eth2/1', dr_prio=10, hello_interval=40, sparse=True, border=True)) + set_module_args(dict(interface='eth2/1', dr_prio=10, hello_interval=40, sparse=True, border=False)) self.execute_module( changed=True, commands=[ 'interface eth2/1', 'ip pim dr-priority 10', 'ip pim hello-interval 40000', - 'ip pim sparse-mode', 'ip pim border'] + 'ip pim sparse-mode', 'no ip pim border'] ) def test_nxos_pim_interface_jp(self):