From f9139be5e6fde4792d15b7a59dbd77499ff48548 Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Tue, 19 Feb 2019 09:49:59 -0800 Subject: [PATCH] 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. --- .../storage/netapp/na_ontap_interface.py | 87 +++++++++++++++---- .../storage/netapp/test_na_ontap_interface.py | 42 ++++++++- 2 files changed, 109 insertions(+), 20 deletions(-) diff --git a/lib/ansible/modules/storage/netapp/na_ontap_interface.py b/lib/ansible/modules/storage/netapp/na_ontap_interface.py index 2ae6a01720..4df4f2f368 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_interface.py +++ b/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. - 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). + - 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'. + - 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): - ''' calling zapi to create interface ''' + def set_protocol_option(self, required_keys): + """ 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 required_keys.issubset(set(self.parameters.keys())): + if not 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: - data_protocols_obj = netapp_utils.zapi.NaElement('data-protocols') + if data_protocols_obj is not None: 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: diff --git a/test/units/modules/storage/netapp/test_na_ontap_interface.py b/test/units/modules/storage/netapp/test_na_ontap_interface.py index 5598700038..acb761c2d3 100644 --- a/test/units/modules/storage/netapp/test_na_ontap_interface.py +++ b/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())