From 2dd03e843476bd149dbfa8e128b5a3be4badc051 Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Wed, 1 Sep 2021 17:08:19 -0400 Subject: [PATCH] Sync bitbucket and GitHub --- CHANGELOG.rst | 21 ++ README.md | 7 +- changelogs/changelog.yaml | 44 ++++- changelogs/fragments/DEVOPS-2459.yaml | 2 + changelogs/fragments/DEVOPS-4227.yaml | 2 + plugins/module_utils/netapp_module.py | 35 ++-- plugins/modules/na_ontap_job_schedule.py | 186 +++++++++--------- plugins/modules/na_ontap_ntp.py | 62 +++++- plugins/modules/na_ontap_ucadapter.py | 5 + plugins/modules/na_ontap_volume.py | 132 ++++++------- .../modules/test_na_ontap_job_schedule.py | 16 +- .../unit/plugins/modules/test_na_ontap_ntp.py | 182 +++++++++++++++++ 12 files changed, 499 insertions(+), 195 deletions(-) create mode 100644 changelogs/fragments/DEVOPS-2459.yaml create mode 100644 changelogs/fragments/DEVOPS-4227.yaml create mode 100644 tests/unit/plugins/modules/test_na_ontap_ntp.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3792ab33..c54d008a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,27 @@ NetApp ONTAP Collection Release Notes .. contents:: Topics +v21.11.0 +======== + +Minor Changes +------------- + +- na_ontap_interface - new option ``from_name`` to rename an interface. +- na_ontap_ntp - Added REST support to the ntp module +- na_ontap_ntp - Added REST support to the ntp module +- na_ontap_software_update - new option ``validate_after_download`` to run ONTAP software update validation checks. +- na_ontap_software_update - remove ``absent`` as a choice for ``state`` as it has no use. +- na_ontap_svm - ignore ``aggr_list`` with ``'*'`` when using REST. +- na_ontap_svm - new option ``ignore_rest_unsupported_options`` to ignore older ZAPI options not available in REST. +- na_ontap_svm - new option ``services`` to allow and/or enable protocol services. + +Bugfixes +-------- + +- na_ontap_job_schedule - fix idempotency issue with REST when job_minutes is set to -1. +- na_ontap_ldap_client - remove limitation on schema so that custom schemas can be used. + v21.10.0 ======== diff --git a/README.md b/README.md index 7a1dc091..e98473ae 100644 --- a/README.md +++ b/README.md @@ -38,10 +38,14 @@ Join our Slack Channel at [Netapp.io](http://netapp.io/slack) ### Minor Changes - na_ontap_software_update - remove `absent` as a choice for `state` as it has no use. - na_ontap_svm - ignore `aggr_list: '*'` when using REST. - + ### Bug Fixes + - na_ontap_job_schedule - fix idempotency issue with REST when job_minutes is set to -1. - na_ontap_ldap_client - remove limitation on schema so that custom schemas can be used. +### Added REST support to existing modules + - na_ontap_ntp - Added REST support to the ntp module + ## 21.10.0 ### Minor Changes @@ -51,6 +55,7 @@ Join our Slack Channel at [Netapp.io](http://netapp.io/slack) - na_ontap_vserver_delete role - delete iSCSI igroups and CIFS server before deleting vserver. - all modules - traceback on ONTAP 9.3 (and earlier) when trying to detect REST support. + ## 21.9.0 ### Minor Changes diff --git a/changelogs/changelog.yaml b/changelogs/changelog.yaml index c9f59611..5476bac6 100644 --- a/changelogs/changelog.yaml +++ b/changelogs/changelog.yaml @@ -26,14 +26,13 @@ releases: - na_ontap_vserver_cifs_security - fix int and boolean options when modifying vserver cifs security. minor_changes: - - "Added REST support to existing modules.\n By default, the module will use\ - \ REST if the target system supports it, and the options are supported. Otherwise,\ - \ it will switch back to ZAPI.\n This behavior can be controlled with the\ - \ ``use_rest`` option.\n Always - to force REST. The module fails and reports\ - \ an error if REST cannot be used.\n Never - to force ZAPI. This could be\ - \ useful if you find some incompatibility with REST, or want to confirm the\ - \ behavior is identical between REST and ZAPI.\n Auto - the default, as described\ - \ above.\n" + - "Added REST support to existing modules.\n By default, the module will use + REST if the target system supports it, and the options are supported. Otherwise, + it will switch back to ZAPI.\n This behavior can be controlled with the ``use_rest`` + option.\n Always - to force REST. The module fails and reports an error + if REST cannot be used.\n Never - to force ZAPI. This could be useful if + you find some incompatibility with REST, or want to confirm the behavior is + identical between REST and ZAPI.\n Auto - the default, as described above.\n" - na_ontap_cluster_config - role updated to support a cleaner playbook - na_ontap_command - ``vserver`` - to allow command to run as either cluster admin or vserver admin. To run as vserver admin you must use the vserver @@ -1061,6 +1060,35 @@ releases: - DEVOPS-4190.yaml - DEVOPS-4231.yaml release_date: '2021-08-12' + 21.11.0: + changes: + bugfixes: + - na_ontap_job_schedule - fix idempotency issue with REST when job_minutes is + set to -1. + - na_ontap_ldap_client - remove limitation on schema so that custom schemas + can be used. + minor_changes: + - na_ontap_interface - new option ``from_name`` to rename an interface. + - na_ontap_ntp - Added REST support to the ntp module + - na_ontap_ntp - Added REST support to the ntp module + - na_ontap_software_update - new option ``validate_after_download`` to run ONTAP + software update validation checks. + - na_ontap_software_update - remove ``absent`` as a choice for ``state`` as + it has no use. + - na_ontap_svm - ignore ``aggr_list`` with ``'*'`` when using REST. + - na_ontap_svm - new option ``ignore_rest_unsupported_options`` to ignore older + ZAPI options not available in REST. + - na_ontap_svm - new option ``services`` to allow and/or enable protocol services. + fragments: + - DEVOPS-2459.yaml + - DEVOPS-2459.yml + - DEVOPS-4218.yaml + - DEVOPS-4227.yaml + - DEVOPS-4235.yaml + - DEVOPS-4243.yaml + - DEVOPS-4255.yaml + - DEVOPS-4256.yaml + release_date: '2021-09-01' 21.2.0: changes: bugfixes: diff --git a/changelogs/fragments/DEVOPS-2459.yaml b/changelogs/fragments/DEVOPS-2459.yaml new file mode 100644 index 00000000..74221fcd --- /dev/null +++ b/changelogs/fragments/DEVOPS-2459.yaml @@ -0,0 +1,2 @@ +minor_changes: + - na_ontap_ntp - Added REST support to the ntp module diff --git a/changelogs/fragments/DEVOPS-4227.yaml b/changelogs/fragments/DEVOPS-4227.yaml new file mode 100644 index 00000000..ade00474 --- /dev/null +++ b/changelogs/fragments/DEVOPS-4227.yaml @@ -0,0 +1,2 @@ +bugfixes: + - na_ontap_job_schedule - fix idempotency issue with REST when job_minutes is set to -1. diff --git a/plugins/module_utils/netapp_module.py b/plugins/module_utils/netapp_module.py index 5d9b1cd8..758186d3 100644 --- a/plugins/module_utils/netapp_module.py +++ b/plugins/module_utils/netapp_module.py @@ -68,24 +68,25 @@ class NetAppModule(object): def __init__(self, module=None): self.module = module - self.log = list() + self.log = [] self.changed = False self.parameters = {'name': 'not initialized'} - self.zapi_string_keys = dict() - self.zapi_bool_keys = dict() - self.zapi_list_keys = dict() - self.zapi_int_keys = dict() - self.zapi_required = dict() + self.zapi_string_keys = {} + self.zapi_bool_keys = {} + self.zapi_list_keys = {} + self.zapi_int_keys = {} + self.zapi_required = {} + self.params_to_rest_api_keys = {} def set_parameters(self, ansible_params): - self.parameters = dict() + self.parameters = {} for param in ansible_params: if ansible_params[param] is not None: self.parameters[param] = ansible_params[param] return self.parameters def check_and_set_parameters(self, module): - self.parameters = dict() + self.parameters = {} check_for_none = netapp_utils.has_feature(module, 'check_required_params_for_none') if check_for_none: required_keys = [key for key, value in module.argument_spec.items() if value.get('required')] @@ -177,11 +178,7 @@ def get_cd_action(self, current, desired): is_present = 'present' action = cd_action(current=is_present, desired = self.desired.state()) ''' - if 'state' in desired: - desired_state = desired['state'] - else: - desired_state = 'present' - + desired_state = desired['state'] if 'state' in desired else 'present' if current is None and desired_state == 'absent': return None if current is not None and desired_state == 'present': @@ -193,7 +190,7 @@ def get_cd_action(self, current, desired): return 'create' def compare_and_update_values(self, current, desired, keys_to_compare): - updated_values = dict() + updated_values = {} is_changed = False for key in keys_to_compare: if key in current: @@ -237,7 +234,7 @@ def compare_lists(current, desired, get_list_diff): desired_diff_list.append(item) # get what in current but not in desired - current_diff_list = list() + current_diff_list = [] for item in current: if item in desired_copy: desired_copy.remove(item) @@ -269,7 +266,7 @@ def get_modified_attributes(self, current, desired, get_list_diff=False): aggregate name) ''' # if the object does not exist, we can't modify it - modified = dict() + modified = {} if current is None: return modified @@ -318,7 +315,7 @@ def is_rename_action(self, source, target): # idempotency (or) new_name_is_already_in_use # alternatively we could delete B and rename A to B return False - if source is None and target is not None: + if source is None: # do nothing, maybe the rename was already done return False # source is not None and target is None: @@ -446,7 +443,7 @@ def filter_out_none_entries(self, list_or_dict): """ if isinstance(list_or_dict, dict): - result = dict() + result = {} for key, value in list_or_dict.items(): if isinstance(value, (list, dict)): sub = self.filter_out_none_entries(value) @@ -459,7 +456,7 @@ def filter_out_none_entries(self, list_or_dict): return result if isinstance(list_or_dict, list): - alist = list() + alist = [] for item in list_or_dict: if isinstance(item, (list, dict)): sub = self.filter_out_none_entries(item) diff --git a/plugins/modules/na_ontap_job_schedule.py b/plugins/modules/na_ontap_job_schedule.py index bde974e6..5917ee30 100644 --- a/plugins/modules/na_ontap_job_schedule.py +++ b/plugins/modules/na_ontap_job_schedule.py @@ -12,11 +12,6 @@ __metaclass__ = type -ANSIBLE_METADATA = {'metadata_version': '1.1', - 'status': ['preview'], - 'supported_by': 'certified'} - - DOCUMENTATION = ''' module: na_ontap_job_schedule short_description: NetApp ONTAP Job Schedule @@ -96,7 +91,7 @@ EXAMPLES = """ - name: Create Job for 11.30PM at 10th of every month - na_ontap_job_schedule: + netapp.ontap.na_ontap_job_schedule: state: present name: jobName job_minutes: 30 @@ -107,7 +102,7 @@ username: "{{ netapp_username }}" password: "{{ netapp_password }}" - name: Create Job for 11.30PM at 10th of January, April, July, October for ZAPI and REST - na_ontap_job_schedule: + netapp.ontap.na_ontap_job_schedule: state: present name: jobName job_minutes: 30 @@ -119,7 +114,7 @@ username: "{{ netapp_username }}" password: "{{ netapp_password }}" - name: Create Job for 11.30PM at 10th of January, April, July, October for ZAPI and REST - na_ontap_job_schedule: + netapp.ontap.na_ontap_job_schedule: state: present name: jobName job_minutes: 30 @@ -131,7 +126,7 @@ username: "{{ netapp_username }}" password: "{{ netapp_password }}" - name: Create Job for 11.30PM at 10th of January when using REST and February when using ZAPI !!! - na_ontap_job_schedule: + netapp.ontap.na_ontap_job_schedule: state: present name: jobName job_minutes: 30 @@ -142,7 +137,7 @@ username: "{{ netapp_username }}" password: "{{ netapp_password }}" - name: Delete Job - na_ontap_job_schedule: + netapp.ontap.na_ontap_job_schedule: state: absent name: jobName hostname: "{{ netapp_hostname }}" @@ -161,9 +156,10 @@ import ansible_collections.netapp.ontap.plugins.module_utils.netapp as netapp_utils from ansible_collections.netapp.ontap.plugins.module_utils.netapp_module import NetAppModule from ansible_collections.netapp.ontap.plugins.module_utils.netapp import OntapRestAPI +from ansible_collections.netapp.ontap.plugins.module_utils import rest_generic -class NetAppONTAPJob(object): +class NetAppONTAPJob(): '''Class with job schedule cron methods''' def __init__(self): @@ -195,12 +191,11 @@ def __init__(self): self.rest_api = OntapRestAPI(self.module) if self.rest_api.is_rest(): self.use_rest = True + elif netapp_utils.has_netapp_lib(): + self.server = netapp_utils.setup_na_ontap_zapi(module=self.module) else: - if netapp_utils.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) + self.module.fail_json( + msg="the python NetApp-Lib module is required") self.month_offset = self.parameters.get('month_offset') if self.month_offset is None: @@ -224,7 +219,7 @@ def set_playbook_zapi_key_map(self): } def set_playbook_api_key_map(self): - self.na_helper.api_list_keys = { + self.na_helper.params_to_rest_api_keys = { 'job_minutes': 'minutes', 'job_months': 'months', 'job_hours': 'hours', @@ -232,6 +227,34 @@ def set_playbook_api_key_map(self): 'job_days_of_week': 'weekdays' } + def get_job_schedule_rest(self): + """ + Return details about the job + :param: + name : Job name + :return: Details about the Job. None if not found. + :rtype: dict + """ + query = {'name': self.parameters['name']} + record, error = rest_generic.get_one_record(self.rest_api, 'cluster/schedules', query, 'uuid,cron') + if error is not None: + self.module.fail_json(msg="Error on fetching job schedule: %s" % error) + if record: + self.uuid = record['uuid'] + job_details = {'name': record['name']} + for param_key, rest_key in self.na_helper.params_to_rest_api_keys.items(): + if rest_key in record['cron']: + job_details[param_key] = record['cron'][rest_key] + # adjust offsets if necessary + if 'job_months' in job_details and self.month_offset == 0: + job_details['job_months'] = [x - 1 for x in job_details['job_months']] + # adjust minutes if necessary, -1 means all in ZAPI and for our user facing parameters + # while REST returns all values + if 'job_minutes' in job_details and len(job_details['job_minutes']) == 60: + job_details['job_minutes'] = [-1] + return job_details + return None + def get_job_schedule(self): """ Return details about the job @@ -241,54 +264,38 @@ def get_job_schedule(self): :rtype: dict """ if self.use_rest: - params = {'name': self.parameters['name']} - api = '/cluster/schedules' - message, error = self.rest_api.get(api, params) - if error is not None: - self.module.fail_json(msg="Error on fetching job schedule: %s" % error) - if message['num_records'] > 0: - self.uuid = message['records'][0]['uuid'] - job_details = dict() - job_details['name'] = message['records'][0]['name'] - for key, value in self.na_helper.api_list_keys.items(): - if value in message['records'][0]['cron']: - job_details[key] = message['records'][0]['cron'][value] - # adjust offsets if necessary - if 'job_months' in job_details and self.month_offset == 0: - job_details['job_months'] = [x - 1 for x in job_details['job_months']] - return job_details + return self.get_job_schedule_rest() - else: - job_get_iter = netapp_utils.zapi.NaElement('job-schedule-cron-get-iter') - job_get_iter.translate_struct({ - 'query': { - 'job-schedule-cron-info': { - 'job-schedule-name': self.parameters['name'] - } + job_get_iter = netapp_utils.zapi.NaElement('job-schedule-cron-get-iter') + job_get_iter.translate_struct({ + 'query': { + 'job-schedule-cron-info': { + 'job-schedule-name': self.parameters['name'] } - }) - result = self.server.invoke_successfully(job_get_iter, True) - job_details = None - # check if job exists - if result.get_child_by_name('num-records') and int(result['num-records']) >= 1: - job_info = result['attributes-list']['job-schedule-cron-info'] - job_details = dict() - for item_key, zapi_key in self.na_helper.zapi_string_keys.items(): - job_details[item_key] = job_info[zapi_key] - for item_key, zapi_key in self.na_helper.zapi_list_keys.items(): - parent, dummy = zapi_key - job_details[item_key] = self.na_helper.get_value_for_list(from_zapi=True, - zapi_parent=job_info.get_child_by_name(parent) - ) - if item_key == 'job_months' and self.month_offset == 1: - job_details[item_key] = [int(x) + 1 for x in job_details[item_key]] - else: - job_details[item_key] = [int(x) for x in job_details[item_key]] - # if any of the job_hours, job_minutes, job_months, job_days are empty: - # it means the value is -1 for ZAPI - if not job_details[item_key]: - job_details[item_key] = [-1] - return job_details + } + }) + result = self.server.invoke_successfully(job_get_iter, True) + job_details = None + # check if job exists + if result.get_child_by_name('num-records') and int(result['num-records']) >= 1: + job_info = result['attributes-list']['job-schedule-cron-info'] + job_details = {} + for item_key, zapi_key in self.na_helper.zapi_string_keys.items(): + job_details[item_key] = job_info[zapi_key] + for item_key, zapi_key in self.na_helper.zapi_list_keys.items(): + parent, dummy = zapi_key + job_details[item_key] = self.na_helper.get_value_for_list(from_zapi=True, + zapi_parent=job_info.get_child_by_name(parent) + ) + if item_key == 'job_months' and self.month_offset == 1: + job_details[item_key] = [int(x) + 1 for x in job_details[item_key]] + else: + job_details[item_key] = [int(x) for x in job_details[item_key]] + # if any of the job_hours, job_minutes, job_months, job_days are empty: + # it means the value is -1 for ZAPI + if not job_details[item_key]: + job_details[item_key] = [-1] + return job_details def add_job_details(self, na_element_object, values): """ @@ -319,30 +326,25 @@ def create_job_schedule(self): """ Creates a job schedule """ - # job_minutes is mandatory for create - if self.parameters.get('job_minutes') is None: - self.module.fail_json(msg='Error: missing required parameter job_minutes for create') - if self.use_rest: - cron = dict() - for key, value in self.na_helper.api_list_keys.items(): + cron = {} + for param_key, rest_key in self.na_helper.params_to_rest_api_keys.items(): # -1 means all in zapi, while empty means all in api. - if self.parameters.get(key): - if len(self.parameters[key]) == 1 and self.parameters[key][0] == -1: + if self.parameters.get(param_key): + if len(self.parameters[param_key]) == 1 and self.parameters[param_key][0] == -1: # need to set empty value for minutes as this is required parameter - if value == 'minutes': - cron[value] = [] + if rest_key == 'minutes': + cron[rest_key] = [] + elif param_key == 'job_months' and self.month_offset == 0: + cron[rest_key] = [x + 1 for x in self.parameters[param_key]] else: - if key == 'job_months' and self.month_offset == 0: - cron[value] = [x + 1 for x in self.parameters[key]] - else: - cron[value] = self.parameters[key] + cron[rest_key] = self.parameters[param_key] params = { 'name': self.parameters['name'], 'cron': cron } - api = '/cluster/schedules' + api = 'cluster/schedules' dummy, error = self.rest_api.post(api, params) if error is not None: self.module.fail_json(msg="Error on creating job schedule: %s" % error) @@ -363,7 +365,7 @@ def delete_job_schedule(self): Delete a job schedule """ if self.use_rest: - api = '/cluster/schedules/' + self.uuid + api = 'cluster/schedules/' + self.uuid dummy, error = self.rest_api.delete(api) if error is not None: self.module.fail_json(msg="Error on deleting job schedule: %s" % error) @@ -383,25 +385,23 @@ def modify_job_schedule(self, params, current): modify a job schedule """ if self.use_rest: - cron = dict() - for key, value in self.na_helper.api_list_keys.items(): + cron = {} + for param_key, rest_key in self.na_helper.params_to_rest_api_keys.items(): # -1 means all in zapi, while empty means all in api. - if params.get(key): - if len(self.parameters[key]) == 1 and self.parameters[key][0] == -1: - pass - else: - if key == 'job_months' and self.month_offset == 0: - cron[value] = [x + 1 for x in self.parameters[key]] + if params.get(param_key): + if self.parameters[param_key] != [-1]: + if param_key == 'job_months' and self.month_offset == 0: + cron[rest_key] = [x + 1 for x in self.parameters[param_key]] else: - cron[value] = self.parameters[key] + cron[rest_key] = self.parameters[param_key] # Usually only include modify attributes, but omitting an attribute means all in api. # Need to add the current attributes in params. - elif current.get(key): - cron[value] = current[key] + elif current.get(param_key): + cron[rest_key] = current[param_key] params = { 'cron': cron } - api = '/cluster/schedules/' + self.uuid + api = 'cluster/schedules/' + self.uuid dummy, error = self.rest_api.patch(api, params) if error is not None: self.module.fail_json(msg="Error on modifying job schedule: %s" % error) @@ -435,6 +435,10 @@ def apply(self): action = self.na_helper.get_cd_action(current, self.parameters) if action is None and self.parameters['state'] == 'present': modify = self.na_helper.get_modified_attributes(current, self.parameters) + if action == 'create' and self.parameters.get('job_minutes') is None: + # job_minutes is mandatory for create + self.module.fail_json(msg='Error: missing required parameter job_minutes for create') + if self.na_helper.changed and not self.module.check_mode: if action == 'create': self.create_job_schedule() diff --git a/plugins/modules/na_ontap_ntp.py b/plugins/modules/na_ontap_ntp.py index 96d67dd4..66a71410 100644 --- a/plugins/modules/na_ontap_ntp.py +++ b/plugins/modules/na_ontap_ntp.py @@ -62,6 +62,9 @@ from ansible.module_utils._text import to_native import ansible_collections.netapp.ontap.plugins.module_utils.netapp as netapp_utils from ansible_collections.netapp.ontap.plugins.module_utils.netapp_module import NetAppModule +from ansible_collections.netapp.ontap.plugins.module_utils.netapp import OntapRestAPI +from ansible_collections.netapp.ontap.plugins.module_utils import rest_generic +import ansible_collections.netapp.ontap.plugins.module_utils.rest_response_helpers as rrh HAS_NETAPP_LIB = netapp_utils.has_netapp_lib() @@ -84,11 +87,15 @@ def __init__(self): self.na_helper = NetAppModule() self.parameters = self.na_helper.set_parameters(self.module.params) - 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) + self.rest_api = OntapRestAPI(self.module) + self.use_rest = self.rest_api.is_rest() + + if not self.use_rest: + 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) def get_ntp_server(self): """ @@ -98,6 +105,8 @@ def get_ntp_server(self): :return: Details about the ntp server. None if not found. :rtype: dict """ + if self.use_rest: + return self.get_ntp_server_rest() ntp_iter = netapp_utils.zapi.NaElement('ntp-server-get-iter') ntp_info = netapp_utils.zapi.NaElement('ntp-server-info') ntp_info.add_new_child('server-name', self.parameters['server_name']) @@ -125,10 +134,21 @@ def get_ntp_server(self): return return_value + def get_ntp_server_rest(self): + api = 'cluster/ntp/servers' + options = {'server': self.parameters['server_name'], + 'fields': 'server,version'} + record, error = rest_generic.get_one_record(self.rest_api, api, options) + if error: + self.module.fail_json(msg=error) + return record + def create_ntp_server(self): """ create ntp server. """ + if self.use_rest: + return self.create_ntp_server_rest() ntp_server_create = netapp_utils.zapi.NaElement.create_node_with_children( 'ntp-server-create', **{'server-name': self.parameters['server_name'], 'version': self.parameters['version'] @@ -142,10 +162,22 @@ def create_ntp_server(self): % (self.parameters['server_name'], to_native(error)), exception=traceback.format_exc()) + def create_ntp_server_rest(self): + api = 'cluster/ntp/servers' + params = { + 'server': self.parameters['server_name'], + 'version': self.parameters['version'] + } + dummy, error = rest_generic.post_async(self.rest_api, api, params) + if error: + self.module.fail_json(msg=error) + def delete_ntp_server(self): """ delete ntp server. """ + if self.use_rest: + return self.delete_ntp_server_rest() ntp_server_delete = netapp_utils.zapi.NaElement.create_node_with_children( 'ntp-server-delete', **{'server-name': self.parameters['server_name']}) @@ -157,26 +189,40 @@ def delete_ntp_server(self): % (self.parameters['server_name'], to_native(error)), exception=traceback.format_exc()) + def delete_ntp_server_rest(self): + dummy, error = rest_generic.delete_async(self.rest_api, 'cluster/ntp/servers', self.parameters['server_name']) + if error: + self.module.fail_json(msg=error) + def modify_version(self): """ modify the version. """ - ntp_modify_versoin = netapp_utils.zapi.NaElement.create_node_with_children( + if self.use_rest: + return self.modify_version_rest() + ntp_modify_version = netapp_utils.zapi.NaElement.create_node_with_children( 'ntp-server-modify', **{'server-name': self.parameters['server_name'], 'version': self.parameters['version']}) try: - self.server.invoke_successfully(ntp_modify_versoin, + self.server.invoke_successfully(ntp_modify_version, enable_tunneling=True) except netapp_utils.zapi.NaApiError as error: self.module.fail_json(msg='Error modifying version for ntp server %s: %s' % (self.parameters['server_name'], to_native(error)), exception=traceback.format_exc()) + def modify_version_rest(self): + body = {'version': self.parameters['version']} + dummy, error = rest_generic.patch_async(self.rest_api, 'cluster/ntp/servers', self.parameters['server_name'], body) + if error: + self.module.fail_json(msg=error) + def apply(self): """Apply action to ntp-server""" modify = None - netapp_utils.ems_log_event_cserver("na_ontap_ntp", self.server, self.module) + if not self.use_rest: + netapp_utils.ems_log_event_cserver("na_ontap_ntp", self.server, self.module) current = self.get_ntp_server() cd_action = self.na_helper.get_cd_action(current, self.parameters) if cd_action is None and self.parameters['state'] == 'present': diff --git a/plugins/modules/na_ontap_ucadapter.py b/plugins/modules/na_ontap_ucadapter.py index 5c5f847b..0211879d 100644 --- a/plugins/modules/na_ontap_ucadapter.py +++ b/plugins/modules/na_ontap_ucadapter.py @@ -205,6 +205,11 @@ def apply(self): changed = False adapter_detail = self.get_adapter() + try: + self.autosupport_log() + except Exception: + pass + def need_to_change(expected, pending, current): if expected is None: return False diff --git a/plugins/modules/na_ontap_volume.py b/plugins/modules/na_ontap_volume.py index 42832c9e..47639138 100644 --- a/plugins/modules/na_ontap_volume.py +++ b/plugins/modules/na_ontap_volume.py @@ -861,7 +861,7 @@ def __init__(self): self.parameters = self.na_helper.check_and_set_parameters(self.module) self.volume_style = None self.volume_created = False - self.issues = list() + self.issues = [] self.sis_keys2zapi_get = dict( efficiency_policy='policy', compression='is-compression-enabled', @@ -1087,9 +1087,9 @@ def create_nas_application_component(self): tiering = self.na_helper.safe_get(self.parameters, ['nas_application_template', 'tiering']) if tiering is not None or self.parameters.get('tiering_policy') is not None: - application_component['tiering'] = dict() + application_component['tiering'] = {} if tiering is None: - tiering = dict() + tiering = {} if 'policy' not in tiering: tiering['policy'] = self.parameters.get('tiering_policy') for attr in ('control', 'policy', 'object_stores'): @@ -1160,7 +1160,7 @@ def create_volume(self): # round off time_out retries = (self.parameters['time_out'] + 5) // 10 is_online = None - errors = list() + errors = [] while not is_online and retries > 0: try: current = self.get_volume() @@ -1512,7 +1512,7 @@ def change_volume_state(self, call_from_delete_vol=False): volume_change_state = netapp_utils.zapi.NaElement.create_node_with_children( vol_state_zapi, **{vol_name_zapi: self.parameters['name']}) - errors = list() + errors = [] if not self.parameters['is_online'] or call_from_delete_vol: # Unmount before offline try: self.server.invoke_successfully(volume_unmount, enable_tunneling=True) @@ -1649,10 +1649,16 @@ def volume_modify_attributes(self, params): failures = result.get_child_by_name('failure-list') # handle error if modify space, policy, or unix-permissions parameter fails if failures is not None: - error_msgs = list() - for return_info in ('volume-modify-iter-info', 'volume-modify-iter-async-info'): - if failures.get_child_by_name(return_info) is not None: - error_msgs.append(failures.get_child_by_name(return_info).get_child_content('error-message')) + error_msgs = [ + failures.get_child_by_name(return_info).get_child_content( + 'error-message' + ) + for return_info in ( + 'volume-modify-iter-info', + 'volume-modify-iter-async-info', + ) + if failures.get_child_by_name(return_info) is not None + ] if error_msgs and any(x is not None for x in error_msgs): self.wrap_fail_json(msg="Error modifying volume %s: %s" % (self.parameters['name'], ' --- '.join(error_msgs)), @@ -1660,10 +1666,11 @@ def volume_modify_attributes(self, params): 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() + results = {} for key in ('status', 'jobid'): if success and success.get_child_by_name(key): results[key] = success[key] + status = results.get('status') if status == 'in_progress' and 'jobid' in results: if self.parameters['time_out'] == 0: @@ -1738,22 +1745,21 @@ def compare_chmod_value(self, current): desire = self.parameters if current is None: return False - octal_value = '' unix_permissions = desire['unix_permissions'] if unix_permissions.isdigit(): return int(current['unix_permissions']) == int(unix_permissions) - else: - if len(unix_permissions) != 12: - return False - if unix_permissions[:3] != '---': + if len(unix_permissions) != 12: + return False + if unix_permissions[:3] != '---': + return False + octal_value = '' + for i in range(3, len(unix_permissions), 3): + if unix_permissions[i] not in ['r', '-'] or unix_permissions[i + 1] not in ['w', '-']\ + or unix_permissions[i + 2] not in ['x', '-']: return False - for i in range(3, len(unix_permissions), 3): - if unix_permissions[i] not in ['r', '-'] or unix_permissions[i + 1] not in ['w', '-']\ - or unix_permissions[i + 2] not in ['x', '-']: - return False - group_permission = self.char_to_octal(unix_permissions[i:i + 3]) - octal_value += str(group_permission) - return int(current['unix_permissions']) == int(octal_value) + group_permission = self.char_to_octal(unix_permissions[i:i + 3]) + octal_value += str(group_permission) + return int(current['unix_permissions']) == int(octal_value) def char_to_octal(self, chars): """ @@ -1838,21 +1844,21 @@ def check_job_status(self, jobid): elif results['job-state'] in ('queued', 'running'): error = 'job completion exceeded expected timer of: %s seconds' % \ self.parameters['time_out'] + elif results['job-completion'] is not None: + error = results['job-completion'] else: - if results['job-completion'] is not None: - error = results['job-completion'] - else: - error = results['job-progress'] + error = results['job-progress'] return error def check_invoke_result(self, result, action): ''' check invoked api call back result. ''' - results = dict() + results = {} for key in ('result-status', 'result-jobid'): if result.get_child_by_name(key): results[key] = result[key] + status = results.get('result-status') if status == 'in_progress' and 'result-jobid' in results: if self.parameters['time_out'] == 0: @@ -1884,9 +1890,7 @@ def set_efficiency_config(self): self.server.invoke_successfully(efficiency_enable, enable_tunneling=True) except netapp_utils.zapi.NaApiError as error: # Error 40043 denotes an Operation has already been enabled. - if to_native(error.code) == "40043": - pass - else: + if to_native(error.code) != "40043": self.wrap_fail_json(msg='Error enable efficiency on volume %s: %s' % (self.parameters['name'], to_native(error)), exception=traceback.format_exc()) @@ -2081,13 +2085,12 @@ def apply(self): self.na_helper.changed = True else: cd_action = self.na_helper.get_cd_action(current, self.parameters) - if self.parameters.get('unix_permissions') is not None: - # current stores unix_permissions' numeric value. - # unix_permission in self.parameter can be either numeric or character. - if self.compare_chmod_value(current) or not self.parameters['is_online']: - # don't change if the values are the same - # can't change permissions if not online - del self.parameters['unix_permissions'] + if self.parameters.get('unix_permissions') is not None and ( + self.compare_chmod_value(current) or not self.parameters['is_online'] + ): + # don't change if the values are the same + # can't change permissions if not online + del self.parameters['unix_permissions'] if cd_action is None and rename is None and rehost is None and self.parameters['state'] == 'present': modify = self.set_modify_dict(current) if self.parameters.get('nas_application_template') is not None: @@ -2099,35 +2102,32 @@ def apply(self): self.na_helper.changed = changed self.module.warn('Modifying an app is not supported at present: ignoring: %s' % str(modify_app)) - if self.na_helper.changed: - if self.module.check_mode: - pass - else: - if rename: - self.rename_volume() - if rehost: - self.rehost_volume() - if snapshot_restore: - self.snapshot_restore_volume() - if cd_action == 'create': - response = self.create_volume() - # if we create using ZAPI and modify only options are set (snapdir_access or atime_update), we need to run a modify. - # The modify also takes care of efficiency (sis) parameters and snapshot_auto_delete. - # If we create using REST application, some options are not available, we may need to run a modify. - current = self.get_volume() - if current: - self.volume_created = True - modify_after_create = self.set_modify_dict(current, after_create=True) - if modify_after_create: - self.take_modify_actions(modify_after_create) - # restore this, as set_modify_dict could set it to False - self.na_helper.changed = True - elif cd_action == 'delete': - self.parameters['uuid'] = current['uuid'] - self.delete_volume(current) - elif modify: - self.parameters['uuid'] = current['uuid'] - self.take_modify_actions(modify) + if self.na_helper.changed and not self.module.check_mode: + if rename: + self.rename_volume() + if rehost: + self.rehost_volume() + if snapshot_restore: + self.snapshot_restore_volume() + if cd_action == 'create': + response = self.create_volume() + # if we create using ZAPI and modify only options are set (snapdir_access or atime_update), we need to run a modify. + # The modify also takes care of efficiency (sis) parameters and snapshot_auto_delete. + # If we create using REST application, some options are not available, we may need to run a modify. + current = self.get_volume() + if current: + self.volume_created = True + modify_after_create = self.set_modify_dict(current, after_create=True) + if modify_after_create: + self.take_modify_actions(modify_after_create) + # restore this, as set_modify_dict could set it to False + self.na_helper.changed = True + elif cd_action == 'delete': + self.parameters['uuid'] = current['uuid'] + self.delete_volume(current) + elif modify: + self.parameters['uuid'] = current['uuid'] + self.take_modify_actions(modify) result = dict( changed=self.na_helper.changed diff --git a/tests/unit/plugins/modules/test_na_ontap_job_schedule.py b/tests/unit/plugins/modules/test_na_ontap_job_schedule.py index b7c1c469..13a2eb6e 100644 --- a/tests/unit/plugins/modules/test_na_ontap_job_schedule.py +++ b/tests/unit/plugins/modules/test_na_ontap_job_schedule.py @@ -95,7 +95,7 @@ def __init__(self, kind=None, data=None): self.params = data self.xml_in = None self.xml_out = None - self.call_log = list() + self.call_log = [] def invoke_successfully(self, xml, enable_tunneling): # pylint: disable=unused-argument ''' mock invoke_successfully returning xml data ''' @@ -105,6 +105,8 @@ def invoke_successfully(self, xml, enable_tunneling): # pylint: disable=unused- xml = self.build_job_schedule_cron_info(self.params) elif self.kind == 'job_multiple': xml = self.build_job_schedule_multiple_cron_info(self.params) + elif self.kind == 'no_job': + xml = self.build_job_schedule_None_info(self.params) self.xml_out = xml return xml @@ -130,6 +132,16 @@ def build_job_schedule_cron_info(job_details): xml.translate_struct(attributes) return xml + @staticmethod + def build_job_schedule_None_info(job_details): + ''' build xml data for vserser-info ''' + xml = netapp_utils.zapi.NaElement('xml') + attributes = { + 'num-records': '0' + } + xml.translate_struct(attributes) + return xml + @staticmethod def build_job_schedule_multiple_cron_info(job_details): ''' build xml data for vserser-info ''' @@ -266,7 +278,7 @@ def test_create_error_missing_param(self): del data['job_minutes'] set_module_args(data) with pytest.raises(AnsibleFailJson) as exc: - self.get_job_mock_object('job').create_job_schedule() + self.get_job_mock_object('no_job').apply() msg = 'Error: missing required parameter job_minutes for create' assert exc.value.args[0]['msg'] == msg diff --git a/tests/unit/plugins/modules/test_na_ontap_ntp.py b/tests/unit/plugins/modules/test_na_ontap_ntp.py new file mode 100644 index 00000000..214cef2b --- /dev/null +++ b/tests/unit/plugins/modules/test_na_ontap_ntp.py @@ -0,0 +1,182 @@ +# (c) 2018-2021, NetApp, Inc +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +''' unit test for ONTAP snmp Ansible module ''' + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type +import json +import pytest + +from ansible.module_utils import basic +from ansible.module_utils._text import to_bytes +from ansible_collections.netapp.ontap.tests.unit.compat.mock import patch + +from ansible_collections.netapp.ontap.plugins.modules.na_ontap_ntp \ + import NetAppOntapNTPServer as my_module, main as uut_main # module under test + + +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""" + + +class AnsibleFailJson(Exception): + """Exception class to be raised by module.fail_json and caught by the test case""" + + +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) + + +WARNINGS = list() + + +def warn(dummy, msg): + WARNINGS.append(msg) + + +def default_args(): + return { + 'state': 'present', + 'hostname': '10.10.10.10', + 'username': 'admin', + 'https': 'true', + 'validate_certs': 'false', + 'password': 'password', + 'use_rest': 'always' + } + + +# REST API canned responses when mocking send_request +SRR = { + # common responses + 'is_rest': (200, dict(version=dict(generation=9, major=9, minor=0, full='dummy')), None), + 'is_rest_9_6': (200, dict(version=dict(generation=9, major=6, minor=0, full='dummy')), None), + 'is_rest_9_8': (200, dict(version=dict(generation=9, major=8, minor=0, full='dummy')), None), + 'is_zapi': (400, {}, "Unreachable"), + 'empty_good': (200, {}, None), + 'zero_record': (200, dict(records=[], num_records=0), None), + 'one_record_uuid': (200, dict(records=[dict(uuid='a1b2c3')], num_records=1), None), + 'end_of_sequence': (500, None, "Unexpected call to send_request"), + 'generic_error': (400, None, "Expected error"), + 'server_record': (200, { + "records": [{ + "server": "0.0.0.0", + "version": "auto", + }], + 'num_records': 1 + }, None), + 'create_server': (200, { + 'job': { + 'uuid': 'fde79888-692a-11ea-80c2-005056b39fe7', + '_links': { + 'self': { + 'href': '/api/cluster/jobs/fde79888-692a-11ea-80c2-005056b39fe7'}}} + }, None), + 'job': (200, { + "uuid": "fde79888-692a-11ea-80c2-005056b39fe7", + "state": "success", + "start_time": "2020-02-26T10:35:44-08:00", + "end_time": "2020-02-26T10:47:38-08:00", + "_links": { + "self": { + "href": "/api/cluster/jobs/fde79888-692a-11ea-80c2-005056b39fe7" + } + } + }, None) +} + + +# using pytest natively, without unittest.TestCase +@pytest.fixture +def patch_ansible(): + with patch.multiple(basic.AnsibleModule, + exit_json=exit_json, + fail_json=fail_json, + warn=warn) as mocks: + global WARNINGS + WARNINGS = [] + yield mocks + + +def test_module_fail_when_required_args_missing(patch_ansible): + ''' required arguments are reported as errors ''' + with pytest.raises(AnsibleFailJson) as exc: + set_module_args(dict(hostname='')) + my_module() + print('Info: %s' % exc.value.args[0]['msg']) + msg = 'missing required arguments: server_name' + assert msg == exc.value.args[0]['msg'] + + +@patch('ansible_collections.netapp.ontap.plugins.module_utils.netapp.OntapRestAPI.send_request') +def test_ensure_get_server_called(mock_request, patch_ansible): + args = dict(default_args()) + args['server_name'] = '0.0.0.0' + set_module_args(args) + mock_request.side_effect = [ + SRR['is_rest_9_8'], # get version + SRR['server_record'], # get + SRR['end_of_sequence'] + ] + my_obj = my_module() + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + print('Info: %s' % exc.value.args[0]) + assert exc.value.args[0]['changed'] is False + assert not WARNINGS + + +@patch('ansible_collections.netapp.ontap.plugins.module_utils.netapp.OntapRestAPI.send_request') +def test_ensure_create_server_called(mock_request, patch_ansible): + args = dict(default_args()) + args['server_name'] = '0.0.0.0' + set_module_args(args) + mock_request.side_effect = [ + SRR['is_rest_9_8'], # get version + SRR['zero_record'], # get + SRR['create_server'], # create + SRR['job'], # Job + SRR['end_of_sequence'] + ] + my_obj = my_module() + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + print('Info: %s' % exc.value.args[0]) + assert exc.value.args[0]['changed'] is True + assert not WARNINGS + + +@patch('ansible_collections.netapp.ontap.plugins.module_utils.netapp.OntapRestAPI.send_request') +def test_ensure_delete_server_called(mock_request, patch_ansible): + args = dict(default_args()) + args['server_name'] = '0.0.0.0' + args['state'] = 'absent' + set_module_args(args) + mock_request.side_effect = [ + SRR['is_rest_9_8'], # get version + SRR['server_record'], # get + SRR['empty_good'], # delete + SRR['end_of_sequence'] + ] + my_obj = my_module() + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + print('Info: %s' % exc.value.args[0]) + assert exc.value.args[0]['changed'] is True + assert not WARNINGS