Rewrite (#49570)

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

@@ -53,6 +53,7 @@ options:
   role:
     description:
     - Specifies the role of the LIF.
+    - When setting role as "intercluster", setting protocol is not supported.
     - Required when C(state=present).
 
   address:
@@ -77,6 +78,14 @@ options:
   failover_policy:
     description:
     - Specifies the failover policy for the LIF.
+    - Possible values are 'disabled', 'system-defined', 'local-only', 'sfo-partner-only', and 'broadcast-domain-wide'
+
+  subnet_name:
+    description:
+    - Subnet where the interface address is allocated from.
+      If the option is not used, the IP address will need to be provided by
+      the administrator during configuration.
+    version_added: '2.8'
 
   admin_status:
     choices: ['up', 'down']
@@ -87,6 +96,7 @@ options:
     description:
        If true, data LIF will revert to its home node under certain circumstances such as startup, and load balancing
        migration capability is disabled automatically
+    type: bool
 
   protocols:
     description:
@@ -133,6 +143,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
 
@@ -141,7 +152,6 @@ HAS_NETAPP_LIB = netapp_utils.has_netapp_lib()
 
 class NetAppOntapInterface(object):
     ''' object to describe  interface info '''
-
     def __init__(self):
 
         self.argument_spec = netapp_utils.na_ontap_host_argument_spec()
@@ -158,32 +168,17 @@ class NetAppOntapInterface(object):
             firewall_policy=dict(required=False, type='str', default=None),
             failover_policy=dict(required=False, type='str', default=None),
             admin_status=dict(required=False, choices=['up', 'down']),
-            is_auto_revert=dict(required=False, type='str', default=None),
+            subnet_name=dict(required=False, type='str'),
+            is_auto_revert=dict(required=False, type=bool, default=None),
             protocols=dict(required=False, type='list')
-
         ))
 
         self.module = AnsibleModule(
             argument_spec=self.argument_spec,
             supports_check_mode=True
         )
