diff --git a/ansible_collections/netapp/ontap/README.md b/ansible_collections/netapp/ontap/README.md index b7bccb21..e15c764f 100644 --- a/ansible_collections/netapp/ontap/README.md +++ b/ansible_collections/netapp/ontap/README.md @@ -35,13 +35,15 @@ Join our Slack Channel at [Netapp.io](http://netapp.io/slack) ### New Options - na_ontap_cluster - `time_out` to wait for cluster creation, adding and removing a node. - na_ontap_debug - connection diagnostics added for invalid ipaddress and DNS hostname errors. - - na_ontap_snapmirror - new option `create_destination` to automatically create destination endpoint (ONTAP 9.7). - - na_ontap_snapmirror - new option `destination_cluster` to automatically create destination SVM for SVM DR (ONTAP 9.7). - - na_ontap_snapmirror - new option `source_cluster` to automatically set SVM peering (ONTAP 9.7). + - na_ontap_lun - `total_size` and `total_size_unit` when using SAN application template. + - na_ontap_snapmirror - `create_destination` to automatically create destination endpoint (ONTAP 9.7). + - na_ontap_snapmirror - `destination_cluster` to automatically create destination SVM for SVM DR (ONTAP 9.7). + - na_ontap_snapmirror - `source_cluster` to automatically set SVM peering (ONTAP 9.7). ### Minor changes - na_ontap_firmware_upgrade - Added a new 'storage' type as default firmware_type. - na_ontap_info - deprecate ``state`` option. + - na_ontap_lun - support increasing lun_count and total_size when using SAN application template. - na_ontap_quota - allow to turn quota on/off without providing quota_target or type. - na_ontap_rest_info - deprecate ``state`` option. - na_ontap_snapmirror - use REST API for create action if target supports it. (ZAPIs are still used for all other actions). @@ -57,6 +59,8 @@ Join our Slack Channel at [Netapp.io](http://netapp.io/slack) - na_ontap_snapmirror - wait up to 5 minutes for abort to complete before issuing a delete. - na_ontap_snmp - SNMP module wrong access_control issue and error handling fix. - na_ontap_volume - REST expects 'all' for tiering policy and not 'backup'. + - na_ontap_volume - detect and report error when attempting to change FlexVol into FlexGroup. + - na_ontap_volume - report error if ``aggregate_name`` option is used with a FlexGroup. ## 20.12.0 diff --git a/ansible_collections/netapp/ontap/changelogs/fragments/DEVOPS-3497.yaml b/ansible_collections/netapp/ontap/changelogs/fragments/DEVOPS-3497.yaml new file mode 100644 index 00000000..2fa289d1 --- /dev/null +++ b/ansible_collections/netapp/ontap/changelogs/fragments/DEVOPS-3497.yaml @@ -0,0 +1,3 @@ +minor_changes: + - na_ontap_lun - support increasing lun_count and total_size when using SAN application template. + - na_ontap_lun - new options ``total_size`` and ``total_size_unit`` when using SAN application template. diff --git a/ansible_collections/netapp/ontap/changelogs/fragments/DEVOPS-3501.yaml b/ansible_collections/netapp/ontap/changelogs/fragments/DEVOPS-3501.yaml new file mode 100644 index 00000000..d8ea25d5 --- /dev/null +++ b/ansible_collections/netapp/ontap/changelogs/fragments/DEVOPS-3501.yaml @@ -0,0 +1,3 @@ +bugfixes: + - na_ontap_volume - detect and report error when attempting to change FlexVol into FlexGroup. + - na_ontap_volume - report error if ``aggregate_name`` option is used with a FlexGroup. diff --git a/ansible_collections/netapp/ontap/plugins/module_utils/netapp_module.py b/ansible_collections/netapp/ontap/plugins/module_utils/netapp_module.py index 056e85e9..273f8275 100644 --- a/ansible_collections/netapp/ontap/plugins/module_utils/netapp_module.py +++ b/ansible_collections/netapp/ontap/plugins/module_utils/netapp_module.py @@ -281,6 +281,10 @@ def get_modified_attributes(self, current, desired, get_list_diff=False): modified_list = self.compare_lists(value, desired[key], get_list_diff) # get modified list from current and desired if modified_list is not None: modified[key] = modified_list + elif isinstance(value, dict): + modified_dict = self.get_modified_attributes(value, desired[key]) + if modified_dict: + modified[key] = modified_dict else: try: result = cmp(value, desired[key]) diff --git a/ansible_collections/netapp/ontap/plugins/module_utils/rest_application.py b/ansible_collections/netapp/ontap/plugins/module_utils/rest_application.py index b0b3f774..9b759ee7 100644 --- a/ansible_collections/netapp/ontap/plugins/module_utils/rest_application.py +++ b/ansible_collections/netapp/ontap/plugins/module_utils/rest_application.py @@ -62,25 +62,44 @@ def get_application_uuid(self): dummy, error = self._set_application_uuid() return self.app_uuid, error - def get_application_details(self): + def get_application_details(self, template=None): """Use REST application/applications to get application details""" uuid, error = self.get_application_uuid() if error: return uuid, error if uuid is None: # not found return None, None + if template: + query = dict(fields='name,%s,statistics' % template) + else: + query = None api = '/application/applications/%s' % uuid - response, error = self.rest_api.get(api) + response, error = self.rest_api.get(api, query) return response, rrh.api_error(api, error) def create_application(self, body): """Use REST application/applications san template to create one or more LUNs""" - self.fail_if_uuid() + dummy, error = self.fail_if_uuid() + if error is not None: + return dummy, error api = '/application/applications' query = {'return_timeout': 30, 'return_records': 'true'} response, error = self.rest_api.post(api, body, params=query) return rrh.check_for_error_and_job_results(api, response, error, self.rest_api) + def patch_application(self, body): + """Use REST application/applications san template to add one or more LUNs""" + dummy, error = self.fail_if_no_uuid() + if error is not None: + return dummy, error + uuid, error = self.get_application_uuid() + if error is not None: + return dummy, error + api = '/application/applications/%s' % uuid + query = {'return_timeout': 30, 'return_records': 'true'} + response, error = self.rest_api.patch(api, body, params=query) + return rrh.check_for_error_and_job_results(api, response, error, self.rest_api) + def create_application_body(self, template_name, template_body, smart_container=True): if not isinstance(smart_container, bool): error = "expecting bool value for smart_container, got: %s" % smart_container @@ -95,7 +114,9 @@ def create_application_body(self, template_name, template_body, smart_container= def delete_application(self): """Use REST application/applications to delete app""" - self.fail_if_no_uuid() + dummy, error = self.fail_if_no_uuid() + if error is not None: + return dummy, error api = '/application/applications/%s' % self.app_uuid query = {'return_timeout': 30} response, error = self.rest_api.delete(api, params=query) @@ -105,7 +126,9 @@ def delete_application(self): def get_application_components(self): """Use REST application/applications to get application components""" - self.fail_if_no_uuid() + dummy, error = self.fail_if_no_uuid() + if error is not None: + return dummy, error api = '/application/applications/%s/components' % self.app_uuid response, error = self.rest_api.get(api) return response, rrh.api_error(api, error) @@ -114,7 +137,9 @@ def get_application_component_uuid(self): """Use REST application/applications to get component uuid Assume a single component per application """ - self.fail_if_no_uuid() + dummy, error = self.fail_if_no_uuid() + if error is not None: + return dummy, error response, error = self.get_application_components() record, error = rrh.check_for_0_or_1_records(None, response, error, None) if error is None and record is not None: @@ -123,7 +148,9 @@ def get_application_component_uuid(self): def get_application_component_details(self, comp_uuid=None): """Use REST application/applications to get application components""" - self.fail_if_no_uuid() + dummy, error = self.fail_if_no_uuid() + if error is not None: + return dummy, error if comp_uuid is None: # assume a single component comp_uuid, error = self.get_application_component_uuid() @@ -141,7 +168,9 @@ def get_application_component_backing_storage(self): Assume a single component per application """ - self.fail_if_no_uuid() + dummy, error = self.fail_if_no_uuid() + if error is not None: + return dummy, error response, error = self.get_application_component_details() if error or response is None: return response, error @@ -152,9 +181,11 @@ def fail_if_no_uuid(self): if self.app_uuid is None: msg = 'function should not be called before application uuid is set.' return None, msg + return None, None def fail_if_uuid(self): """Prevent a logic error.""" if self.app_uuid is not None: msg = 'function should not be called when application uuid is set.' return None, msg + return None, None diff --git a/ansible_collections/netapp/ontap/plugins/modules/na_ontap_lun.py b/ansible_collections/netapp/ontap/plugins/modules/na_ontap_lun.py index 97e0563b..329160da 100644 --- a/ansible_collections/netapp/ontap/plugins/modules/na_ontap_lun.py +++ b/ansible_collections/netapp/ontap/plugins/modules/na_ontap_lun.py @@ -59,7 +59,7 @@ size: description: - The size of the LUN in C(size_unit). - - Required when C(state=present). + - Required when creating a single LUN if application template is not used. type: int size_unit: @@ -183,6 +183,21 @@ description: list of object store names for tiering. type: list elements: str + total_size: + description: + - The total size of the application component, split across the member LUNs in C(total_size_unit). + - Recommended when C(lun_count) is present. + - Required when C(lun_count) is present and greater than 1. + - Note - if lun_count is equal to 1, and total_size is not present, size is used to maintain backward compatibility. + type: int + version_added: 21.1.0 + total_size_unit: + description: + - The unit used to interpret the total_size parameter. + - Defaults to size_unit if not present. + choices: ['bytes', 'b', 'kb', 'mb', 'gb', 'tb', 'pb', 'eb', 'zb', 'yb'] + type: str + version_added: 21.1.0 use_san_application: description: - Whether to use the application/applications REST/API to create LUNs. @@ -244,6 +259,7 @@ """ +import copy import traceback from ansible.module_utils.basic import AnsibleModule @@ -292,6 +308,9 @@ def __init__(self): policy=dict(type='str', choices=['all', 'auto', 'none', 'snapshot-only']), object_stores=dict(type='list', elements='str') # create only )), + total_size=dict(type='int'), + total_size_unit=dict(choices=['bytes', 'b', 'kb', 'mb', 'gb', 'tb', + 'pb', 'eb', 'zb', 'yb'], type='str'), )) )) @@ -303,8 +322,18 @@ def __init__(self): # set up state variables self.na_helper = NetAppModule() self.parameters = self.na_helper.set_parameters(self.module.params) + if self.parameters.get('size') is not None: self.parameters['size'] *= netapp_utils.POW2_BYTE_MAP[self.parameters['size_unit']] + if self.na_helper.safe_get(self.parameters, ['san_application_template', 'total_size']) is not None: + unit = self.na_helper.safe_get(self.parameters, ['san_application_template', 'total_size_unit']) + if unit is None: + unit = self.parameters['size_unit'] + self.parameters['san_application_template']['total_size'] *= netapp_utils.POW2_BYTE_MAP[unit] + + self.warnings = list() + self.debug = dict() + # self.debug['got'] = 'empty' # uncomment to enable collecting data if HAS_NETAPP_LIB is False: self.module.fail_json(msg="the python NetApp-Lib module is required") @@ -473,31 +502,39 @@ def get_lun_path_from_backend(self, name): return path return None - def create_san_app_component(self): + def create_san_app_component(self, modify): '''Create SAN application component''' - required_options = ('name', 'size') + if modify: + required_options = ['name'] + action = 'modify' + if 'lun_count' in modify: + required_options.append('total_size') + else: + required_options = ('name', 'total_size') + action = 'create' for option in required_options: if self.parameters.get(option) is None: - self.module.fail_json(msg='Error: "%s" is required to create san application.' % option) + self.module.fail_json(msg='Error: "%s" is required to %s a san application.' % (option, action)) + + application_component = dict(name=self.parameters['name']) + if not modify: + application_component['lun_count'] = 1 # default value for create, may be overriden below - application_component = dict( - name=self.parameters['name'], - total_size=self.parameters['size'], - lun_count=1 # default value, may be overriden below - ) for attr in ('igroup_name', 'lun_count', 'storage_service'): - value = self.na_helper.safe_get(self.parameters, ['san_application_template', attr]) - if value is not None: - application_component[attr] = value - for attr in ('os_type', 'qos_policy_group'): - value = self.na_helper.safe_get(self.parameters, [attr]) - if value is not None: - if attr == 'qos_policy_group': - attr = 'qos' - value = dict(policy=dict(name=value)) - application_component[attr] = value + if not modify or attr in modify: + value = self.na_helper.safe_get(self.parameters, ['san_application_template', attr]) + if value is not None: + application_component[attr] = value + for attr in ('os_type', 'qos_policy_group', 'total_size'): + if not modify or attr in modify: + value = self.na_helper.safe_get(self.parameters, [attr]) + if value is not None: + if attr == 'qos_policy_group': + attr = 'qos' + value = dict(policy=dict(name=value)) + application_component[attr] = value tiering = self.na_helper.safe_get(self.parameters, ['nas_application_template', 'tiering']) - if tiering is not None: + if tiering is not None and not modify: application_component['tiering'] = dict() for attr in ('control', 'policy', 'object_stores'): value = tiering.get(attr) @@ -507,26 +544,28 @@ def create_san_app_component(self): application_component['tiering'][attr] = value return application_component - def create_san_app_body(self): + def create_san_app_body(self, modify=None): '''Create body for san template''' # TODO: # Should we support new_igroups? # It may raise idempotency issues if the REST call fails if the igroup already exists. # And we already have na_ontap_igroups. san = { - 'application_components': [self.create_san_app_component()], + 'application_components': [self.create_san_app_component(modify)], } for attr in ('protection_type',): - value = self.na_helper.safe_get(self.parameters, ['san_application_template', attr]) - if value is not None: - # we expect value to be a dict, but maybe an empty dict - value = self.na_helper.filter_out_none_entries(value) - if value: - san[attr] = value + if not modify or attr in modify: + value = self.na_helper.safe_get(self.parameters, ['san_application_template', attr]) + if value is not None: + # we expect value to be a dict, but maybe an empty dict + value = self.na_helper.filter_out_none_entries(value) + if value: + san[attr] = value for attr in ('os_type',): - value = self.na_helper.safe_get(self.parameters, [attr]) - if value is not None: - san[attr] = value + if not modify: # not supported for modify operation, but required at applicaiton component level + value = self.na_helper.safe_get(self.parameters, [attr]) + if value is not None: + san[attr] = value body, error = self.rest_app.create_application_body('san', san) return body, error @@ -537,6 +576,17 @@ def create_san_application(self): dummy, error = self.rest_app.create_application(body) self.fail_on_error(error) + def modify_san_application(self, modify): + '''Use REST application/applications san template to add one or more LUNs''' + body, error = self.create_san_app_body(modify) + self.fail_on_error(error) + # these cannot be present when using PATCH + body.pop('name') + body.pop('svm') + body.pop('smart_container') + dummy, error = self.rest_app.patch_application(body) + self.fail_on_error(error) + def delete_san_application(self): '''Use REST application/applications san template to delete one or more LUNs''' dummy, error = self.rest_app.delete_application() @@ -662,40 +712,138 @@ def fail_on_error(self, error, stack=False): elements['stack'] = traceback.format_stack() self.module.fail_json(**elements) + def set_total_size(self, validate): + # fix total_size attribute, report error if total_size is misisng (or size is mssing) + attr = 'total_size' + value = self.na_helper.safe_get(self.parameters, ['san_application_template', attr]) + if value is not None or not validate: + self.parameters[attr] = value + return + lun_count = self.na_helper.safe_get(self.parameters, ['san_application_template', 'lun_count']) + value = self.parameters.get('size') + if value is not None and (lun_count is None or lun_count == 1): + self.parameters[attr] = value + return + self.module.fail_json("Error: 'total_size' is a required SAN application template attribute when creating a LUN application") + + def validate_app_create(self): + # fix total_size attribute + self.set_total_size(validate=True) + + def validate_app_changes(self, modify, warning): + errors = list() + for key in modify: + if key not in ('lun_count', 'total_size'): + errors.append("Error: the following application parameter cannot be modified: %s: %s." + % (key, modify[key])) + if 'lun_count' in modify: + for attr in ('total_size', 'os_type', 'igroup_name'): + value = self.parameters.get(attr) + if value is None: + value = self.na_helper.safe_get(self.parameters['san_application_template'], [attr]) + if value is None: + errors.append('Error: %s is a required parameter when increasing lun_count.' % attr) + else: + modify[attr] = value + if warning: + errors.append('Error: %s' % warning) + if errors: + self.module.fail_json(msg='\n'.join(errors)) + if 'total_size' in modify: + self.set_total_size(validate=False) + if warning: + # can't change total_size, let's ignore it + self.warnings.append(warning) + modify.pop('total_size') + + def app_changes(self): + # find and validate app changes + app_current, error = self.rest_app.get_application_details('san') + self.fail_on_error(error) + # save application name, as it is overriden in the flattening operation + app_name = app_current['name'] + # there is an issue with total_size not reflecting the real total_size, and some additional overhead + provisioned_size = self.na_helper.safe_get(app_current, ['statistics', 'space', 'provisioned']) + if provisioned_size is None: + provisioned_size = 0 + if self.debug: + self.debug['app_current'] = app_current # will be updated below as it is mutable + self.debug['got'] = copy.deepcopy(app_current) # fixed copy + # flatten + app_current = app_current['san'] # app template + app_current.update(app_current['application_components'][0]) # app component + del app_current['application_components'] + # if component name does not match, assume a change at LUN level + comp_name = app_current['name'] + if comp_name != self.parameters['name']: + return None, "name: %s does not match component name: %s" % (self.parameters['name'], comp_name) + + # restore app name + app_current['name'] = app_name + + # ready to compare, except for a quirk in size handling + total_size = app_current['total_size'] + desired = dict(self.parameters['san_application_template']) + desired_size = desired.get('total_size') + + warning = None + if desired_size is not None: + if desired_size < total_size: + self.module.fail_json("Error: can't reduce size: total_size=%d, provisioned=%d, requested=%d" + % (total_size, provisioned_size, desired_size)) + elif desired_size > total_size and desired_size < provisioned_size: + # we can't increase, but we can't say it is a problem, as the size is already bigger! + warning = "requested size is too small: total_size=%d, provisioned=%d, requested=%d" % (total_size, provisioned_size, desired_size) + + # preserve change state before calling modify in case an ignorable total_size change is the only change + changed = self.na_helper.changed + app_modify = self.na_helper.get_modified_attributes(app_current, desired) + self.validate_app_changes(app_modify, warning) + if not app_modify: + self.na_helper.changed = changed + app_modify = None + return app_modify, None + def apply(self): results = dict() - warnings = list() netapp_utils.ems_log_event("na_ontap_lun", self.server) - app_cd_action = None + app_cd_action, app_modify, lun_cd_action, lun_modify, lun_rename = None, None, None, None, None + app_modify_warning = None + actions = list() if self.rest_app: app_current, error = self.rest_app.get_application_uuid() self.fail_on_error(error) app_cd_action = self.na_helper.get_cd_action(app_current, self.parameters) - if app_cd_action == 'create' and self.parameters.get('size') is None: - self.module.fail_json(msg="size is a required parameter for create.") - - # For LUNs created using a SAN application, we're getting lun paths from the backing storage - lun_path, from_lun_path = None, None - from_name = self.parameters.get('from_name') - if self.rest_app and app_cd_action is None and app_current: - lun_path = self.get_lun_path_from_backend(self.parameters['name']) - if from_name is not None: - from_lun_path = self.get_lun_path_from_backend(from_name) - - if app_cd_action is None: + if app_cd_action is not None: + actions.append('app_%s' % app_cd_action) + if app_cd_action == 'create': + self.validate_app_create() + if app_cd_action is None and app_current is not None: + app_modify, app_modify_warning = self.app_changes() + if app_modify: + actions.append('app_modify') + results['app_modify'] = dict(app_modify) + + if app_cd_action is None and app_modify is None: # actions at LUN level + lun_path, from_lun_path = None, None + from_name = self.parameters.get('from_name') + if self.rest_app and app_current: + # For LUNs created using a SAN application, we're getting lun paths from the backing storage + lun_path = self.get_lun_path_from_backend(self.parameters['name']) + if from_name is not None: + from_lun_path = self.get_lun_path_from_backend(from_name) current = self.get_lun(self.parameters['name'], lun_path) if current is not None and lun_path is None: lun_path = current['path'] - cd_action = self.na_helper.get_cd_action(current, self.parameters) - modify, rename = None, None - if cd_action == 'create' and from_name is not None: + lun_cd_action = self.na_helper.get_cd_action(current, self.parameters) + if lun_cd_action == 'create' and from_name is not None: # create by renaming existing LUN, if it really exists old_lun = self.get_lun(from_name, from_lun_path) - rename = self.na_helper.is_rename_action(old_lun, current) - if rename is None: + lun_rename = self.na_helper.is_rename_action(old_lun, current) + if lun_rename is None: self.module.fail_json(msg="Error renaming lun: %s does not exist" % from_name) - if rename: + if lun_rename: current = old_lun if from_lun_path is None: from_lun_path = current['path'] @@ -703,47 +851,61 @@ def apply(self): if tail: self.module.fail_json(msg="Error renaming lun: %s does not match lun_path %s" % (from_name, from_lun_path)) lun_path = head + self.parameters['name'] - results['renamed'] = True - cd_action = None - if cd_action == 'create' and self.parameters.get('size') is None: - self.module.fail_json(msg="size is a required parameter for create.") - if cd_action is None and self.parameters['state'] == 'present': + lun_cd_action = None + actions.append('lun_rename') + app_modify_warning = None # reset warning as we found a match + if lun_cd_action is not None: + actions.append(actions.append('lun_%s' % lun_cd_action)) + if lun_cd_action is None and self.parameters['state'] == 'present': # we already handled rename if required current.pop('name', None) - modify = self.na_helper.get_modified_attributes(current, self.parameters) - results['modify'] = dict(modify) - if cd_action and self.rest_app and app_cd_action is None and app_current: + lun_modify = self.na_helper.get_modified_attributes(current, self.parameters) + if lun_modify: + actions.append('lun_modify') + results['lun_modify'] = dict(lun_modify) + app_modify_warning = None # reset warning as we found a match + if lun_cd_action and self.rest_app and app_current: msg = 'This module does not support %s a LUN by name %s a SAN application.' %\ - ('adding', 'to') if cd_action == 'create' else ('removing', 'from') - warnings.append(msg) - cd_action = None + ('adding', 'to') if lun_cd_action == 'create' else ('removing', 'from') + self.warnings.append(msg) + lun_cd_action = None self.na_helper.changed = False + if lun_cd_action == 'create' and self.parameters.get('size') is None: + self.module.fail_json(msg="size is a required parameter for create.") if self.na_helper.changed and not self.module.check_mode: if app_cd_action == 'create': self.create_san_application() elif app_cd_action == 'delete': self.rest_app.delete_application() - elif cd_action == 'create': + elif app_modify: + self.modify_san_application(app_modify) + elif lun_cd_action == 'create': self.create_lun() - elif cd_action == 'delete': + elif lun_cd_action == 'delete': self.delete_lun(lun_path) else: - if rename: + if lun_rename: self.rename_lun(from_lun_path, lun_path) size_changed = False - if modify and 'size' in modify: + if lun_modify and 'size' in lun_modify: # Ensure that size was actually changed. Please # read notes in 'resize_lun' function for details. size_changed = self.resize_lun(lun_path) - modify.pop('size') - if modify: - self.modify_lun(lun_path, modify) - if not modify and not rename: + lun_modify.pop('size') + if lun_modify: + self.modify_lun(lun_path, lun_modify) + if not lun_modify and not lun_rename: # size may not have changed self.na_helper.changed = size_changed + if app_modify_warning: + self.warnings.append(app_modify_warning) results['changed'] = self.na_helper.changed + results['actions'] = actions + if self.warnings: + results['warnings'] = self.warnings + results.update(self.debug) self.module.exit_json(**results) diff --git a/ansible_collections/netapp/ontap/plugins/modules/na_ontap_volume.py b/ansible_collections/netapp/ontap/plugins/modules/na_ontap_volume.py index 85efd923..72c04bcd 100644 --- a/ansible_collections/netapp/ontap/plugins/modules/na_ontap_volume.py +++ b/ansible_collections/netapp/ontap/plugins/modules/na_ontap_volume.py @@ -313,7 +313,7 @@ time_out: description: - - time to wait for flexGroup creation, modification, or deletion in seconds. + - time to wait for Flexgroup creation, modification, or deletion in seconds. - Error out if task is not completed in defined time. - if 0, the request is asynchronous. - default is set to 3 minutes. @@ -567,7 +567,7 @@ username: "{{ netapp_username }}" password: "{{ netapp_password }}" - - name: Create flexGroup volume manually + - name: Create Flexgroup volume manually na_ontap_volume: state: present name: ansibleVolume @@ -587,7 +587,7 @@ snapshot_policy: default time_out: 0 - - name: Create flexGroup volume auto provsion as flex group + - name: Create Flexgroup volume auto provsion as flex group na_ontap_volume: state: present name: ansibleVolume @@ -1117,7 +1117,7 @@ def create_nas_application_component(self): name=self.parameters['name'], total_size=self.parameters['size'], share_count=1, # 1 is the maximum value for nas - scale_out=(self.volume_style == 'flexGroup'), + scale_out=(self.volume_style == 'flexgroup'), ) name = self.na_helper.safe_get(self.parameters, ['nas_application_template', 'storage_service']) if name is not None: @@ -1184,7 +1184,7 @@ def create_volume(self): '''Create ONTAP volume''' if self.rest_app: return self.create_nas_application() - if self.volume_style == 'flexGroup': + if self.volume_style == 'flexgroup': return self.create_volume_async() options = self.create_volume_options() @@ -1244,7 +1244,7 @@ def create_volume_async(self): def create_volume_options(self): '''Set volume options for create operation''' options = {} - if self.volume_style == 'flexGroup': + if self.volume_style == 'flexgroup': options['volume-name'] = self.parameters['name'] if self.parameters.get('aggr_list_multiplier') is not None: options['aggr-list-multiplier'] = str(self.parameters['aggr_list_multiplier']) @@ -1321,7 +1321,7 @@ def delete_volume(self, current): '''Delete ONTAP volume''' if self.use_rest and self.parameters['uuid'] is not None: return self.rest_delete_volume() - if self.parameters.get('is_infinite') or self.volume_style == 'flexGroup': + if self.parameters.get('is_infinite') or self.volume_style == 'flexgroup': if current['is_online']: self.change_volume_state(call_from_delete_vol=True) volume_delete = netapp_utils.zapi.NaElement.create_node_with_children( @@ -1331,7 +1331,7 @@ def delete_volume(self, current): 'volume-destroy', **{'name': self.parameters['name'], 'unmount-and-offline': 'true'}) try: result = self.server.invoke_successfully(volume_delete, enable_tunneling=True) - if self.parameters.get('is_infinite') or self.volume_style == 'flexGroup': + if self.parameters.get('is_infinite') or self.volume_style == 'flexgroup': self.check_invoke_result(result, 'delete') self.ems_log_event("volume-delete") except netapp_utils.zapi.NaApiError as error: @@ -1352,9 +1352,10 @@ def move_volume(self): enable_tunneling=True) self.ems_log_event("volume-move") except netapp_utils.zapi.NaApiError as error: - if not self.move_volume_with_rest_passthrough(): - self.module.fail_json(msg='Error moving volume %s: %s' - % (self.parameters['name'], to_native(error)), + rest_error = self.move_volume_with_rest_passthrough() + if rest_error is not None: + self.module.fail_json(msg='Error moving volume %s: %s - Retry failed with REST error: %s' + % (self.parameters['name'], to_native(error), rest_error), exception=traceback.format_exc()) def move_volume_with_rest_passthrough(self): @@ -1365,13 +1366,13 @@ def move_volume_with_rest_passthrough(self): return False # if REST exists let's try moving using the passthrough CLI api = 'private/cli/volume/move/start' - data = {'volume:': self.parameters['name'], - 'destination-aggregate': self.parameters['aggregate_name'], - 'vserver': self.parameters['vserver']} - dummy, error = self.rest_api.patch(api, data) - if error is not None: - self.module.fail_json(msg='Error moving volume %s: %s' % (self.parameters['name'], error)) - return True + data = {'destination-aggregate': self.parameters['aggregate_name'] + } + query = {'volume': self.parameters['name'], + 'vserver': self.parameters['vserver'] + } + dummy, error = self.rest_api.patch(api, data, query) + return error def wait_for_volume_move(self): waiting = True @@ -1453,7 +1454,7 @@ def resize_volume(self): return self.rest_resize_volume() vol_size_zapi, vol_name_zapi = ['volume-size-async', 'volume-name']\ - if (self.parameters['is_infinite'] or self.volume_style == 'flexGroup')\ + if (self.parameters['is_infinite'] or self.volume_style == 'flexgroup')\ else ['volume-size', 'volume'] volume_resize = netapp_utils.zapi.NaElement.create_node_with_children( vol_size_zapi, **{vol_name_zapi: self.parameters['name'], @@ -1475,11 +1476,11 @@ def change_volume_state(self, call_from_delete_vol=False): """ if self.parameters['is_online'] and not call_from_delete_vol: # Desired state is online, setup zapi APIs respectively vol_state_zapi, vol_name_zapi, action = ['volume-online-async', 'volume-name', 'online']\ - if (self.parameters['is_infinite'] or self.volume_style == 'flexGroup')\ + if (self.parameters['is_infinite'] or self.volume_style == 'flexgroup')\ else ['volume-online', 'name', 'online'] else: # Desired state is offline, setup zapi APIs respectively vol_state_zapi, vol_name_zapi, action = ['volume-offline-async', 'volume-name', 'offline']\ - if (self.parameters['is_infinite'] or self.volume_style == 'flexGroup')\ + if (self.parameters['is_infinite'] or self.volume_style == 'flexgroup')\ else ['volume-offline', 'name', 'offline'] volume_unmount = netapp_utils.zapi.NaElement.create_node_with_children( 'volume-unmount', **{'volume-name': self.parameters['name']}) @@ -1494,7 +1495,7 @@ def change_volume_state(self, call_from_delete_vol=False): errors.append('Error unmounting volume %s: %s' % (self.parameters['name'], to_native(error))) try: result = self.server.invoke_successfully(volume_change_state, enable_tunneling=True) - if self.volume_style == 'flexGroup' or self.parameters['is_infinite']: + if self.volume_style == 'flexgroup' or self.parameters['is_infinite']: self.check_invoke_result(result, action) self.ems_log_event("change-state") except netapp_utils.zapi.NaApiError as error: @@ -1524,7 +1525,7 @@ def volume_modify_attributes(self, params): modify volume parameter 'export_policy','unix_permissions','snapshot_policy','space_guarantee', 'percent_snapshot_space', 'qos_policy_group', 'qos_adaptive_policy_group' """ - if self.volume_style == 'flexGroup' or self.parameters['is_infinite']: + if self.volume_style == 'flexgroup' or self.parameters['is_infinite']: vol_mod_iter = netapp_utils.zapi.NaElement('volume-modify-iter-async') else: vol_mod_iter = netapp_utils.zapi.NaElement('volume-modify-iter') @@ -1629,7 +1630,7 @@ def volume_modify_attributes(self, params): self.module.fail_json(msg="Error modifying volume %s: %s" % (self.parameters['name'], ' --- '.join(error_msgs)), exception=traceback.format_exc()) - if self.volume_style == 'flexGroup' or self.parameters['is_infinite']: + if self.volume_style == 'flexgroup' or self.parameters['is_infinite']: success = result.get_child_by_name('success-list') success = success.get_child_by_name('volume-modify-iter-async-info') results = dict() @@ -1742,15 +1743,10 @@ def char_to_octal(self, chars): def get_volume_style(self, current): '''Get volume style, infinite or standard flexvol''' - if current is None: - if self.parameters.get('aggr_list') or self.parameters.get('aggr_list_multiplier') or self.parameters.get('auto_provision_as'): - return 'flexGroup' - else: - if current.get('style_extended'): - if current['style_extended'] == 'flexgroup': - return 'flexGroup' - else: - return current['style_extended'] + if current is not None: + return current.get('style_extended') + if self.parameters.get('aggr_list') or self.parameters.get('aggr_list_multiplier') or self.parameters.get('auto_provision_as'): + return 'flexgroup' return None def get_job(self, jobid, server): @@ -2000,7 +1996,14 @@ def set_modify_dict(self, current, after_create=False): self.adjust_size(current, after_create) modify = self.na_helper.get_modified_attributes(current, self.parameters) if modify is not None and 'type' in modify: - self.module.fail_json(msg="Changing the same volume from one type to another is not allowed.") + msg = "Error: changing a volume from one type to another is not allowed." + msg += ' Current: %s, desired: %s.' % (current['type'], self.parameters['type']) + self.module.fail_json(msg=msg) + desired_style = self.get_volume_style(None) + if desired_style is not None and desired_style != self.volume_style: + msg = "Error: changing a volume from one backend to another is not allowed." + msg += ' Current: %s, desired: %s.' % (self.volume_style, desired_style) + self.module.fail_json(msg=msg) if self.parameters.get('snapshot_auto_delete') is not None: auto_delete_modify = self.na_helper.get_modified_attributes(auto_delete_info, self.parameters['snapshot_auto_delete']) @@ -2017,7 +2020,7 @@ def take_modify_actions(self, modify): self.modify_volume(modify) if any([modify.get(key) is not None for key in self.sis_keys2zapi_get]): - if self.parameters.get('is_infinite') or self.volume_style == 'flexGroup': + if self.parameters.get('is_infinite') or self.volume_style == 'flexgroup': efficiency_config_modify = 'async' else: efficiency_config_modify = 'sync' @@ -2029,8 +2032,10 @@ def apply(self): modify_after_create = None current = self.get_volume() self.volume_style = self.get_volume_style(current) - # rename and create are mutually exclusive + if self.volume_style == 'flexgroup' and self.parameters.get('aggregate_name') is not None: + self.module.fail_json(msg='Error: aggregate_name option cannot be used with FlexGroups.') rename, rehost, snapshot_restore, cd_action, modify = None, None, None, None, None + # rename and create are mutually exclusive if self.parameters.get('from_name'): rename = self.na_helper.is_rename_action(self.get_volume(self.parameters['from_name']), current) elif self.parameters.get('from_vserver'): diff --git a/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_lun.py b/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_lun.py index 07d3c2f3..87e58735 100644 --- a/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_lun.py +++ b/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_lun.py @@ -164,7 +164,7 @@ def test_successful_rename(self): with pytest.raises(AnsibleExitJson) as exc: self.get_lun_mock_object('lun', 'lun_from_name').apply() assert exc.value.args[0]['changed'] - assert 'renamed' in exc.value.args[0] + assert 'lun_rename' in exc.value.args[0]['actions'] def test_failed_rename(self): ''' Test failed rename ''' diff --git a/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_lun_rest.py b/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_lun_rest.py index 9ed07535..4eee1ce1 100644 --- a/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_lun_rest.py +++ b/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_lun_rest.py @@ -5,6 +5,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import copy import json import pytest @@ -75,6 +76,12 @@ def fail_json(*args, **kwargs): # pylint: disable=unused-argument }, None ), + 'get_app_details': (200, + dict(name='san_appli', uuid='1234', san=dict( + application_components=[dict(name='lun_name', lun_count=3, total_size=1000)] + )), + None + ), 'get_app_component_details': (200, {'backing_storage': dict(luns=[]), }, @@ -208,12 +215,13 @@ def test_successful_create_appli(self, mock_request): @patch('ansible_collections.netapp.ontap.plugins.module_utils.netapp.OntapRestAPI.send_request') def test_successful_create_appli_idem(self, mock_request): ''' Test successful create idempotent ''' - mock_request.side_effect = [ + mock_request.side_effect = copy.deepcopy([ SRR['get_apps_found'], # GET application/applications + SRR['get_app_details'], # GET application/applications/ SRR['get_apps_found'], # GET application/applications//components SRR['get_app_component_details'], # GET application/applications//components/ SRR['end_of_sequence'] - ] + ]) data = dict(self.mock_args()) data['size'] = 5 data.pop('flexvol_name') @@ -226,11 +234,12 @@ def test_successful_create_appli_idem(self, mock_request): @patch('ansible_collections.netapp.ontap.plugins.module_utils.netapp.OntapRestAPI.send_request') def test_successful_create_appli_idem_no_comp(self, mock_request): ''' Test successful create idempotent ''' - mock_request.side_effect = [ + mock_request.side_effect = copy.deepcopy([ SRR['get_apps_found'], # GET application/applications + SRR['get_app_details'], # GET application/applications/ SRR['get_apps_empty'], # GET application/applications//components SRR['end_of_sequence'] - ] + ]) data = dict(self.mock_args()) data['size'] = 5 data.pop('flexvol_name') @@ -238,6 +247,7 @@ def test_successful_create_appli_idem_no_comp(self, mock_request): set_module_args(data) with pytest.raises(AnsibleFailJson) as exc: self.get_lun_mock_object().apply() + # print(mock_request.call_args_list) msg = 'Error: no component for application san_appli' assert msg == exc.value.args[0]['msg'] @@ -261,7 +271,7 @@ def test_successful_delete_appli(self, mock_request): @patch('ansible_collections.netapp.ontap.plugins.module_utils.netapp.OntapRestAPI.send_request') def test_successful_delete_appli_idem(self, mock_request): - ''' Test successful deelte idempotent ''' + ''' Test successful delete idempotent ''' mock_request.side_effect = [ SRR['get_apps_empty'], # GET application/applications SRR['end_of_sequence'] @@ -275,3 +285,69 @@ def test_successful_delete_appli_idem(self, mock_request): with pytest.raises(AnsibleExitJson) as exc: self.get_lun_mock_object().apply() assert not exc.value.args[0]['changed'] + + @patch('ansible_collections.netapp.ontap.plugins.module_utils.netapp.OntapRestAPI.send_request') + def test_successful_modify_appli(self, mock_request): + ''' Test successful modify application ''' + mock_request.side_effect = copy.deepcopy([ + SRR['get_apps_found'], # GET application/applications + SRR['get_app_details'], # GET application/applications/ + # SRR['get_apps_found'], # GET application/applications//components + # SRR['get_app_component_details'], # GET application/applications//components/ + SRR['empty_good'], # PATCH application/applications/ + SRR['end_of_sequence'] + ]) + data = dict(self.mock_args()) + data['os_type'] = 'xyz' + data.pop('flexvol_name') + data['san_application_template'] = dict(name='san_appli', lun_count=5, total_size=1000, igroup_name='abc') + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_lun_mock_object().apply() + print(exc.value.args[0]) + # print(mock_request.call_args_list) + assert exc.value.args[0]['changed'] + + @patch('ansible_collections.netapp.ontap.plugins.module_utils.netapp.OntapRestAPI.send_request') + def test_error_modify_appli_missing_igroup(self, mock_request): + ''' Test successful modify application ''' + mock_request.side_effect = copy.deepcopy([ + SRR['get_apps_found'], # GET application/applications + SRR['get_app_details'], # GET application/applications/ + # SRR['get_apps_found'], # GET application/applications//components + # SRR['get_app_component_details'], # GET application/applications//components/ + SRR['end_of_sequence'] + ]) + data = dict(self.mock_args()) + data['size'] = 5 + data.pop('flexvol_name') + data['san_application_template'] = dict(name='san_appli', lun_count=5) + set_module_args(data) + with pytest.raises(AnsibleFailJson) as exc: + self.get_lun_mock_object().apply() + msg = 'Error: igroup_name is a required parameter when increasing lun_count.' + assert msg in exc.value.args[0]['msg'] + msg = 'Error: total_size is a required parameter when increasing lun_count.' + assert msg in exc.value.args[0]['msg'] + msg = 'Error: os_type is a required parameter when increasing lun_count.' + assert msg in exc.value.args[0]['msg'] + + @patch('ansible_collections.netapp.ontap.plugins.module_utils.netapp.OntapRestAPI.send_request') + def test_successful_no_action(self, mock_request): + ''' Test successful modify application ''' + mock_request.side_effect = copy.deepcopy([ + SRR['get_apps_found'], # GET application/applications + SRR['get_app_details'], # GET application/applications/ + SRR['get_apps_found'], # GET application/applications//components + SRR['get_app_component_details'], # GET application/applications//components/ + SRR['end_of_sequence'] + ]) + data = dict(self.mock_args()) + data['name'] = 'unknown' + data.pop('flexvol_name') + data['san_application_template'] = dict(name='san_appli', lun_count=5) + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_lun_mock_object().apply() + print(exc.value.args[0]) + assert not exc.value.args[0]['changed'] diff --git a/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_volume.py b/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_volume.py index bc31c561..d1e95ba9 100644 --- a/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_volume.py +++ b/ansible_collections/netapp/ontap/tests/unit/plugins/modules/test_na_ontap_volume.py @@ -87,6 +87,8 @@ def invoke_successfully(self, xml, enable_tunneling): # pylint: disable=unused- if kind == 'volume': xml = self.build_volume_info(self.params) + if kind == 'flexgroup': + xml = self.build_flexgroup_info(self.params) elif kind == 'job_info': xml = self.build_job_info(self.job_error) elif kind == 'error_modify': @@ -168,7 +170,7 @@ def build_volume_info(vol_details): return xml @staticmethod - def build_flex_group_info(vol_details): + def build_flexgroup_info(vol_details): ''' build xml data for flexGroup volume-attributes ''' xml = netapp_utils.zapi.NaElement('xml') attributes = { @@ -191,7 +193,7 @@ def build_flex_group_info(vol_details): 'is-atime-update-enabled': 'true' }, 'volume-state-attributes': { - 'state': "online" + 'state': "online", }, 'volume-space-attributes': { 'space-guarantee': 'none', @@ -204,6 +206,9 @@ def build_flex_group_info(vol_details): 'volume-security-unix-attributes': { 'permissions': vol_details['unix_permissions'] } + }, + 'volume-snapshot-autodelete-attributes': { + 'commitment': 'try' } } } @@ -946,7 +951,7 @@ def test_failure_modify_unix_permissions_flex_group(self, get_volume): 'password': 'test_pass!', 'name': self.mock_vol['name'], 'vserver': self.mock_vol['vserver'], - 'style_extended': 'flexvol', + 'style_extended': 'flexgroup', 'unix_permissions': '777', 'uuid': '1234' } @@ -1199,3 +1204,23 @@ def test_error_volume_restore(self): with pytest.raises(AnsibleFailJson) as exc: self.get_volume_mock_object('zapi_error').snapshot_restore_volume() assert exc.value.args[0]['msg'] == 'Error restoring volume test_vol: NetApp API failed. Reason - test:error' + + def test_error_modify_flexvol_to_flexgroup(self): + ''' Test successful modify vserver_dr_protection ''' + data = self.mock_args() + data['auto_provision_as'] = 'flexgroup' + set_module_args(data) + with pytest.raises(AnsibleFailJson) as exc: + self.get_volume_mock_object('volume').apply() + msg = 'Error: changing a volume from one backend to another is not allowed. Current: flexvol, desired: flexgroup.' + assert msg == exc.value.args[0]['msg'] + + def test_error_modify_flexgroup_to_flexvol(self): + ''' Test successful modify vserver_dr_protection ''' + data = self.mock_args() + data['aggregate_name'] = 'nothing' + set_module_args(data) + with pytest.raises(AnsibleFailJson) as exc: + self.get_volume_mock_object('flexgroup').apply() + msg = 'Error: aggregate_name option cannot be used with FlexGroups.' + assert msg == exc.value.args[0]['msg']