Added support for nvme, and bug fixes for na_ontap_interface (#52283)

* Revert "changes to clusteR"

This reverts commit 33ee1b71e4bc8435fb315762a871f8c4cb6c5f80.

* add interface

* updates

* fix author

* Revert "add interface"

This reverts commit 26ef11602e549fee0a6070c1e983c325d08da8df.

* Revert "Revert "add interface""

This reverts commit 4518d537a2b876a8cb0eae4e4cc02f5232c813f3.

* Revert "updates"

This reverts commit 47b58dde021c1dd6aeca6080bffc26b78cf944ea.

* Revert "Revert "changes to clusteR""

This reverts commit 2c517792115179665d43d74780dc356ca41b9abe.

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

@@ -1,7 +1,7 @@
 #!/usr/bin/python
 """ this is interface module
 
- (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)
 """
 
@@ -43,7 +43,7 @@ options:
   home_node:
     description:
     - Specifies the LIF's home node.
-    - Required when C(state=present).
+    - By default, the first node from the cluster is considered as home node
 
   home_port:
     description:
@@ -100,9 +100,10 @@ options:
 
   protocols:
     description:
-       Specifies the list of data protocols configured on the LIF. By default, the values in this element are nfs, cifs and fcache.
+    - Specifies the list of data protocols configured on the LIF. By default, the values in this element are nfs, cifs and fcache.
-       Other supported protocols are iscsi and fcp. A LIF can be configured to not support any data protocols by specifying 'none'.
+    - Other supported protocols are iscsi and fcp. A LIF can be configured to not support any data protocols by specifying 'none'.
-       Protocol values of none, iscsi or fcp can't be combined with any other data protocol(s).
+    - Protocol values of none, iscsi, fc-nvme or fcp can't be combined with any other data protocol(s).
+    - address, netmask and firewall_policy parameters are not supported for 'fc-nvme' option.
 
 '''
 
@@ -204,7 +205,6 @@ class NetAppOntapInterface(object):
         interface_info.add_child_elem(query)
         result = self.server.invoke_successfully(interface_info, True)
         return_value = None
-
         if result.get_child_by_name('num-records') and \
                 int(result.get_child_content('num-records')) >= 1:
 
@@ -215,12 +215,15 @@ class NetAppOntapInterface(object):
                 '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,
             }
+            if interface_attributes.get_child_by_name('address'):
+                return_value['address'] = interface_attributes['address']
+            if interface_attributes.get_child_by_name('netmask'):
+                return_value['netmask'] = interface_attributes['netmask']
+            if interface_attributes.get_child_by_name('firewall-policy'):
+                return_value['firewall_policy'] = interface_attributes['firewall-policy']
         return return_value
 
     def set_options(self, options, parameters):
@@ -242,28 +245,76 @@ class NetAppOntapInterface(object):
         if parameters.get('admin_status') is not None:
             options['administrative-status'] = parameters['admin_status']
 
-    def create_interface(self):
+    def set_protocol_option(self, required_keys):
-        ''' calling zapi to create interface '''
+        """ set protocols for create """
+        if self.parameters.get('protocols') is not None:
+            data_protocols_obj = netapp_utils.zapi.NaElement('data-protocols')
+            for protocol in self.parameters.get('protocols'):
+                if protocol.lower() == 'fc-nvme':
+                    required_keys.remove('address')
+                    required_keys.remove('home_port')
+                    required_keys.remove('netmask')
+                    not_required_params = set(['address', 'netmask', 'firewall_policy'])
+                    if not not_required_params.isdisjoint(set(self.parameters.keys())):
+                        self.module.fail_json(msg='Error: Following parameters for creating interface are not supported'
+                                                  ' for data-protocol fc-nvme: %s' % ', '.join(not_required_params))
+                data_protocols_obj.add_new_child('data-protocol', protocol)
+            return data_protocols_obj
+        return None
+
+    def get_home_node_for_cluster(self):
+        ''' get the first node name from this cluster '''
+        get_node = netapp_utils.zapi.NaElement('cluster-node-get-iter')
+        attributes = {
+            'query': {
+                'cluster-node-info': {}
+            }
+        }
+        get_node.translate_struct(attributes)
+        try:
+            result = self.server.invoke_successfully(get_node, enable_tunneling=True)
+        except netapp_utils.zapi.NaApiError as exc:
+            self.module.fail_json(msg='Error fetching node for interface %s: %s' %
+                                  (self.parameters['interface_name'], to_native(exc)),
+                                  exception=traceback.format_exc())
+        if result.get_child_by_name('num-records') and int(result.get_child_content('num-records')) >= 1:
+            attributes = result.get_child_by_name('attributes-list')
+            return attributes.get_child_by_name('cluster-node-info').get_child_content('node-name')
+        return None
+
+    def validate_create_parameters(self, keys):
+        '''
+            Validate if required parameters for create are present.
+            Parameter requirement might vary based on given data-protocol.
+            :return: None
+        '''
+        if self.parameters.get('home_node') is None:
+            node = self.get_home_node_for_cluster()
+            if node is not None:
+                self.parameters['home_node'] = node
         # validate if mandatory parameters are present for create
-        required_keys = set(['role', 'address', 'home_node', 'home_port', 'netmask'])
+        if not keys.issubset(set(self.parameters.keys())):
-        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))
+                                      % ', '.join(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')
+
+    def create_interface(self):
+        ''' calling zapi to create interface '''
+        required_keys = set(['role', 'address', 'home_port', 'netmask'])
+        data_protocols_obj = self.set_protocol_option(required_keys)
+        self.validate_create_parameters(required_keys)
+
         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:
+        if data_protocols_obj is not None:
-            data_protocols_obj = netapp_utils.zapi.NaElement('data-protocols')
             interface_create.add_child_elem(data_protocols_obj)
-            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)
         except netapp_utils.zapi.NaApiError as exc:

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

@@ -17,7 +17,7 @@ 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')
+    pytestmark = pytest.mark.skip('skipping as missing required netapp_lib')
 
 
 def set_module_args(args):
@@ -84,7 +84,8 @@ class MockONTAPConnection(object):
                     'home-port': data['home_port'],
                     'address': data['address'],
                     'netmask': data['netmask'],
-                    'role': data['role']
+                    'role': data['role'],
+                    'protocols': data['protocols'] if data.get('protocols') else None
                 }
             }
         }
@@ -181,6 +182,43 @@ class TestMyModule(unittest.TestCase):
             self.get_interface_mock_object().apply()
         assert exc.value.args[0]['changed']
 
+    def test_successful_create_for_NVMe(self):
+        ''' Test successful create for NVMe protocol'''
+        data = self.mock_args()
+        data['protocols'] = 'fc-nvme'
+        del data['address']
+        del data['netmask']
+        del data['home_port']
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_interface_mock_object().apply()
+        assert exc.value.args[0]['changed']
+
+    def test_create_idempotency_for_NVMe(self):
+        ''' Test create idempotency for NVMe protocol '''
+        data = self.mock_args()
+        data['protocols'] = 'fc-nvme'
+        del data['address']
+        del data['netmask']
+        del data['home_port']
+        set_module_args(data)
+        with pytest.raises(AnsibleExitJson) as exc:
+            self.get_interface_mock_object('interface').apply()
+        assert not exc.value.args[0]['changed']
+
+    def test_create_error_for_NVMe(self):
+        ''' Test if create throws an error if required param 'protocols' uses NVMe'''
+        data = self.mock_args()
+        data['protocols'] = 'fc-nvme'
+        set_module_args(data)
+        with pytest.raises(AnsibleFailJson) as exc:
+            self.get_interface_mock_object('interface').create_interface()
+        msg = 'Error: Following parameters for creating interface are not supported for data-protocol fc-nvme: ' \
+              'netmask, firewall_policy, address'
+        expected = sorted(','.split(msg))
+        received = sorted(','.split(exc.value.args[0]['msg']))
+        assert expected == received
+
     def test_create_idempotency(self):
         ''' Test create idempotency '''
         set_module_args(self.mock_args())