-
-        params = self.module.params
-
-        # set up state variables
-        self.state = params['state']
-        self.interface_name = params['interface_name']
-        self.home_node = params['home_node']
-        self.home_port = params['home_port']
-        self.role = params['role']
-        self.vserver = params['vserver']
-        self.address = params['address']
-        self.netmask = params['netmask']
-        self.admin_status = params['admin_status']
-        self.failover_policy = params['failover_policy']
-        self.firewall_policy = params['firewall_policy']
-        self.is_auto_revert = params['is_auto_revert']
-        self.protocols = params['protocols']
+        self.na_helper = NetAppModule()
+        self.parameters = self.na_helper.set_parameters(self.module.params)
 
         if HAS_NETAPP_LIB is False:
             self.module.fail_json(
@@ -191,7 +186,7 @@ class NetAppOntapInterface(object):
         else:
             self.server = netapp_utils.setup_na_ontap_zapi(module=self.module)
 
-    def get_interface(self):
+    def get_interface(self, interface_name=None):
         """
         Return details about the interface
         :param:
@@ -200,11 +195,11 @@ class NetAppOntapInterface(object):
         :return: Details about the interface. None if not found.
         :rtype: dict
         """
+        if interface_name is None:
+            interface_name = self.parameters['interface_name']
         interface_info = netapp_utils.zapi.NaElement('net-interface-get-iter')
-        interface_attributes = netapp_utils.zapi.NaElement(
-            'net-interface-info')
-        interface_attributes.add_new_child(
-            'interface-name', self.interface_name)
+        interface_attributes = netapp_utils.zapi.NaElement('net-interface-info')
+        interface_attributes.add_new_child('interface-name', interface_name)
         query = netapp_utils.zapi.NaElement('query')
         query.add_child_elem(interface_attributes)
         interface_info.add_child_elem(query)
@@ -217,142 +212,119 @@ class NetAppOntapInterface(object):
             interface_attributes = result.get_child_by_name('attributes-list').\
                 get_child_by_name('net-interface-info')
             return_value = {
-                'interface_name': self.interface_name,
-                'admin_status': interface_attributes.get_child_content('administrative-status'),
-                'home_port': interface_attributes.get_child_content('home-port'),
-                'home_node': interface_attributes.get_child_content('home-node'),
-                'address': interface_attributes.get_child_content('address'),
-                'netmask': interface_attributes.get_child_content('netmask'),
-                'failover_policy': interface_attributes.get_child_content('failover-policy'),
-                'firewall_policy': interface_attributes.get_child_content('firewall-policy'),
-                'is_auto_revert': interface_attributes.get_child_content('is-auto-revert'),
+                'interface_name': self.parameters['interface_name'],
+                'admin_status': interface_attributes['administrative-status'],
+                'home_port': interface_attributes['home-port'],
+                'home_node': interface_attributes['home-node'],
+                'address': interface_attributes['address'],
+                'netmask': interface_attributes['netmask'],
+                'failover_policy': interface_attributes['failover-policy'].replace('_', '-'),
+                'firewall_policy': interface_attributes['firewall-policy'],
+                'is_auto_revert': True if interface_attributes['is-auto-revert'] == 'true' else False,
             }
         return return_value
 
+    def set_options(self, options, parameters):
+        """ set attributes for create or modify """
+        if parameters.get('home_port') is not None:
+            options['home-port'] = parameters['home_port']
+        if parameters.get('subnet_name') is not None:
+            options['subnet-name'] = parameters['subnet_name']
+        if parameters.get('address') is not None:
+            options['address'] = parameters['address']
+        if parameters.get('netmask') is not None:
+            options['netmask'] = parameters['netmask']
+        if parameters.get('failover_policy') is not None:
+            options['failover-policy'] = parameters['failover_policy']
+        if parameters.get('firewall_policy') is not None:
+            options['firewall-policy'] = parameters['firewall_policy']
+        if parameters.get('is_auto_revert') is not None:
+            options['is-auto-revert'] = 'true' if parameters['is_auto_revert'] is True else 'false'
+        if parameters.get('admin_status') is not None:
+            options['administrative-status'] = parameters['admin_status']
+
     def create_interface(self):
         ''' calling zapi to create interface '''
-
-        options = {'interface-name': self.interface_name,
-                   'vserver': self.vserver}
-        if self.home_port is not None:
-            options['home-port'] = self.home_port
-        if self.home_node is not None:
-            options['home-node'] = self.home_node
-        if self.address is not None:
-            options['address'] = self.address
-        if self.netmask is not None:
-            options['netmask'] = self.netmask
-        if self.role is not None:
-            options['role'] = self.role
-        if self.failover_policy is not None:
-            options['failover-policy'] = self.failover_policy
-        if self.firewall_policy is not None:
-            options['firewall-policy'] = self.firewall_policy
-        if self.is_auto_revert is not None:
-            options['is-auto-revert'] = self.is_auto_revert
-        if self.admin_status is not None:
-            options['administrative-status'] = self.admin_status
-
-        interface_create = netapp_utils.zapi.NaElement.create_node_with_children(
-            'net-interface-create', **options)
-        if self.protocols is not None:
+        # validate if mandatory parameters are present for create
+        required_keys = set(['role', 'address', 'home_node', 'home_port', 'netmask'])
+        if not required_keys.issubset(set(self.parameters.keys())):
+            self.module.fail_json(msg='Error: Missing one or more required parameters for creating interface: %s'
+                                  % ', '.join(required_keys))
+        # if role is intercluster, protocol cannot be specified
+        if self.parameters['role'] == "intercluster" and self.parameters.get('protocols') is not None:
+            self.module.fail_json(msg='Error: Protocol cannot be specified for intercluster role,'
+                                      'failed to create interface')
+        options = {'interface-name': self.parameters['interface_name'],
+                   'role': self.parameters['role'],
+                   'home-node': self.parameters.get('home_node'),
+                   'vserver': self.parameters['vserver']}
+        self.set_options(options, self.parameters)
+        interface_create = netapp_utils.zapi.NaElement.create_node_with_children('net-interface-create', **options)
+        if self.parameters.get('protocols') is not None:
             data_protocols_obj = netapp_utils.zapi.NaElement('data-protocols')
             interface_create.add_child_elem(data_protocols_obj)
-            for protocol in self.protocols:
+            for protocol in self.parameters.get('protocols'):
                 data_protocols_obj.add_new_child('data-protocol', protocol)
-
         try:
-            self.server.invoke_successfully(interface_create,
-                                            enable_tunneling=True)
+            self.server.invoke_successfully(interface_create, enable_tunneling=True)
         except netapp_utils.zapi.NaApiError as exc:
             self.module.fail_json(msg='Error Creating interface %s: %s' %
-                                  (self.interface_name, to_native(exc)), exception=traceback.format_exc())
+                                  (self.parameters['interface_name'], to_native(exc)), exception=traceback.format_exc())
 
     def delete_interface(self, current_status):
         ''' calling zapi to delete interface '''
         if current_status == 'up':
-            self.admin_status = 'down'
-            self.modify_interface()
+            self.parameters['admin_status'] = 'down'
+            self.modify_interface({'admin_status': 'down'})
 
         interface_delete = netapp_utils.zapi.NaElement.create_node_with_children(
-            'net-interface-delete', **{'interface-name': self.interface_name,
-                                       'vserver': self.vserver})
+            'net-interface-delete', **{'interface-name': self.parameters['interface_name'],
+                                       'vserver': self.parameters['vserver']})
         try:
