From 38680f38f1b6a06265c92dfe8149bf7acefce0c0 Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Thu, 16 Aug 2018 05:36:00 -0700 Subject: [PATCH] Bug Fixes for ontap_volume.py (#44224) * Bug Fixes for ontap_volume.py * Fix issues --- .../modules/storage/netapp/na_ontap_volume.py | 415 +++++++----------- 1 file changed, 169 insertions(+), 246 deletions(-) diff --git a/lib/ansible/modules/storage/netapp/na_ontap_volume.py b/lib/ansible/modules/storage/netapp/na_ontap_volume.py index 32c217fb49d..e651d5683cf 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_volume.py +++ b/lib/ansible/modules/storage/netapp/na_ontap_volume.py @@ -20,8 +20,7 @@ short_description: Manage NetApp ONTAP volumes. extends_documentation_fragment: - netapp.na_ontap version_added: '2.6' -author: -- Sumit Kumar (sumit4@netapp.com), Suhas Bangalore Shekar (bsuhas@netapp.com) +author: NetApp Ansible Team (ng-ansibleteam@netapp.com) description: - Create or destroy or modify volumes on NetApp ONTAP. @@ -44,9 +43,10 @@ options: - Name of the vserver to use. required: true - new_name: + from_name: description: - - New name of the volume to be renamed. + - Name of the existing volume to be renamed to name. + version_added: '2.7' is_infinite: type: bool @@ -102,6 +102,18 @@ options: choices: ['mixed', 'ntfs', 'unified', 'unix'] default: 'mixed' + encrypt: + type: bool + description: + - Whether or not to enable Volume Encryption. + default: False + version_added: '2.7' + + efficiency_policy: + description: + - Allows a storage efficiency policy to be set on volume creation. + version_added: '2.7' + ''' EXAMPLES = """ @@ -139,6 +151,7 @@ RETURN = """ import traceback import ansible.module_utils.netapp as netapp_utils +from ansible.module_utils.netapp_module import NetAppModule from ansible.module_utils.basic import AnsibleModule from ansible.module_utils._text import to_native @@ -169,7 +182,7 @@ class NetAppOntapVolume(object): 'present', 'absent'], default='present'), name=dict(required=True, type='str'), vserver=dict(required=True, type='str'), - new_name=dict(required=False, type='str'), + from_name=dict(required=False, type='str'), is_infinite=dict(required=False, type='bool', default=False), is_online=dict(required=False, type='bool', @@ -186,81 +199,74 @@ class NetAppOntapVolume(object): percent_snapshot_space=dict(type='str', default=None), volume_security_style=dict(choices=['mixed', 'ntfs', 'unified', 'unix'], - default='mixed') + default='mixed'), + encrypt=dict(required=False, type='bool', default=False), + efficiency_policy=dict(required=False, type='str'), )) - self.module = AnsibleModule( argument_spec=self.argument_spec, supports_check_mode=True ) + self.na_helper = NetAppModule() + self.parameters = self.na_helper.set_parameters(self.module.params) - parameters = self.module.params - - # set up state variables - self.state = parameters['state'] - self.name = parameters['name'] - self.new_name = parameters['new_name'] - self.is_infinite = parameters['is_infinite'] - self.is_online = parameters['is_online'] - self.size_unit = parameters['size_unit'] - self.vserver = parameters['vserver'] - self.type = parameters['type'] - self.policy = parameters['policy'] - self.junction_path = parameters['junction_path'] - self.space_guarantee = parameters['space_guarantee'] - self.percent_snapshot_space = parameters['percent_snapshot_space'] - self.aggregate_name = parameters['aggregate_name'] - self.volume_security_style = parameters['volume_security_style'] - - if parameters['size'] is not None: - self.size = parameters['size'] * \ - self._size_unit_map[self.size_unit] - else: - self.size = None - + if self.parameters.get('size'): + self.parameters['size'] = self.parameters['size'] * \ + self._size_unit_map[self.parameters['size_unit']] if HAS_NETAPP_LIB is False: self.module.fail_json( msg="the python NetApp-Lib module is required") else: self.server = netapp_utils.setup_na_ontap_zapi( - module=self.module, vserver=self.vserver) + module=self.module, vserver=self.parameters['vserver']) self.cluster = netapp_utils.setup_na_ontap_zapi(module=self.module) - def get_volume(self): + def volume_get_iter(self, vol_name=None): """ - Return details about the volume - :param: - name : Name of the volume - - :return: Details about the volume. None if not found. - :rtype: dict + Return volume-get-iter query results + :param vol_name: name of the volume + :return: NaElement """ volume_info = netapp_utils.zapi.NaElement('volume-get-iter') volume_attributes = netapp_utils.zapi.NaElement('volume-attributes') - volume_id_attributes = netapp_utils.zapi.NaElement( - 'volume-id-attributes') - volume_id_attributes.add_new_child('name', self.name) + volume_id_attributes = netapp_utils.zapi.NaElement('volume-id-attributes') + volume_id_attributes.add_new_child('name', vol_name) volume_attributes.add_child_elem(volume_id_attributes) - query = netapp_utils.zapi.NaElement('query') query.add_child_elem(volume_attributes) - volume_info.add_child_elem(query) - result = self.server.invoke_successfully(volume_info, True) + try: + result = self.server.invoke_successfully(volume_info, True) + except netapp_utils.zapi.NaApiError as error: + self.module.fail_json(msg='Error fetching volume %s : %s' + % (self.parameters['name'], to_native(error)), + exception=traceback.format_exc()) + return result - return_value = None + def get_volume(self, vol_name=None): + """ + Return details about the volume + :param: + name : Name of the volume - if result.get_child_by_name('num-records') and \ - int(result.get_child_content('num-records')) >= 1: + :return: Details about the volume. None if not found. + :rtype: dict + """ + if vol_name is None: + vol_name = self.parameters['name'] + volume_get_iter = self.volume_get_iter(vol_name) + return_value = None + if volume_get_iter.get_child_by_name('num-records') and \ + int(volume_get_iter.get_child_content('num-records')) > 0: - volume_attributes = result.get_child_by_name( + volume_attributes = volume_get_iter.get_child_by_name( 'attributes-list').get_child_by_name( 'volume-attributes') # Get volume's current size volume_space_attributes = volume_attributes.get_child_by_name( 'volume-space-attributes') - current_size = volume_space_attributes.get_child_content('size') + current_size = int(volume_space_attributes.get_child_content('size')) # Get volume's state (online/offline) volume_state_attributes = volume_attributes.get_child_by_name( @@ -270,106 +276,88 @@ class NetAppOntapVolume(object): 'volume-id-attributes') aggregate_name = volume_id_attributes.get_child_content( 'containing-aggregate-name') - junction_path = volume_id_attributes.get_child_content( - 'junction-path') - volume_type = volume_id_attributes.get_child_content('type') volume_export_attributes = volume_attributes.get_child_by_name( 'volume-export-attributes') policy = volume_export_attributes.get_child_content('policy') space_guarantee = volume_space_attributes.get_child_content( 'space-guarantee') - percent_snapshot_space = volume_space_attributes.get_child_by_name( - 'percentage-snapshot-reserve') - - is_online = None - if current_state == "online": - is_online = True - elif current_state == "offline": - is_online = False + + is_online = (current_state == "online") return_value = { - 'name': self.name, + 'name': vol_name, 'size': current_size, 'is_online': is_online, 'aggregate_name': aggregate_name, 'policy': policy, 'space_guarantee': space_guarantee, - 'percent_snapshot_space': percent_snapshot_space, - 'type': volume_type, - 'junction_path': junction_path - } return return_value def create_volume(self): '''Create ONTAP volume''' - if self.aggregate_name is None: + if self.parameters.get('aggregate_name') is None: self.module.fail_json(msg='Error provisioning volume %s: \ aggregate_name is required' - % self.name) - options = {'volume': self.name, - 'containing-aggr-name': self.aggregate_name, - 'size': str(self.size)} - if self.percent_snapshot_space is not None: - options['percentage-snapshot-reserve'] = self.percent_snapshot_space - if self.type is not None: - options['volume-type'] = self.type - if self.policy is not None: - options['export-policy'] = self.policy - if self.junction_path is not None: - options['junction-path'] = self.junction_path - if self.space_guarantee is not None: - options['space-reserve'] = self.space_guarantee - if self.volume_security_style is not None: - options['volume-security-style'] = self.volume_security_style - - volume_create = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-create', **options) + % self.parameters['name']) + options = {'volume': self.parameters['name'], + 'containing-aggr-name': self.parameters['aggregate_name'], + 'size': str(self.parameters['size'])} + if self.parameters.get('percent_snapshot_space'): + options['percentage-snapshot-reserve'] = self.parameters['percent_snapshot_space'] + if self.parameters.get('type'): + options['volume-type'] = self.parameters['type'] + if self.parameters.get('policy'): + options['export-policy'] = self.parameters['policy'] + if self.parameters.get('junction_path'): + options['junction-path'] = self.parameters['junction_path'] + if self.parameters.get('space_guarantee'): + options['space-reserve'] = self.parameters['space_guarantee'] + if self.parameters.get('volume_security_style'): + options['volume-security-style'] = self.parameters['volume_security_style'] + volume_create = netapp_utils.zapi.NaElement.create_node_with_children('volume-create', **options) try: self.server.invoke_successfully(volume_create, enable_tunneling=True) - self.ems_log_event("create") + self.ems_log_event("volume-create") except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error provisioning volume %s \ of size %s: %s' - % (self.name, self.size, to_native(error)), + % (self.parameters['name'], self.parameters['size'], to_native(error)), exception=traceback.format_exc()) def delete_volume(self): '''Delete ONTAP volume''' - if self.is_infinite: + if self.parameters.get('is_infinite'): volume_delete = netapp_utils.zapi\ .NaElement.create_node_with_children( - 'volume-destroy-async', **{'volume-name': self.name}) + 'volume-destroy-async', **{'volume-name': self.parameters['name']}) else: volume_delete = netapp_utils.zapi\ .NaElement.create_node_with_children( - 'volume-destroy', **{'name': self.name, + 'volume-destroy', **{'name': self.parameters['name'], 'unmount-and-offline': 'true'}) try: - self.server.invoke_successfully(volume_delete, - enable_tunneling=True) + self.server.invoke_successfully(volume_delete, enable_tunneling=True) self.ems_log_event("delete") except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error deleting volume %s: %s' - % (self.name, to_native(error)), + % (self.parameters['name'], to_native(error)), exception=traceback.format_exc()) def move_volume(self): '''Move volume from source aggregate to destination aggregate''' - volume_move = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-move-start', **{'source-volume': self.name, - 'vserver': self.vserver, - 'dest-aggr': self.aggregate_name}) + volume_move = netapp_utils.zapi.NaElement.create_node_with_children( + 'volume-move-start', **{'source-volume': self.parameters['name'], + 'vserver': self.parameters['vserver'], + 'dest-aggr': self.parameters['aggregate_name']}) try: self.cluster.invoke_successfully(volume_move, enable_tunneling=True) - self.ems_log_event("move") + self.ems_log_event("volume-move") except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error moving volume %s: %s' - % (self.name, to_native(error)), + % (self.parameters['name'], to_native(error)), exception=traceback.format_exc()) def rename_volume(self): @@ -379,24 +367,18 @@ class NetAppOntapVolume(object): Note: 'is_infinite' needs to be set to True in order to rename an Infinite Volume. """ - if self.is_infinite: - volume_rename = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-rename-async', - **{'volume-name': self.name, 'new-volume-name': str( - self.new_name)}) - else: - volume_rename = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-rename', **{'volume': self.name, - 'new-volume-name': str(self.new_name)}) + vol_rename_zapi, vol_name_zapi = ['volume-rename-async', 'volume-name'] if self.parameters['is_infinite']\ + else ['volume-rename', 'volume'] + volume_rename = netapp_utils.zapi.NaElement.create_node_with_children( + vol_rename_zapi, **{vol_name_zapi: self.parameters['from_name'], + 'new-volume-name': str(self.parameters['name'])}) try: self.server.invoke_successfully(volume_rename, enable_tunneling=True) - self.ems_log_event("rename") + self.ems_log_event("volume-rename") except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error renaming volume %s: %s' - % (self.name, to_native(error)), + % (self.parameters['name'], to_native(error)), exception=traceback.format_exc()) def resize_volume(self): @@ -406,186 +388,127 @@ class NetAppOntapVolume(object): Note: 'is_infinite' needs to be set to True in order to rename an Infinite Volume. """ - if self.is_infinite: - volume_resize = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-size-async', - **{'volume-name': self.name, 'new-size': str( - self.size)}) - else: - volume_resize = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-size', **{'volume': self.name, 'new-size': str( - self.size)}) + vol_size_zapi, vol_name_zapi = ['volume-size-async', 'volume-name'] if self.parameters['is_infinite']\ + else ['volume-size', 'volume'] + volume_resize = netapp_utils.zapi.NaElement.create_node_with_children( + vol_size_zapi, **{vol_name_zapi: self.parameters['name'], + 'new-size': str(self.parameters['size'])}) try: - self.server.invoke_successfully(volume_resize, - enable_tunneling=True) - self.ems_log_event("resize") + self.server.invoke_successfully(volume_resize, enable_tunneling=True) + self.ems_log_event("volume-resize") except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error re-sizing volume %s: %s' - % (self.name, to_native(error)), + % (self.parameters['name'], to_native(error)), exception=traceback.format_exc()) def change_volume_state(self): """ Change volume's state (offline/online). - """ - state_requested = None - if self.is_online: - # Requested state is 'online'. - state_requested = "online" - if self.is_infinite: - volume_change_state = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-online-async', - **{'volume-name': self.name}) - else: - volume_change_state = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-online', - **{'name': self.name}) - else: - # Requested state is 'offline'. - state_requested = "offline" - volume_unmount = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-unmount', **{'volume-name': self.name}) - if self.is_infinite: - volume_change_state = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-offline-async', - **{'volume-name': self.name}) - else: - volume_change_state = netapp_utils.zapi\ - .NaElement.create_node_with_children( - 'volume-offline', - **{'name': self.name}) + if self.parameters['is_online']: # Desired state is online, setup zapi APIs respectively + vol_state_zapi, vol_name_zapi = ['volume-online-async', 'volume-name'] if self.parameters['is_infinite']\ + else ['volume-online', 'name'] + else: # Desired state is offline, setup zapi APIs respectively + vol_state_zapi, vol_name_zapi = ['volume-offline-async', 'volume-name'] if self.parameters['is_infinite']\ + else ['volume-offline', 'name'] + volume_unmount = netapp_utils.zapi.NaElement.create_node_with_children( + 'volume-unmount', **{'volume-name': self.parameters['name']}) + volume_change_state = netapp_utils.zapi.NaElement.create_node_with_children( + vol_state_zapi, **{vol_name_zapi: self.parameters['name']}) try: - if state_requested == "offline": - self.server.invoke_successfully(volume_unmount, - enable_tunneling=True) - self.server.invoke_successfully(volume_change_state, - enable_tunneling=True) - self.ems_log_event("change") + if not self.parameters['is_online']: # Unmount before offline + self.server.invoke_successfully(volume_unmount, enable_tunneling=True) + self.server.invoke_successfully(volume_change_state, enable_tunneling=True) + self.ems_log_event("change-state") except netapp_utils.zapi.NaApiError as error: - self.module.fail_json(msg='Error changing the state of \ - volume %s to %s: %s' - % (self.name, state_requested, - to_native(error)), + state = "online" if self.parameters['is_online'] else "offline" + self.module.fail_json(msg='Error changing the state of volume %s to %s: %s' + % (self.parameters['name'], state, to_native(error)), exception=traceback.format_exc()) - def volume_modify(self): + def volume_modify_policy_space(self): """ - modify volume parameter 'policy' + modify volume parameter 'policy' or 'space_guarantee' """ + # TODO: refactor this method vol_mod_iter = netapp_utils.zapi.NaElement('volume-modify-iter') attributes = netapp_utils.zapi.NaElement('attributes') vol_mod_attributes = netapp_utils.zapi.NaElement('volume-attributes') - if self.policy is not None: + if self.parameters.get('policy'): vol_export_attributes = netapp_utils.zapi.NaElement( 'volume-export-attributes') - vol_export_attributes.add_new_child('policy', self.policy) + vol_export_attributes.add_new_child('policy', self.parameters['policy']) vol_mod_attributes.add_child_elem(vol_export_attributes) - if self.space_guarantee is not None: + if self.parameters.get('space_guarantee'): vol_space_attributes = netapp_utils.zapi.NaElement( 'volume-space-attributes') vol_space_attributes.add_new_child( - 'space-guarantee', self.space_guarantee) + 'space-guarantee', self.parameters['space_guarantee']) vol_mod_attributes.add_child_elem(vol_space_attributes) attributes.add_child_elem(vol_mod_attributes) query = netapp_utils.zapi.NaElement('query') vol_query_attributes = netapp_utils.zapi.NaElement('volume-attributes') vol_id_attributes = netapp_utils.zapi.NaElement('volume-id-attributes') - vol_id_attributes.add_new_child('name', self.name) + vol_id_attributes.add_new_child('name', self.parameters['name']) vol_query_attributes.add_child_elem(vol_id_attributes) query.add_child_elem(vol_query_attributes) vol_mod_iter.add_child_elem(attributes) vol_mod_iter.add_child_elem(query) - try: - self.server.invoke_successfully(vol_mod_iter, - enable_tunneling=True) - self.ems_log_event("modify") + result = self.server.invoke_successfully(vol_mod_iter, enable_tunneling=True) + failures = result.get_child_by_name('failure-list') + # handle error if modify space or policy parameter fails + if failures is not None and failures.get_child_by_name('volume-modify-iter-info') is not None: + error_msg = failures.get_child_by_name('volume-modify-iter-info').get_child_content('error-message') + self.module.fail_json(msg="Error modifying volume %s: %s" + % (self.parameters['name'], error_msg), + exception=traceback.format_exc()) + self.ems_log_event("volume-modify") except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error modifying volume %s: %s' - % (self.name, to_native(error)), + % (self.parameters['name'], to_native(error)), exception=traceback.format_exc()) + def modify_volume(self, modify): + for attribute in modify.keys(): + if attribute == 'size': + self.resize_volume() + elif attribute == 'is_online': + self.change_volume_state() + elif attribute == 'aggregate_name': + self.move_volume() + else: + self.volume_modify_policy_space() + def apply(self): '''Call create/modify/delete operations''' - changed = False - volume_exists = False - rename_volume = False - resize_volume = False - move_volume = False - modify_volume = False - state_change = False - volume_detail = self.get_volume() - if volume_detail: - volume_exists = True - - if self.state == 'absent': - changed = True - elif self.state == 'present': - if self.aggregate_name is not None and \ - volume_detail['aggregate_name'] != self.aggregate_name: - move_volume = True - changed = True - if self.size is not None and \ - str(volume_detail['size']) != str(self.size): - resize_volume = True - changed = True - if (volume_detail['is_online'] is not None) and \ - (volume_detail['is_online'] != self.is_online): - state_change = True - changed = True - if self.new_name is not None and \ - self.name != self.new_name: - rename_volume = True - changed = True - if self.policy is not None and \ - self.policy != volume_detail['policy']: - modify_volume = True - changed = True - if self.space_guarantee is not None and \ - self.space_guarantee != volume_detail['space_guarantee']: - modify_volume = True - changed = True + current = self.get_volume() + # rename and create are mutually exclusive + rename, cd_action = None, None + if self.parameters.get('from_name'): + rename = self.na_helper.is_rename_action(self.get_volume(self.parameters['from_name']), current) else: - if self.state == 'present': - changed = True - if changed: + 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 not volume_exists: - self.create_volume() - else: - if resize_volume: - self.resize_volume() - if state_change: - self.change_volume_state() - if modify_volume: - self.volume_modify() - # Ensure re-naming is the last change made. - if rename_volume: - self.rename_volume() - # Ensure volume move is the very last change made. - if move_volume: - self.move_volume() - - elif self.state == 'absent': + if rename: + self.rename_volume() + if cd_action == 'create': + self.create_volume() + elif cd_action == 'delete': self.delete_volume() - - self.module.exit_json(changed=changed) + elif modify: + self.modify_volume(modify) + self.module.exit_json(changed=self.na_helper.changed) def ems_log_event(self, state): '''Autosupport log event''' if state == 'create': message = "A Volume has been created, size: " + \ - str(self.size) + str(self.size_unit) + str(self.parameters['size']) + str(self.parameters['size_unit']) elif state == 'delete': message = "A Volume has been deleted" elif state == 'move': @@ -594,7 +517,7 @@ class NetAppOntapVolume(object): message = "A Volume has been renamed" elif state == 'resize': message = "A Volume has been resized to: " + \ - str(self.size) + str(self.size_unit) + str(self.parameters['size']) + str(self.parameters['size_unit']) elif state == 'change': message = "A Volume state has been changed" else: