New Options for Autosupport (#50773)

* new option

* spelling

* fix minor issue

* Fix Doc line

* Add update netapp_module

* fix issue with autosupport

* Fix docuemntation

lib/ansible/module_utils/netapp_module.py

@@ -28,6 +28,8 @@
 
 ''' Support class for NetApp ansible modules '''
 
+import ansible.module_utils.netapp as netapp_utils
+
 
 def cmp(a, b):
     """
@@ -36,6 +38,18 @@ def cmp(a, b):
     :param b: second object to check
     :return:
     """
+    # convert to lower case for string comparison.
+    if a is None:
+        return -1
+    if type(a) is str and type(b) is str:
+        a = a.lower()
+        b = b.lower()
+    # if list has string element, convert string to lower case.
+    if type(a) is list and type(b) is list:
+        a = [x.lower() if type(x) is str else x for x in a]
+        b = [x.lower() if type(x) is str else x for x in b]
+        a.sort()
+        b.sort()
     return (a > b) - (a < b)
 
 
@@ -50,6 +64,11 @@ class NetAppModule(object):
         self.log = list()
         self.changed = False
         self.parameters = {'name': 'not intialized'}
+        self.zapi_string_keys = dict()
+        self.zapi_bool_keys = dict()
+        self.zapi_list_keys = dict()
+        self.zapi_int_keys = dict()
+        self.zapi_required = dict()
 
     def set_parameters(self, ansible_params):
         self.parameters = dict()
@@ -58,6 +77,64 @@ class NetAppModule(object):
                 self.parameters[param] = ansible_params[param]
         return self.parameters
 
+    def get_value_for_bool(self, from_zapi, value):
+        """
+        Convert boolean values to string or vice-versa
+        If from_zapi = True, value is converted from string (as it appears in ZAPI) to boolean
+        If from_zapi = False, value is converted from boolean to string
+        For get() method, from_zapi = True
+        For modify(), create(), from_zapi = False
+        :param from_zapi: convert the value from ZAPI or to ZAPI acceptable type
+        :param value: value of the boolean attribute
+        :return: string or boolean
+        """
+        if value is None:
+            return None
+        if from_zapi:
+            return True if value == 'true' else False
+        else:
+            return 'true' if value else 'false'
+
+    def get_value_for_int(self, from_zapi, value):
+        """
+        Convert integer values to string or vice-versa
+        If from_zapi = True, value is converted from string (as it appears in ZAPI) to integer
+        If from_zapi = False, value is converted from integer to string
+        For get() method, from_zapi = True
+        For modify(), create(), from_zapi = False
+        :param from_zapi: convert the value from ZAPI or to ZAPI acceptable type
+        :param value: value of the integer attribute
+        :return: string or integer
+        """
+        if value is None:
+            return None
+        if from_zapi:
+            return int(value)
+        else:
+            return str(value)
+
+    def get_value_for_list(self, from_zapi, zapi_parent, zapi_child=None, data=None):
+        """
+        Convert a python list() to NaElement or vice-versa
+        If from_zapi = True, value is converted from NaElement (parent-children structure) to list()
+        If from_zapi = False, value is converted from list() to NaElement
+        :param zapi_parent: ZAPI parent key or the ZAPI parent NaElement
+        :param zapi_child: ZAPI child key
+        :param data: list() to be converted to NaElement parent-children object
+        :param from_zapi: convert the value from ZAPI or to ZAPI acceptable type
+        :return: list() or NaElement
+        """
+        if from_zapi:
+            if zapi_parent is None:
+                return []
+            else:
+                return [zapi_child.get_content() for zapi_child in zapi_parent.get_children()]
+        else:
+            zapi_parent = netapp_utils.zapi.NaElement(zapi_parent)
+            for item in data:
+                zapi_parent.add_new_child(zapi_child, item)
+            return zapi_parent
+
     def get_cd_action(self, current, desired):
         ''' takes a desired state and a current state, and return an action:
             create, delete, None
@@ -91,11 +168,16 @@ class NetAppModule(object):
         '''
         pass
 
-    def get_modified_attributes(self, current, desired):
-        ''' takes two lists of attributes and return a list of attributes that are
-            not in the desired state
+    def get_modified_attributes(self, current, desired, get_list_diff=False):
+        ''' takes two dicts of attributes and return a dict of attributes that are
+            not in the current state
             It is expected that all attributes of interest are listed in current and
             desired.
+            :param: current: current attributes in ONTAP
+            :param: desired: attributes from playbook
+            :param: get_list_diff: specifies whether to have a diff of desired list w.r.t current list for an attribute
+            :return: dict of attributes to be modified
+            :rtype: dict
 
             NOTE: depending on the attribute, the caller may need to do a modify or a
             different operation (eg move volume if the modified attribute is an
@@ -116,7 +198,10 @@ class NetAppModule(object):
                     value.sort()
                     desired[key].sort()
                 if cmp(value, desired[key]) != 0:
-                    modified[key] = desired[key]
+                    if not get_list_diff:
+                        modified[key] = desired[key]
+                    else:
+                        modified[key] = [item for item in desired[key] if item not in value]
         if modified:
             self.changed = True
         return modified

lib/ansible/modules/storage/netapp/na_ontap_autosupport.py

@@ -3,7 +3,7 @@
 create Autosupport module to enable, disable or modify
 """
 
-# (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)
 from __future__ import absolute_import, division, print_function
 __metaclass__ = type
@@ -24,58 +24,79 @@ module: na_ontap_autosupport
 options:
   state:
     description:
-      - Specifies whether the AutoSupport daemon is present or absent.
-      - When this setting is absent, delivery of all AutoSupport messages is turned off.
+    - Specifies whether the AutoSupport daemon is present or absent.
+    - When this setting is absent, delivery of all AutoSupport messages is turned off.
     choices: ['present', 'absent']
     default: present
   node_name:
     description:
-      - The name fo the filer that owns the AutoSupport Configuration.
+    - The name of the filer that owns the AutoSupport Configuration.
     required: true
   transport:
     description:
-      - The name of the transport protocol used to deliver AutoSupport messages
+    - The name of the transport protocol used to deliver AutoSupport messages
     choices: ['http', 'https', 'smtp']
   noteto:
     description:
-      - Specifies up to five recipients of full AutoSupport e-mail messages.
+    - Specifies up to five recipients of short AutoSupport e-mail messages.
   post_url:
     description:
-      - The URL used to deliver AutoSupport messages via HTTP POST
+    - The URL used to deliver AutoSupport messages via HTTP POST
   mail_hosts:
     description:
-      - List of mail server(s) used to deliver AutoSupport messages via SMTP.
-      - Both host names and IP addresses may be used as valid input.
+    - List of mail server(s) used to deliver AutoSupport messages via SMTP.
+    - Both host names and IP addresses may be used as valid input.
   support:
     description:
-      - Specifies whether AutoSupport notification to technical support is enabled.
+    - Specifies whether AutoSupport notification to technical support is enabled.
     type: bool
-short_description: "NetApp ONTAP manage Autosupport"
+  from_address:
+    description:
+    - specify the e-mail address from which the node sends AutoSupport messages
+    version_added: 2.8
+  partner_addresses:
+    description:
+    - Specifies up to five partner vendor recipients of full AutoSupport e-mail messages.
+    version_added: 2.8
+  to_addresses:
+    description:
+    - Specifies up to five recipients of full AutoSupport e-mail messages.
+    version_added: 2.8
+  proxy_url:
+    description:
+    - specify an HTTP or HTTPS proxy if the 'transport' parameter is set to HTTP or HTTPS and your organization uses a proxy
+    version_added: 2.8
+  hostname_in_subject:
+    description:
+    - Specify whether the hostname of the node is included in the subject line of the AutoSupport message.
+    type: bool
+    version_added: 2.8
+short_description: NetApp ONTAP Autosupport
 version_added: "2.7"
 
 """
 
 EXAMPLES = """
-   - name: Enable autosupport
-     na_ontap_autosupport:
-       hostname: "{{ hostname }}"
-       username: "{{ username }}"
-       password: "{{ password }}"
-       state: present
-       node_name: test
-       transport: https
-       noteto: abc@def.com,def@ghi.com
-       mail_hosts: 1.2.3.4,5.6.7.8
-       support: False
-       post_url: url/1.0/post
+    - name: Enable autosupport
+      na_ontap_autosupport:
+        hostname: "{{ hostname }}"
+        username: "{{ username }}"
+        password: "{{ password }}"
+        state: present
+        node_name: test
+        transport: https
+        noteto: abc@def.com,def@ghi.com
+        mail_hosts: 1.2.3.4,5.6.7.8
+        support: False
+        post_url: url/1.0/post
 
-   - name: Disable autosupport
-     na_ontap_autosupport:
-       hostname: "{{ hostname }}"
-       username: "{{ username }}"
-       password: "{{ password }}"
-       state: absent
-       node_name: test
+    - name: Disable autosupport
+      na_ontap_autosupport:
+        hostname: "{{ hostname }}"
+        username: "{{ username }}"
+        password: "{{ password }}"
+        state: absent
+        node_name: test
 
 """
 
@@ -104,7 +125,12 @@ class NetAppONTAPasup(object):
             noteto=dict(required=False, type='list'),
             post_url=dict(reuired=False, type='str'),
             support=dict(required=False, type='bool'),
-            mail_hosts=dict(required=False, type='list')
+            mail_hosts=dict(required=False, type='list'),
+            from_address=dict(required=False, type='str'),
+            partner_addresses=dict(required=False, type='list'),
+            to_addresses=dict(required=False, type='list'),
+            proxy_url=dict(required=False, type='str'),
+            hostname_in_subject=dict(required=False, type='bool'),
         ))
 
         self.module = AnsibleModule(
@@ -116,16 +142,36 @@ class NetAppONTAPasup(object):
         self.parameters = self.na_helper.set_parameters(self.module.params)
         # present or absent requires modifying state to enabled or disabled
         self.parameters['service_state'] = 'started' if self.parameters['state'] == 'present' else 'stopped'
+        self.set_playbook_zapi_key_map()
 
         if HAS_NETAPP_LIB is False:
             self.module.fail_json(msg="the python NetApp-Lib module is required")
         else:
             self.server = netapp_utils.setup_ontap_zapi(module=self.module)
 
+    def set_playbook_zapi_key_map(self):
+        self.na_helper.zapi_string_keys = {
+            'node_name': 'node-name',
+            'transport': 'transport',
+            'post_url': 'post-url',
+            'from_address': 'from',
+            'proxy_url': 'proxy-url'
+        }
+        self.na_helper.zapi_list_keys = {
+            'noteto': ('noteto', 'mail-address'),
+            'mail_hosts': ('mail-hosts', 'string'),
+            'partner_addresses': ('partner-address', 'mail-address'),
+            'to_addresses': ('to', 'mail-address'),
+        }
+        self.na_helper.zapi_bool_keys = {
+            'support': 'is-support-enabled',
+            'hostname_in_subject': 'is-node-in-subject'
+        }
+
     def get_autosupport_config(self):
         """
-        Invoke zapi - get current autosupport status
-        @return: 'true' or 'false' / FAILURE with an error_message
+        Invoke zapi - get current autosupport details
+        :return: dict()
         """
         asup_details = netapp_utils.zapi.NaElement('autosupport-config-get')
         asup_details.add_new_child('node-name', self.parameters['node_name'])
@@ -137,26 +183,17 @@ class NetAppONTAPasup(object):
                                   exception=traceback.format_exc())
         # zapi invoke successful
         asup_attr_info = result.get_child_by_name('attributes').get_child_by_name('autosupport-config-info')
