From 079b7cb1fee29edd03118492217abcf34580b05b Mon Sep 17 00:00:00 2001 From: Trishna Guha Date: Tue, 8 May 2018 12:38:22 +0530 Subject: [PATCH] nxos and ios bugfix stable-2.5 (#39845) * nxos_linkagg normalize interface (#39591) Signed-off-by: Trishna Guha (cherry picked from commit 3c35dd4f7f8e3ce163451fc879ce8e29f1be801b) * use show run instead of section pipeline ios_l2_interface (#39658) Signed-off-by: Trishna Guha (cherry picked from commit dddcbb7198f68756e5743c4e43e3c33849630ce5) * add changelog Signed-off-by: Trishna Guha * fix nxos_snmp_user issues (#39760) * fix nxos_snmp_user issues * shipppable fix (cherry picked from commit e3bfbe5875f429e48014b362659b0e4023288a95) * changelog Signed-off-by: Trishna Guha * fix shippable Signed-off-by: Trishna Guha --- .../fragments/ios_l2_interface_fix.yaml | 1 + changelogs/fragments/nxos_bugfixes.yaml | 2 + .../modules/network/ios/ios_l2_interface.py | 5 +- .../modules/network/nxos/nxos_linkagg.py | 52 ++++++++- .../modules/network/nxos/nxos_snmp_user.py | 98 ++++++++++------ .../nxos_snmp_user/tests/common/sanity.yaml | 105 +++++++++++++----- test/sanity/validate-modules/ignore.txt | 2 - 7 files changed, 193 insertions(+), 72 deletions(-) diff --git a/changelogs/fragments/ios_l2_interface_fix.yaml b/changelogs/fragments/ios_l2_interface_fix.yaml index 4655915fa9f..bb61e823807 100644 --- a/changelogs/fragments/ios_l2_interface_fix.yaml +++ b/changelogs/fragments/ios_l2_interface_fix.yaml @@ -1,2 +1,3 @@ bugfixes: - ios_l2_interface - fix removal of trunk vlans (https://github.com/ansible/ansible/pull/37389) +- ios_l2_interface - use show run instead of section pipeline ios_l2_interface (https://github.com/ansible/ansible/pull/39658) diff --git a/changelogs/fragments/nxos_bugfixes.yaml b/changelogs/fragments/nxos_bugfixes.yaml index 4a2393fd44f..a95ea3d23a0 100644 --- a/changelogs/fragments/nxos_bugfixes.yaml +++ b/changelogs/fragments/nxos_bugfixes.yaml @@ -27,3 +27,5 @@ bugfixes: - nxos_l2_interface - Add aggregate example in nxos_l2_interface module doc (https://github.com/ansible/ansible/pull/39275) - nxos_snmp_host - Fix for nxos_snmp_host issues (https://github.com/ansible/ansible/pull/39642) - nxos_snmp_traps - Fix nxos_snmp_traps issues (https://github.com/ansible/ansible/pull/39444) +- nxos_linkagg - nxos_linkagg abbreviated form issue (https://github.com/ansible/ansible/pull/39591) +- nxos_snmp_user - Fix nxos_snmp_user (https://github.com/ansible/ansible/pull/39760) diff --git a/lib/ansible/modules/network/ios/ios_l2_interface.py b/lib/ansible/modules/network/ios/ios_l2_interface.py index 27292bf1c4b..361e5259c6a 100644 --- a/lib/ansible/modules/network/ios/ios_l2_interface.py +++ b/lib/ansible/modules/network/ios/ios_l2_interface.py @@ -146,10 +146,9 @@ def is_switchport(name, module): def interface_is_portchannel(name, module): if get_interface_type(name) == 'ethernet': - config = get_config(module, flags=[' | section interface']) - if 'channel group' in config: + config = run_commands(module, ['show run interface {0}'.format(name)])[0] + if any(c in config for c in ['channel group', 'channel-group']): return True - return False diff --git a/lib/ansible/modules/network/nxos/nxos_linkagg.py b/lib/ansible/modules/network/nxos/nxos_linkagg.py index af2e21818c1..bd5032fa2b1 100644 --- a/lib/ansible/modules/network/nxos/nxos_linkagg.py +++ b/lib/ansible/modules/network/nxos/nxos_linkagg.py @@ -138,6 +138,46 @@ from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.network.common.utils import remove_default_spec +def normalize_interface(name): + """Return the normalized interface name + """ + if not name: + return + + def _get_number(name): + digits = '' + for char in name: + if char.isdigit() or char in '/.': + digits += char + return digits + + if name.lower().startswith('et'): + if_type = 'Ethernet' + elif name.lower().startswith('vl'): + if_type = 'Vlan' + elif name.lower().startswith('lo'): + if_type = 'loopback' + elif name.lower().startswith('po'): + if_type = 'port-channel' + elif name.lower().startswith('nv'): + if_type = 'nve' + else: + if_type = None + + number_list = name.split(' ') + if len(number_list) == 2: + number = number_list[-1].strip() + else: + number = _get_number(name) + + if if_type: + proper_interface = if_type + number + else: + proper_interface = name + + return proper_interface + + def execute_show_command(command, module): device_info = get_capabilities(module) network_api = device_info.get('network_api', 'nxapi') @@ -250,14 +290,20 @@ def map_params_to_obj(module): d = item.copy() d['group'] = str(d['group']) d['min_links'] = str(d['min_links']) + if d['members']: + d['members'] = [normalize_interface(i) for i in d['members']] obj.append(d) else: + members = None + if module.params['members']: + members = [normalize_interface(i) for i in module.params['members']] + obj.append({ 'group': str(module.params['group']), 'mode': module.params['mode'], 'min_links': str(module.params['min_links']), - 'members': module.params['members'], + 'members': members, 'state': module.params['state'] }) @@ -296,10 +342,10 @@ def get_members(channel): return list() if isinstance(interfaces, dict): - members.append(interfaces.get('port')) + members.append(normalize_interface(interfaces.get('port'))) elif isinstance(interfaces, list): for i in interfaces: - members.append(i.get('port')) + members.append(normalize_interface(i.get('port'))) return members diff --git a/lib/ansible/modules/network/nxos/nxos_snmp_user.py b/lib/ansible/modules/network/nxos/nxos_snmp_user.py index 2978a6767bc..4264197abb6 100644 --- a/lib/ansible/modules/network/nxos/nxos_snmp_user.py +++ b/lib/ansible/modules/network/nxos/nxos_snmp_user.py @@ -42,33 +42,34 @@ options: group: description: - Group to which the user will belong to. - required: true + If state = present, and the user is existing, + the group is added to the user. If the user + is not existing, user entry is created with this + group argument. + If state = absent, only the group is removed from the + user entry. However, to maintain backward compatibility, + if the existing user belongs to only one group, and if + group argument is same as the existing user's group, + then the user entry also is deleted. authentication: description: - Authentication parameters for the user. - required: false - default: null choices: ['md5', 'sha'] pwd: description: - Authentication password when using md5 or sha. - required: false - default: null + This is not idempotent privacy: description: - Privacy password for the user. - required: false - default: null + This is not idempotent encrypt: description: - Enables AES-128 bit encryption when using privacy password. - required: false - default: null - choices: ['true','false'] + type: bool state: description: - Manage the state of the resource. - required: false default: present choices: ['present','absent'] ''' @@ -157,8 +158,18 @@ def get_snmp_user(user, module): privkey = 'priv' grpkey = 'group' - resource_table = body[0][tablekey][rowkey] - resource['user'] = str(resource_table['user']) + rt = body[0][tablekey][rowkey] + # on some older platforms, all groups except the 1st one + # are in list elements by themselves and they are + # indexed by 'user'. This is due to a platform bug. + # Get first element if rt is a list due to the bug + # or if there is no bug, parse rt directly + if isinstance(rt, list): + resource_table = rt[0] + else: + resource_table = rt + + resource['user'] = user resource['authentication'] = str(resource_table[authkey]).strip() encrypt = str(resource_table[privkey]).strip() if encrypt.startswith('aes'): @@ -175,6 +186,15 @@ def get_snmp_user(user, module): except TypeError: groups.append(str(group_table[grpkey]).strip()) + # Now for the platform bug case, get the groups + if isinstance(rt, list): + # remove 1st element from the list as this is parsed already + rt.pop(0) + # iterate through other elements indexed by + # 'user' and add it to groups. + for each in rt: + groups.append(each['user'].strip()) + resource['group'] = groups except (KeyError, AttributeError, IndexError, TypeError): @@ -183,22 +203,23 @@ def get_snmp_user(user, module): return resource -def remove_snmp_user(user): - return ['no snmp-server user {0}'.format(user)] +def remove_snmp_user(user, group=None): + if group: + return ['no snmp-server user {0} {1}'.format(user, group)] + else: + return ['no snmp-server user {0}'.format(user)] -def config_snmp_user(proposed, user, reset, new): - if reset and not new: +def config_snmp_user(proposed, user, reset): + if reset: commands = remove_snmp_user(user) else: commands = [] - group = proposed.get('group', None) - - cmd = '' - - if group: + if proposed.get('group'): cmd = 'snmp-server user {0} {group}'.format(user, **proposed) + else: + cmd = 'snmp-server user {0}'.format(user) auth = proposed.get('authentication', None) pwd = proposed.get('pwd', None) @@ -223,7 +244,7 @@ def config_snmp_user(proposed, user, reset, new): def main(): argument_spec = dict( user=dict(required=True, type='str'), - group=dict(type='str', required=True), + group=dict(type='str'), pwd=dict(type='str'), privacy=dict(type='str'), authentication=dict(choices=['md5', 'sha']), @@ -260,19 +281,28 @@ def main(): existing = get_snmp_user(user, module) - if existing: - if group not in existing['group']: - existing['group'] = None + if state == 'present' and existing: + if group: + if group not in existing['group']: + existing['group'] = None + else: + existing['group'] = group else: - existing['group'] = group + existing['group'] = None commands = [] if state == 'absent' and existing: - commands.append(remove_snmp_user(user)) + if group: + if group in existing['group']: + if len(existing['group']) == 1: + commands.append(remove_snmp_user(user)) + else: + commands.append(remove_snmp_user(user, group)) + else: + commands.append(remove_snmp_user(user)) elif state == 'present': - new = False reset = False args = dict(user=user, pwd=pwd, group=group, privacy=privacy, @@ -282,7 +312,7 @@ def main(): if not existing: if encrypt: proposed['encrypt'] = 'aes-128' - commands.append(config_snmp_user(proposed, user, reset, new)) + commands.append(config_snmp_user(proposed, user, reset)) elif existing: if encrypt and not existing['encrypt'].startswith('aes'): @@ -294,14 +324,12 @@ def main(): if delta.get('pwd'): delta['authentication'] = authentication - if delta: - delta['group'] = group - if delta and encrypt: delta['encrypt'] = 'aes-128' - command = config_snmp_user(delta, user, reset, new) - commands.append(command) + if delta: + command = config_snmp_user(delta, user, reset) + commands.append(command) cmds = flatten_list(commands) if cmds: diff --git a/test/integration/targets/nxos_snmp_user/tests/common/sanity.yaml b/test/integration/targets/nxos_snmp_user/tests/common/sanity.yaml index 73a5e836f26..b3a4c32e566 100644 --- a/test/integration/targets/nxos_snmp_user/tests/common/sanity.yaml +++ b/test/integration/targets/nxos_snmp_user/tests/common/sanity.yaml @@ -3,41 +3,88 @@ - debug: msg="Using provider={{ connection.transport }}" when: ansible_connection == "local" -- name: Create snmp user - nxos_snmp_user: &create - user: ntc - group: network-operator - authentication: md5 - pwd: N$tOpe%1 - privacy: HelloU$er1 - encrypt: true - provider: "{{ connection }}" - register: result - -- assert: &true - that: - - "result.changed == true" - -- name: delete snmp user +- name: Remove snmp user nxos_snmp_user: &remove user: ntc - group: network-operator - authentication: md5 - pwd: Testing1% - privacy: HelloU$er1 - encrypt: true state: absent provider: "{{ connection }}" - register: result -- assert: *true +- pause: + seconds: 5 + +- block: + - name: Create snmp user + nxos_snmp_user: &create + user: ntc + group: network-operator + authentication: md5 + pwd: N$tOpe%1 + privacy: HelloU$er1 + encrypt: true + provider: "{{ connection }}" + register: result + + - assert: &true + that: + - "result.changed == true" + + - name: Add another group to user + nxos_snmp_user: &chg + user: ntc + group: network-admin + provider: "{{ connection }}" + register: result + + - assert: *true + + - name: "Check Idempotence" + nxos_snmp_user: *chg + register: result + + - assert: &false + that: + - "result.changed == false" + + - name: Remove group from user + nxos_snmp_user: &remg + user: ntc + group: network-admin + state: absent + provider: "{{ connection }}" + register: result + + - assert: *true + + - pause: + seconds: 5 + + - name: "Check Idempotence" + nxos_snmp_user: *remg + register: result + + - assert: *false + + - name: delete snmp user + nxos_snmp_user: &remove1 + user: ntc + group: network-operator + state: absent + provider: "{{ connection }}" + register: result + + - assert: *true + + - pause: + seconds: 5 + + - name: "Remove Idempotence" + nxos_snmp_user: *remove1 + register: result -- name: "Remove Idempotence" - nxos_snmp_user: *remove - register: result + - assert: *false -- assert: &false - that: - - "result.changed == false" + always: + - name: delete snmp user + nxos_snmp_user: *remove - debug: msg="END connection={{ ansible_connection }} nxos_snmp_user sanity test" diff --git a/test/sanity/validate-modules/ignore.txt b/test/sanity/validate-modules/ignore.txt index c331edf3378..238919bc867 100644 --- a/test/sanity/validate-modules/ignore.txt +++ b/test/sanity/validate-modules/ignore.txt @@ -1748,8 +1748,6 @@ lib/ansible/modules/network/nxos/nxos_reboot.py E325 lib/ansible/modules/network/nxos/nxos_smu.py E324 lib/ansible/modules/network/nxos/nxos_snapshot.py E325 lib/ansible/modules/network/nxos/nxos_snapshot.py E326 -lib/ansible/modules/network/nxos/nxos_snmp_user.py E325 -lib/ansible/modules/network/nxos/nxos_snmp_user.py E326 lib/ansible/modules/network/nxos/nxos_system.py E325 lib/ansible/modules/network/nxos/nxos_udld.py E325 lib/ansible/modules/network/nxos/nxos_udld.py E326