From ad91793428736f7bb1b9e3e89f82de944cdab2fb Mon Sep 17 00:00:00 2001 From: Michael Price Date: Fri, 24 Aug 2018 09:44:59 -0500 Subject: [PATCH] Resolve issues in NetApp E-Series Host module (#39748) * Resolve issues in NetApp E-Series Host module The E-Series host module had some bugs relating to the update/creation of host definitions when iSCSI initiators when included in the configuration. This patch resolves this and other minor issues with correctly detecting updates. There were also several minor issues found that were causing issues with truly idepotent updates/changes to the host definition. This patch also provides some unit tests and integration tests to help catch future issues in these areas. fixes #28272 * Improve NetApp E-Series Host module testing The NetApp E-Series Host module integration test lacked feature test verification to verify the changes made to the storage array. The NetApp E-Series rest api was used to verify host create, update, and remove changes made to the NetApp E-Series storage arrays. --- .../modules/storage/netapp/netapp_e_host.py | 442 +++++++++++++----- .../targets/netapp_eseries_host/aliases | 10 + .../netapp_eseries_host/tasks/main.yml | 1 + .../targets/netapp_eseries_host/tasks/run.yml | 276 +++++++++++ test/sanity/validate-modules/ignore.txt | 2 - test/units/modules/storage/__init__.py | 0 test/units/modules/storage/netapp/__init__.py | 0 .../storage/netapp/test_netapp_e_host.py | 190 ++++++++ 8 files changed, 807 insertions(+), 114 deletions(-) create mode 100644 test/integration/targets/netapp_eseries_host/aliases create mode 100644 test/integration/targets/netapp_eseries_host/tasks/main.yml create mode 100644 test/integration/targets/netapp_eseries_host/tasks/run.yml create mode 100644 test/units/modules/storage/__init__.py create mode 100644 test/units/modules/storage/netapp/__init__.py create mode 100644 test/units/modules/storage/netapp/test_netapp_e_host.py diff --git a/lib/ansible/modules/storage/netapp/netapp_e_host.py b/lib/ansible/modules/storage/netapp/netapp_e_host.py index 8775a0aa7b5..920685ee9e5 100644 --- a/lib/ansible/modules/storage/netapp/netapp_e_host.py +++ b/lib/ansible/modules/storage/netapp/netapp_e_host.py @@ -1,17 +1,16 @@ #!/usr/bin/python - -# (c) 2016, NetApp, Inc +# -*- coding: utf-8 -*- +# +# (c) 2018, NetApp Inc. # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) - from __future__ import absolute_import, division, print_function -__metaclass__ = type +__metaclass__ = type ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], 'supported_by': 'community'} - DOCUMENTATION = """ --- module: netapp_e_host @@ -25,44 +24,141 @@ extends_documentation_fragment: options: name: description: - - If the host doesn't yet exist, the label to assign at creation time. - - If the hosts already exists, this is what is used to identify the host to apply any desired changes + - If the host doesn't yet exist, the label/name to assign at creation time. + - If the hosts already exists, this will be used to uniquely identify the host to make any required changes required: True + aliases: + - label + state: + description: + - Set to absent to remove an existing host + - Set to present to modify or create a new host definition + choices: + - absent + - present + default: present + version_added: 2.7 host_type_index: description: - The index that maps to host type you wish to create. It is recommended to use the M(netapp_e_facts) module to gather this information. Alternatively you can use the WSP portal to retrieve the information. - required: True + - Required when C(state=present) + aliases: + - host_type ports: description: - - a list of of dictionaries of host ports you wish to associate with the newly created host + - A list of host ports you wish to associate with the host. + - Host ports are uniquely identified by their WWN or IQN. Their assignments to a particular host are + uniquely identified by a label and these must be unique. required: False + suboptions: + type: + description: + - The interface type of the port to define. + - Acceptable choices depend on the capabilities of the target hardware/software platform. + required: true + choices: + - iscsi + - sas + - fc + - ib + - nvmeof + - ethernet + label: + description: + - A unique label to assign to this port assignment. + required: true + port: + description: + - The WWN or IQN of the hostPort to assign to this port definition. + required: true + force_port: + description: + - Allow ports that are already assigned to be re-assigned to your current host + required: false + type: bool + version_added: 2.7 group: description: - - the group you want the host to be a member of + - The unique identifier of the host-group you want the host to be a member of; this is used for clustering. required: False + aliases: + - cluster + log_path: + description: + - A local path to a file to be used for debug logging + required: False + version_added: 2.7 """ EXAMPLES = """ - - name: Set Host Info + - name: Define or update an existing host named 'Host1' + netapp_e_host: + ssid: "1" + api_url: "10.113.1.101:8443" + api_username: "admin" + api_password: "myPassword" + name: "Host1" + state: present + host_type_index: 28 + ports: + - type: 'iscsi' + label: 'PORT_1' + port: 'iqn.1996-04.de.suse:01:56f86f9bd1fe' + - type: 'fc' + label: 'FC_1' + port: '10:00:FF:7C:FF:FF:FF:01' + - type: 'fc' + label: 'FC_2' + port: '10:00:FF:7C:FF:FF:FF:00' + + - name: Ensure a host named 'Host2' doesn't exist netapp_e_host: - ssid: "{{ ssid }}" - api_url: "{{ netapp_api_url }}" - api_username: "{{ netapp_api_username }}" - api_password: "{{ netapp_api_password }}" - name: "{{ host_name }}" - host_type_index: "{{ host_type_index }}" + ssid: "1" + api_url: "10.113.1.101:8443" + api_username: "admin" + api_password: "myPassword" + name: "Host2" + state: absent + """ RETURN = """ msg: - description: Success message - returned: success + description: + - A user-readable description of the actions performed. + returned: on success type: string sample: The host has been created. +id: + description: + - the unique identifier of the host on the E-Series storage-system + returned: on success when state=present + type: string + sample: 00000000600A098000AAC0C3003004700AD86A52 + version_added: "2.6" + +ssid: + description: + - the unique identifer of the E-Series storage-system with the current api + returned: on success + type: string + sample: 1 + version_added: "2.6" + +api_url: + description: + - the url of the API that this request was proccessed by + returned: on success + type: string + sample: https://webservices.example.com:8443 + version_added: "2.6" + """ import json +import logging +from pprint import pformat from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.netapp import request, eseries_host_argument_spec @@ -78,15 +174,22 @@ class Host(object): def __init__(self): argument_spec = eseries_host_argument_spec() argument_spec.update(dict( - state=dict(type='str', required=True, choices=['absent', 'present']), - group=dict(type='str', required=False), + state=dict(type='str', default='present', choices=['absent', 'present']), + group=dict(type='str', required=False, aliases=['cluster']), ports=dict(type='list', required=False), force_port=dict(type='bool', default=False), - name=dict(type='str', required=True), - host_type_index=dict(type='int', required=True) + name=dict(type='str', required=True, aliases=['label']), + host_type_index=dict(type='int', aliases=['host_type']), + log_path=dict(type='str', required=False), )) - self.module = AnsibleModule(argument_spec=argument_spec) + required_if = [ + ["state", "absent", ["name"]], + ["state", "present", ["name", "host_type"]] + ] + + self.module = AnsibleModule(argument_spec=argument_spec, supports_check_mode=True, required_if=required_if) + self.check_mode = self.module.check_mode args = self.module.params self.group = args['group'] self.ports = args['ports'] @@ -99,12 +202,32 @@ class Host(object): self.user = args['api_username'] self.pwd = args['api_password'] self.certs = args['validate_certs'] - self.ports = args['ports'] self.post_body = dict() + self.all_hosts = list() + self.newPorts = list() + self.portsForUpdate = list() + self.force_port_update = False + + log_path = args['log_path'] + + # logging setup + self._logger = logging.getLogger(self.__class__.__name__) + + if log_path: + logging.basicConfig( + level=logging.DEBUG, filename=log_path, filemode='w', + format='%(relativeCreated)dms %(levelname)s %(module)s.%(funcName)s:%(lineno)d\n %(message)s') + if not self.url.endswith('/'): self.url += '/' + # Fix port representation if they are provided with colons + if self.ports is not None: + for port in self.ports: + if port['type'] != 'iscsi': + port['port'] = port['port'].replace(':', '') + @property def valid_host_type(self): try: @@ -115,40 +238,25 @@ class Host(object): msg="Failed to get host types. Array Id [%s]. Error [%s]." % (self.ssid, to_native(err))) try: - match = filter(lambda host_type: host_type['index'] == self.host_type_index, host_types)[0] + match = list(filter(lambda host_type: host_type['index'] == self.host_type_index, host_types))[0] return True except IndexError: self.module.fail_json(msg="There is no host type with index %s" % self.host_type_index) @property - def hostports_available(self): - used_ids = list() - try: - (rc, self.available_ports) = request(self.url + 'storage-systems/%s/unassociated-host-ports' % self.ssid, - url_password=self.pwd, url_username=self.user, - validate_certs=self.certs, - headers=HEADERS) - except Exception as err: - self.module.fail_json( - msg="Failed to get unassociated host ports. Array Id [%s]. Error [%s]." % (self.ssid, to_native(err))) - - if len(self.available_ports) > 0 and len(self.ports) <= len(self.available_ports): - for port in self.ports: - for free_port in self.available_ports: - # Desired Type matches but also make sure we haven't already used the ID - if not free_port['id'] in used_ids: - # update the port arg to have an id attribute - used_ids.append(free_port['id']) - break - - if len(used_ids) != len(self.ports) and not self.force_port: - self.module.fail_json( - msg="There are not enough free host ports with the specified port types to proceed") - else: - return True - - else: - self.module.fail_json(msg="There are no host ports available OR there are not enough unassigned host ports") + def host_ports_available(self): + """Determine if the hostPorts requested have already been assigned""" + for host in self.all_hosts: + if host['label'] != self.name: + for host_port in host['hostSidePorts']: + for port in self.ports: + if (port['port'] == host_port['address'] or port['label'] == host_port['label']): + if not self.force_port: + self.module.fail_json( + msg="There are no host ports available OR there are not enough unassigned host ports") + else: + return False + return True @property def group_id(self): @@ -162,7 +270,7 @@ class Host(object): msg="Failed to get host groups. Array Id [%s]. Error [%s]." % (self.ssid, to_native(err))) try: - group_obj = filter(lambda group: group['name'] == self.group, all_groups)[0] + group_obj = list(filter(lambda group: group['name'] == self.group, all_groups))[0] return group_obj['id'] except IndexError: self.module.fail_json(msg="No group with the name: %s exists" % self.group) @@ -172,6 +280,10 @@ class Host(object): @property def host_exists(self): + """Determine if the requested host exists + As a side effect, set the full list of defined hosts in 'all_hosts', and the target host in 'host_obj'. + """ + all_hosts = list() try: (rc, all_hosts) = request(self.url + 'storage-systems/%s/hosts' % self.ssid, url_password=self.pwd, url_username=self.user, validate_certs=self.certs, headers=HEADERS) @@ -180,8 +292,20 @@ class Host(object): msg="Failed to determine host existence. Array Id [%s]. Error [%s]." % (self.ssid, to_native(err))) self.all_hosts = all_hosts + + # Augment the host objects + for host in all_hosts: + # Augment hostSidePorts with their ID (this is an omission in the API) + host_side_ports = host['hostSidePorts'] + initiators = dict((port['label'], port['id']) for port in host['initiators']) + ports = dict((port['label'], port['id']) for port in host['ports']) + ports.update(initiators) + for port in host_side_ports: + if port['label'] in ports: + port['id'] = ports[port['label']] + try: # Try to grab the host object - self.host_obj = filter(lambda host: host['label'] == self.name, all_hosts)[0] + self.host_obj = list(filter(lambda host: host['label'] == self.name, all_hosts))[0] return True except IndexError: # Host with the name passed in does not exist @@ -189,28 +313,51 @@ class Host(object): @property def needs_update(self): + """Determine whether we need to update the Host object + As a side effect, we will set the ports that we need to update (portsForUpdate), and the ports we need to add + (newPorts), on self. + :return: + """ needs_update = False - self.force_port_update = False - if self.host_obj['clusterRef'] != self.group_id or \ - self.host_obj['hostTypeIndex'] != self.host_type_index: + if self.host_obj['clusterRef'] != self.group_id or self.host_obj['hostTypeIndex'] != self.host_type_index: + self._logger.info("Either hostType or the clusterRef doesn't match, an update is required.") needs_update = True if self.ports: - if not self.host_obj['ports']: + self._logger.debug("Determining if ports need to be updated.") + # Find the names of all defined ports + port_names = set(port['label'] for port in self.host_obj['hostSidePorts']) + port_addresses = set(port['address'] for port in self.host_obj['hostSidePorts']) + + # If we have ports defined and there are no ports on the host object, we need an update + if not self.host_obj['hostSidePorts']: needs_update = True for arg_port in self.ports: # First a quick check to see if the port is mapped to a different host if not self.port_on_diff_host(arg_port): - for obj_port in self.host_obj['ports']: - if arg_port['label'] == obj_port['label']: - # Confirmed that port arg passed in exists on the host - # port_id = self.get_port_id(obj_port['label']) - if arg_port['type'] != obj_port['portId']['ioInterfaceType']: - needs_update = True - if 'iscsiChapSecret' in arg_port: - # No way to know the current secret attr, so always return True just in case - needs_update = True + # The port (as defined), currently does not exist + if arg_port['label'] not in port_names: + needs_update = True + # This port has not been defined on the host at all + if arg_port['port'] not in port_addresses: + self.newPorts.append(arg_port) + # A port label update has been requested + else: + self.portsForUpdate.append(arg_port) + # The port does exist, does it need to be updated? + else: + for obj_port in self.host_obj['hostSidePorts']: + if arg_port['label'] == obj_port['label']: + # Confirmed that port arg passed in exists on the host + # port_id = self.get_port_id(obj_port['label']) + if arg_port['type'] != obj_port['type']: + needs_update = True + self.portsForUpdate.append(arg_port) + if 'iscsiChapSecret' in arg_port: + # No way to know the current secret attr, so always return True just in case + needs_update = True + self.portsForUpdate.append(arg_port) else: # If the user wants the ports to be reassigned, do it if self.force_port: @@ -218,106 +365,165 @@ class Host(object): needs_update = True else: self.module.fail_json( - msg="The port you specified:\n%s\n is associated with a different host. Specify force_port as True or try a different " - "port spec" % arg_port + msg="The port you specified:\n%s\n is associated with a different host. Specify force_port" + " as True or try a different port spec" % arg_port ) - + self._logger.debug("Is an update required ?=%s", needs_update) return needs_update + def get_ports_on_host(self): + """Retrieve the hostPorts that are defined on the target host + :return: a list of hostPorts with their labels and ids + Example: + [ + { + 'name': 'hostPort1', + 'id': '0000000000000000000000' + } + ] + """ + ret = dict() + for host in self.all_hosts: + if host['name'] == self.name: + ports = host['hostSidePorts'] + for port in ports: + ret[port['address']] = {'label': port['label'], 'id': port['id'], 'address': port['address']} + return ret + def port_on_diff_host(self, arg_port): """ Checks to see if a passed in port arg is present on a different host """ for host in self.all_hosts: # Only check 'other' hosts - if self.host_obj['name'] != self.name: - for port in host['ports']: + if host['name'] != self.name: + for port in host['hostSidePorts']: # Check if the port label is found in the port dict list of each host - if arg_port['label'] == port['label']: + if arg_port['label'] == port['label'] or arg_port['port'] == port['address']: self.other_host = host return True return False + def get_port(self, label, address): + for host in self.all_hosts: + for port in host['hostSidePorts']: + if port['label'] == label or port['address'] == address: + return port + def reassign_ports(self, apply=True): - if not self.post_body: - self.post_body = dict( - portsToUpdate=dict() - ) + post_body = dict( + portsToUpdate=dict() + ) for port in self.ports: if self.port_on_diff_host(port): - self.post_body['portsToUpdate'].update(dict( - portRef=self.other_host['hostPortRef'], + host_port = self.get_port(port['label'], port['port']) + post_body['portsToUpdate'].update(dict( + portRef=host_port['id'], hostRef=self.host_obj['id'], + label=port['label'] # Doesn't yet address port identifier or chap secret )) + self._logger.info("reassign_ports: %s", pformat(post_body)) + if apply: try: (rc, self.host_obj) = request( self.url + 'storage-systems/%s/hosts/%s' % (self.ssid, self.host_obj['id']), url_username=self.user, url_password=self.pwd, headers=HEADERS, - validate_certs=self.certs, method='POST', data=json.dumps(self.post_body)) + validate_certs=self.certs, method='POST', data=json.dumps(post_body)) except Exception as err: self.module.fail_json( msg="Failed to reassign host port. Host Id [%s]. Array Id [%s]. Error [%s]." % ( self.host_obj['id'], self.ssid, to_native(err))) + return post_body + def update_host(self): - if self.ports: - if self.hostports_available: - if self.force_port_update is True: - self.reassign_ports(apply=False) - # Make sure that only ports that arent being reassigned are passed into the ports attr - self.ports = [port for port in self.ports if not self.port_on_diff_host(port)] + self._logger.debug("Beginning the update for host=%s.", self.name) - self.post_body['ports'] = self.ports + if self.ports: + self._logger.info("Requested ports: %s", pformat(self.ports)) + if self.host_ports_available or self.force_port: + self.reassign_ports(apply=True) + # Make sure that only ports that aren't being reassigned are passed into the ports attr + host_ports = self.get_ports_on_host() + ports_for_update = list() + self._logger.info("Ports on host: %s", pformat(host_ports)) + for port in self.portsForUpdate: + if port['port'] in host_ports: + defined_port = host_ports.get(port['port']) + defined_port.update(port) + defined_port['portRef'] = defined_port['id'] + ports_for_update.append(defined_port) + self._logger.info("Ports to update: %s", pformat(ports_for_update)) + self._logger.info("Ports to define: %s", pformat(self.newPorts)) + self.post_body['portsToUpdate'] = ports_for_update + self.post_body['ports'] = self.newPorts + else: + self._logger.debug("No host ports were defined.") if self.group: self.post_body['groupId'] = self.group_id self.post_body['hostType'] = dict(index=self.host_type_index) - try: - (rc, self.host_obj) = request(self.url + 'storage-systems/%s/hosts/%s' % (self.ssid, self.host_obj['id']), - url_username=self.user, url_password=self.pwd, headers=HEADERS, - validate_certs=self.certs, method='POST', data=json.dumps(self.post_body)) - except Exception as err: - self.module.fail_json(msg="Failed to update host. Array Id [%s]. Error [%s]." % (self.ssid, to_native(err))) + api = self.url + 'storage-systems/%s/hosts/%s' % (self.ssid, self.host_obj['id']) + self._logger.info("POST => url=%s, body=%s.", api, pformat(self.post_body)) + + if not self.check_mode: + try: + (rc, self.host_obj) = request(api, url_username=self.user, url_password=self.pwd, headers=HEADERS, + validate_certs=self.certs, method='POST', data=json.dumps(self.post_body)) + except Exception as err: + self.module.fail_json( + msg="Failed to update host. Array Id [%s]. Error [%s]." % (self.ssid, to_native(err))) - self.module.exit_json(changed=True, **self.host_obj) + payload = self.build_success_payload(self.host_obj) + self.module.exit_json(changed=True, **payload) def create_host(self): + self._logger.info("Creating host definition.") + needs_reassignment = False post_body = dict( name=self.name, hostType=dict(index=self.host_type_index), groupId=self.group_id, - ports=self.ports ) if self.ports: # Check that all supplied port args are valid - if self.hostports_available: + if self.host_ports_available: + self._logger.info("The host-ports requested are available.") post_body.update(ports=self.ports) elif not self.force_port: self.module.fail_json( - msg="You supplied ports that are already in use. Supply force_port to True if you wish to reassign the ports") + msg="You supplied ports that are already in use." + " Supply force_port to True if you wish to reassign the ports") + else: + needs_reassignment = True + + api = self.url + "storage-systems/%s/hosts" % self.ssid + self._logger.info('POST => url=%s, body=%s', api, pformat(post_body)) - if not self.host_exists: + if not (self.host_exists and self.check_mode): try: - (rc, create_resp) = request(self.url + "storage-systems/%s/hosts" % self.ssid, method='POST', - url_username=self.user, url_password=self.pwd, validate_certs=self.certs, - data=json.dumps(post_body), headers=HEADERS) + (rc, self.host_obj) = request(api, method='POST', + url_username=self.user, url_password=self.pwd, validate_certs=self.certs, + data=json.dumps(post_body), headers=HEADERS) except Exception as err: self.module.fail_json( msg="Failed to create host. Array Id [%s]. Error [%s]." % (self.ssid, to_native(err))) else: + payload = self.build_success_payload(self.host_obj) self.module.exit_json(changed=False, - msg="Host already exists. Id [%s]. Host [%s]." % (self.ssid, self.name)) + msg="Host already exists. Id [%s]. Host [%s]." % (self.ssid, self.name), **payload) - self.host_obj = create_resp - - if self.ports and self.force_port: + self._logger.info("Created host, beginning port re-assignment.") + if needs_reassignment: self.reassign_ports() - self.module.exit_json(changed=True, **self.host_obj) + payload = self.build_success_payload(self.host_obj) + + self.module.exit_json(changed=True, msg='Host created.', **payload) def remove_host(self): try: @@ -326,25 +532,37 @@ class Host(object): url_username=self.user, url_password=self.pwd, validate_certs=self.certs) except Exception as err: self.module.fail_json( - msg="Failed to remote host. Host[%s]. Array Id [%s]. Error [%s]." % (self.host_obj['id'], + msg="Failed to remove host. Host[%s]. Array Id [%s]. Error [%s]." % (self.host_obj['id'], self.ssid, to_native(err))) + def build_success_payload(self, host=None): + keys = ['id'] + if host is not None: + result = dict((key, host[key]) for key in keys) + else: + result = dict() + result['ssid'] = self.ssid + result['api_url'] = self.url + return result + def apply(self): if self.state == 'present': if self.host_exists: if self.needs_update and self.valid_host_type: self.update_host() else: - self.module.exit_json(changed=False, msg="Host already present.", id=self.ssid, label=self.name) + payload = self.build_success_payload(self.host_obj) + self.module.exit_json(changed=False, msg="Host already present; no changes required.", **payload) elif self.valid_host_type: self.create_host() else: + payload = self.build_success_payload() if self.host_exists: self.remove_host() - self.module.exit_json(changed=True, msg="Host removed.") + self.module.exit_json(changed=True, msg="Host removed.", **payload) else: - self.module.exit_json(changed=False, msg="Host already absent.", id=self.ssid, label=self.name) + self.module.exit_json(changed=False, msg="Host already absent.", **payload) def main(): diff --git a/test/integration/targets/netapp_eseries_host/aliases b/test/integration/targets/netapp_eseries_host/aliases new file mode 100644 index 00000000000..d314d14a748 --- /dev/null +++ b/test/integration/targets/netapp_eseries_host/aliases @@ -0,0 +1,10 @@ +# This test is not enabled by default, but can be utilized by defining required variables in integration_config.yml +# Example integration_config.yml: +# --- +#netapp_e_api_host: 10.113.1.111:8443 +#netapp_e_api_username: admin +#netapp_e_api_password: myPass +#netapp_e_ssid: 1 + +unsupported +netapp/eseries diff --git a/test/integration/targets/netapp_eseries_host/tasks/main.yml b/test/integration/targets/netapp_eseries_host/tasks/main.yml new file mode 100644 index 00000000000..996354c8860 --- /dev/null +++ b/test/integration/targets/netapp_eseries_host/tasks/main.yml @@ -0,0 +1 @@ +- include_tasks: run.yml diff --git a/test/integration/targets/netapp_eseries_host/tasks/run.yml b/test/integration/targets/netapp_eseries_host/tasks/run.yml new file mode 100644 index 00000000000..fd0a8d5fa20 --- /dev/null +++ b/test/integration/targets/netapp_eseries_host/tasks/run.yml @@ -0,0 +1,276 @@ +--- +# Test code for the netapp_e_host module +# (c) 2018, NetApp, Inc +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +- name: NetApp Test Host module + fail: + msg: 'Please define netapp_e_api_username, netapp_e_api_password, netapp_e_api_host, and netapp_e_ssid.' + when: netapp_e_api_username is undefined or netapp_e_api_password is undefined or + netapp_e_api_host is undefined or netapp_e_ssid is undefined + vars: + gather_facts: yes + credentials: &creds + api_url: "https://{{ netapp_e_api_host }}/devmgr/v2" + api_username: "{{ netapp_e_api_username }}" + api_password: "{{ netapp_e_api_password }}" + ssid: "{{ netapp_e_ssid }}" + validate_certs: no + hosts: &hosts + 1: + host_type: 27 + update_host_type: 28 + ports: + - type: 'iscsi' + label: 'I_1' + port: 'iqn.1996-04.de.suse:01:56f86f9bd1fe' + - type: 'iscsi' + label: 'I_2' + port: 'iqn.1996-04.de.suse:01:56f86f9bd1ff' + ports2: + - type: 'iscsi' + label: 'I_1' + port: 'iqn.1996-04.de.suse:01:56f86f9bd1fe' + - type: 'iscsi' + label: 'I_2' + port: 'iqn.1996-04.de.suse:01:56f86f9bd1ff' + - type: 'fc' + label: 'FC_3' + port: '10:00:8C:7C:FF:1A:B9:01' + 2: + host_type: 27 + update_host_type: 28 + ports: + - type: 'fc' + label: 'FC_1' + port: '10:00:8C:7C:FF:1A:B9:01' + - type: 'fc' + label: 'FC_2' + port: '10:00:8C:7C:FF:1A:B9:00' + ports2: + - type: 'fc' + label: 'FC_6' + port: '10:00:8C:7C:FF:1A:B9:01' + - type: 'fc' + label: 'FC_4' + port: '10:00:8C:7C:FF:1A:B9:00' + + +# ******************************************** +# *** Ensure jmespath package is installed *** +# ******************************************** +# NOTE: jmespath must be installed for the json_query filter +- name: Ensure that jmespath is installed + pip: + name: jmespath + state: present + register: jmespath +- fail: + msg: "Restart playbook, the jmespath package was installed and is need for the playbook's execution." + when: jmespath.changed + + +# ***************************************** +# *** Set credential and host variables *** +# ***************************************** +- name: Set hosts variable + set_fact: + hosts: *hosts +- name: set credentials + set_fact: + credentials: *creds +- name: Show some debug information + debug: + msg: "Using user={{ credentials.api_username }} on server={{ credentials.api_url }}." + +# *** Remove any existing hosts to set initial state and verify state *** +- name: Remove any existing hosts + netapp_e_host: + <<: *creds + state: absent + name: "{{ item.key }}" + with_dict: *hosts + +# Retrieve array host definitions +- name: HTTP request for all host definitions from array + uri: + url: "{{ credentials.api_url }}/storage-systems/{{ credentials.ssid }}/hosts" + user: "{{ credentials.api_username }}" + password: "{{ credentials.api_password }}" + body_format: json + validate_certs: no + register: result + +# Verify that host 1 and 2 host objects do not exist +- name: Collect host side port labels + set_fact: + host_labels: "{{ result | json_query('json[*].label') }}" +- name: Assert hosts were removed + assert: + that: "'{{ item.key }}' not in host_labels" + msg: "Host, {{ item.key }}, failed to be removed from the hosts!" + loop: "{{ lookup('dict', hosts) }}" + + +# ***************************************************************** +# *** Create host definitions and validate host object creation *** +# ***************************************************************** +- name: Define hosts + netapp_e_host: + <<: *creds + state: present + host_type: "{{ item.value.host_type }}" + ports: "{{ item.value.ports }}" + name: "{{ item.key }}" + with_dict: *hosts + +# Retrieve array host definitions +- name: https request to validate host definitions were created + uri: + url: "{{ credentials.api_url }}/storage-systems/{{ credentials.ssid }}/hosts" + user: "{{ credentials.api_username }}" + password: "{{ credentials.api_password }}" + body_format: json + validate_certs: no + register: result + +# Verify hosts were indeed created +- name: Collect host label list + set_fact: + hosts_labels: "{{ result | json_query('json[*].label') }}" +- name: Validate hosts were in fact created + assert: + that: "'{{ item.key }}' in hosts_labels" + msg: "host, {{ item.key }}, not define on array!" + loop: "{{ lookup('dict', hosts) }}" + +# *** Update with no state changes results in no changes *** +- name: Redefine hosts, expecting no changes + netapp_e_host: + <<: *creds + state: present + host_type: "{{ item.value.host_type }}" + ports: "{{ item.value.ports }}" + name: "{{ item.key }}" + with_dict: *hosts + register: result + +# Verify that no changes occurred +- name: Ensure no change occurred + assert: + msg: "A change was not detected!" + that: "not result.changed" + + +# *********************************************************************************** +# *** Redefine hosts using ports2 host definitions and validate the updated state *** +# *********************************************************************************** +- name: Redefine hosts, expecting changes + netapp_e_host: + <<: *creds + state: present + host_type: "{{ item.value.host_type }}" + ports: "{{ item.value.ports2 }}" + name: "{{ item.key }}" + force_port: yes + with_dict: *hosts + register: result + +# Request from the array all host definitions +- name: HTTP request for port information + uri: + url: "{{ credentials.api_url }}/storage-systems/{{ credentials.ssid }}/hosts" + user: "{{ credentials.api_username }}" + password: "{{ credentials.api_password }}" + body_format: json + validate_certs: no + register: result + +# Compile a list of array host port information for verifying changes +- name: Compile array host port information list + set_fact: + tmp: [] + +# Append each loop to the previous extraction. Each loop consists of host definitions and the filters will perform +# the following: grab host side port lists; combine to each list a dictionary containing the host name(label); +# lastly, convert the zip_longest object into a list +- set_fact: + tmp: "{{ tmp }} + {{ [item | json_query('hostSidePorts[*]')] | + zip_longest([], fillvalue={'host_name': item.label}) | list }}" + loop: "{{ result.json }}" + +# Make new list, port_info, by combining each list entry's dictionaries into a single dictionary +- name: Create port information list + set_fact: + port_info: [] +- set_fact: + port_info: "{{ port_info }} + [{{ item[0] |combine(item[1]) }}]" + loop: "{{ tmp }}" + +# Compile list of expected host port information for verifying changes +- name: Create expected port information list + set_fact: + tmp: [] + +# Append each loop to the previous extraction. Each loop consists of host definitions and the filters will perform +# the following: grab host side port lists; combine to each list a dictionary containing the host name(label); +# lastly, convert the zip_longest object into a list +- set_fact: + tmp: "{{ tmp }} + {{ [item | json_query('value.ports2[*]')]| + zip_longest([], fillvalue={'host_name': item.key|string}) | list }}" + loop: "{{ lookup('dict', hosts) }}" + +# Make new list, expected_port_info, by combining each list entry's dictionaries into a single dictionary +- name: Create expected port information list + set_fact: + expected_port_info: [] +- set_fact: + expected_port_info: "{{ expected_port_info }} + [{{ item[0] |combine(item[1]) }}]" + loop: "{{ tmp }}" + +# Verify that each host object has the expected protocol type and address/port +- name: Assert hosts information was updated with new port information + assert: + that: "{{ item[0].host_name != item[1].host_name or + item[0].label != item[1].label or + (item[0].type == item[1].type and + (item[0].address|regex_replace(':','')) == (item[1].port|regex_replace(':',''))) }}" + msg: "port failed to be updated!" + loop: "{{ query('nested', port_info, expected_port_info) }}" + + +# **************************************************** +# *** Remove any existing hosts and verify changes *** +# **************************************************** +- name: Remove any existing hosts + netapp_e_host: + <<: *creds + state: absent + name: "{{ item.key }}" + with_dict: *hosts + +# Request all host object definitions +- name: HTTP request for all host definitions from array + uri: + url: "{{ credentials.api_url }}/storage-systems/{{ credentials.ssid }}/hosts" + user: "{{ credentials.api_username }}" + password: "{{ credentials.api_password }}" + body_format: json + validate_certs: no + register: results + +# Collect port label information +- name: Collect host side port labels + set_fact: + host_side_port_labels: "{{ results | json_query('json[*].hostSidePorts[*].label') }}" + +- name: Collect removed port labels + set_fact: + removed_host_side_port_labels: "{{ hosts | json_query('*.ports[*].label') }}" + +# Verify host 1 and 2 objects were removed +- name: Assert hosts were removed + assert: + that: item not in host_side_port_labels + msg: "Host {{ item }} failed to be removed from the hosts!" + loop: "{{ removed_host_side_port_labels }}" diff --git a/test/sanity/validate-modules/ignore.txt b/test/sanity/validate-modules/ignore.txt index 8d5c99955dd..5ee9695b00a 100644 --- a/test/sanity/validate-modules/ignore.txt +++ b/test/sanity/validate-modules/ignore.txt @@ -1074,9 +1074,7 @@ lib/ansible/modules/storage/netapp/netapp_e_facts.py E325 lib/ansible/modules/storage/netapp/netapp_e_flashcache.py E322 lib/ansible/modules/storage/netapp/netapp_e_flashcache.py E325 lib/ansible/modules/storage/netapp/netapp_e_flashcache.py E326 -lib/ansible/modules/storage/netapp/netapp_e_host.py E322 lib/ansible/modules/storage/netapp/netapp_e_host.py E325 -lib/ansible/modules/storage/netapp/netapp_e_host.py E326 lib/ansible/modules/storage/netapp/netapp_e_lun_mapping.py E325 lib/ansible/modules/storage/netapp/netapp_e_snapshot_group.py E322 lib/ansible/modules/storage/netapp/netapp_e_snapshot_group.py E325 diff --git a/test/units/modules/storage/__init__.py b/test/units/modules/storage/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/units/modules/storage/netapp/__init__.py b/test/units/modules/storage/netapp/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/units/modules/storage/netapp/test_netapp_e_host.py b/test/units/modules/storage/netapp/test_netapp_e_host.py new file mode 100644 index 00000000000..24971ecf877 --- /dev/null +++ b/test/units/modules/storage/netapp/test_netapp_e_host.py @@ -0,0 +1,190 @@ +# (c) 2018, NetApp Inc. +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from mock import MagicMock + +from ansible.module_utils import basic, netapp +from ansible.modules.storage.netapp import netapp_e_host +from ansible.modules.storage.netapp.netapp_e_host import Host +from units.modules.utils import AnsibleExitJson, AnsibleFailJson, ModuleTestCase, set_module_args + +__metaclass__ = type +import unittest +import mock +import pytest +import json +from ansible.compat.tests.mock import patch +from ansible.module_utils._text import to_bytes + + +class HostTest(ModuleTestCase): + REQUIRED_PARAMS = { + 'api_username': 'rw', + 'api_password': 'password', + 'api_url': 'http://localhost', + 'ssid': '1', + 'name': '1', + } + HOST = { + 'name': '1', + 'label': '1', + 'id': '0' * 30, + 'clusterRef': 40 * '0', + 'hostTypeIndex': 28, + 'hostSidePorts': [], + 'initiators': [], + 'ports': [], + } + HOST_ALT = { + 'name': '2', + 'label': '2', + 'id': '1' * 30, + 'clusterRef': '1', + 'hostSidePorts': [], + 'initiators': [], + 'ports': [], + } + REQ_FUNC = 'ansible.modules.storage.netapp.netapp_e_host.request' + + def _set_args(self, args): + module_args = self.REQUIRED_PARAMS.copy() + module_args.update(args) + set_module_args(module_args) + + def test_delete_host(self): + """Validate removing a host object""" + self._set_args({ + 'state': 'absent' + }) + host = Host() + with self.assertRaises(AnsibleExitJson) as result: + # We expect 2 calls to the API, the first to retrieve the host objects defined, + # the second to remove the host definition. + with mock.patch(self.REQ_FUNC, side_effect=[(200, [self.HOST]), (204, {})]) as request: + host.apply() + self.assertEquals(request.call_count, 2) + # We expect the module to make changes + self.assertEquals(result.exception.args[0]['changed'], True) + + def test_delete_host_no_changes(self): + """Ensure that removing a host that doesn't exist works correctly.""" + self._set_args({ + 'state': 'absent' + }) + host = Host() + with self.assertRaises(AnsibleExitJson) as result: + # We expect a single call to the API: retrieve the defined hosts. + with mock.patch(self.REQ_FUNC, return_value=(200, [])): + host.apply() + # We should not mark changed=True + self.assertEquals(result.exception.args[0]['changed'], False) + + def test_host_exists(self): + """Test host_exists method""" + self._set_args({ + 'state': 'absent' + }) + host = Host() + with mock.patch(self.REQ_FUNC, return_value=(200, [self.HOST])) as request: + host_exists = host.host_exists + self.assertTrue(host_exists, msg="This host should exist!") + + def test_host_exists_negative(self): + """Test host_exists method with no matching hosts to return""" + self._set_args({ + 'state': 'absent' + }) + host = Host() + with mock.patch(self.REQ_FUNC, return_value=(200, [self.HOST_ALT])) as request: + host_exists = host.host_exists + self.assertFalse(host_exists, msg="This host should exist!") + + def test_host_exists_fail(self): + """Ensure we do not dump a stack trace if we fail to make the request""" + self._set_args({ + 'state': 'absent' + }) + host = Host() + with self.assertRaises(AnsibleFailJson): + with mock.patch(self.REQ_FUNC, side_effect=Exception("http_error")) as request: + host_exists = host.host_exists + + def test_needs_update_host_type(self): + """Ensure a changed host_type triggers an update""" + self._set_args({ + 'state': 'present', + 'host_type': 27 + }) + host = Host() + host.host_obj = self.HOST + with mock.patch(self.REQ_FUNC, return_value=(200, [self.HOST])) as request: + needs_update = host.needs_update + self.assertTrue(needs_update, msg="An update to the host should be required!") + + def test_needs_update_cluster(self): + """Ensure a changed group_id triggers an update""" + self._set_args({ + 'state': 'present', + 'host_type': self.HOST['hostTypeIndex'], + 'group': '1', + }) + host = Host() + host.host_obj = self.HOST + with mock.patch(self.REQ_FUNC, return_value=(200, [self.HOST])) as request: + needs_update = host.needs_update + self.assertTrue(needs_update, msg="An update to the host should be required!") + + def test_needs_update_no_change(self): + """Ensure no changes do not trigger an update""" + self._set_args({ + 'state': 'present', + 'host_type': self.HOST['hostTypeIndex'], + }) + host = Host() + host.host_obj = self.HOST + with mock.patch(self.REQ_FUNC, return_value=(200, [self.HOST])) as request: + needs_update = host.needs_update + self.assertFalse(needs_update, msg="An update to the host should be required!") + + def test_needs_update_ports(self): + """Ensure added ports trigger an update""" + self._set_args({ + 'state': 'present', + 'host_type': self.HOST['hostTypeIndex'], + 'ports': [{'label': 'abc', 'type': 'iscsi', 'port': '0'}], + }) + host = Host() + host.host_obj = self.HOST + with mock.patch.object(host, 'all_hosts', [self.HOST]): + needs_update = host.needs_update + self.assertTrue(needs_update, msg="An update to the host should be required!") + + def test_needs_update_changed_ports(self): + """Ensure changed ports trigger an update""" + self._set_args({ + 'state': 'present', + 'host_type': self.HOST['hostTypeIndex'], + 'ports': [{'label': 'abc', 'type': 'iscsi', 'port': '0'}], + }) + host = Host() + host.host_obj = self.HOST.copy() + host.host_obj['hostSidePorts'] = [{'label': 'xyz', 'type': 'iscsi', 'port': '0', 'address': 'iqn:0'}] + + with mock.patch.object(host, 'all_hosts', [self.HOST]): + needs_update = host.needs_update + self.assertTrue(needs_update, msg="An update to the host should be required!") + + def test_needs_update_changed_negative(self): + """Ensure a ports update with no changes does not trigger an update""" + self._set_args({ + 'state': 'present', + 'host_type': self.HOST['hostTypeIndex'], + 'ports': [{'label': 'abc', 'type': 'iscsi', 'port': '0'}], + }) + host = Host() + host.host_obj = self.HOST.copy() + host.host_obj['hostSidePorts'] = [{'label': 'xyz', 'type': 'iscsi', 'port': '0', 'address': 'iqn:0'}] + + with mock.patch.object(host, 'all_hosts', [self.HOST]): + needs_update = host.needs_update + self.assertTrue(needs_update, msg="An update to the host should be required!")