MOTD was not idempotent (#57372)

* fix issue

* fix issues

* fix issues

* fix issues

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 <piotr.olczak@redhat.com>
 # 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) <polczak@redhat.com>
+author:
+ - Piotr Olczak (@dprts) <polczak@redhat.com>
+ - NetApp Ansible Team (@carchi8py) <ng-ansibleteam@netapp.com>
 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):
+    def motd_get_iter(self):
-        api_call = netapp_utils.zapi.NaElement('vserver-motd-modify-iter')
+        """
-        api_call.add_new_child('message', self.parameters['message'])
+        Compose NaElement object to query current motd
-        api_call.add_new_child('is-cluster-message-enabled', 'true' if self.parameters['show_cluster_motd'] else 'false')
+        :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)
+        motd_create.add_child_elem(query)
-        return api_call
+        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):
+    def apply(self):
-        results = netapp_utils.get_cserver(self.server)
+        """
-        cserver = netapp_utils.setup_na_ontap_zapi(module=self.module, vserver=results)
+        Applies action from playbook
-        netapp_utils.ems_log_event("na_ontap_motd", cserver)
+        """
+        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()
+        if self.na_helper.changed:
-
+            if self.module.check_mode:
-        try:
+                pass
-            _call_result = self.server.invoke_successfully(call, enable_tunneling=False)
+            else:
-        except netapp_utils.zapi.NaApiError as err:
+                self.modify_motd()
-            self.module.fail_json(msg="Error calling API %s: %s" %
+        self.module.exit_json(changed=self.na_helper.changed)
-                                  ('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)
 
 
 def main():
-    ansible_call = CDotMotd()
+    motd_obj = NetAppONTAPMotd()
-    ansible_call.commit_changes()
+    motd_obj.apply()
 
 
 if __name__ == '__main__':

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

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']