Fix for nxos_snmp_host issues (#39642)

* fix snmp_host issues

* source files

* fix shippable

* remove defaults to match arg spec

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

@@ -42,9 +42,8 @@ options:
         required: true
     version:
         description:
-            - SNMP version.
+            - SNMP version. If this is not specified, v1 is used.
-        default: v2c
+        choices: ['v1', 'v2c', 'v3']
-        choices: ['v2c', 'v3']
     v3:
         description:
             - Use this when verion is v3. SNMPv3 Security level.
@@ -55,23 +54,33 @@ options:
     udp:
         description:
             - UDP port number (0-65535).
+        default: 162
     snmp_type:
         description:
-            - type of message to send to host.
+            - type of message to send to host. If this is not
-        default: trap
+              specified, trap type is used.
         choices: ['trap', 'inform']
     vrf:
         description:
             - VRF to use to source traffic to source.
+              If state = absent, the vrf is removed.
     vrf_filter:
         description:
             - Name of VRF to filter.
+              If state = absent, the vrf is removed from the filter.
     src_intf:
         description:
-            - Source interface.
+            - Source interface. Must be fully qualified interface name.
+              If state = absent, the interface is removed.
     state:
         description:
-            - Manage the state of the resource.
+            - Manage the state of the resource. If state = present, the
+              host is added to the configuration. If only vrf and/or
+              vrf_filter and/or src_intf are given, they will be added to
+              the existing host configuration. If state = absent, the
+              host is removed if community parameter is given. It is possible
+              to remove only vrf and/or src_int and/or vrf_filter
+              by providing only those parameters and no community parameter.
         default: present
         choices: ['present','absent']
 '''
@@ -131,7 +140,7 @@ def flatten_list(command_lists):
     return flat_command_list
 
 
-def get_snmp_host(host, module):
+def get_snmp_host(host, udp, module):
     body = execute_show_command('show snmp host', module)
 
     host_map = {
@@ -160,7 +169,7 @@ def get_snmp_host(host, module):
                 resource_table = [resource_table]
 
             for each in resource_table:
-                key = str(each['host'])
+                key = str(each['host']) + '_' + str(each['port']).strip()
                 src = each.get('src_intf')
                 host_resource = apply_key_map(host_map, each)
 
@@ -188,7 +197,7 @@ def get_snmp_host(host, module):
                     resource_table = [resource_table]
 
                 for each in resource_table:
-                    key = str(each['address'])
+                    key = str(each['address']) + '_' + str(each['port']).strip()
                     src = each.get('src_intf')
                     host_resource = apply_key_map(host_map_5k, each)
 
@@ -197,22 +206,23 @@ def get_snmp_host(host, module):
                         if re.search(r'interface:', src):
                             host_resource['src_intf'] = src.split(':')[1].strip()
 
+                    vrf = each.get('use_vrf_name')
+                    if vrf:
+                        host_resource['vrf'] = vrf.strip()
+
                     vrf_filt = each.get('TABLE_filter_vrf')
                     if vrf_filt:
                         vrf_filter = vrf_filt['ROW_filter_vrf']['filter_vrf_name'].split(',')
                         filters = [vrf.strip() for vrf in vrf_filter]
                         host_resource['vrf_filter'] = filters
 
-                    vrf = each.get('use_vrf_name')
-                    if vrf:
-                        host_resource['vrf'] = vrf.strip()
                     resource[key] = host_resource
             except (KeyError, AttributeError, TypeError):
                 return resource
         except (AttributeError, TypeError):
             return resource
 
-        find = resource.get(host)
+        find = resource.get(host + '_' + udp)
 
         if find:
             fix_find = {}
@@ -226,24 +236,54 @@ def get_snmp_host(host, module):
     return {}
 
 
-def remove_snmp_host(host, existing):
+def remove_snmp_host(host, udp, existing):
     commands = []
     if existing['version'] == 'v3':
         existing['version'] = '3'
         command = 'no snmp-server host {0} {snmp_type} version \
-                    {version} {v3} {community}'.format(host, **existing)
+                    {version} {v3} {community} udp-port {1}'.format(host, udp, **existing)
 
     elif existing['version'] == 'v2c':
         existing['version'] = '2c'
         command = 'no snmp-server host {0} {snmp_type} version \
-                    {version} {community}'.format(host, **existing)
+                    {version} {community} udp-port {1}'.format(host, udp, **existing)
+
+    elif existing['version'] == 'v1':
+        existing['version'] = '1'
+        command = 'no snmp-server host {0} {snmp_type} version \
+                    {version} {community} udp-port {1}'.format(host, udp, **existing)
 
     if command:
         commands.append(command)
     return commands
 
 
-def config_snmp_host(delta, proposed, existing, module):
+def remove_vrf(host, udp, proposed, existing):
+    commands = []
+    if existing.get('vrf'):
+        commands.append('no snmp-server host {0} use-vrf \
+                    {1} udp-port {2}'.format(host, proposed.get('vrf'), udp))
+    return commands
+
+
+def remove_filter(host, udp, proposed, existing):
+    commands = []
+    if existing.get('vrf_filter'):
+        if proposed.get('vrf_filter') in existing.get('vrf_filter'):
+            commands.append('no snmp-server host {0} filter-vrf \
+                    {1} udp-port {2}'.format(host, proposed.get('vrf_filter'), udp))
+    return commands
+
+
+def remove_src(host, udp, proposed, existing):
+    commands = []
+    if existing.get('src_intf'):
+        commands.append('no snmp-server host {0} source-interface \
+                    {1} udp-port {2}'.format(host, proposed.get('src_intf'), udp))
+    return commands
+
+
+def config_snmp_host(delta, udp, proposed, existing, module):
     commands = []
     command_builder = []
     host = proposed['snmp_host']
@@ -262,7 +302,9 @@ def config_snmp_host(delta, proposed, existing, module):
 
         version = version or existing.get('version')
         if version:
-            if version == 'v2c':
+            if version == 'v1':
+                vn = '1'
+            elif version == 'v2c':
                 vn = '2c'
             elif version == 'v3':
                 vn = '3'
@@ -278,21 +320,23 @@ def config_snmp_host(delta, proposed, existing, module):
             community_string = community or existing.get('community')
             command_builder.append(community_string)
 
+        udp_string = ' udp-port {0}'.format(udp)
+        command_builder.append(udp_string)
+
         cmd = ' '.join(command_builder)
 
         commands.append(cmd)
 
     CMDS = {
-        'vrf_filter': 'snmp-server host {0} filter-vrf {vrf_filter}',
+        'vrf_filter': 'snmp-server host {0} filter-vrf {vrf_filter} udp-port {1}',
-        'vrf': 'snmp-server host {0} use-vrf {vrf}',
+        'vrf': 'snmp-server host {0} use-vrf {vrf} udp-port {1}',
-        'udp': 'snmp-server host {0} udp-port {udp}',
+        'src_intf': 'snmp-server host {0} source-interface {src_intf} udp-port {1}'
-        'src_intf': 'snmp-server host {0} source-interface {src_intf}'
     }
 
     for key in delta:
         command = CMDS.get(key)
         if command:
-            cmd = command.format(host, **delta)
+            cmd = command.format(host, udp, **delta)
             commands.append(cmd)
     return commands
 
@@ -301,13 +345,13 @@ def main():
     argument_spec = dict(
         snmp_host=dict(required=True, type='str'),
         community=dict(type='str'),
-        udp=dict(type='str'),
+        udp=dict(type='str', default='162'),
-        version=dict(choices=['v2c', 'v3'], default='v2c'),
+        version=dict(choices=['v1', 'v2c', 'v3']),
         src_intf=dict(type='str'),
         v3=dict(choices=['noauth', 'auth', 'priv']),
         vrf_filter=dict(type='str'),
         vrf=dict(type='str'),
-        snmp_type=dict(choices=['trap', 'inform'], default='trap'),
+        snmp_type=dict(choices=['trap', 'inform']),
         state=dict(choices=['absent', 'present'], default='present'),
     )
 
@@ -330,17 +374,35 @@ def main():
     snmp_type = module.params['snmp_type']
     state = module.params['state']
 
-    if snmp_type == 'inform' and version != 'v3':
+    existing = get_snmp_host(snmp_host, udp, module)
-        module.fail_json(msg='inform requires snmp v3')
+
+    if version is None:
+        if existing:
+            version = existing.get('version')
+        else:
+            version = 'v1'
+
+    if snmp_type is None:
+        if existing:
+            snmp_type = existing.get('snmp_type')
+        else:
+            snmp_type = 'trap'
+
+    if v3 is None:
+        if version == 'v3' and existing:
+            v3 = existing.get('v3')
+
+    if snmp_type == 'inform' and version == 'v1':
+        module.fail_json(msg='inform requires snmp v2c or v3')
 
-    if version == 'v2c' and v3:
+    if (version == 'v1' or version == 'v2c') and v3:
         module.fail_json(msg='param: "v3" should not be used when '
-                             'using version v2c')
+                             'using version v1 or v2c')
 
-    if not any([vrf_filter, vrf, udp, src_intf]):
+    if not any([vrf_filter, vrf, src_intf]):
-        if not all([snmp_type, version, community]):
+        if not all([snmp_type, version, community, udp]):
             module.fail_json(msg='when not configuring options like '
-                                 'vrf_filter, vrf, udp, and src_intf,'
+                                 'vrf_filter, vrf, and src_intf,'
                                  'the following params are required: '
                                  'type, version, community')
 
@@ -348,8 +410,6 @@ def main():
         module.fail_json(msg='when using version=v3, the param v3 '
                              '(options: auth, noauth, priv) is also required')
 
-    existing = get_snmp_host(snmp_host, module)
-
     # existing returns the list of vrfs configured for a given host
     # checking to see if the proposed is in the list
     store = existing.get('vrf_filter')
@@ -360,11 +420,6 @@ def main():
             existing['vrf_filter'] = vrf_filter
     commands = []
 
-    if state == 'absent' and existing:
-        command = remove_snmp_host(snmp_host, existing)
-        commands.append(command)
-
-    elif state == 'present':
     args = dict(
         community=community,
         snmp_host=snmp_host,
@@ -378,9 +433,21 @@ def main():
     )
     proposed = dict((k, v) for k, v in args.items() if v is not None)
 
+    if state == 'absent' and existing:
+        if proposed.get('community'):
+            commands.append(remove_snmp_host(snmp_host, udp, existing))
+        else:
+            if proposed.get('src_intf'):
+                commands.append(remove_src(snmp_host, udp, proposed, existing))
+            if proposed.get('vrf'):
+                commands.append(remove_vrf(snmp_host, udp, proposed, existing))
+            if proposed.get('vrf_filter'):
+                commands.append(remove_filter(snmp_host, udp, proposed, existing))
+
+    elif state == 'present':
         delta = dict(set(proposed.items()).difference(existing.items()))
         if delta:
-            command = config_snmp_host(delta, proposed, existing, module)
+            command = config_snmp_host(delta, udp, proposed, existing, module)
             commands.append(command)
 
     cmds = flatten_list(commands)

test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v1_trap.yaml

@@ -0,0 +1,129 @@
+---
+- set_fact: snmp_type="trap"
+- set_fact: snmp_version="v1"
+
+- debug: msg="START connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }} sanity test"
+- debug: msg="Using provider={{ connection.transport }}"
+  when: ansible_connection == "local"
+
+# Select interface for test
+- set_fact: intname="{{ nxos_int1 }}"
+  when: not (platform is match("N5K"))
+
+- name: Setup - Remove snmp_host if configured
+  nxos_snmp_host: &remove
+    snmp_host: 3.3.3.3
+    community: TESTING
+    version: "{{ snmp_version }}" 
+    snmp_type: "{{ snmp_type }}" 
+    vrf: management
+    vrf_filter: management
+    src_intf: "{{ intname|default(omit) }}"
+    udp: 222
+    state: absent 
+    provider: "{{ connection }}"
+  ignore_errors: yes
+
+- block:
+
+  - name: Configure snmp host 
+    nxos_snmp_host: &config
+      snmp_host: 3.3.3.3
+      community: TESTING
+      version: "{{ snmp_version }}" 
+      snmp_type: "{{ snmp_type }}" 
+      vrf: management
+      vrf_filter: management
+      src_intf: "{{ intname|default(omit) }}"
+      udp: 222
+      state: present 
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: &true
+      that:
+        - "result.changed == true"
+
+  - name: Idempotence Check
+    nxos_snmp_host: *config
+    register: result
+
+  - assert: &false
+      that:
+        - "result.changed == false"
+
+  - block:
+    - name: Add another vrf to filter
+      nxos_snmp_host: &config1
+        snmp_host: 3.3.3.3
+        vrf_filter: default
+        udp: 222
+        state: present 
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: Idempotence Check
+      nxos_snmp_host: *config1
+      register: result
+
+    - assert: *false
+    when: not (platform is match('N35|N5K'))
+
+  - name: remove some configuration
+    nxos_snmp_host: &rem1
+      snmp_host: 3.3.3.3
+      udp: 222
+      src_intf: "{{ intname|default(omit) }}"
+      vrf: management
+      vrf_filter: management
+      state: absent
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: Idempotence Check
+    nxos_snmp_host: *rem1
+    register: result
+
+  - assert: *false
+
+  - block:
+    - name: remove some more configuration
+      nxos_snmp_host: &rem2
+        snmp_host: 3.3.3.3
+        udp: 222
+        vrf_filter: default
+        state: absent
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: Idempotence Check
+      nxos_snmp_host: *rem2
+      register: result
+
+    - assert: *false
+    when: not (platform is match('N35|N5K'))
+
+
+  - name: Cleanup
+    nxos_snmp_host: *remove
+    register: result
+
+  - assert: *true
+
+  - name: Cleanup Idempotence
+    nxos_snmp_host: *remove
+    register: result
+
+  - assert: *false
+
+  always:
+  - name: Cleanup
+    nxos_snmp_host: *remove
+
+  - debug: msg="END connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }} sanity test"

test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v2_inform.yaml

@@ -1,8 +1,8 @@
 ---
-- set_fact: snmp_type="trap"
+- set_fact: snmp_type="inform"
 - set_fact: snmp_version="v2c"
 
-- debug: msg="START connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }}sanity test"
+- debug: msg="START connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }} sanity test"
 - debug: msg="Using provider={{ connection.transport }}"
   when: ansible_connection == "local"
 
@@ -19,6 +19,7 @@
     vrf: management
     vrf_filter: management
     src_intf: "{{ intname|default(omit) }}"
+    udp: 222
     state: absent 
     provider: "{{ connection }}"
   ignore_errors: yes
@@ -34,6 +35,7 @@
       vrf: management
       vrf_filter: management
       src_intf: "{{ intname|default(omit) }}"
+      udp: 222
       state: present 
       provider: "{{ connection }}"
     register: result
@@ -50,7 +52,63 @@
       that:
         - "result.changed == false"
 
-  always:
+  - block:
+    - name: Add another vrf to filter
+      nxos_snmp_host: &config1
+        snmp_host: 3.3.3.3
+        vrf_filter: default
+        udp: 222
+        state: present 
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: Idempotence Check
+      nxos_snmp_host: *config1
+      register: result
+
+    - assert: *false
+    when: not (platform is match('N35|N5K'))
+
+  - name: remove some configuration
+    nxos_snmp_host: &rem1
+      snmp_host: 3.3.3.3
+      udp: 222
+      src_intf: "{{ intname|default(omit) }}"
+      vrf: management
+      vrf_filter: management
+      state: absent
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: Idempotence Check
+    nxos_snmp_host: *rem1
+    register: result
+
+  - assert: *false
+
+  - block:
+    - name: remove some more configuration
+      nxos_snmp_host: &rem2
+        snmp_host: 3.3.3.3
+        udp: 222
+        vrf_filter: default
+        state: absent
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: Idempotence Check
+      nxos_snmp_host: *rem2
+      register: result
+
+    - assert: *false
+    when: not (platform is match('N35|N5K'))
+
   - name: Cleanup
     nxos_snmp_host: *remove
     register: result
@@ -63,4 +121,8 @@
 
   - assert: *false
 
-  - debug: msg="END connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }}sanity test"
+  always:
+  - name: Cleanup
+    nxos_snmp_host: *remove
+
+  - debug: msg="END connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }} sanity test"

test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v3_inform.yaml

@@ -3,7 +3,7 @@
 - set_fact: snmp_version="v3"
 - set_fact: snmp_auth="noauth"
 
-- debug: msg="START connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }}sanity test"
+- debug: msg="START connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }} sanity test"
 - debug: msg="Using provider={{ connection.transport }}"
   when: ansible_connection == "local"
 
@@ -11,13 +11,17 @@
 - set_fact: intname="{{ nxos_int1 }}"
   when: not (platform is match("N5K"))
 
+- set_fact: run="true"
+- set_fact: run="false"
+  when: platform is match('N35')
+
 - name: Setup - Remove snmp_host if configured
   nxos_snmp_host: &remove
     snmp_host: 3.3.3.3
     community: TESTING
-    v3: "{{ snmp_auth|default(omit) }}"
     version: "{{ snmp_version }}" 
     snmp_type: "{{ snmp_type }}" 
+    v3: "{{ snmp_auth }}" 
     vrf: management
     vrf_filter: management
     src_intf: "{{ intname|default(omit) }}"
@@ -53,9 +57,60 @@
       that:
         - "result.changed == false"
 
-  when: not (platform is match('N35'))
+  - block:
+    - name: Add another vrf to filter
+      nxos_snmp_host: &config1
+        snmp_host: 3.3.3.3
+        vrf_filter: default
+        state: present 
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: Idempotence Check
+      nxos_snmp_host: *config1
+      register: result
+
+    - assert: *false
+    when: not (platform is match('N35|N5K'))
+
+  - name: remove some configuration
+    nxos_snmp_host: &rem1
+      snmp_host: 3.3.3.3
+      src_intf: "{{ intname|default(omit) }}"
+      vrf: management
+      vrf_filter: management
+      state: absent
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: Idempotence Check
+    nxos_snmp_host: *rem1
+    register: result
+
+  - assert: *false
+
+  - block:
+    - name: remove some more configuration
+      nxos_snmp_host: &rem2
+        snmp_host: 3.3.3.3
+        vrf_filter: default
+        state: absent
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: Idempotence Check
+      nxos_snmp_host: *rem2
+      register: result
+
+    - assert: *false
+    when: not (platform is match('N35|N5K'))
 
-  always:
   - name: Cleanup
     nxos_snmp_host: *remove
     register: result
@@ -68,4 +123,11 @@
 
   - assert: *false
 
-  - debug: msg="END connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }}sanity test"
+  when: run
+
+  always:
+  - name: Cleanup
+    nxos_snmp_host: *remove
+    register: result
+
+  - debug: msg="END connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }} sanity test"

test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v3_trap.yaml

@@ -1,21 +1,27 @@
 ---
 - set_fact: snmp_type="trap"
 - set_fact: snmp_version="v3"
-- set_fact: snmp_auth="noauth"
+- set_fact: snmp_auth="priv"
 
-- debug: msg="START connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }}sanity test"
+- debug: msg="START connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }} sanity test"
 - debug: msg="Using provider={{ connection.transport }}"
   when: ansible_connection == "local"
 
+# Select interface for test
+- set_fact: intname="{{ nxos_int1 }}"
+  when: not (platform is match("N5K"))
+
 - name: Setup - Remove snmp_host if configured
   nxos_snmp_host: &remove
     snmp_host: 3.3.3.3
     community: TESTING
+    udp: 222
     v3: "{{ snmp_auth|default(omit) }}"
     version: "{{ snmp_version }}" 
     snmp_type: "{{ snmp_type }}" 
     vrf: management
     vrf_filter: management
+    src_intf: "{{ intname|default(omit) }}"
     state: absent 
     provider: "{{ connection }}"
   ignore_errors: yes
@@ -26,11 +32,13 @@
     nxos_snmp_host: &config
       snmp_host: 3.3.3.3
       community: TESTING
+      udp: 222
       v3: "{{ snmp_auth|default(omit) }}"
       version: "{{ snmp_version }}" 
       snmp_type: "{{ snmp_type }}" 
       vrf: management
       vrf_filter: management
+      src_intf: "{{ intname|default(omit) }}"
       state: present 
       provider: "{{ connection }}"
     register: result
@@ -47,7 +55,63 @@
       that:
         - "result.changed == false"
 
-  always:
+  - block:
+    - name: Add another vrf to filter
+      nxos_snmp_host: &config1
+        snmp_host: 3.3.3.3
+        udp: 222
+        vrf_filter: default
+        state: present 
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: Idempotence Check
+      nxos_snmp_host: *config1
+      register: result
+
+    - assert: *false
+    when: not (platform is match('N35|N5K'))
+
+  - name: remove some configuration
+    nxos_snmp_host: &rem1
+      snmp_host: 3.3.3.3
+      udp: 222
+      src_intf: "{{ intname|default(omit) }}"
+      vrf: management
+      vrf_filter: management
+      state: absent
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: Idempotence Check
+    nxos_snmp_host: *rem1
+    register: result
+
+  - assert: *false
+
+  - block:
+    - name: remove some more configuration
+      nxos_snmp_host: &rem2
+        snmp_host: 3.3.3.3
+        udp: 222
+        vrf_filter: default
+        state: absent
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: Idempotence Check
+      nxos_snmp_host: *rem2
+      register: result
+
+    - assert: *false
+    when: not (platform is match('N35|N5K'))
+
   - name: Cleanup
     nxos_snmp_host: *remove
     register: result
@@ -60,4 +124,9 @@
 
   - assert: *false
 
-  - debug: msg="END connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }}sanity test"
+  always:
+  - name: Cleanup
+    nxos_snmp_host: *remove
+    register: result
+
+  - debug: msg="END connection={{ ansible_connection }} nxos_snmp_host {{ snmp_type }} {{ snmp_version }} sanity test"