From 64bad56484b15c05d4aecfdb5fae90695723b11c Mon Sep 17 00:00:00 2001 From: saichint Date: Thu, 3 May 2018 09:04:25 -0700 Subject: [PATCH] fix nxos_snmp_traps issues (#39444) * fix snmp_traps code * add IT cases * fix shippable * fix shippable without ignore --- .../modules/network/nxos/nxos_snmp_traps.py | 136 +++++++----------- test/integration/nxos.yaml | 9 ++ .../nxos_snmp_traps/defaults/main.yaml | 2 + .../targets/nxos_snmp_traps/meta/main.yml | 2 + .../targets/nxos_snmp_traps/tasks/cli.yaml | 33 +++++ .../targets/nxos_snmp_traps/tasks/main.yaml | 7 + .../targets/nxos_snmp_traps/tasks/nxapi.yaml | 27 ++++ .../nxos_snmp_traps/tests/common/sanity.yaml | 84 +++++++++++ 8 files changed, 216 insertions(+), 84 deletions(-) create mode 100644 test/integration/targets/nxos_snmp_traps/defaults/main.yaml create mode 100644 test/integration/targets/nxos_snmp_traps/meta/main.yml create mode 100644 test/integration/targets/nxos_snmp_traps/tasks/cli.yaml create mode 100644 test/integration/targets/nxos_snmp_traps/tasks/main.yaml create mode 100644 test/integration/targets/nxos_snmp_traps/tasks/nxapi.yaml create mode 100644 test/integration/targets/nxos_snmp_traps/tests/common/sanity.yaml diff --git a/lib/ansible/modules/network/nxos/nxos_snmp_traps.py b/lib/ansible/modules/network/nxos/nxos_snmp_traps.py index 146614be7eccbd..d42eca87bed625 100644 --- a/lib/ansible/modules/network/nxos/nxos_snmp_traps.py +++ b/lib/ansible/modules/network/nxos/nxos_snmp_traps.py @@ -42,9 +42,10 @@ description: - Case sensitive group. required: true - choices: ['aaa', 'bridge', 'callhome', 'cfs', 'config', 'entity', - 'feature-control', 'hsrp', 'license', 'link', 'lldp', 'ospf', 'pim', - 'rf', 'rmon', 'snmp', 'storm-control', 'stpx', 'sysmgr', 'system', + choices: ['aaa', 'bfd', 'bgp', 'bridge', 'callhome', 'cfs', 'config', + 'eigrp', 'entity', 'feature-control', 'generic', 'hsrp', 'license', + 'link', 'lldp', 'mmode', 'ospf', 'pim', 'rf', 'rmon', 'snmp', + 'storm-control', 'stpx', 'switchfabric', 'syslog', 'sysmgr', 'system', 'upgrade', 'vtp', 'all'] state: description: @@ -83,25 +84,12 @@ def execute_show_command(command, module): command = { 'command': command, - 'output': 'json', + 'output': 'text', } return run_commands(module, command) -def apply_key_map(key_map, table): - new_dict = {} - for key, value in table.items(): - new_key = key_map.get(key) - if new_key: - value = table.get(key) - if value: - new_dict[new_key] = str(value) - else: - new_dict[new_key] = value - return new_dict - - def flatten_list(command_lists): flat_command_list = [] for command in command_lists: @@ -113,61 +101,44 @@ def flatten_list(command_lists): def get_snmp_traps(group, module): - body = execute_show_command('show snmp trap', module) - - trap_key = { - 'description': 'trap', - 'isEnabled': 'enabled' - } - - trap_key_5k = { - 'trap': 'trap', - 'enabled': 'enabled' - } + body = execute_show_command('show run snmp all', module)[0].split('\n') resource = {} - feature_list = ['aaa', 'bridge', 'callhome', 'cfs', 'config', - 'entity', 'feature-control', 'hsrp', 'license', - 'link', 'lldp', 'ospf', 'pim', 'rf', 'rmon', - 'snmp', 'storm-control', 'stpx', 'sysmgr', - 'system', 'upgrade', 'vtp'] - try: - resource_table = body[0]['TABLE_snmp_trap']['ROW_snmp_trap'] - - for each_feature in feature_list: - resource[each_feature] = [] - - for each_resource in resource_table: - key = str(each_resource['trap_type']) - mapped_trap = apply_key_map(trap_key, each_resource) - - if key != 'Generic': - resource[key].append(mapped_trap) - except KeyError: - try: - resource_table = body[0]['TABLE_mib']['ROW_mib'] - - for each_feature in feature_list: - resource[each_feature] = [] - - for each_resource in resource_table: - key = str(each_resource['mib']) - each_resource = each_resource['TABLE_trap']['ROW_trap'] - mapped_trap = apply_key_map(trap_key_5k, each_resource) - - if key != 'Generic': - resource[key].append(mapped_trap) - except (KeyError, AttributeError): - return resource - except AttributeError: - return resource + feature_list = ['aaa', 'bfd', 'bgp', 'bridge', 'callhome', 'cfs', 'config', + 'eigrp', 'entity', 'feature-control', 'generic', 'hsrp', + 'license', 'link', 'lldp', 'mmode', 'ospf', 'pim', + 'rf', 'rmon', 'snmp', 'storm-control', 'stpx', + 'switchfabric', 'syslog', 'sysmgr', 'system', 'upgrade', + 'vtp'] + for each in feature_list: + for line in body: + if each == 'ospf': + # ospf behaves differently when routers are present + if 'snmp-server enable traps ospf' == line: + resource[each] = True + break + else: + if 'enable traps {0}'.format(each) in line: + if 'no ' in line: + resource[each] = False + break + else: + resource[each] = True + + for each in feature_list: + if resource.get(each) is None: + # on some platforms, the 'no' cmd does not + # show up and so check if the feature is enabled + body = execute_show_command('show run | inc feature', module)[0] + if 'feature {0}'.format(each) in body: + resource[each] = False find = resource.get(group, None) if group == 'all'.lower(): return resource - elif find: - trap_resource = {group: resource[group]} + elif find is not None: + trap_resource = {group: find} return trap_resource else: # if 'find' is None, it means that 'group' is a @@ -183,26 +154,22 @@ def get_trap_commands(group, state, existing, module): if group == 'all': if state == 'disabled': for feature in existing: - trap_commands = ['no snmp-server enable traps {0}'.format(feature) for - trap in existing[feature] if trap['enabled'] == 'Yes'] - trap_commands = list(set(trap_commands)) - commands.append(trap_commands) + if existing[feature]: + trap_command = 'no snmp-server enable traps {0}'.format(feature) + commands.append(trap_command) elif state == 'enabled': for feature in existing: - trap_commands = ['snmp-server enable traps {0}'.format(feature) for - trap in existing[feature] if trap['enabled'] == 'No'] - trap_commands = list(set(trap_commands)) - commands.append(trap_commands) + if existing[feature] is False: + trap_command = 'snmp-server enable traps {0}'.format(feature) + commands.append(trap_command) else: if group in existing: - for each_trap in existing[group]: - check = each_trap['enabled'] - if check.lower() == 'yes': - enabled = True - if check.lower() == 'no': - disabled = True + if existing[group]: + enabled = True + else: + disabled = True if state == 'disabled' and enabled: commands.append(['no snmp-server enable traps {0}'.format(group)]) @@ -218,11 +185,12 @@ def get_trap_commands(group, state, existing, module): def main(): argument_spec = dict( state=dict(choices=['enabled', 'disabled'], default='enabled'), - group=dict(choices=['aaa', 'bridge', 'callhome', 'cfs', 'config', - 'entity', 'feature-control', 'hsrp', - 'license', 'link', 'lldp', 'ospf', 'pim', 'rf', - 'rmon', 'snmp', 'storm-control', 'stpx', - 'sysmgr', 'system', 'upgrade', 'vtp', 'all'], + group=dict(choices=['aaa', 'bfd', 'bgp', 'bridge', 'callhome', 'cfs', 'config', + 'eigrp', 'entity', 'feature-control', 'generic', 'hsrp', + 'license', 'link', 'lldp', 'mmode', 'ospf', 'pim', + 'rf', 'rmon', 'snmp', 'storm-control', 'stpx', + 'switchfabric', 'syslog', 'sysmgr', 'system', 'upgrade', + 'vtp', 'all'], required=True), ) diff --git a/test/integration/nxos.yaml b/test/integration/nxos.yaml index 509dad851d49b7..67ac2f796ab050 100644 --- a/test/integration/nxos.yaml +++ b/test/integration/nxos.yaml @@ -554,6 +554,15 @@ failed_modules: "{{ failed_modules }} + [ 'nxos_snmp_location' ]" test_failed: true + - block: + - include_role: + name: nxos_snmp_traps + when: "limit_to in ['*', 'nxos_snmp_traps']" + rescue: + - set_fact: + failed_modules: "{{ failed_modules }} + [ 'nxos_snmp_traps' ]" + test_failed: true + - block: - include_role: name: nxos_snmp_host diff --git a/test/integration/targets/nxos_snmp_traps/defaults/main.yaml b/test/integration/targets/nxos_snmp_traps/defaults/main.yaml new file mode 100644 index 00000000000000..5f709c5aac15f1 --- /dev/null +++ b/test/integration/targets/nxos_snmp_traps/defaults/main.yaml @@ -0,0 +1,2 @@ +--- +testcase: "*" diff --git a/test/integration/targets/nxos_snmp_traps/meta/main.yml b/test/integration/targets/nxos_snmp_traps/meta/main.yml new file mode 100644 index 00000000000000..ae741cbdc71660 --- /dev/null +++ b/test/integration/targets/nxos_snmp_traps/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: + - prepare_nxos_tests diff --git a/test/integration/targets/nxos_snmp_traps/tasks/cli.yaml b/test/integration/targets/nxos_snmp_traps/tasks/cli.yaml new file mode 100644 index 00000000000000..edbff7dfafbb0c --- /dev/null +++ b/test/integration/targets/nxos_snmp_traps/tasks/cli.yaml @@ -0,0 +1,33 @@ +--- +- name: collect common cli test cases + find: + paths: "{{ role_path }}/tests/common" + patterns: "{{ testcase }}.yaml" + connection: local + register: test_cases + +- name: collect cli test cases + find: + paths: "{{ role_path }}/tests/cli" + patterns: "{{ testcase }}.yaml" + connection: local + register: cli_cases + +- set_fact: + test_cases: + files: "{{ test_cases.files }} + {{ cli_cases.files }}" + +- name: set test_items + set_fact: test_items="{{ test_cases.files | map(attribute='path') | list }}" + +- name: run test cases (connection=network_cli) + include: "{{ test_case_to_run }} ansible_connection=network_cli connection={}" + with_items: "{{ test_items }}" + loop_control: + loop_var: test_case_to_run + +- name: run test case (connection=local) + include: "{{ test_case_to_run }} ansible_connection=local connection={{ cli }}" + with_first_found: "{{ test_items }}" + loop_control: + loop_var: test_case_to_run diff --git a/test/integration/targets/nxos_snmp_traps/tasks/main.yaml b/test/integration/targets/nxos_snmp_traps/tasks/main.yaml new file mode 100644 index 00000000000000..fea9337c14c955 --- /dev/null +++ b/test/integration/targets/nxos_snmp_traps/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_snmp_traps/tasks/nxapi.yaml b/test/integration/targets/nxos_snmp_traps/tasks/nxapi.yaml new file mode 100644 index 00000000000000..68e96a29420612 --- /dev/null +++ b/test/integration/targets/nxos_snmp_traps/tasks/nxapi.yaml @@ -0,0 +1,27 @@ +--- +- name: collect common nxapi test cases + find: + paths: "{{ role_path }}/tests/common" + patterns: "{{ testcase }}.yaml" + connection: local + register: test_cases + +- name: collect nxapi test cases + find: + paths: "{{ role_path }}/tests/nxapi" + patterns: "{{ testcase }}.yaml" + connection: local + register: nxapi_cases + +- set_fact: + test_cases: + files: "{{ test_cases.files }} + {{ nxapi_cases.files }}" + +- name: set test_items + set_fact: test_items="{{ test_cases.files | map(attribute='path') | list }}" + +- name: run test cases (connection=local) + include: "{{ test_case_to_run }} ansible_connection=local connection={{ nxapi }}" + with_items: "{{ test_items }}" + loop_control: + loop_var: test_case_to_run diff --git a/test/integration/targets/nxos_snmp_traps/tests/common/sanity.yaml b/test/integration/targets/nxos_snmp_traps/tests/common/sanity.yaml new file mode 100644 index 00000000000000..4286813c8667e7 --- /dev/null +++ b/test/integration/targets/nxos_snmp_traps/tests/common/sanity.yaml @@ -0,0 +1,84 @@ +--- +- debug: msg="START connection={{ ansible_connection }} nxos_snmp_traps sanity test" +- debug: msg="Using provider={{ connection.transport }}" + when: ansible_connection == "local" + +- name: Setup - Remove snmp_traps if configured + nxos_snmp_traps: &remove + group: all + state: disabled + provider: "{{ connection }}" + +- block: + - name: Configure one snmp trap group + nxos_snmp_traps: &config + group: bridge + state: enabled + provider: "{{ connection }}" + register: result + + - assert: &true + that: + - "result.changed == true" + + - name: Idempotence Check + nxos_snmp_traps: *config + register: result + + - assert: &false + that: + - "result.changed == false" + + - name: Remove snmp trap group + nxos_snmp_traps: &rem1 + group: bridge + state: disabled + provider: "{{ connection }}" + register: result + + - assert: *true + + - name: Idempotence Check + nxos_snmp_traps: *rem1 + register: result + + - assert: *false + + - name: Configure all snmp trap groups + nxos_snmp_traps: &config1 + group: all + state: enabled + provider: "{{ connection }}" + register: result + + - assert: *true + + - block: + # On I2, link command does not work properly + # On D1, callhome command does not work properly + # skip for these older platforms + - name: Idempotence Check + nxos_snmp_traps: *config1 + register: result + when: imagetag is not search("I2|D1") + + - assert: *false + when: imagetag is not search("I2|D1") + + - name: Cleanup + nxos_snmp_traps: *remove + register: result + + - assert: *true + + - name: Cleanup Idempotence + nxos_snmp_traps: *remove + register: result + + - assert: *false + + always: + - name: Cleanup + nxos_snmp_traps: *remove + + - debug: msg="END connection={{ ansible_connection }} nxos_snmp_traps sanity test"