From f3aac3a11291fdc926729d3734b12aa8d904856e Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Fri, 12 Jul 2019 13:47:42 -0700 Subject: [PATCH] MOTD was not idempotent (#57372) * fix issue * fix issues * fix issues * fix issues --- .../modules/storage/netapp/na_ontap_motd.py | 134 ++++++++----- test/sanity/validate-modules/ignore.txt | 1 - .../storage/netapp/test_na_ontap_motd.py | 181 ++++++++++++++++++ 3 files changed, 271 insertions(+), 45 deletions(-) create mode 100644 test/units/modules/storage/netapp/test_na_ontap_motd.py diff --git a/lib/ansible/modules/storage/netapp/na_ontap_motd.py b/lib/ansible/modules/storage/netapp/na_ontap_motd.py index bdc83ff7d30..7f48ebcb8d9 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_motd.py +++ b/lib/ansible/modules/storage/netapp/na_ontap_motd.py @@ -1,5 +1,6 @@ #!/usr/bin/python +# (c) 2018-2019, NetApp, Inc # (c) 2018 Piotr Olczak # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) @@ -13,12 +14,15 @@ ANSIBLE_METADATA = {'metadata_version': '1.1', DOCUMENTATION = ''' module: na_ontap_motd -author: Piotr Olczak (@dprts) +author: + - Piotr Olczak (@dprts) + - NetApp Ansible Team (@carchi8py) extends_documentation_fragment: - netapp.na_ontap -short_description: Setup motd on cDOT +short_description: Setup motd description: - - This module allows you to manipulate motd on cDOT + - This module allows you to manipulate motd for a vserver + - It also allows to manipulate motd at the cluster level by using the cluster vserver (cserver) version_added: "2.7" requirements: - netapp_lib @@ -31,10 +35,12 @@ options: message: description: - MOTD Text message, required when C(state=present). + type: str vserver: description: - The name of the SVM motd should be set for. required: true + type: str show_cluster_motd: description: - Set to I(false) if Cluster-level Message of the Day should not be shown @@ -66,6 +72,15 @@ EXAMPLES = ''' show_cluster_motd: False https: true +- name: Remove Cluster-Level MOTD + na_ontap_motd: + vserver: my_ontap_cluster + hostname: "{{ netapp_hostname }}" + username: "{{ netapp_username }}" + password: "{{ netapp_password }}" + state: absent + https: true + ''' RETURN = ''' @@ -78,17 +93,11 @@ from ansible.module_utils._text import to_native import ansible.module_utils.netapp as netapp_utils from ansible.module_utils.netapp_module import NetAppModule -try: - import xmltodict - HAS_XMLTODICT_LIB = True -except ImportError: - HAS_XMLTODICT_LIB = False - HAS_NETAPP_LIB = netapp_utils.has_netapp_lib() -class CDotMotd(object): +class NetAppONTAPMotd(object): def __init__(self): argument_spec = netapp_utils.na_ontap_host_argument_spec() @@ -107,56 +116,93 @@ class CDotMotd(object): self.na_helper = NetAppModule() self.parameters = self.na_helper.set_parameters(self.module.params) - if HAS_XMLTODICT_LIB is False: - self.module.fail_json(msg="the python xmltodict module is required") - if HAS_NETAPP_LIB is False: self.module.fail_json(msg="the python NetApp-Lib module is required") - self.server = netapp_utils.setup_na_ontap_zapi(module=self.module) + self.server = netapp_utils.setup_na_ontap_zapi(module=self.module, vserver=self.parameters['vserver']) - def _create_call(self): - api_call = netapp_utils.zapi.NaElement('vserver-motd-modify-iter') - api_call.add_new_child('message', self.parameters['message']) - api_call.add_new_child('is-cluster-message-enabled', 'true' if self.parameters['show_cluster_motd'] else 'false') + def motd_get_iter(self): + """ + Compose NaElement object to query current motd + :return: NaElement object for vserver-motd-get-iter + """ + motd_get_iter = netapp_utils.zapi.NaElement('vserver-motd-get-iter') + query = netapp_utils.zapi.NaElement('query') + motd_info = netapp_utils.zapi.NaElement('vserver-motd-info') + motd_info.add_new_child('is-cluster-message-enabled', str(self.parameters['show_cluster_motd'])) + motd_info.add_new_child('vserver', self.parameters['vserver']) + query.add_child_elem(motd_info) + motd_get_iter.add_child_elem(query) + return motd_get_iter + + def motd_get(self): + """ + Get current motd + :return: Dictionary of current motd details if query successful, else None + """ + motd_get_iter = self.motd_get_iter() + motd_result = dict() + try: + result = self.server.invoke_successfully(motd_get_iter, enable_tunneling=True) + except netapp_utils.zapi.NaApiError as error: + self.module.fail_json(msg='Error fetching motd info: %s' % to_native(error), + exception=traceback.format_exc()) + if result.get_child_by_name('num-records') and \ + int(result.get_child_content('num-records')) > 0: + motd_info = result.get_child_by_name('attributes-list').get_child_by_name( + 'vserver-motd-info') + motd_result['message'] = motd_info.get_child_content('message') + motd_result['message'] = str(motd_result['message']).rstrip() + motd_result['show_cluster_motd'] = True if motd_info.get_child_content( + 'is-cluster-message-enabled') == 'true' else False + motd_result['vserver'] = motd_info.get_child_content('vserver') + return motd_result + return None + + def modify_motd(self): + motd_create = netapp_utils.zapi.NaElement('vserver-motd-modify-iter') + motd_create.add_new_child('message', self.parameters['message']) + motd_create.add_new_child( + 'is-cluster-message-enabled', 'true' if self.parameters['show_cluster_motd'] is True else 'false') query = netapp_utils.zapi.NaElement('query') motd_info = netapp_utils.zapi.NaElement('vserver-motd-info') motd_info.add_new_child('vserver', self.parameters['vserver']) query.add_child_elem(motd_info) - api_call.add_child_elem(query) - return api_call + motd_create.add_child_elem(query) + try: + self.server.invoke_successfully(motd_create, enable_tunneling=False) + except netapp_utils.zapi.NaApiError as err: + self.module.fail_json(msg="Error creating motd: %s" % (to_native(err)), exception=traceback.format_exc()) + return motd_create - def commit_changes(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_motd", cserver) + def apply(self): + """ + Applies action from playbook + """ + netapp_utils.ems_log_event("na_ontap_motd", self.server) + current = self.motd_get() + if self.parameters['state'] == 'present' and self.parameters['message'] == "": + self.module.fail_json(msg="message parameter cannot be empty") if self.parameters['state'] == 'absent': # Just make sure it is empty self.parameters['message'] = '' + if current['message'] == 'None': + current = None + cd_action = self.na_helper.get_cd_action(current, self.parameters) + if cd_action is None and self.parameters['state'] == 'present': + self.na_helper.get_modified_attributes(current, self.parameters) - call = self._create_call() - - try: - _call_result = self.server.invoke_successfully(call, enable_tunneling=False) - except netapp_utils.zapi.NaApiError as err: - self.module.fail_json(msg="Error calling API %s: %s" % - ('vserver-motd-modify-iter', to_native(err)), exception=traceback.format_exc()) - - _dict_num_succeeded = xmltodict.parse( - _call_result.get_child_by_name('num-succeeded').to_string(), - xml_attribs=False) - - num_succeeded = int(_dict_num_succeeded['num-succeeded']) - - changed = bool(num_succeeded >= 1) - - result = {'state': self.parameters['state'], 'changed': changed} - self.module.exit_json(**result) + if self.na_helper.changed: + if self.module.check_mode: + pass + else: + self.modify_motd() + self.module.exit_json(changed=self.na_helper.changed) def main(): - ansible_call = CDotMotd() - ansible_call.commit_changes() + motd_obj = NetAppONTAPMotd() + motd_obj.apply() if __name__ == '__main__': diff --git a/test/sanity/validate-modules/ignore.txt b/test/sanity/validate-modules/ignore.txt index 63c493bf630..5c14f2adcb1 100644 --- a/test/sanity/validate-modules/ignore.txt +++ b/test/sanity/validate-modules/ignore.txt @@ -3412,7 +3412,6 @@ lib/ansible/modules/storage/netapp/na_ontap_lun_map.py E337 lib/ansible/modules/storage/netapp/na_ontap_lun_map.py E338 lib/ansible/modules/storage/netapp/na_ontap_lun.py E337 lib/ansible/modules/storage/netapp/na_ontap_lun.py E338 -lib/ansible/modules/storage/netapp/na_ontap_motd.py E337 lib/ansible/modules/storage/netapp/na_ontap_motd.py E338 lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py E337 lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py E338 diff --git a/test/units/modules/storage/netapp/test_na_ontap_motd.py b/test/units/modules/storage/netapp/test_na_ontap_motd.py new file mode 100644 index 00000000000..a3b031aa739 --- /dev/null +++ b/test/units/modules/storage/netapp/test_na_ontap_motd.py @@ -0,0 +1,181 @@ +# (c) 2019, NetApp, Inc +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +""" unit tests for Ansible module: na_ontap_motd """ + +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_motd \ + import NetAppONTAPMotd as my_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): + ''' save arguments ''' + self.type = kind + 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 == 'motd': + xml = self.build_motd_info() + elif self.type == 'motd_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_motd_info(): + ''' build xml data for motd ''' + xml = netapp_utils.zapi.NaElement('xml') + data = {'num-records': 1, + 'attributes-list': {'vserver-motd-info': {'message': 'ansible', + 'vserver': 'ansible', + 'is-cluster-message-enabled': 'true'}}} + xml.translate_struct(data) + print(xml.to_string()) + 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() + # whether to use a mock or a simulator + self.onbox = False + + def set_default_args(self): + if self.onbox: + hostname = '10.10.10.10' + username = 'admin' + password = 'password' + message = 'ansible' + vserver = 'ansible' + show_cluster_motd = 'true' + else: + hostname = 'hostname' + username = 'username' + password = 'password' + message = 'ansible' + vserver = 'ansible' + show_cluster_motd = 'true' + return dict({ + 'hostname': hostname, + 'username': username, + 'password': password, + 'message': message, + 'vserver': vserver, + 'show_cluster_motd': show_cluster_motd + }) + + def call_command(self, module_args): + ''' utility function to call apply ''' + module_args.update(self.set_default_args()) + set_module_args(module_args) + my_obj = my_module() + if not self.onbox: + my_obj.server = MockONTAPConnection('motd') + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + return exc.value.args[0]['changed'] + + def test_module_fail_when_required_args_missing(self): + ''' required arguments are reported as errors ''' + with pytest.raises(AnsibleFailJson) as exc: + set_module_args({}) + my_module() + print('Info: %s' % exc.value.args[0]['msg']) + + def test_ensure_motd_get_called(self): + ''' fetching details of motd ''' + set_module_args(self.set_default_args()) + my_obj = my_module() + my_obj.server = self.server + assert my_obj.motd_get() is None + + def test_ensure_get_called_existing(self): + ''' test for existing motd''' + set_module_args(self.set_default_args()) + my_obj = my_module() + my_obj.server = MockONTAPConnection(kind='motd') + assert my_obj.motd_get() + + def test_motd_create(self): + ''' test for creating motd''' + set_module_args(self.set_default_args()) + my_obj = my_module() + if not self.onbox: + my_obj.server = MockONTAPConnection(kind='motd') + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + assert not exc.value.args[0]['changed'] + + def test_motd_delete(self): + ''' test for deleting motd''' + module_args = { + 'state': 'absent', + } + changed = self.call_command(module_args) + assert changed + + def test_if_all_methods_catch_exception(self): + set_module_args(self.set_default_args()) + my_obj = my_module() + if not self.onbox: + my_obj.server = MockONTAPConnection('motd_fail') + with pytest.raises(AnsibleFailJson) as exc: + my_obj.motd_get() + assert '' in exc.value.args[0]['msg'] + with pytest.raises(AnsibleFailJson) as exc: + my_obj.modify_motd() + assert 'Error creating motd: ' in exc.value.args[0]['msg']