From f99bae1776cdc87e0a3f190b76c41d77fb7dd94b Mon Sep 17 00:00:00 2001 From: saichint Date: Thu, 3 May 2018 09:06:33 -0700 Subject: [PATCH] Fix for nxos_snmp_host issues (#39642) * fix snmp_host issues * source files * fix shippable * remove defaults to match arg spec --- .../modules/network/nxos/nxos_snmp_host.py | 171 ++++++++++++------ .../tests/common/sanity_snmp_v1_trap.yaml | 129 +++++++++++++ ...2_trap.yaml => sanity_snmp_v2_inform.yaml} | 70 ++++++- .../tests/common/sanity_snmp_v3_inform.yaml | 72 +++++++- .../tests/common/sanity_snmp_v3_trap.yaml | 77 +++++++- 5 files changed, 454 insertions(+), 65 deletions(-) create mode 100644 test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v1_trap.yaml rename test/integration/targets/nxos_snmp_host/tests/common/{sanity_snmp_v2_trap.yaml => sanity_snmp_v2_inform.yaml} (51%) diff --git a/lib/ansible/modules/network/nxos/nxos_snmp_host.py b/lib/ansible/modules/network/nxos/nxos_snmp_host.py index a87b52bb421..acbbd65088b 100644 --- a/lib/ansible/modules/network/nxos/nxos_snmp_host.py +++ b/lib/ansible/modules/network/nxos/nxos_snmp_host.py @@ -42,9 +42,8 @@ options: required: true version: description: - - SNMP version. - default: v2c - choices: ['v2c', 'v3'] + - SNMP version. If this is not specified, v1 is used. + choices: ['v1', '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. - default: trap + - type of message to send to host. If this is not + 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': 'snmp-server host {0} use-vrf {vrf}', - 'udp': 'snmp-server host {0} udp-port {udp}', - 'src_intf': 'snmp-server host {0} source-interface {src_intf}' + 'vrf_filter': 'snmp-server host {0} filter-vrf {vrf_filter} udp-port {1}', + 'vrf': 'snmp-server host {0} use-vrf {vrf} udp-port {1}', + 'src_intf': 'snmp-server host {0} source-interface {src_intf} udp-port {1}' } 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'), - version=dict(choices=['v2c', 'v3'], default='v2c'), + udp=dict(type='str', default='162'), + 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': - module.fail_json(msg='inform requires snmp v3') + existing = get_snmp_host(snmp_host, udp, module) - if version == 'v2c' and 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 == '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 all([snmp_type, version, community]): + if not any([vrf_filter, vrf, src_intf]): + 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,27 +420,34 @@ def main(): existing['vrf_filter'] = vrf_filter commands = [] + args = dict( + community=community, + snmp_host=snmp_host, + udp=udp, + version=version, + src_intf=src_intf, + vrf_filter=vrf_filter, + v3=v3, + vrf=vrf, + snmp_type=snmp_type + ) + proposed = dict((k, v) for k, v in args.items() if v is not None) + if state == 'absent' and existing: - command = remove_snmp_host(snmp_host, existing) - commands.append(command) + 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': - args = dict( - community=community, - snmp_host=snmp_host, - udp=udp, - version=version, - src_intf=src_intf, - vrf_filter=vrf_filter, - v3=v3, - vrf=vrf, - snmp_type=snmp_type - ) - proposed = dict((k, v) for k, v in args.items() if v is not None) - 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) diff --git a/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v1_trap.yaml b/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v1_trap.yaml new file mode 100644 index 00000000000..e7de1ef55fc --- /dev/null +++ b/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" diff --git a/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v2_trap.yaml b/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v2_inform.yaml similarity index 51% rename from test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v2_trap.yaml rename to test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v2_inform.yaml index 16c1843574d..7c7e100077b 100644 --- a/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v2_trap.yaml +++ b/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" diff --git a/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v3_inform.yaml b/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v3_inform.yaml index 84b41a2222c..9a481e57701 100644 --- a/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v3_inform.yaml +++ b/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" diff --git a/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v3_trap.yaml b/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v3_trap.yaml index 0d92d3b94d6..2262a6637f0 100644 --- a/test/integration/targets/nxos_snmp_host/tests/common/sanity_snmp_v3_trap.yaml +++ b/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"