diff --git a/lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py b/lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py index dea6f833f6..a31c94a855 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py +++ b/lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py @@ -19,7 +19,7 @@ extends_documentation_fragment: version_added: '2.6' author: NetApp Ansible Team (@carchi8py) description: -- Create, modify, destroy the network interface group +- Create, modify ports, destroy the network interface group options: state: description: @@ -46,32 +46,48 @@ options: - Specifies the name of node. required: true - port: + ports: + aliases: + - port description: - - Adds the specified port. - + - List of expected ports to be present in the interface group. + - If a port is present in this list, but not on the target, it will be added. + - If a port is not in the list, but present on the target, it will be removed. + - Make sure the list contains all ports you want to see on the target. + version_added: '2.8' """ EXAMPLES = """ - name: create ifgrp na_ontap_net_ifgrp: - state=present - username={{ netapp_username }} - password={{ netapp_password }} - hostname={{ netapp_hostname }} - distribution_function=ip - name=a0c - port=e0d - mode=multimode - node={{ Vsim node name }} + state: present + username: "{{ netapp_username }}" + password: "{{ netapp_password }}" + hostname: "{{ netapp_hostname }}" + distribution_function: ip + name: a0c + ports: [e0a] + mode: multimode + node: "{{ Vsim node name }}" + - name: modify ports in an ifgrp + na_ontap_net_ifgrp: + state: present + username: "{{ netapp_username }}" + password: "{{ netapp_password }}" + hostname: "{{ netapp_hostname }}" + distribution_function: ip + name: a0c + port: [e0a, e0c] + mode: multimode + node: "{{ Vsim node name }}" - name: delete ifgrp na_ontap_net_ifgrp: - state=absent - username={{ netapp_username }} - password={{ netapp_password }} - hostname={{ netapp_hostname }} - name=a0c - node={{ Vsim node name }} + state: absent + username: "{{ netapp_username }}" + password: "{{ netapp_password }}" + hostname: "{{ netapp_hostname }}" + name: a0c + node: "{{ Vsim node name }}" """ RETURN = """ @@ -81,6 +97,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 @@ -102,7 +119,7 @@ class NetAppOntapIfGrp(object): name=dict(required=True, type='str'), mode=dict(required=False, type='str'), node=dict(required=True, type='str'), - port=dict(required=False, type='str'), + ports=dict(required=False, type='list', aliases=["port"]), )) self.module = AnsibleModule( @@ -113,15 +130,9 @@ class NetAppOntapIfGrp(object): supports_check_mode=True ) - parameters = self.module.params - - # set up state variables - self.state = parameters['state'] - self.distribution_function = parameters['distribution_function'] - self.name = parameters['name'] - self.mode = parameters['mode'] - self.node = parameters['node'] - self.port = parameters['port'] + # set up variables + self.na_helper = NetAppModule() + self.parameters = self.na_helper.set_parameters(self.module.params) if HAS_NETAPP_LIB is False: self.module.fail_json(msg="the python NetApp-Lib module is required") @@ -140,31 +151,27 @@ class NetAppOntapIfGrp(object): """ if_group_iter = netapp_utils.zapi.NaElement('net-port-get-iter') if_group_info = netapp_utils.zapi.NaElement('net-port-info') - if_group_info.add_new_child('port', self.name) + if_group_info.add_new_child('port', self.parameters['name']) if_group_info.add_new_child('port-type', 'if_group') + if_group_info.add_new_child('node', self.parameters['node']) query = netapp_utils.zapi.NaElement('query') query.add_child_elem(if_group_info) if_group_iter.add_child_elem(query) - result = self.server.invoke_successfully(if_group_iter, True) + try: + result = self.server.invoke_successfully(if_group_iter, True) + except netapp_utils.zapi.NaApiError as error: + self.module.fail_json(msg='Error getting if_group %s: %s' % (self.parameters['name'], to_native(error)), + exception=traceback.format_exc()) return_value = None - if result.get_child_by_name('num-records') and \ - int(result.get_child_content('num-records')) >= 1: - - if_group_attributes = result.get_child_by_name('attributes-list').get_child_by_name('net-port-info') - distribution_function = if_group_attributes.get_child_content('ifgrp-distribution-function') - name = if_group_attributes.get_child_content('port') - mode = if_group_attributes.get_child_content('ifgrp-mode') - ports = if_group_attributes.get_child_content('ifgrp-port') - node = if_group_attributes.get_child_content('node') - + if result.get_child_by_name('num-records') and int(result['num-records']) >= 1: + if_group_attributes = result['attributes-list']['net-port-info'] return_value = { - 'name': name, - 'distribution_function': distribution_function, - 'mode': mode, - 'node': node, - 'ports': ports + 'name': if_group_attributes['port'], + 'distribution_function': if_group_attributes['ifgrp-distribution-function'], + 'mode': if_group_attributes['ifgrp-mode'], + 'node': if_group_attributes['node'], } return return_value @@ -174,141 +181,116 @@ class NetAppOntapIfGrp(object): Return ports of the if_group :param: name : Name of the if_group - :return: Ports of the if_group. None if not found. :rtype: dict """ if_group_iter = netapp_utils.zapi.NaElement('net-port-ifgrp-get') - if_group_iter.add_new_child('ifgrp-name', self.name) - if_group_iter.add_new_child('node', self.node) - result = self.server.invoke_successfully(if_group_iter, True) - - return_value = None + if_group_iter.add_new_child('ifgrp-name', self.parameters['name']) + if_group_iter.add_new_child('node', self.parameters['node']) + try: + result = self.server.invoke_successfully(if_group_iter, True) + except netapp_utils.zapi.NaApiError as error: + self.module.fail_json(msg='Error getting if_group ports %s: %s' % (self.parameters['name'], to_native(error)), + exception=traceback.format_exc()) + port_list = [] if result.get_child_by_name('attributes'): - if_group_attributes = result.get_child_by_name('attributes').get_child_by_name('net-ifgrp-info') - name = if_group_attributes.get_child_content('ifgrp-name') - mode = if_group_attributes.get_child_content('mode') - port_list = [] + if_group_attributes = result['attributes']['net-ifgrp-info'] if if_group_attributes.get_child_by_name('ports'): ports = if_group_attributes.get_child_by_name('ports').get_children() for each in ports: port_list.append(each.get_content()) - node = if_group_attributes.get_child_content('node') - return_value = { - 'name': name, - 'mode': mode, - 'node': node, - 'ports': port_list - } - return return_value + return {'ports': port_list} def create_if_grp(self): """ Creates a new ifgrp """ route_obj = netapp_utils.zapi.NaElement("net-port-ifgrp-create") - route_obj.add_new_child("distribution-function", self.distribution_function) - route_obj.add_new_child("ifgrp-name", self.name) - route_obj.add_new_child("mode", self.mode) - route_obj.add_new_child("node", self.node) + route_obj.add_new_child("distribution-function", self.parameters['distribution_function']) + route_obj.add_new_child("ifgrp-name", self.parameters['name']) + route_obj.add_new_child("mode", self.parameters['mode']) + route_obj.add_new_child("node", self.parameters['node']) try: self.server.invoke_successfully(route_obj, True) except netapp_utils.zapi.NaApiError as error: - self.module.fail_json(msg='Error creating if_group %s: %s' % (self.name, to_native(error)), + self.module.fail_json(msg='Error creating if_group %s: %s' % (self.parameters['name'], to_native(error)), exception=traceback.format_exc()) + for port in self.parameters.get('ports'): + self.add_port_to_if_grp(port) def delete_if_grp(self): """ Deletes a ifgrp """ route_obj = netapp_utils.zapi.NaElement("net-port-ifgrp-destroy") - route_obj.add_new_child("ifgrp-name", self.name) - route_obj.add_new_child("node", self.node) + route_obj.add_new_child("ifgrp-name", self.parameters['name']) + route_obj.add_new_child("node", self.parameters['node']) try: self.server.invoke_successfully(route_obj, True) except netapp_utils.zapi.NaApiError as error: - self.module.fail_json(msg='Error deleting if_group %s: %s' % (self.name, to_native(error)), + self.module.fail_json(msg='Error deleting if_group %s: %s' % (self.parameters['name'], to_native(error)), exception=traceback.format_exc()) - def add_port_to_if_grp(self): + def add_port_to_if_grp(self, port): """ adds port to a ifgrp """ route_obj = netapp_utils.zapi.NaElement("net-port-ifgrp-add-port") - route_obj.add_new_child("ifgrp-name", self.name) - route_obj.add_new_child("port", self.port) - route_obj.add_new_child("node", self.node) + route_obj.add_new_child("ifgrp-name", self.parameters['name']) + route_obj.add_new_child("port", port) + route_obj.add_new_child("node", self.parameters['node']) try: self.server.invoke_successfully(route_obj, True) except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error adding port %s to if_group %s: %s' % - (self.port, self.name, to_native(error)), + (port, self.parameters['name'], to_native(error)), exception=traceback.format_exc()) - def remove_port_to_if_grp(self): + def modify_ports(self, current_ports): + for port in current_ports: + self.remove_port_to_if_grp(port) + for port in self.parameters['ports']: + self.add_port_to_if_grp(port) + + def remove_port_to_if_grp(self, port): """ removes port from a ifgrp """ route_obj = netapp_utils.zapi.NaElement("net-port-ifgrp-remove-port") - route_obj.add_new_child("ifgrp-name", self.name) - route_obj.add_new_child("port", self.port) - route_obj.add_new_child("node", self.node) + route_obj.add_new_child("ifgrp-name", self.parameters['name']) + route_obj.add_new_child("port", port) + route_obj.add_new_child("node", self.parameters['node']) try: self.server.invoke_successfully(route_obj, True) except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error removing port %s to if_group %s: %s' % - (self.port, self.name, to_native(error)), + (port, self.parameters['name'], to_native(error)), exception=traceback.format_exc()) - def apply(self): - changed = False - ifgroup_exists = False - add_ports_exists = True - remove_ports_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_net_ifgrp", cserver) - if_group_detail = self.get_if_grp() - if if_group_detail: - ifgroup_exists = True - ifgrp_ports_detail = self.get_if_grp_ports() - if self.state == 'absent': - changed = True - if self.port: - if self.port in ifgrp_ports_detail['ports']: - remove_ports_exists = True - elif self.state == 'present': - if self.port: - if not ifgrp_ports_detail['ports']: - add_ports_exists = False - changed = True - else: - if self.port not in ifgrp_ports_detail['ports']: - add_ports_exists = False - changed = True - else: - if self.state == 'present': - changed = True - if changed: + def apply(self): + self.autosupport_log() + current = self.get_if_grp() + cd_action = self.na_helper.get_cd_action(current, self.parameters) + if current and self.parameters['state'] == 'present': + current_ports = self.get_if_grp_ports() + modify = self.na_helper.get_modified_attributes(current_ports, self.parameters) + if self.na_helper.changed: if self.module.check_mode: pass else: - if self.state == 'present': - if not ifgroup_exists: - self.create_if_grp() - if self.port: - self.add_port_to_if_grp() - else: - if not add_ports_exists: - self.add_port_to_if_grp() - elif self.state == 'absent': - if remove_ports_exists: - self.remove_port_to_if_grp() + if cd_action == 'create': + self.create_if_grp() + elif cd_action == 'delete': self.delete_if_grp() - - self.module.exit_json(changed=changed) + elif modify: + self.modify_ports(current_ports['ports']) + self.module.exit_json(changed=self.na_helper.changed) def main(): diff --git a/test/units/modules/storage/netapp/test_na_ontap_net_ifgrp.py b/test/units/modules/storage/netapp/test_na_ontap_net_ifgrp.py new file mode 100644 index 0000000000..0b678ba3fa --- /dev/null +++ b/test/units/modules/storage/netapp/test_na_ontap_net_ifgrp.py @@ -0,0 +1,286 @@ +# (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_net_ifgrp \ + import NetAppOntapIfGrp as ifgrp_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.kind = 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.kind == 'ifgrp': + xml = self.build_ifgrp_info(self.params) + elif self.kind == 'ifgrp-ports': + xml = self.build_ifgrp_ports_info(self.params) + elif self.kind == 'ifgrp-fail': + raise netapp_utils.zapi.NaApiError(code='TEST', message="This exception is from the unit test") + self.xml_out = xml + return xml + + @staticmethod + def build_ifgrp_info(ifgrp_details): + ''' build xml data for ifgrp-info ''' + xml = netapp_utils.zapi.NaElement('xml') + attributes = { + 'num-records': 1, + 'attributes-list': { + 'net-port-info': { + 'port': ifgrp_details['name'], + 'ifgrp-distribution-function': 'mac', + 'ifgrp-mode': ifgrp_details['mode'], + 'node': ifgrp_details['node'] + } + } + } + xml.translate_struct(attributes) + return xml + + @staticmethod + def build_ifgrp_ports_info(data): + ''' build xml data for ifgrp-ports ''' + xml = netapp_utils.zapi.NaElement('xml') + attributes = { + 'attributes': { + 'net-ifgrp-info': { + 'ports': [ + {'lif-bindable': data['ports'][0]}, + {'lif-bindable': data['ports'][1]}, + {'lif-bindable': data['ports'][2]} + ] + } + } + } + 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_ifgrp = { + 'name': 'test', + 'port': 'a1', + 'node': 'test_vserver', + 'mode': 'something' + } + + def mock_args(self): + return { + 'name': self.mock_ifgrp['name'], + 'distribution_function': 'mac', + 'ports': [self.mock_ifgrp['port']], + 'node': self.mock_ifgrp['node'], + 'mode': self.mock_ifgrp['mode'], + 'hostname': 'test', + 'username': 'test_user', + 'password': 'test_pass!' + } + + def get_ifgrp_mock_object(self, kind=None, data=None): + """ + Helper method to return an na_ontap_net_ifgrp object + :param kind: passes this param to MockONTAPConnection() + :return: na_ontap_net_ifgrp object + """ + obj = ifgrp_module() + obj.autosupport_log = Mock(return_value=None) + if data is None: + data = self.mock_ifgrp + obj.server = MockONTAPConnection(kind=kind, data=data) + return 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({}) + ifgrp_module() + print('Info: %s' % exc.value.args[0]['msg']) + + def test_get_nonexistent_ifgrp(self): + ''' Test if get_ifgrp returns None for non-existent ifgrp ''' + set_module_args(self.mock_args()) + result = self.get_ifgrp_mock_object().get_if_grp() + assert result is None + + def test_get_existing_ifgrp(self): + ''' Test if get_ifgrp returns details for existing ifgrp ''' + set_module_args(self.mock_args()) + result = self.get_ifgrp_mock_object('ifgrp').get_if_grp() + assert result['name'] == self.mock_ifgrp['name'] + + def test_successful_create(self): + ''' Test successful create ''' + data = self.mock_args() + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_ifgrp_mock_object().apply() + assert exc.value.args[0]['changed'] + + def test_successful_delete(self): + ''' Test delete existing volume ''' + data = self.mock_args() + data['state'] = 'absent' + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_ifgrp_mock_object('ifgrp').apply() + assert exc.value.args[0]['changed'] + + def test_successful_modify(self): + ''' Test delete existing volume ''' + data = self.mock_args() + data['ports'] = ['1', '2', '3'] + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_ifgrp_mock_object('ifgrp').apply() + assert exc.value.args[0]['changed'] + + @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.get_if_grp') + @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.create_if_grp') + def test_create_called(self, create_ifgrp, get_ifgrp): + data = self.mock_args() + set_module_args(data) + get_ifgrp.return_value = None + with pytest.raises(AnsibleExitJson) as exc: + self.get_ifgrp_mock_object().apply() + get_ifgrp.assert_called_with() + create_ifgrp.assert_called_with() + + @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.add_port_to_if_grp') + def test_if_ports_are_added_after_create(self, add_ports): + ''' Test successful create ''' + data = self.mock_args() + set_module_args(data) + self.get_ifgrp_mock_object().create_if_grp() + add_ports.assert_called_with('a1') + + @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.get_if_grp') + @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.delete_if_grp') + def test_delete_called(self, delete_ifgrp, get_ifgrp): + data = self.mock_args() + data['state'] = 'absent' + set_module_args(data) + get_ifgrp.return_value = Mock() + with pytest.raises(AnsibleExitJson) as exc: + self.get_ifgrp_mock_object().apply() + get_ifgrp.assert_called_with() + delete_ifgrp.assert_called_with() + + def test_get_return_value(self): + data = self.mock_args() + set_module_args(data) + result = self.get_ifgrp_mock_object('ifgrp').get_if_grp() + assert result['name'] == data['name'] + assert result['mode'] == data['mode'] + assert result['node'] == data['node'] + + def test_get_ports_list(self): + data = self.mock_args() + data['ports'] = ['e0a', 'e0b', 'e0c'] + set_module_args(data) + result = self.get_ifgrp_mock_object('ifgrp-ports', data).get_if_grp_ports() + assert result['ports'] == data['ports'] + + def test_add_port_packet(self): + data = self.mock_args() + set_module_args(data) + obj = self.get_ifgrp_mock_object('ifgrp') + obj.add_port_to_if_grp('addme') + assert obj.server.xml_in['port'] == 'addme' + + @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.remove_port_to_if_grp') + @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.add_port_to_if_grp') + def test_modify_ports_calls(self, add_port, remove_port): + data = self.mock_args() + data['ports'] = ['1', '2', '3'] + set_module_args(data) + self.get_ifgrp_mock_object('ifgrp').modify_ports(current_ports=['1', '2']) + assert remove_port.call_count == 2 + assert add_port.call_count == 3 + + def test_get_ports_returns_none(self): + set_module_args(self.mock_args()) + result = self.get_ifgrp_mock_object().get_if_grp_ports() + assert result['ports'] == [] + result = self.get_ifgrp_mock_object().get_if_grp() + assert result is None + + def test_if_all_methods_catch_exception(self): + set_module_args(self.mock_args()) + with pytest.raises(AnsibleFailJson) as exc: + self.get_ifgrp_mock_object('ifgrp-fail').get_if_grp() + assert 'Error getting if_group test' in exc.value.args[0]['msg'] + with pytest.raises(AnsibleFailJson) as exc: + self.get_ifgrp_mock_object('ifgrp-fail').create_if_grp() + assert 'Error creating if_group test' in exc.value.args[0]['msg'] + with pytest.raises(AnsibleFailJson) as exc: + self.get_ifgrp_mock_object('ifgrp-fail').get_if_grp_ports() + assert 'Error getting if_group ports test' in exc.value.args[0]['msg'] + with pytest.raises(AnsibleFailJson) as exc: + self.get_ifgrp_mock_object('ifgrp-fail').add_port_to_if_grp('test-port') + assert 'Error adding port test-port to if_group test' in exc.value.args[0]['msg'] + with pytest.raises(AnsibleFailJson) as exc: + self.get_ifgrp_mock_object('ifgrp-fail').remove_port_to_if_grp('test-port') + assert 'Error removing port test-port to if_group test' in exc.value.args[0]['msg'] + with pytest.raises(AnsibleFailJson) as exc: + self.get_ifgrp_mock_object('ifgrp-fail').delete_if_grp() + assert 'Error deleting if_group test' in exc.value.args[0]['msg']