fix nxos_snmp_traps issues (ansible#39444)

* fix snmp_traps code

* add IT cases

* fix shippable

* fix shippable without ignore

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),
     )
 

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

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

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

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

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

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

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'] }

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

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"