-        current_state = asup_attr_info.get_child_content('is-enabled')
-        if current_state == 'true':
-            asup_info['service_state'] = 'started'
-        elif current_state == 'false':
-            asup_info['service_state'] = 'stopped'
-        current_support = asup_attr_info.get_child_content('is-support-enabled')
-        if current_support == 'true':
-            asup_info['support'] = True
-        elif current_support == 'false':
-            asup_info['support'] = False
-        asup_info['transport'] = asup_attr_info.get_child_content('transport')
-        asup_info['post_url'] = asup_attr_info.get_child_content('post-url')
-        mail_hosts = asup_attr_info.get_child_by_name('mail-hosts')
-        # mail hosts has one valid entry always
-        if mail_hosts is not None:
-            # get list of mail hosts
-            asup_info['mail_hosts'] = [mail.get_content() for mail in mail_hosts.get_children()]
-        email_list = asup_attr_info.get_child_by_name('noteto')
-        # if email_list is empty, noteto is also empty
-        asup_info['noteto'] = [] if email_list is None else [email.get_content() for email in email_list.get_children()]
+        asup_info['service_state'] = 'started' if asup_attr_info['is-enabled'] == 'true' else 'stopped'
+        for item_key, zapi_key in self.na_helper.zapi_string_keys.items():
+            asup_info[item_key] = asup_attr_info[zapi_key]
+        for item_key, zapi_key in self.na_helper.zapi_bool_keys.items():
+            asup_info[item_key] = self.na_helper.get_value_for_bool(from_zapi=True,
+                                                                    value=asup_attr_info[zapi_key])
+        for item_key, zapi_key in self.na_helper.zapi_list_keys.items():
+            parent, dummy = zapi_key
+            asup_info[item_key] = self.na_helper.get_value_for_list(from_zapi=True,
+                                                                    zapi_parent=asup_attr_info.get_child_by_name(parent)
+                                                                    )
         return asup_info
 
     def modify_autosupport_config(self, modify):
