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.

lib/ansible/modules/storage/netapp/netapp_e_host.py

@@ -1,17 +1,16 @@
 #!/usr/bin/python
-
+# -*- coding: utf-8 -*-
-# (c) 2016, NetApp, Inc
+#
+# (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 host doesn't yet exist, the label/name 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 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: "{{ ssid }}"
+        ssid: "1"
-        api_url: "{{ netapp_api_url }}"
+        api_url: "10.113.1.101:8443"
-        api_username: "{{ netapp_api_username }}"
+        api_username: "admin"
-        api_password: "{{ netapp_api_password }}"
+        api_password: "myPassword"
-        name: "{{ host_name }}"
+        name: "Host1"
-        host_type_index: "{{ host_type_index }}"
+        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: "1"
+        api_url: "10.113.1.101:8443"
+        api_username: "admin"
+        api_password: "myPassword"
+        name: "Host2"
+        state: absent
+
 """
 
 RETURN = """
 msg:
-    description: Success message
+    description:
-    returned: success
+        - 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']),
+            state=dict(type='str', default='present', choices=['absent', 'present']),
-            group=dict(type='str', required=False),
+            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),
+            name=dict(type='str', required=True, aliases=['label']),
-            host_type_index=dict(type='int', required=True)
+            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):
+    def host_ports_available(self):
-        used_ids = list()
+        """Determine if the hostPorts requested have already been assigned"""
-        try:
+        for host in self.all_hosts:
-            (rc, self.available_ports) = request(self.url + 'storage-systems/%s/unassociated-host-ports' % self.ssid,
+            if host['label'] != self.name:
-                                                 url_password=self.pwd, url_username=self.user,
+                for host_port in host['hostSidePorts']:
-                                                 validate_certs=self.certs,
+                    for port in self.ports:
-                                                 headers=HEADERS)
+                        if (port['port'] == host_port['address'] or port['label'] == host_port['label']):
-        except Exception as err:
+                            if not self.force_port:
-            self.module.fail_json(
+                                self.module.fail_json(
-                msg="Failed to get unassociated host ports. Array Id [%s]. Error [%s]." % (self.ssid, to_native(err)))
+                                    msg="There are no host ports available OR there are not enough unassigned host ports")
-
+                            else:
-        if len(self.available_ports) > 0 and len(self.ports) <= len(self.available_ports):
+                                return False
-            for port in self.ports:
+        return True
-                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")
 
     @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 \
+        if self.host_obj['clusterRef'] != self.group_id or self.host_obj['hostTypeIndex'] != self.host_type_index:
-                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']:
+                    # The port (as defined), currently does not exist
-                        if arg_port['label'] == obj_port['label']:
+                    if arg_port['label'] not in port_names:
-                            # Confirmed that port arg passed in exists on the host
+                        needs_update = True
-                            # port_id = self.get_port_id(obj_port['label'])
+                        # This port has not been defined on the host at all
-                            if arg_port['type'] != obj_port['portId']['ioInterfaceType']:
+                        if arg_port['port'] not in port_addresses:
-                                needs_update = True
+                            self.newPorts.append(arg_port)
-                            if 'iscsiChapSecret' in arg_port:
+                        # A port label update has been requested
-                                # No way to know the current secret attr, so always return True just in case
+                        else:
-                                needs_update = True
+                            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 "
+                            msg="The port you specified:\n%s\n is associated with a different host. Specify force_port"
-                                "port spec" % arg_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:
+            if host['name'] != self.name:
-                for port in host['ports']:
+                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:
+        post_body = dict(
-            self.post_body = dict(
+            portsToUpdate=dict()
-                portsToUpdate=dict()
+        )
-            )
 
         for port in self.ports:
             if self.port_on_diff_host(port):
-                self.post_body['portsToUpdate'].update(dict(
+                host_port = self.get_port(port['label'], port['port'])
-                    portRef=self.other_host['hostPortRef'],
+                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)))
 
-    def update_host(self):
+        return post_body
-        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.post_body['ports'] = self.ports
+    def update_host(self):
+        self._logger.debug("Beginning the update for host=%s.", self.name)
+
+        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:
+        api = self.url + 'storage-systems/%s/hosts/%s' % (self.ssid, self.host_obj['id'])
-            (rc, self.host_obj) = request(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))
-                                          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)
+        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)))
+
+        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
 
-        if not self.host_exists:
+        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 and self.check_mode):
             try:
-                (rc, create_resp) = request(self.url + "storage-systems/%s/hosts" % self.ssid, method='POST',
+                (rc, self.host_obj) = request(api, method='POST',
-                                            url_username=self.user, url_password=self.pwd, validate_certs=self.certs,
+                                              url_username=self.user, url_password=self.pwd, validate_certs=self.certs,
-                                            data=json.dumps(post_body), headers=HEADERS)
+                                              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
+        self._logger.info("Created host, beginning port re-assignment.")
-
+        if needs_reassignment:
-        if self.ports and self.force_port:
             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():

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

test/integration/targets/netapp_eseries_host/tasks/main.yml

@@ -0,0 +1 @@
+- include_tasks: run.yml

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 }}"

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

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!")