getVLUN() method is misleading

[acaso]

getVLUN(volumeName) method receives a volume name as its unique parameter and is described in the API as

    Get information about a VLUN.

    :param volumeName: The volume name of the VLUN to find
    :returns: VLUN

This description seems to imply that a volume can only have one associated VLUN which frequently is not the case (a volume can be exported to one or several hosts or one or several hosts sets). Indeed, in its implementation, this method gets the list of VLUNs matching the volumeName but only returns the first one (on purpose, as stated by the comment "Return the first VLUN found for the volume"): https://github.com/hpe-storage/python-3parclient/blob/de7c4a21d6fe11b616c2382cf5587ae5db66fcc9/hpe3parclient/client.py#L2128-L2136 Ideally, this method should request enough parameters to univocally reference the VLUN to get (similar feature in WSAPI requires volume name and host name in versions up to up to 1.2, and volume name, lun ID and a combination of hostname and/or port in recent versions). As maybe a signature change will cause more harm that good, at least its behavior should be stated clearly in the doc to avoid confusion.

Related note: as this library does not expose WSAPI's filtering support, the only current way to get an actual list of the VLUNs of a volume is to use getVLUNs() to get ALL the volumes and filter it client-side.

[ainamori]

I think it needs to add "hostname" argument for getVLUN method. Using getHostVLUNs works well.

    def getVLUN(self, volumeName, hostname=None):
        """Get information about a VLUN.

        :param volumeName: The volume name of the VLUN to find
        :type name: str

        :returns: VLUN

        :raises: :class:`~hpe3parclient.exceptions.HTTPNotFound`
            -  NON_EXISTENT_VLUN - VLUN doesn't exist

        """
        # Check if the WSAPI supports VLUN querying. If it is supported
        # request only the VLUNs that are associated with the volume.
        if self.vlun_query_supported:
            query = '"volumeName EQ %s"' % volumeName
            response, body = self.http.get('/vluns?query=%s' %
                                           quote(query.encode("utf8")))

            # Return the first VLUN found for the volume.
            for vlun in body.get('members', []):
                return vlun
        else:
            vluns = self.getHostVLUNs(hostname=hostname)
            if len(vluns) > 0:
                for vlun in vluns:
                    if vlun['volumeName'] == volumeName:
                        return vlun

        raise exceptions.HTTPNotFound({'code': 'NON_EXISTENT_VLUN',
                                       'desc': "VLUN '%s' was not found" %
                                               volumeName})

[ainamori]

Sorry, I found that the variable param hostname is required. Besides, it needs to add hostname check for response from query parameter.

    def getVLUN(self, volumeName, hostname):
        """Get information about a VLUN.

        :param volumeName: The volume name of the VLUN to find
        :type name: str

        :returns: VLUN

        :raises: :class:`~hpe3parclient.exceptions.HTTPNotFound`
            -  NON_EXISTENT_VLUN - VLUN doesn't exist

        """
        # Check if the WSAPI supports VLUN querying. If it is supported
        # request only the VLUNs that are associated with the volume.
        if self.vlun_query_supported:
            query = '"volumeName EQ %s"' % volumeName
            response, body = self.http.get('/vluns?query=%s' %
                                           quote(query.encode("utf8")))

            # Return the first VLUN found for the volume.
            for vlun in body.get('members', []):
                if vlun['hostname'] == hostName:
                    return vlun
        else:
            vluns = self.getHostVLUNs(hostname=hostname)
            if len(vluns) > 0:
                for vlun in vluns:
                    if vlun['volumeName'] == volumeName:
                        return vlun

        raise exceptions.HTTPNotFound({'code': 'NON_EXISTENT_VLUN',
                                       'desc': "VLUN '%s' was not found" %
                                               volumeName})