From cf491e8b5b4cb90f220b927bad22cccd45ea4f3b Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Thu, 21 Feb 2019 07:34:24 -0800 Subject: [PATCH] Modify na_ontap_service_processor_network to use NetAppModule, add new option to wait for completion, and unit tests. (#52292) * Revert "changes to clusteR" This reverts commit 33ee1b71e4bc8435fb315762a871f8c4cb6c5f80. * updates * fix docs * Revert "Revert "changes to clusteR"" This reverts commit 8e56b999e6cf6a65de339e516f7134a6b6b39cba. * fix docs --- .../na_ontap_service_processor_network.py | 247 ++++++++++-------- ...test_na_ontap_service_processor_network.py | 232 ++++++++++++++++ 2 files changed, 365 insertions(+), 114 deletions(-) create mode 100644 test/units/modules/storage/netapp/test_na_ontap_service_processor_network.py diff --git a/lib/ansible/modules/storage/netapp/na_ontap_service_processor_network.py b/lib/ansible/modules/storage/netapp/na_ontap_service_processor_network.py index 9db67784166..efb13f9281f 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_service_processor_network.py +++ b/lib/ansible/modules/storage/netapp/na_ontap_service_processor_network.py @@ -1,6 +1,6 @@ #!/usr/bin/python -# (c) 2018, NetApp, Inc +# (c) 2018-2019, NetApp, Inc # GNU General Public License v3.0+ # (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) @@ -57,20 +57,27 @@ options: prefix_length: description: - Specify the service processor prefix_length. + wait_for_completion: + description: + - Set this parameter to 'true' for synchronous execution (wait until SP status is successfully updated) + - Set this parameter to 'false' for asynchronous execution + - For asynchronous, execution exits as soon as the request is sent, without checking SP status + type: bool + default: false + version_added: '2.8' ''' EXAMPLES = """ - name: Modify Service Processor Network na_ontap_service_processor_network: - state=present - address_type=ipv4 - is_enabled=true - dhcp=v4 - node=FPaaS-A300-01 - node={{ netapp_node }} - username={{ netapp_username }} - password={{ netapp_password }} - hostname={{ netapp_hostname }} + state: present + address_type: ipv4 + is_enabled: true + dhcp: v4 + node: "{{ netapp_node }}" + username: "{{ netapp_username }}" + password: "{{ netapp_password }}" + hostname: "{{ netapp_hostname }}" """ RETURN = """ @@ -80,6 +87,8 @@ import traceback from ansible.module_utils.basic import AnsibleModule from ansible.module_utils._text import to_native import ansible.module_utils.netapp as netapp_utils +from ansible.module_utils.netapp_module import NetAppModule +import time HAS_NETAPP_LIB = netapp_utils.has_netapp_lib() @@ -104,7 +113,7 @@ class NetAppOntapServiceProcessorNetwork(object): ip_address=dict(required=False, type='str'), netmask=dict(required=False, type='str'), prefix_length=dict(required=False, type='int'), - + wait_for_completion=dict(required=False, type='bool', default=False) )) self.module = AnsibleModule( @@ -112,18 +121,9 @@ class NetAppOntapServiceProcessorNetwork(object): supports_check_mode=True ) - parameters = self.module.params - - # set up state variables - self.state = parameters['state'] - self.address_type = parameters['address_type'] - self.dhcp = parameters['dhcp'] - self.gateway_ip_address = parameters['gateway_ip_address'] - self.ip_address = parameters['ip_address'] - self.is_enabled = parameters['is_enabled'] - self.netmask = parameters['netmask'] - self.node = parameters['node'] - self.prefix_length = parameters['prefix_length'] + self.na_helper = NetAppModule() + self.parameters = self.na_helper.set_parameters(self.module.params) + self.set_playbook_zapi_key_map() if HAS_NETAPP_LIB is False: self.module.fail_json( @@ -133,121 +133,140 @@ class NetAppOntapServiceProcessorNetwork(object): module=self.module, vserver=None) return + def set_playbook_zapi_key_map(self): + self.na_helper.zapi_string_keys = { + 'address_type': 'address-type', + 'node': 'node', + 'dhcp': 'dhcp', + 'gateway_ip_address': 'gateway-ip-address', + 'ip_address': 'ip-address', + 'netmask': 'netmask' + } + self.na_helper.zapi_int_keys = { + 'prefix_length': 'prefix-length' + } + self.na_helper.zapi_bool_keys = { + 'is_enabled': 'is-enabled', + } + self.na_helper.zapi_required = { + 'address_type': 'address-type', + 'node': 'node', + 'is_enabled': 'is-enabled' + } + + def get_sp_network_status(self): + """ + Return status of service processor network + :param: + name : name of the node + :return: Status of the service processor network + :rtype: dict + """ + spn_get_iter = netapp_utils.zapi.NaElement('service-processor-network-get-iter') + query_info = { + 'query': { + 'service-processor-network-info': { + 'node': self.parameters['node'], + 'address-type': self.parameters['address_type'] + } + } + } + spn_get_iter.translate_struct(query_info) + result = self.server.invoke_successfully(spn_get_iter, True) + if int(result['num-records']) >= 1: + sp_attr_info = result['attributes-list']['service-processor-network-info'] + return sp_attr_info.get_child_content('setup-status') + return None + def get_service_processor_network(self): """ Return details about service processor network :param: - name : name of the vserver + name : name of the node :return: Details about service processor network. None if not found. :rtype: dict """ - spn_get_iter = netapp_utils.zapi.NaElement( - 'service-processor-network-get-iter') - spn_info = netapp_utils.zapi.NaElement( - 'service-processor-network-info') - spn_info.add_new_child('node', self.node) - query = netapp_utils.zapi.NaElement('query') - query.add_child_elem(spn_info) - spn_get_iter.add_child_elem(query) - result = self.server.invoke_successfully(spn_get_iter, True) - sp_network_details = None - # check if job exists - if result.get_child_by_name('num-records') and \ - int(result.get_child_content('num-records')) >= 1: - attributes_list = result.get_child_by_name('attributes-list').\ - get_child_by_name('service-processor-network-info') - node_value = attributes_list.get_child_content('node') - address_type_value = attributes_list.get_child_content( - 'address-type') - dhcp_value = attributes_list.get_child_content('dhcp') - gateway_ip_address_value = attributes_list.get_child_content( - 'gateway-ip-address') - ip_address_value = attributes_list.get_child_content('ip-address') - is_enabled_value = attributes_list.get_child_content('is-enabled') - netmask_value = attributes_list.get_child_content('netmask') - prefix_length_value = attributes_list.get_child_content( - 'prefix-length') - sp_network_details = { - 'node_value': node_value, - 'address_type_value': address_type_value, - 'dhcp_value': dhcp_value, - 'gateway_ip_address_value': gateway_ip_address_value, - 'ip_address_value': ip_address_value, - 'is_enabled_value': is_enabled_value, - 'netmask_value': netmask_value, - 'prefix_length_value': prefix_length_value + spn_get_iter = netapp_utils.zapi.NaElement('service-processor-network-get-iter') + query_info = { + 'query': { + 'service-processor-network-info': { + 'node': self.parameters['node'] + } } - return sp_network_details + } + spn_get_iter.translate_struct(query_info) + result = self.server.invoke_successfully(spn_get_iter, True) + sp_details = None + # check if job exists + if int(result['num-records']) >= 1: + sp_details = dict() + sp_attr_info = result['attributes-list']['service-processor-network-info'] + for item_key, zapi_key in self.na_helper.zapi_string_keys.items(): + sp_details[item_key] = sp_attr_info.get_child_content(zapi_key) + for item_key, zapi_key in self.na_helper.zapi_bool_keys.items(): + sp_details[item_key] = self.na_helper.get_value_for_bool(from_zapi=True, + value=sp_attr_info.get_child_content(zapi_key)) + for item_key, zapi_key in self.na_helper.zapi_int_keys.items(): + sp_details[item_key] = self.na_helper.get_value_for_int(from_zapi=True, + value=sp_attr_info.get_child_content(zapi_key)) + return sp_details - def modify_service_processor_network(self): + def modify_service_processor_network(self, params=None): """ Modify a service processor network """ - service_obj = netapp_utils.zapi.NaElement( - 'service-processor-network-modify') - service_obj.add_new_child("node", self.node) - service_obj.add_new_child("address-type", self.address_type) - service_obj.add_new_child("is-enabled", str(self.is_enabled).lower()) - - if self.dhcp: - service_obj.add_new_child("dhcp", self.dhcp) - if self.gateway_ip_address: - service_obj.add_new_child( - "gateway-ip-address", self.gateway_ip_address) - if self.ip_address: - service_obj.add_new_child("ip-address", self.ip_address) - if self.netmask: - service_obj.add_new_child("netmask", self.netmask) - if self.prefix_length is not None: - service_obj.add_new_child("prefix-length", str(self.prefix_length)) + if params.get('is_enabled') is None: # is-enabled is mandatory for ZAPI service-processor-network-modify + params['is_enabled'] = self.parameters['is_enabled'] + if params['is_enabled'] is False: + if len(params) > 1: + self.module.fail_json(msg='Error: Cannot modify any other parameter for a disabled service processor network') + sp_modify = netapp_utils.zapi.NaElement('service-processor-network-modify') + sp_modify.add_new_child("node", self.parameters['node']) + sp_modify.add_new_child("address-type", self.parameters['address_type']) + sp_attributes = dict() + for item_key in params: + if item_key in self.na_helper.zapi_string_keys: + zapi_key = self.na_helper.zapi_string_keys.get(item_key) + sp_attributes[zapi_key] = params[item_key] + elif item_key in self.na_helper.zapi_bool_keys: + zapi_key = self.na_helper.zapi_bool_keys.get(item_key) + sp_attributes[zapi_key] = self.na_helper.get_value_for_bool(from_zapi=False, value=params[item_key]) + elif item_key in self.na_helper.zapi_bool_keys: + zapi_key = self.na_helper.zapi_int_keys.get(item_key) + sp_attributes[zapi_key] = self.na_helper.get_value_for_int(from_zapi=False, value=params[item_key]) + sp_modify.translate_struct(sp_attributes) try: - result = self.server.invoke_successfully(service_obj, - enable_tunneling=True) + self.server.invoke_successfully(sp_modify, enable_tunneling=True) + if self.parameters.get('wait_for_completion'): + retries = 10 + while self.get_sp_network_status() == 'in_progress' and retries > 0: + time.sleep(10) + retries = retries - 1 except netapp_utils.zapi.NaApiError as error: - self.module.fail_json(msg='Error modifying \ - service processor network: %s' - % (to_native(error)), + self.module.fail_json(msg='Error modifying service processor network: %s' % (to_native(error)), exception=traceback.format_exc()) + 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_service_processor_network", cserver) + def apply(self): """ Run Module based on play book """ - changed = False - results = netapp_utils.get_cserver(self.server) - cserver = netapp_utils.setup_ontap_zapi( - module=self.module, vserver=results) - netapp_utils.ems_log_event( - "na_ontap_service_processor_network", cserver) - spn_details = self.get_service_processor_network() - spn_exists = False - if spn_details: - spn_exists = True - if self.state == 'present': # modify - if (self.dhcp is not None and - self.dhcp != spn_details['dhcp_value']) or \ - (self.gateway_ip_address is not None and - self.gateway_ip_address != spn_details['gateway_ip_address_value']) or \ - (self.ip_address is not None and - self.ip_address != spn_details['ip_address_value']) or \ - (self.netmask is not None and - self.netmask != spn_details['netmask_value']) or \ - (self.prefix_length is not None and str(self.prefix_length) - != spn_details['prefix_length_value']) or \ - (self.is_enabled is not None and str(self.is_enabled).lower() - != spn_details['is_enabled_value']): - changed = True - else: - self.module.fail_json(msg='Error No Service Processor for node: %s' % self.node) - if changed: + self.autosupport_log() + current = self.get_service_processor_network() + modify = self.na_helper.get_modified_attributes(current, self.parameters) + if not current: + self.module.fail_json(msg='Error No Service Processor for node: %s' % self.parameters['node']) + if self.na_helper.changed: if self.module.check_mode: pass else: - if self.state == 'present': # execute modify - if spn_exists: - self.modify_service_processor_network() - self.module.exit_json(changed=changed) + self.modify_service_processor_network(modify) + self.module.exit_json(changed=self.na_helper.changed) def main(): diff --git a/test/units/modules/storage/netapp/test_na_ontap_service_processor_network.py b/test/units/modules/storage/netapp/test_na_ontap_service_processor_network.py new file mode 100644 index 00000000000..ffbf41077e7 --- /dev/null +++ b/test/units/modules/storage/netapp/test_na_ontap_service_processor_network.py @@ -0,0 +1,232 @@ +''' unit test template for ONTAP Ansible module ''' + +from __future__ import print_function +import json +import pytest + +import time +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_service_processor_network \ + import NetAppOntapServiceProcessorNetwork as sp_module # module under test + +if not netapp_utils.has_netapp_lib(): + pytestmark = pytest.mark.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.data = 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 == 'sp-enabled': + xml = self.build_sp_info(self.data) + elif self.kind == 'sp-disabled': + xml = self.build_sp_disabled_info(self.data) + else: + xml = self.build_info() + self.xml_out = xml + return xml + + @staticmethod + def build_sp_info(sp): + ''' build xml data for vserser-info ''' + xml = netapp_utils.zapi.NaElement('xml') + attributes = { + 'num-records': 1, + 'attributes-list': + { + 'service-processor-network-info': + { + 'node-name': sp['node'], + 'is-enabled': 'true', + 'address-type': sp['address_type'], + 'dhcp': 'v4', + 'gateway-ip-address': sp['gateway_ip_address'], + 'netmask': sp['netmask'], + 'ip-address': sp['ip_address'], + 'setup-status': 'succeeded', + } + } + } + xml.translate_struct(attributes) + return xml + + @staticmethod + def build_info(): + ''' build xml data for vserser-info ''' + xml = netapp_utils.zapi.NaElement('xml') + attributes = { + 'num-records': 0 + } + xml.translate_struct(attributes) + return xml + + @staticmethod + def build_sp_disabled_info(sp): + ''' build xml data for vserser-info ''' + xml = netapp_utils.zapi.NaElement('xml') + attributes = { + 'num-records': 1, + 'attributes-list': + { + 'service-processor-network-info': + { + 'node-name': sp['node'], + 'is-enabled': 'false', + 'address-type': sp['address_type'], + 'setup-status': 'not_setup', + } + } + } + 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.server = MockONTAPConnection() + self.mock_sp = { + 'node': 'test-vsim1', + 'gateway_ip_address': '2.2.2.2', + 'address_type': 'ipv4', + 'ip_address': '1.1.1.1', + 'netmask': '255.255.248.0', + 'dhcp': 'v4' + } + + def mock_args(self, enable=True): + data = { + 'node': self.mock_sp['node'], + 'is_enabled': enable, + 'address_type': self.mock_sp['address_type'], + 'hostname': 'host', + 'username': 'admin', + 'password': 'password', + } + if enable is True: + data['ip_address'] = self.mock_sp['ip_address'] + data['gateway_ip_address'] = self.mock_sp['gateway_ip_address'] + data['netmask'] = self.mock_sp['netmask'] + data['dhcp'] = 'v4' + return data + + def get_sp_mock_object(self, kind=None): + """ + Helper method to return an na_ontap_volume object + :param kind: passes this param to MockONTAPConnection() + :return: na_ontap_volume object + """ + sp_obj = sp_module() + sp_obj.autosupport_log = Mock(return_value=None) + if kind is None: + sp_obj.server = MockONTAPConnection() + else: + sp_obj.server = MockONTAPConnection(kind=kind, data=self.mock_sp) + return sp_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({}) + sp_module() + print('Info: %s' % exc.value.args[0]['msg']) + + def test_modify_error_on_disabled_sp(self): + ''' a more interesting test ''' + data = self.mock_args(enable=False) + set_module_args(data) + with pytest.raises(AnsibleFailJson) as exc: + self.get_sp_mock_object('sp-disabled').apply() + assert 'Error: Cannot modify any other parameter for a disabled service processor network' in \ + exc.value.args[0]['msg'] + + def test_modify_sp(self): + ''' a more interesting test ''' + data = self.mock_args() + data['ip_address'] = '3.3.3.3' + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_sp_mock_object('sp-enabled').apply() + assert exc.value.args[0]['changed'] + + def test_modify_sp_wait(self): + ''' a more interesting test ''' + data = self.mock_args() + data['ip_address'] = '3.3.3.3' + data['wait_for_completion'] = True + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_sp_mock_object('sp-enabled').apply() + assert exc.value.args[0]['changed'] + + @patch('ansible.modules.storage.netapp.na_ontap_service_processor_network.NetAppOntapServiceProcessorNetwork.' + 'get_service_processor_network') + def test_non_existing_sp(self, get_sp): + set_module_args(self.mock_args()) + get_sp.return_value = None + with pytest.raises(AnsibleFailJson) as exc: + self.get_sp_mock_object().apply() + assert 'Error No Service Processor for node: test-vsim1' in exc.value.args[0]['msg'] + + @patch('ansible.modules.storage.netapp.na_ontap_service_processor_network.NetAppOntapServiceProcessorNetwork.' + 'get_sp_network_status') + @patch('time.sleep') + def test_wait_on_sp_status(self, get_sp, sleep): + data = self.mock_args() + data['gateway_ip_address'] = '4.4.4.4' + data['wait_for_completion'] = True + set_module_args(data) + get_sp.side_effect = ['in_progress', 'done'] + with pytest.raises(AnsibleExitJson) as exc: + self.get_sp_mock_object('sp-enabled').apply() + sleep.assert_called_once_with() + assert exc.value.args[0]['changed']