@@ -164,55 +201,46 @@ class NetAppONTAPasup(object):
         Invoke zapi - modify autosupport config
         @return: NaElement object / FAILURE with an error_message
         """
-        asup_details = netapp_utils.zapi.NaElement('autosupport-config-modify')
-        asup_details.add_new_child('node-name', self.parameters['node_name'])
+        asup_details = {'node-name': self.parameters['node_name']}
         if modify.get('service_state'):
-            if modify.get('service_state') == 'started':
-                asup_details.add_new_child('is-enabled', 'true')
-            elif modify.get('service_state') == 'stopped':
-                asup_details.add_new_child('is-enabled', 'false')
-        if modify.get('support') is not None:
-            if modify.get('support') is True:
-                asup_details.add_new_child('is-support-enabled', 'true')
-            elif modify.get('support') is False:
-                asup_details.add_new_child('is-support-enabled', 'false')
-        if modify.get('transport'):
-            asup_details.add_new_child('transport', modify['transport'])
-        if modify.get('post_url'):
-            asup_details.add_new_child('post-url', modify['post_url'])
-        if modify.get('noteto'):
-            asup_email = netapp_utils.zapi.NaElement('noteto')
-            asup_details.add_child_elem(asup_email)
-            for email in modify.get('noteto'):
-                asup_email.add_new_child('mail-address', email)
-        if modify.get('mail_hosts'):
-            asup_mail_hosts = netapp_utils.zapi.NaElement('mail-hosts')
-            asup_details.add_child_elem(asup_mail_hosts)
-            for mail in modify.get('mail_hosts'):
-                asup_mail_hosts.add_new_child('string', mail)
+            asup_details['is-enabled'] = 'true' if modify.get('service_state') == 'started' else 'false'
+        asup_config = netapp_utils.zapi.NaElement('autosupport-config-modify')
+        for item_key in modify:
+            if item_key in self.na_helper.zapi_string_keys:
+                zapi_key = self.na_helper.zapi_string_keys.get(item_key)
+                asup_details[zapi_key] = modify[item_key]
+            elif item_key in self.na_helper.zapi_bool_keys:
+                zapi_key = self.na_helper.zapi_bool_keys.get(item_key)
+                asup_details[zapi_key] = self.na_helper.get_value_for_bool(from_zapi=False,
+                                                                           value=modify[item_key])
+            elif item_key in self.na_helper.zapi_list_keys:
+                parent_key, child_key = self.na_helper.zapi_list_keys.get(item_key)
+                asup_config.add_child_elem(self.na_helper.get_value_for_list(from_zapi=False,
+                                                                             zapi_parent=parent_key,
+                                                                             zapi_child=child_key,
+                                                                             data=modify.get(item_key)))
+        asup_config.translate_struct(asup_details)
         try:
-            result = self.server.invoke_successfully(asup_details, enable_tunneling=True)
-            return result
+            return self.server.invoke_successfully(asup_config, enable_tunneling=True)
         except netapp_utils.zapi.NaApiError as error:
-            self.module.fail_json(msg='%s' % to_native(error),
-                                  exception=traceback.format_exc())
+            self.module.fail_json(msg='%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_autosupport", cserver)
 
     def apply(self):
         """
         Apply action to autosupport
         """
-        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_autosupport", cserver)
-
         current = self.get_autosupport_config()
         modify = self.na_helper.get_modified_attributes(current, self.parameters)
         if self.na_helper.changed:
             if self.module.check_mode:
                 pass
             else:
-                if modify:
-                    self.modify_autosupport_config(modify)
+                self.modify_autosupport_config(modify)
         self.module.exit_json(changed=self.na_helper.changed)
 
 

test/units/modules/storage/netapp/test_na_ontap_autosupport.py

@@ -0,0 +1,244 @@
+# (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_autosupport \
+    import NetAppONTAPasup as asup_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.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 == 'asup':
+            xml = self.build_asup_config_info(self.params)
+        self.xml_out = xml
+        return xml
+
+    @staticmethod
+    def build_asup_config_info(asup_data):
+        ''' build xml data for asup-config '''
+        xml = netapp_utils.zapi.NaElement('xml')
+        attributes = {'attributes': {'autosupport-config-info': {
+            'node-name': asup_data['node_name'],
+            'is-enabled': asup_data['is_enabled'],
+            'is-support-enabled': asup_data['support'],
+            'proxy-url': asup_data['proxy_url'],
+            'post-url': asup_data['post_url'],
+            'transport': asup_data['transport'],
+            'is-node-in-subject': 'false',
+            'from': 'test',
+            'mail-hosts': [{'string': '1.2.3.4'}, {'string': '4.5.6.8'}],
+            'noteto': [{'mail-address': 'abc@test.com'},
+                       {'mail-address': 'def@test.com'}],
+        }}}
+        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_asup = {
+            'node_name': 'test-vsim1',
+            'transport': 'https',
+            'support': 'false',
+            'post_url': 'testbed.netapp.com/asupprod/post/1.0/postAsup',
+            'proxy_url': 'something.com',
+        }
+
+    def mock_args(self):
+        return {
+            'node_name': self.mock_asup['node_name'],
+            'transport': self.mock_asup['transport'],
+            'support': self.mock_asup['support'],
+            'post_url': self.mock_asup['post_url'],
+            'proxy_url': self.mock_asup['proxy_url'],
+            'hostname': 'host',
+            'username': 'admin',
+            'password': 'password',
+        }
+
+    def get_asup_mock_object(self, kind=None, enabled='false'):
+        """
+        Helper method to return an na_ontap_volume object
+        :param kind: passes this param to MockONTAPConnection()
+        :return: na_ontap_volume object
+        """
+        asup_obj = asup_module()
+        asup_obj.autosupport_log = Mock(return_value=None)
+        if kind is None:
+            asup_obj.server = MockONTAPConnection()
+        else:
+            data = self.mock_asup
+            data['is_enabled'] = enabled
+            asup_obj.server = MockONTAPConnection(kind='asup', data=data)
+        return asup_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({})
+            asup_module()
+        print('Info: %s' % exc.value.args[0]['msg'])
+
+    def test_enable_asup(self):
+        ''' a more interesting test '''
+        data = self.mock_args()
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_asup_mock_object('asup').apply()
+        assert exc.value.args[0]['changed']
+
+    def test_disable_asup(self):
+        ''' a more interesting test '''
+        # enable asup
+        data = self.mock_args()
+        data['state'] = 'absent'
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_asup_mock_object(kind='asup', enabled='true').apply()
+        assert exc.value.args[0]['changed']
+
+    def test_result_from_get(self):
+        ''' Check boolean and service_state conversion from get '''
+        data = self.mock_args()
+        set_module_args(data)
+        obj = self.get_asup_mock_object(kind='asup', enabled='true')
+        # constructed based on valued passed in self.mock_asup and build_asup_config_info()
+        expected_dict = {
+            'node_name': 'test-vsim1',
+            'service_state': 'started',
+            'support': False,
+            'hostname_in_subject': False,
+            'transport': self.mock_asup['transport'],
+            'post_url': self.mock_asup['post_url'],
+            'proxy_url': self.mock_asup['proxy_url'],
+            'from_address': 'test',
+            'mail_hosts': ['1.2.3.4', '4.5.6.8'],
+            'partner_addresses': [],
+            'to_addresses': [],
+            'noteto': ['abc@test.com', 'def@test.com']
+        }
+        result = obj.get_autosupport_config()
+        assert result == expected_dict
+
+    def test_modify_config(self):
+        ''' Check boolean and service_state conversion from get '''
+        data = self.mock_args()
+        data['transport'] = 'http'
+        data['post_url'] = 'somethingelse.com'
+        data['proxy_url'] = 'somethingelse.com'
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_asup_mock_object('asup').apply()
+        assert exc.value.args[0]['changed']
+
+    @patch('ansible.modules.storage.netapp.na_ontap_autosupport.NetAppONTAPasup.get_autosupport_config')
+    def test_get_called(self, get_asup):
+        data = self.mock_args()
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_asup_mock_object('asup').apply()
+        get_asup.assert_called_with()
+
+    @patch('ansible.modules.storage.netapp.na_ontap_autosupport.NetAppONTAPasup.modify_autosupport_config')
+    def test_modify_called(self, modify_asup):
+        data = self.mock_args()
+        data['transport'] = 'http'
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_asup_mock_object('asup').apply()
+        modify_asup.assert_called_with({'transport': 'http', 'service_state': 'started'})
+
+    @patch('ansible.modules.storage.netapp.na_ontap_autosupport.NetAppONTAPasup.modify_autosupport_config')
+    @patch('ansible.modules.storage.netapp.na_ontap_autosupport.NetAppONTAPasup.get_autosupport_config')
+    def test_modify_not_called(self, get_asup, modify_asup):
+        data = self.mock_args()
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_asup_mock_object('asup').apply()
+        get_asup.assert_called_with()
+        modify_asup.assert_not_called()
+
+    def test_modify_packet(self):
+        '''check XML construction for nested attributes like mail-hosts, noteto, partner-address, and to'''
+        data = self.mock_args()
+        set_module_args(data)
+        obj = self.get_asup_mock_object(kind='asup', enabled='true')
+        modify_dict = {
+            'noteto': ['one@test.com'],
+            'partner_addresses': ['firstpartner@test.com'],
+            'mail_hosts': ['1.1.1.1'],
+            'to_addresses': ['first@test.com']
+        }
+        obj.modify_autosupport_config(modify_dict)
+        xml = obj.server.xml_in
+        for key in ['noteto', 'to', 'partner-address']:
+            assert xml[key] is not None
+            assert xml[key]['mail-address'] is not None
+        assert xml['noteto']['mail-address'] == modify_dict['noteto'][0]
+        assert xml['to']['mail-address'] == modify_dict['to_addresses'][0]
+        assert xml['partner-address']['mail-address'] == modify_dict['partner_addresses'][0]
+        assert xml['mail-hosts'] is not None
+        assert xml['mail-hosts']['string'] is not None
+        assert xml['mail-hosts']['string'] == modify_dict['mail_hosts'][0]