From 1e5a0982b9575f31969e0c76b1515d5882a67ab3 Mon Sep 17 00:00:00 2001 From: Trishna Guha Date: Fri, 19 May 2017 22:10:42 +0530 Subject: [PATCH] Fix nxos_hsrp and add unit test (#24770) * nxos_hsrp fix Signed-off-by: Trishna Guha * unit test nxos_hsrp Signed-off-by: Trishna Guha * ansibot told me to do this * revert apply_key_map and simplify method --- lib/ansible/modules/network/nxos/nxos_hsrp.py | 190 +++++++----------- test/sanity/pep8/legacy-files.txt | 1 - .../modules/network/nxos/test_nxos_hsrp.py | 60 ++++++ 3 files changed, 138 insertions(+), 113 deletions(-) create mode 100644 test/units/modules/network/nxos/test_nxos_hsrp.py diff --git a/lib/ansible/modules/network/nxos/nxos_hsrp.py b/lib/ansible/modules/network/nxos/nxos_hsrp.py index 73861f12ea..8428fddd17 100644 --- a/lib/ansible/modules/network/nxos/nxos_hsrp.py +++ b/lib/ansible/modules/network/nxos/nxos_hsrp.py @@ -17,10 +17,11 @@ # -ANSIBLE_METADATA = {'metadata_version': '1.0', - 'status': ['preview'], - 'supported_by': 'community'} - +ANSIBLE_METADATA = { + 'metadata_version': '1.0', + 'status': ['preview'], + 'supported_by': 'community' +} DOCUMENTATION = ''' --- @@ -29,60 +30,60 @@ extends_documentation_fragment: nxos version_added: "2.2" short_description: Manages HSRP configuration on NX-OS switches. description: - - Manages HSRP configuration on NX-OS switches. + - Manages HSRP configuration on NX-OS switches. author: - - Jason Edelman (@jedelman8) - - Gabriele Gerbino (@GGabriele) + - Jason Edelman (@jedelman8) + - Gabriele Gerbino (@GGabriele) notes: - - HSRP feature needs to be enabled first on the system. - - SVIs must exist before using this module. - - Interface must be a L3 port before using this module. - - HSRP cannot be configured on loopback interfaces. - - MD5 authentication is only possible with HSRPv2 while it is ignored if - HSRPv1 is used instead, while it will not raise any error. Here we allow - MD5 authentication only with HSRPv2 in order to enforce better practice. + - HSRP feature needs to be enabled first on the system. + - SVIs must exist before using this module. + - Interface must be a L3 port before using this module. + - HSRP cannot be configured on loopback interfaces. + - MD5 authentication is only possible with HSRPv2 while it is ignored if + HSRPv1 is used instead, while it will not raise any error. Here we allow + MD5 authentication only with HSRPv2 in order to enforce better practice. options: - group: - description: - - HSRP group number. - required: true - interface: - description: - - Full name of interface that is being managed for HSRP. - required: true - version: - description: - - HSRP version. - required: false - default: 2 - choices: ['1','2'] - priority: - description: - - HSRP priority. - required: false - default: null - vip: - description: - - HSRP virtual IP address. - required: false - default: null - auth_string: - description: - - Authentication string. - required: false - default: null - auth_type: - description: - - Authentication type. - required: false - default: null - choices: ['text','md5'] - state: - description: - - Specify desired state of the resource. - required: false - choices: ['present','absent'] - default: 'present' + group: + description: + - HSRP group number. + required: true + interface: + description: + - Full name of interface that is being managed for HSRP. + required: true + version: + description: + - HSRP version. + required: false + default: 2 + choices: ['1','2'] + priority: + description: + - HSRP priority. + required: false + default: null + vip: + description: + - HSRP virtual IP address. + required: false + default: null + auth_string: + description: + - Authentication string. + required: false + default: null + auth_type: + description: + - Authentication type. + required: false + default: null + choices: ['text','md5'] + state: + description: + - Specify desired state of the resource. + required: false + choices: ['present','absent'] + default: 'present' ''' EXAMPLES = ''' @@ -116,42 +117,19 @@ EXAMPLES = ''' ''' RETURN = ''' -proposed: - description: k/v pairs of parameters passed into module - returned: always - type: dict - sample: {"group": "30", "version": "2", "vip": "10.30.1.1"} -existing: - description: k/v pairs of existing hsrp info on the interface - returned: always - type: dict - sample: {} -end_state: - description: k/v pairs of hsrp after module execution - returned: always - type: dict - sample: {"auth_string": "cisco", "auth_type": "text", - "group": "30", "interface": "vlan10", "preempt": "disabled", - "priority": "100", "version": "2", "vip": "10.30.1.1"} -updates: +commands: description: commands sent to the device returned: always type: list sample: ["interface vlan10", "hsrp version 2", "hsrp 30", "ip 10.30.1.1"] -changed: - description: check to see if a change was made on the device - returned: always - type: boolean - sample: true ''' -from ansible.module_utils.nxos import get_config, load_config, run_commands +from ansible.module_utils.nxos import load_config, run_commands from ansible.module_utils.nxos import nxos_argument_spec, check_args from ansible.module_utils.basic import AnsibleModule - -def execute_show_command(command, module, command_type='cli_show'): +def execute_show_command(command, module): if module.params['transport'] == 'cli': command += ' | json' cmds = [command] @@ -165,7 +143,7 @@ def execute_show_command(command, module, command_type='cli_show'): def apply_key_map(key_map, table): new_dict = {} - for key, value in table.items(): + for key in table: new_key = key_map.get(key) if new_key: value = table.get(key) @@ -197,9 +175,12 @@ def get_interface_mode(interface, intf_type, module): command = 'show interface {0}'.format(interface) interface = {} mode = 'unknown' + try: + body = execute_show_command(command, module)[0] + except IndexError: + return None if intf_type in ['ethernet', 'portchannel']: - body = execute_show_command(command, module)[0] interface_table = body['TABLE_interface']['ROW_interface'] mode = str(interface_table.get('eth_mode', 'layer3')) if mode == 'access' or mode == 'trunk': @@ -211,12 +192,12 @@ def get_interface_mode(interface, intf_type, module): def get_hsrp_groups_on_interfaces(device, module): command = 'show hsrp all' - body = execute_show_command(command, module) hsrp = {} try: - get_data = body[0]['TABLE_grp_detail']['ROW_grp_detail'] - except (KeyError, AttributeError): + body = execute_show_command(command, module)[0] + get_data = body['TABLE_grp_detail']['ROW_grp_detail'] + except (IndexError, KeyError, AttributeError): return {} for entry in get_data: @@ -232,7 +213,6 @@ def get_hsrp_groups_on_interfaces(device, module): def get_hsrp_group(group, interface, module): command = 'show hsrp group {0}'.format(group) - body = execute_show_command(command, module) hsrp = {} hsrp_key = { @@ -247,7 +227,8 @@ def get_hsrp_group(group, interface, module): } try: - hsrp_table = body[0]['TABLE_grp_detail']['ROW_grp_detail'] + body = execute_show_command(command, module)[0] + hsrp_table = body['TABLE_grp_detail']['ROW_grp_detail'] except (AttributeError, IndexError, TypeError): return {} @@ -271,9 +252,7 @@ def get_hsrp_group(group, interface, module): def get_commands_remove_hsrp(group, interface): - commands = [] - commands.append('interface {0}'.format(interface)) - commands.append('no hsrp {0}'.format(group)) + commands = ['interface {0}'.format(interface), 'no hsrp {0}'.format(group)] return commands @@ -295,7 +274,7 @@ def get_commands_config_hsrp(delta, interface, args): elif preempt == 'disabled': delta['preempt'] = 'no preempt' - for key, value in delta.items(): + for key in delta: command = config_args.get(key, 'DNE').format(**delta) if command and command != 'DNE': if key == 'group': @@ -391,13 +370,11 @@ def main(): interface=dict(required=True), version=dict(choices=['1', '2'], default='2', required=False), priority=dict(type='str', required=False), - preempt=dict(type='str', choices=['disabled', 'enabled'], - required=False), + preempt=dict(type='str', choices=['disabled', 'enabled'], required=False), vip=dict(type='str', required=False), auth_type=dict(choices=['text', 'md5'], required=False), auth_string=dict(type='str', required=False), - state=dict(choices=['absent', 'present'], required=False, - default='present'), + state=dict(choices=['absent', 'present'], required=False, default='present'), include_defaults=dict(default=True), config=dict(), save=dict(type='bool', default=False) @@ -405,12 +382,11 @@ def main(): argument_spec.update(nxos_argument_spec) - module = AnsibleModule(argument_spec=argument_spec, - supports_check_mode=True) + module = AnsibleModule(argument_spec=argument_spec, supports_check_mode=True) warnings = list() check_args(module, warnings) - + results = dict(changed=False, warnings=warnings) interface = module.params['interface'].lower() group = module.params['group'] @@ -470,8 +446,6 @@ def main(): module.fail_json(msg="Existing auth_type is md5. It's recommended " "to use HSRP v2 when using md5") - changed = False - end_state = existing commands = [] if state == 'present': delta = dict( @@ -487,28 +461,20 @@ def main(): if commands: if module.check_mode: - module.exit_json(changed=True, commands=commands) + module.exit_json(**results) else: load_config(module, commands) if transport == 'cli': body = run_commands(module, commands) validate_config(body, vip, module) - changed = True + results['changed'] = True end_state = get_hsrp_group(group, interface, module) if 'configure' in commands: commands.pop(0) - results = {} - results['proposed'] = proposed - results['existing'] = existing - results['end_state'] = end_state - results['updates'] = commands - results['changed'] = changed - results['warnings'] = warnings - + results['commands'] = commands module.exit_json(**results) if __name__ == '__main__': main() - diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index 81ca18db09..528e095eb4 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/test/sanity/pep8/legacy-files.txt @@ -473,7 +473,6 @@ lib/ansible/modules/network/nxos/nxos_facts.py lib/ansible/modules/network/nxos/nxos_feature.py lib/ansible/modules/network/nxos/nxos_gir.py lib/ansible/modules/network/nxos/nxos_gir_profile_management.py -lib/ansible/modules/network/nxos/nxos_hsrp.py lib/ansible/modules/network/nxos/nxos_igmp.py lib/ansible/modules/network/nxos/nxos_igmp_interface.py lib/ansible/modules/network/nxos/nxos_igmp_snooping.py diff --git a/test/units/modules/network/nxos/test_nxos_hsrp.py b/test/units/modules/network/nxos/test_nxos_hsrp.py new file mode 100644 index 0000000000..e1b8fbe2d4 --- /dev/null +++ b/test/units/modules/network/nxos/test_nxos_hsrp.py @@ -0,0 +1,60 @@ +# (c) 2016 Red Hat Inc. +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +# Make coding more python3-ish +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import json + +from ansible.compat.tests.mock import patch +from ansible.modules.network.nxos import nxos_hsrp +from .nxos_module import TestNxosModule, load_fixture, set_module_args + + +class TestNxosHsrpModule(TestNxosModule): + + module = nxos_hsrp + + def setUp(self): + self.mock_run_commands = patch('ansible.modules.network.nxos.nxos_hsrp.run_commands') + self.run_commands = self.mock_run_commands.start() + + self.mock_load_config = patch('ansible.modules.network.nxos.nxos_hsrp.load_config') + self.load_config = self.mock_load_config.start() + + def tearDown(self): + self.mock_run_commands.stop() + self.mock_load_config.stop() + + def load_fixtures(self, commands=None): + self.load_config.return_value = None + + def test_nxos_hsrp(self): + set_module_args(dict(group='10', + vip='192.0.2.2', + priority='150', + interface='Ethernet1/2', + preempt='enabled', + host='192.0.2.1')) + result = self.execute_module(changed=True) + self.assertEqual(sorted(result['commands']), sorted(['interface ethernet1/2', + 'hsrp version 2', + 'hsrp 10', + 'priority 150', + 'ip 192.0.2.2', + 'preempt']))