-            self.server.invoke_successfully(interface_delete,
-                                            enable_tunneling=True)
+            self.server.invoke_successfully(interface_delete, enable_tunneling=True)
         except netapp_utils.zapi.NaApiError as exc:
-            self.module.fail_json(msg='Error deleting interface %s: %s' % (self.interface_name, to_native(exc)),
+            self.module.fail_json(msg='Error deleting interface %s: %s' % (self.parameters['interface_name'], to_native(exc)),
                                   exception=traceback.format_exc())
 
-    def modify_interface(self):
+    def modify_interface(self, modify):
         """
         Modify the interface.
         """
-        options = {'interface-name': self.interface_name,
-                   'vserver': self.vserver
+        options = {'interface-name': self.parameters['interface_name'],
+                   'vserver': self.parameters['vserver']
                    }
-        if self.admin_status is not None:
-            options['administrative-status'] = self.admin_status
-        if self.failover_policy is not None:
-            options['failover-policy'] = self.failover_policy
-        if self.firewall_policy is not None:
-            options['firewall-policy'] = self.firewall_policy
-        if self.is_auto_revert is not None:
-            options['is-auto-revert'] = self.is_auto_revert
-        if self.netmask is not None:
-            options['netmask'] = self.netmask
-        if self.address is not None:
-            options['address'] = self.address
-
-        interface_modify = netapp_utils.zapi.NaElement.create_node_with_children(
-            'net-interface-modify', **options)
+        self.set_options(options, modify)
+        interface_modify = netapp_utils.zapi.NaElement.create_node_with_children('net-interface-modify', **options)
         try:
-            self.server.invoke_successfully(interface_modify,
-                                            enable_tunneling=True)
-        except netapp_utils.zapi.NaApiError as e:
-            self.module.fail_json(msg='Error modifying interface %s: %s' % (self.interface_name, to_native(e)),
-                                  exception=traceback.format_exc())
+            self.server.invoke_successfully(interface_modify, enable_tunneling=True)
+        except netapp_utils.zapi.NaApiError as err:
+            self.module.fail_json(msg='Error modifying interface %s: %s' % (self.parameters['interface_name'],
+                                  to_native(err)), exception=traceback.format_exc())
 
-    def apply(self):
-        ''' calling all interface features '''
-        changed = False
-        interface_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_interface", cserver)
-        interface_detail = self.get_interface()
-        if interface_detail:
-            interface_exists = True
-            if self.state == 'absent':
-                changed = True
 
-            elif self.state == 'present':
-                if (self.admin_status is not None and self.admin_status != interface_detail['admin_status']) or \
-                   (self.address is not None and self.address != interface_detail['address']) or \
-                   (self.netmask is not None and self.netmask != interface_detail['netmask']) or \
-                   (self.failover_policy is not None and self.failover_policy != interface_detail['failover_policy']) or \
-                   (self.firewall_policy is not None and self.firewall_policy != interface_detail['firewall_policy']) or \
-                   (self.is_auto_revert is not None and self.is_auto_revert != interface_detail['is_auto_revert']):
-                    changed = True
-        else:
-            if self.state == 'present':
-                changed = True
-
-        if changed:
+    def apply(self):
+        ''' calling all interface features '''
+        self.autosupport_log()
+        current = self.get_interface()
+        # rename and create are mutually exclusive
+        cd_action = self.na_helper.get_cd_action(current, self.parameters)
+        modify = self.na_helper.get_modified_attributes(current, self.parameters)
+        if self.na_helper.changed:
             if self.module.check_mode:
                 pass
             else:
-                if self.state == 'present':
-                    if interface_exists is False:
-                        self.create_interface()
-                    else:
-                        self.modify_interface()
-
-                elif self.state == 'absent':
-                    self.delete_interface(interface_detail['admin_status'])
-
-        self.module.exit_json(changed=changed)
+                if cd_action == 'create':
+                    self.create_interface()
+                elif cd_action == 'delete':
+                    self.delete_interface(current['admin_status'])
+                elif modify:
+                    self.modify_interface(modify)
+        self.module.exit_json(changed=self.na_helper.changed)
 
 
 def main():

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

@@ -0,0 +1,225 @@
+# (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_interface \
+    import NetAppOntapInterface as interface_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.type = 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.type == 'interface':
+            xml = self.build_interface_info(self.params)
+        self.xml_out = xml
+        return xml
+
+    @staticmethod
+    def build_interface_info(data):
+        ''' build xml data for vserser-info '''
+        xml = netapp_utils.zapi.NaElement('xml')
+        attributes = {
+            'num-records': 1,
+            'attributes-list': {
+                'net-interface-info': {
+                    'interface-name': data['name'],
+                    'administrative-status': data['administrative-status'],
+                    'failover-policy': data['failover-policy'],
+                    'firewall-policy': data['firewall-policy'],
+                    'is-auto-revert': data['is-auto-revert'],
+                    'home-node': data['home_node'],
+                    'home-port': data['home_port'],
+                    'address': data['address'],
+                    'netmask': data['netmask'],
+                    'role': data['role']
+                }
+            }
+        }
+        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_interface = {
+            'name': 'test_lif',
+            'administrative-status': 'up',
+            'failover-policy': 'up',
+            'firewall-policy': 'up',
+            'is-auto-revert': 'true',
+            'home_node': 'node',
+            'role': 'test',
+            'home_port': 'e0c',
+            'address': '2.2.2.2',
+            'netmask': '1.1.1.1',
+        }
+
+    def mock_args(self):
+        return {
+            'vserver': 'vserver',
+            'interface_name': self.mock_interface['name'],
+            'home_node': self.mock_interface['home_node'],
+            'role': self.mock_interface['role'],
+            'home_port': self.mock_interface['home_port'],
+            'address': self.mock_interface['address'],
+            'netmask': self.mock_interface['netmask'],
+            'hostname': 'hostname',
+            'username': 'username',
+            'password': 'password',
+        }
+
+    def get_interface_mock_object(self, kind=None):
+        """
+        Helper method to return an na_ontap_interface object
+        :param kind: passes this param to MockONTAPConnection()
+        :return: na_ontap_interface object
+        """
+        interface_obj = interface_module()
+        interface_obj.autosupport_log = Mock(return_value=None)
+        if kind is None:
+            interface_obj.server = MockONTAPConnection()
+        else:
+            interface_obj.server = MockONTAPConnection(kind=kind, data=self.mock_interface)
+        return interface_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({})
+            interface_module()
+        print('Info: %s' % exc.value.args[0]['msg'])
+
+    def test_create_error_missing_param(self):
+        ''' Test if create throws an error if required param 'role' is not specified'''
+        data = self.mock_args()
+        del data['role']
+        set_module_args(data)
+        with pytest.raises(AnsibleFailJson) as exc:
+            self.get_interface_mock_object('interface').create_interface()
+        msg = 'Error: Missing one or more required parameters for creating interface: ' \
+              'home_port, netmask, role, home_node, address'
+        expected = sorted(','.split(msg))
+        received = sorted(','.split(exc.value.args[0]['msg']))
+        assert expected == received
+
+    def test_get_nonexistent_interface(self):
+        ''' Test if get_interface returns None for non-existent interface '''
+        set_module_args(self.mock_args())
+        result = self.get_interface_mock_object().get_interface()
+        assert result is None
+
+    def test_get_existing_interface(self):
+        ''' Test if get_interface returns None for existing interface '''
+        set_module_args(self.mock_args())
+        result = self.get_interface_mock_object(kind='interface').get_interface()
+        assert result['interface_name'] == self.mock_interface['name']
+
+    def test_successful_create(self):
+        ''' Test successful create '''
+        set_module_args(self.mock_args())
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_interface_mock_object().apply()
+        assert exc.value.args[0]['changed']
+
+    def test_create_idempotency(self):
+        ''' Test create idempotency '''
+        set_module_args(self.mock_args())
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_interface_mock_object('interface').apply()
+        assert not exc.value.args[0]['changed']
+
+    def test_successful_delete(self):
+        ''' Test delete existing interface '''
+        data = self.mock_args()
+        data['state'] = 'absent'
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_interface_mock_object('interface').apply()
+        assert exc.value.args[0]['changed']
+
+    def test_delete_idempotency(self):
+        ''' Test delete idempotency '''
+        data = self.mock_args()
+        data['state'] = 'absent'
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_interface_mock_object().apply()
+        assert not exc.value.args[0]['changed']
+
+    def test_successful_modify(self):
+        ''' Test successful modify interface_minutes '''
+        data = self.mock_args()
+        data['home_port'] = 'new_port'
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            interface_obj = self.get_interface_mock_object('interface')
+            interface_obj.apply()
+        assert exc.value.args[0]['changed']
+
+    def test_modify_idempotency(self):
+        ''' Test modify idempotency '''
+        data = self.mock_args()
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_interface_mock_object('interface').apply()
+        assert not exc.value.args[0]['changed']