From 39b1f1c363a0f46dc8b67cb5a49b49f2a3b6078e Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Thu, 20 Dec 2018 04:19:32 -0800 Subject: [PATCH] Rewrite (#49570) --- .../storage/netapp/na_ontap_interface.py | 224 ++++++++--------- .../storage/netapp/test_na_ontap_interface.py | 225 ++++++++++++++++++ 2 files changed, 323 insertions(+), 126 deletions(-) create mode 100644 test/units/modules/storage/netapp/test_na_ontap_interface.py diff --git a/lib/ansible/modules/storage/netapp/na_ontap_interface.py b/lib/ansible/modules/storage/netapp/na_ontap_interface.py index 3cb6e9305da..94668c47ef3 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_interface.py +++ b/lib/ansible/modules/storage/netapp/na_ontap_interface.py @@ -53,6 +53,7 @@ options: role: description: - Specifies the role of the LIF. + - When setting role as "intercluster", setting protocol is not supported. - Required when C(state=present). address: @@ -77,6 +78,14 @@ options: failover_policy: description: - Specifies the failover policy for the LIF. + - Possible values are 'disabled', 'system-defined', 'local-only', 'sfo-partner-only', and 'broadcast-domain-wide' + + subnet_name: + description: + - Subnet where the interface address is allocated from. + If the option is not used, the IP address will need to be provided by + the administrator during configuration. + version_added: '2.8' admin_status: choices: ['up', 'down'] @@ -87,6 +96,7 @@ options: description: If true, data LIF will revert to its home node under certain circumstances such as startup, and load balancing migration capability is disabled automatically + type: bool protocols: description: @@ -133,6 +143,7 @@ RETURN = """ import traceback from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.netapp_module import NetAppModule from ansible.module_utils._text import to_native import ansible.module_utils.netapp as netapp_utils @@ -141,7 +152,6 @@ HAS_NETAPP_LIB = netapp_utils.has_netapp_lib() class NetAppOntapInterface(object): ''' object to describe interface info ''' - def __init__(self): self.argument_spec = netapp_utils.na_ontap_host_argument_spec() @@ -158,32 +168,17 @@ class NetAppOntapInterface(object): firewall_policy=dict(required=False, type='str', default=None), failover_policy=dict(required=False, type='str', default=None), admin_status=dict(required=False, choices=['up', 'down']), - is_auto_revert=dict(required=False, type='str', default=None), + subnet_name=dict(required=False, type='str'), + is_auto_revert=dict(required=False, type=bool, default=None), protocols=dict(required=False, type='list') - )) self.module = AnsibleModule( argument_spec=self.argument_spec, supports_check_mode=True ) - - params = self.module.params - - # set up state variables - self.state = params['state'] - self.interface_name = params['interface_name'] - self.home_node = params['home_node'] - self.home_port = params['home_port'] - self.role = params['role'] - self.vserver = params['vserver'] - self.address = params['address'] - self.netmask = params['netmask'] - self.admin_status = params['admin_status'] - self.failover_policy = params['failover_policy'] - self.firewall_policy = params['firewall_policy'] - self.is_auto_revert = params['is_auto_revert'] - self.protocols = params['protocols'] + self.na_helper = NetAppModule() + self.parameters = self.na_helper.set_parameters(self.module.params) if HAS_NETAPP_LIB is False: self.module.fail_json( @@ -191,7 +186,7 @@ class NetAppOntapInterface(object): else: self.server = netapp_utils.setup_na_ontap_zapi(module=self.module) - def get_interface(self): + def get_interface(self, interface_name=None): """ Return details about the interface :param: @@ -200,11 +195,11 @@ class NetAppOntapInterface(object): :return: Details about the interface. None if not found. :rtype: dict """ + if interface_name is None: + interface_name = self.parameters['interface_name'] interface_info = netapp_utils.zapi.NaElement('net-interface-get-iter') - interface_attributes = netapp_utils.zapi.NaElement( - 'net-interface-info') - interface_attributes.add_new_child( - 'interface-name', self.interface_name) + interface_attributes = netapp_utils.zapi.NaElement('net-interface-info') + interface_attributes.add_new_child('interface-name', interface_name) query = netapp_utils.zapi.NaElement('query') query.add_child_elem(interface_attributes) interface_info.add_child_elem(query) @@ -217,142 +212,119 @@ class NetAppOntapInterface(object): interface_attributes = result.get_child_by_name('attributes-list').\ get_child_by_name('net-interface-info') return_value = { - 'interface_name': self.interface_name, - 'admin_status': interface_attributes.get_child_content('administrative-status'), - 'home_port': interface_attributes.get_child_content('home-port'), - 'home_node': interface_attributes.get_child_content('home-node'), - 'address': interface_attributes.get_child_content('address'), - 'netmask': interface_attributes.get_child_content('netmask'), - 'failover_policy': interface_attributes.get_child_content('failover-policy'), - 'firewall_policy': interface_attributes.get_child_content('firewall-policy'), - 'is_auto_revert': interface_attributes.get_child_content('is-auto-revert'), + 'interface_name': self.parameters['interface_name'], + 'admin_status': interface_attributes['administrative-status'], + 'home_port': interface_attributes['home-port'], + 'home_node': interface_attributes['home-node'], + 'address': interface_attributes['address'], + 'netmask': interface_attributes['netmask'], + 'failover_policy': interface_attributes['failover-policy'].replace('_', '-'), + 'firewall_policy': interface_attributes['firewall-policy'], + 'is_auto_revert': True if interface_attributes['is-auto-revert'] == 'true' else False, } return return_value + def set_options(self, options, parameters): + """ set attributes for create or modify """ + if parameters.get('home_port') is not None: + options['home-port'] = parameters['home_port'] + if parameters.get('subnet_name') is not None: + options['subnet-name'] = parameters['subnet_name'] + if parameters.get('address') is not None: + options['address'] = parameters['address'] + if parameters.get('netmask') is not None: + options['netmask'] = parameters['netmask'] + if parameters.get('failover_policy') is not None: + options['failover-policy'] = parameters['failover_policy'] + if parameters.get('firewall_policy') is not None: + options['firewall-policy'] = parameters['firewall_policy'] + if parameters.get('is_auto_revert') is not None: + options['is-auto-revert'] = 'true' if parameters['is_auto_revert'] is True else 'false' + if parameters.get('admin_status') is not None: + options['administrative-status'] = parameters['admin_status'] + def create_interface(self): ''' calling zapi to create interface ''' - - options = {'interface-name': self.interface_name, - 'vserver': self.vserver} - if self.home_port is not None: - options['home-port'] = self.home_port - if self.home_node is not None: - options['home-node'] = self.home_node - if self.address is not None: - options['address'] = self.address - if self.netmask is not None: - options['netmask'] = self.netmask - if self.role is not None: - options['role'] = self.role - if self.failover_policy is not None: - options['failover-policy'] = self.failover_policy - if self.firewall_policy is not None: - options['firewall-policy'] = self.firewall_policy - if self.is_auto_revert is not None: - options['is-auto-revert'] = self.is_auto_revert - if self.admin_status is not None: - options['administrative-status'] = self.admin_status - - interface_create = netapp_utils.zapi.NaElement.create_node_with_children( - 'net-interface-create', **options) - if self.protocols is not None: + # validate if mandatory parameters are present for create + required_keys = set(['role', 'address', 'home_node', 'home_port', 'netmask']) + if not required_keys.issubset(set(self.parameters.keys())): + self.module.fail_json(msg='Error: Missing one or more required parameters for creating interface: %s' + % ', '.join(required_keys)) + # if role is intercluster, protocol cannot be specified + if self.parameters['role'] == "intercluster" and self.parameters.get('protocols') is not None: + self.module.fail_json(msg='Error: Protocol cannot be specified for intercluster role,' + 'failed to create interface') + options = {'interface-name': self.parameters['interface_name'], + 'role': self.parameters['role'], + 'home-node': self.parameters.get('home_node'), + 'vserver': self.parameters['vserver']} + self.set_options(options, self.parameters) + interface_create = netapp_utils.zapi.NaElement.create_node_with_children('net-interface-create', **options) + if self.parameters.get('protocols') is not None: data_protocols_obj = netapp_utils.zapi.NaElement('data-protocols') interface_create.add_child_elem(data_protocols_obj) - for protocol in self.protocols: + for protocol in self.parameters.get('protocols'): data_protocols_obj.add_new_child('data-protocol', protocol) - try: - self.server.invoke_successfully(interface_create, - enable_tunneling=True) + self.server.invoke_successfully(interface_create, enable_tunneling=True) except netapp_utils.zapi.NaApiError as exc: self.module.fail_json(msg='Error Creating interface %s: %s' % - (self.interface_name, to_native(exc)), exception=traceback.format_exc()) + (self.parameters['interface_name'], to_native(exc)), exception=traceback.format_exc()) def delete_interface(self, current_status): ''' calling zapi to delete interface ''' if current_status == 'up': - self.admin_status = 'down' - self.modify_interface() + self.parameters['admin_status'] = 'down' + self.modify_interface({'admin_status': 'down'}) interface_delete = netapp_utils.zapi.NaElement.create_node_with_children( - 'net-interface-delete', **{'interface-name': self.interface_name, - 'vserver': self.vserver}) + 'net-interface-delete', **{'interface-name': self.parameters['interface_name'], + 'vserver': self.parameters['vserver']}) try: - self.server.invoke_successfully(interface_delete, - enable_tunneling=True) + self.server.invoke_successfully(interface_delete, enable_tunneling=True) except netapp_utils.zapi.NaApiError as exc: - self.module.fail_json(msg='Error deleting interface %s: %s' % (self.interface_name, to_native(exc)), + self.module.fail_json(msg='Error deleting interface %s: %s' % (self.parameters['interface_name'], to_native(exc)), exception=traceback.format_exc()) - def modify_interface(self): + def modify_interface(self, modify): """ Modify the interface. """ - options = {'interface-name': self.interface_name, - 'vserver': self.vserver + options = {'interface-name': self.parameters['interface_name'], + 'vserver': self.parameters['vserver'] } - if self.admin_status is not None: - options['administrative-status'] = self.admin_status - if self.failover_policy is not None: - options['failover-policy'] = self.failover_policy - if self.firewall_policy is not None: - options['firewall-policy'] = self.firewall_policy - if self.is_auto_revert is not None: - options['is-auto-revert'] = self.is_auto_revert - if self.netmask is not None: - options['netmask'] = self.netmask - if self.address is not None: - options['address'] = self.address - - interface_modify = netapp_utils.zapi.NaElement.create_node_with_children( - 'net-interface-modify', **options) + self.set_options(options, modify) + interface_modify = netapp_utils.zapi.NaElement.create_node_with_children('net-interface-modify', **options) try: - self.server.invoke_successfully(interface_modify, - enable_tunneling=True) - except netapp_utils.zapi.NaApiError as e: - self.module.fail_json(msg='Error modifying interface %s: %s' % (self.interface_name, to_native(e)), - exception=traceback.format_exc()) + self.server.invoke_successfully(interface_modify, enable_tunneling=True) + except netapp_utils.zapi.NaApiError as err: + self.module.fail_json(msg='Error modifying interface %s: %s' % (self.parameters['interface_name'], + to_native(err)), exception=traceback.format_exc()) - def apply(self): - ''' calling all interface features ''' - changed = False - interface_exists = False + def autosupport_log(self): results = netapp_utils.get_cserver(self.server) cserver = netapp_utils.setup_na_ontap_zapi( module=self.module, vserver=results) netapp_utils.ems_log_event("na_ontap_interface", cserver) - interface_detail = self.get_interface() - if interface_detail: - interface_exists = True - if self.state == 'absent': - changed = True - - elif self.state == 'present': - if (self.admin_status is not None and self.admin_status != interface_detail['admin_status']) or \ - (self.address is not None and self.address != interface_detail['address']) or \ - (self.netmask is not None and self.netmask != interface_detail['netmask']) or \ - (self.failover_policy is not None and self.failover_policy != interface_detail['failover_policy']) or \ - (self.firewall_policy is not None and self.firewall_policy != interface_detail['firewall_policy']) or \ - (self.is_auto_revert is not None and self.is_auto_revert != interface_detail['is_auto_revert']): - changed = True - else: - if self.state == 'present': - changed = True - if changed: + def apply(self): + ''' calling all interface features ''' + self.autosupport_log() + current = self.get_interface() + # rename and create are mutually exclusive + cd_action = self.na_helper.get_cd_action(current, self.parameters) + modify = self.na_helper.get_modified_attributes(current, self.parameters) + if self.na_helper.changed: if self.module.check_mode: pass else: - if self.state == 'present': - if interface_exists is False: - self.create_interface() - else: - self.modify_interface() - - elif self.state == 'absent': - self.delete_interface(interface_detail['admin_status']) - - self.module.exit_json(changed=changed) + if cd_action == 'create': + self.create_interface() + elif cd_action == 'delete': + self.delete_interface(current['admin_status']) + elif modify: + self.modify_interface(modify) + self.module.exit_json(changed=self.na_helper.changed) def main(): diff --git a/test/units/modules/storage/netapp/test_na_ontap_interface.py b/test/units/modules/storage/netapp/test_na_ontap_interface.py new file mode 100644 index 00000000000..55987000387 --- /dev/null +++ b/test/units/modules/storage/netapp/test_na_ontap_interface.py @@ -0,0 +1,225 @@ +# (c) 2018, NetApp, Inc +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +''' unit test template for ONTAP Ansible module ''' + +from __future__ import print_function +import json +import pytest + +from units.compat import unittest +from units.compat.mock import patch, Mock +from ansible.module_utils import basic +from ansible.module_utils._text import to_bytes +import ansible.module_utils.netapp as netapp_utils + +from ansible.modules.storage.netapp.na_ontap_interface \ + import NetAppOntapInterface as interface_module # module under test + +if not netapp_utils.has_netapp_lib(): + pytestmark = pytest.skip('skipping as missing required netapp_lib') + + +def set_module_args(args): + """prepare arguments so that they will be picked up during module creation""" + args = json.dumps({'ANSIBLE_MODULE_ARGS': args}) + basic._ANSIBLE_ARGS = to_bytes(args) # pylint: disable=protected-access + + +class AnsibleExitJson(Exception): + """Exception class to be raised by module.exit_json and caught by the test case""" + pass + + +class AnsibleFailJson(Exception): + """Exception class to be raised by module.fail_json and caught by the test case""" + pass + + +def exit_json(*args, **kwargs): # pylint: disable=unused-argument + """function to patch over exit_json; package return data into an exception""" + if 'changed' not in kwargs: + kwargs['changed'] = False + raise AnsibleExitJson(kwargs) + + +def fail_json(*args, **kwargs): # pylint: disable=unused-argument + """function to patch over fail_json; package return data into an exception""" + kwargs['failed'] = True + raise AnsibleFailJson(kwargs) + + +class MockONTAPConnection(object): + ''' mock server connection to ONTAP host ''' + + def __init__(self, kind=None, data=None): + ''' save arguments ''' + self.type = kind + self.params = data + self.xml_in = None + self.xml_out = None + + def invoke_successfully(self, xml, enable_tunneling): # pylint: disable=unused-argument + ''' mock invoke_successfully returning xml data ''' + self.xml_in = xml + if self.type == 'interface': + xml = self.build_interface_info(self.params) + self.xml_out = xml + return xml + + @staticmethod + def build_interface_info(data): + ''' build xml data for vserser-info ''' + xml = netapp_utils.zapi.NaElement('xml') + attributes = { + 'num-records': 1, + 'attributes-list': { + 'net-interface-info': { + 'interface-name': data['name'], + 'administrative-status': data['administrative-status'], + 'failover-policy': data['failover-policy'], + 'firewall-policy': data['firewall-policy'], + 'is-auto-revert': data['is-auto-revert'], + 'home-node': data['home_node'], + 'home-port': data['home_port'], + 'address': data['address'], + 'netmask': data['netmask'], + 'role': data['role'] + } + } + } + xml.translate_struct(attributes) + return xml + + +class TestMyModule(unittest.TestCase): + ''' a group of related Unit Tests ''' + + def setUp(self): + self.mock_module_helper = patch.multiple(basic.AnsibleModule, + exit_json=exit_json, + fail_json=fail_json) + self.mock_module_helper.start() + self.addCleanup(self.mock_module_helper.stop) + self.mock_interface = { + 'name': 'test_lif', + 'administrative-status': 'up', + 'failover-policy': 'up', + 'firewall-policy': 'up', + 'is-auto-revert': 'true', + 'home_node': 'node', + 'role': 'test', + 'home_port': 'e0c', + 'address': '2.2.2.2', + 'netmask': '1.1.1.1', + } + + def mock_args(self): + return { + 'vserver': 'vserver', + 'interface_name': self.mock_interface['name'], + 'home_node': self.mock_interface['home_node'], + 'role': self.mock_interface['role'], + 'home_port': self.mock_interface['home_port'], + 'address': self.mock_interface['address'], + 'netmask': self.mock_interface['netmask'], + 'hostname': 'hostname', + 'username': 'username', + 'password': 'password', + } + + def get_interface_mock_object(self, kind=None): + """ + Helper method to return an na_ontap_interface object + :param kind: passes this param to MockONTAPConnection() + :return: na_ontap_interface object + """ + interface_obj = interface_module() + interface_obj.autosupport_log = Mock(return_value=None) + if kind is None: + interface_obj.server = MockONTAPConnection() + else: + interface_obj.server = MockONTAPConnection(kind=kind, data=self.mock_interface) + return interface_obj + + def test_module_fail_when_required_args_missing(self): + ''' required arguments are reported as errors ''' + with pytest.raises(AnsibleFailJson) as exc: + set_module_args({}) + interface_module() + print('Info: %s' % exc.value.args[0]['msg']) + + def test_create_error_missing_param(self): + ''' Test if create throws an error if required param 'role' is not specified''' + data = self.mock_args() + del data['role'] + set_module_args(data) + with pytest.raises(AnsibleFailJson) as exc: + self.get_interface_mock_object('interface').create_interface() + msg = 'Error: Missing one or more required parameters for creating interface: ' \ + 'home_port, netmask, role, home_node, address' + expected = sorted(','.split(msg)) + received = sorted(','.split(exc.value.args[0]['msg'])) + assert expected == received + + def test_get_nonexistent_interface(self): + ''' Test if get_interface returns None for non-existent interface ''' + set_module_args(self.mock_args()) + result = self.get_interface_mock_object().get_interface() + assert result is None + + def test_get_existing_interface(self): + ''' Test if get_interface returns None for existing interface ''' + set_module_args(self.mock_args()) + result = self.get_interface_mock_object(kind='interface').get_interface() + assert result['interface_name'] == self.mock_interface['name'] + + def test_successful_create(self): + ''' Test successful create ''' + set_module_args(self.mock_args()) + with pytest.raises(AnsibleExitJson) as exc: + self.get_interface_mock_object().apply() + assert exc.value.args[0]['changed'] + + def test_create_idempotency(self): + ''' Test create idempotency ''' + set_module_args(self.mock_args()) + with pytest.raises(AnsibleExitJson) as exc: + self.get_interface_mock_object('interface').apply() + assert not exc.value.args[0]['changed'] + + def test_successful_delete(self): + ''' Test delete existing interface ''' + data = self.mock_args() + data['state'] = 'absent' + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_interface_mock_object('interface').apply() + assert exc.value.args[0]['changed'] + + def test_delete_idempotency(self): + ''' Test delete idempotency ''' + data = self.mock_args() + data['state'] = 'absent' + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_interface_mock_object().apply() + assert not exc.value.args[0]['changed'] + + def test_successful_modify(self): + ''' Test successful modify interface_minutes ''' + data = self.mock_args() + data['home_port'] = 'new_port' + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + interface_obj = self.get_interface_mock_object('interface') + interface_obj.apply() + assert exc.value.args[0]['changed'] + + def test_modify_idempotency(self): + ''' Test modify idempotency ''' + data = self.mock_args() + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_interface_mock_object('interface').apply() + assert not exc.value.args[0]['changed']