From 56c8665d3d342ce90f5d9433966c0f244063b4c1 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Tue, 4 Apr 2017 20:23:58 +0200 Subject: [PATCH] Refactor iSCSI connect This patch refactors iSCSI connect code changing the approach to one that relies primarily on sysfs, instead of CLI tools, to retrieve all the required information: devices from the connection, multipath system device name, multipath name, the WWN for the block devices... By doing so, not only do we fix a good number of bugs, but we also improve the reliability and speed of the mechanism. A good example of improvements and benefits achieved by this patch are: - Clean all leftovers on exceptions on a connection. - Parallelize logins on multipath to increase speed on flaky network connections. - As long as there is at least one good path when working with multipath we will return a multipath device instead of a single path device, which helps with temporary flaky connections. - Don't use the rescan retry parameter as log in retry on multipath connections so both single and multipath cases are consistent. - We no longer rely on just one device to get the wwn and look for the multipath. This would be problematic with flaky connections. - No more querying iSCSI devices for their WWN (page 0x83) removing delays and issue on flaky connections. - It's no longer a problem for the mechanism the fact that a device exists but is not accessible. - We use links in `/dev/disk/by-id` to get the WWID on connect, so we make sure there are no leftovers on disconnect, but we no longer use symlinks from `/dev/disk/by-path`, `/dev/disk/by-id`, or `/dev/mapper` to find devices. - We no longer need to rely on the WWN to determine the multipath, we have the session and the LUN, so we trace the devices and from those we get if they belong to a multipath. - Stop polluting the logs with unnecessary exceptions from checking if the node or session exist. - Action retries will now only log the final exception instead of logging all the exceptions. - Warn when a multipath could not be formed and a single device is being used, as performance will be degraded. - We no longer do global rescans on single connections that could be bringing in unrelated and unwanted devices (`iscsiadm -T iqn -p portal --rescan`). - Fix scan mechanism that would not request all scans when the same iqn was shareed between portals and could send a scan request to the wrong IP if they shared the same iqn. - When doing single pathed connections we could end with a multipath because we didn't clean up unfruitful connections. It's worth mentioning that this patch does not touch the extend operation. Change-Id: Ia1c47bfaa7bc3544a5feba6a8a30faf2f132b8d7 --- os_brick/exception.py | 8 - os_brick/initiator/connectors/iscsi.py | 585 +++++----- os_brick/initiator/linuxscsi.py | 133 ++- os_brick/privileged/rootwrap.py | 27 + .../tests/initiator/connectors/test_iscsi.py | 1040 +++++++++-------- os_brick/tests/initiator/test_linuxscsi.py | 186 ++- os_brick/tests/privileged/test_rootwrap.py | 30 + ...factor_iscsi_connect-dfbb24305a954783.yaml | 5 + 8 files changed, 1266 insertions(+), 748 deletions(-) create mode 100644 releasenotes/notes/refactor_iscsi_connect-dfbb24305a954783.yaml diff --git a/os_brick/exception.py b/os_brick/exception.py index c047ea1d2..294eb3751 100644 --- a/os_brick/exception.py +++ b/os_brick/exception.py @@ -163,14 +163,6 @@ class InvalidConnectorProtocol(ValueError): pass -class HostChannelsTargetsNotFound(BrickException): - message = _('Unable to find host, channel, and target for %(iqns)s.') - - def __init__(self, message=None, iqns=None, found=None): - super(HostChannelsTargetsNotFound, self).__init__(message, iqns=iqns) - self.found = found - - class ExceptionChainer(BrickException): """A Exception that can contain a group of exceptions. diff --git a/os_brick/initiator/connectors/iscsi.py b/os_brick/initiator/connectors/iscsi.py index d4e21a557..6c94796d8 100644 --- a/os_brick/initiator/connectors/iscsi.py +++ b/os_brick/initiator/connectors/iscsi.py @@ -14,18 +14,18 @@ import collections -import copy import glob import os +import threading import time from oslo_concurrency import lockutils from oslo_concurrency import processutils as putils from oslo_log import log as logging +from oslo_utils import excutils from oslo_utils import strutils from os_brick import exception -from os_brick.i18n import _ from os_brick import initiator from os_brick.initiator.connectors import base from os_brick.initiator.connectors import base_iscsi @@ -94,11 +94,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): # didn't exist previously. # We are simply trying to find any existing volumes with # already connected sessions. - host_devices, target_props = self._get_potential_volume_paths( - connection_properties, - connect_to_portal=False, - use_rescan=False) - + host_devices = self._get_potential_volume_paths(connection_properties) for path in host_devices: if os.path.exists(path): volume_paths.append(path) @@ -163,9 +159,45 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): # entry: [tcp, [1], 192.168.121.250:3260,1 ...] return [entry[2] for entry in self._get_iscsi_sessions_full()] - def _get_potential_volume_paths(self, connection_properties, - connect_to_portal=True, - use_rescan=True): + def _get_ips_iqns_luns(self, connection_properties): + """Build a list of ips, iqns, and luns. + + :param connection_properties: The dictionary that describes all + of the target volume attributes. + :type connection_properties: dict + :returns: list of tuples of (ip, iqn, lun) + """ + try: + ips_iqns_luns = self._discover_iscsi_portals(connection_properties) + except Exception: + if 'target_portals' in connection_properties: + raise exception.TargetPortalsNotFound( + target_portals=connection_properties['target_portals']) + if 'target_portal' in connection_properties: + raise exception.TargetPortalNotFound( + target_portal=connection_properties['target_portal']) + raise + + if not connection_properties.get('target_iqns'): + # There are two types of iSCSI multipath devices. One which + # shares the same iqn between multiple portals, and the other + # which use different iqns on different portals. + # Try to identify the type by checking the iscsiadm output + # if the iqn is used by multiple portals. If it is, it's + # the former, so use the supplied iqn. Otherwise, it's the + # latter, so try the ip,iqn combinations to find the targets + # which constitutes the multipath device. + main_iqn = connection_properties['target_iqn'] + all_portals = {(ip, lun) for ip, iqn, lun in ips_iqns_luns} + match_portals = {(ip, lun) for ip, iqn, lun in ips_iqns_luns + if iqn == main_iqn} + if len(all_portals) == len(match_portals): + ips_iqns_luns = [(p[0], main_iqn, p[1]) + for p in all_portals] + + return ips_iqns_luns + + def _get_potential_volume_paths(self, connection_properties): """Build a list of potential volume paths that exist. Given a list of target_portals in the connection_properties, @@ -173,10 +205,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): This method's job is to build that list of potential paths for a volume that might show up. - This is used during connect_volume time, in which case we want - to connect to the iSCSI target portal. - - During get_volume_paths time, we are looking to + This is only used during get_volume_paths time, we are looking to find a list of existing volume paths for the connection_properties. In this case, we don't want to connect to the portal. If we blindly try and connect to a portal, it could create a new iSCSI @@ -185,96 +214,28 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): :param connection_properties: The dictionary that describes all of the target volume attributes. :type connection_properties: dict - :param connect_to_portal: Do we want to try a new connection to the - target portal(s)? Set this to False if you - want to search for existing volumes, not - discover new volumes. - :param connect_to_portal: bool - :param use_rescan: Issue iSCSI rescan during discovery? - :type use_rescan: bool - :returns: dict + :returns: list """ - - target_props = None - connected_to_portal = False if self.use_multipath: LOG.info("Multipath discovery for iSCSI enabled") # Multipath installed, discovering other targets if available - try: - ips_iqns_luns = self._discover_iscsi_portals( - connection_properties) - except Exception: - if 'target_portals' in connection_properties: - raise exception.TargetPortalsNotFound( - target_portals=connection_properties['target_portals']) - elif 'target_portal' in connection_properties: - raise exception.TargetPortalNotFound( - target_portal=connection_properties['target_portal']) - else: - raise - - if not connection_properties.get('target_iqns'): - # There are two types of iSCSI multipath devices. One which - # shares the same iqn between multiple portals, and the other - # which use different iqns on different portals. - # Try to identify the type by checking the iscsiadm output - # if the iqn is used by multiple portals. If it is, it's - # the former, so use the supplied iqn. Otherwise, it's the - # latter, so try the ip,iqn combinations to find the targets - # which constitutes the multipath device. - main_iqn = connection_properties['target_iqn'] - all_portals = {(ip, lun) for ip, iqn, lun in ips_iqns_luns} - match_portals = {(ip, lun) for ip, iqn, lun in ips_iqns_luns - if iqn == main_iqn} - if len(all_portals) == len(match_portals): - ips_iqns_luns = [(p[0], main_iqn, p[1]) - for p in all_portals] - - for ip, iqn, lun in ips_iqns_luns: - props = copy.deepcopy(connection_properties) - props['target_portal'] = ip - props['target_iqn'] = iqn - if connect_to_portal: - if self._connect_to_iscsi_portal(props): - connected_to_portal = True - - if use_rescan: - self._rescan_iscsi(ips_iqns_luns) - host_devices = self._get_device_path(connection_properties) else: LOG.info("Multipath discovery for iSCSI not enabled.") - iscsi_sessions = [] - if not connect_to_portal: - iscsi_sessions = self._get_iscsi_sessions() + iscsi_sessions = self._get_iscsi_sessions() + iscsi_portals_with_sessions = [s[2] for s in iscsi_sessions] - host_devices = [] - target_props = connection_properties + host_devices = set() for props in self._iterate_all_targets(connection_properties): - if connect_to_portal: - if self._connect_to_iscsi_portal(props): - target_props = props - connected_to_portal = True - host_devices = self._get_device_path(props) - break - else: - LOG.warning( - 'Failed to connect to iSCSI portal %(portal)s.', - {'portal': props['target_portal']}) - else: - # If we aren't trying to connect to the portal, we - # want to find ALL possible paths from all of the - # alternate portals - if props['target_portal'] in iscsi_sessions: - paths = self._get_device_path(props) - host_devices = list(set(paths + host_devices)) + # If we aren't trying to connect to the portal, we + # want to find ALL possible paths from all of the + # alternate portals + if props['target_portal'] in iscsi_portals_with_sessions: + paths = self._get_device_path(props) + host_devices.update(paths) + host_devices = list(host_devices) - if connect_to_portal and not connected_to_portal: - msg = _("Could not login to any iSCSI portal.") - LOG.error(msg) - raise exception.FailedISCSITargetPortalLogin(message=msg) - - return host_devices, target_props + return host_devices def set_execute(self, execute): super(ISCSIConnector, self).set_execute(execute) @@ -424,7 +385,6 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): @utils.trace @synchronized('connect_volume') - @utils.retry(exceptions=(exception.VolumeDeviceNotFound)) def connect_volume(self, connection_properties): """Attach the volume to instance_name. @@ -447,66 +407,224 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): target_lun(s) - LUN id of the volume Note that plural keys may be used when use_multipath=True """ - - device_info = {'type': 'block'} - - # At this point the host_devices may be an empty list - host_devices, target_props = self._get_potential_volume_paths( - connection_properties) - - # The /dev/disk/by-path/... node is not always present immediately - # TODO(justinsb): This retry-with-delay is a pattern, move to utils? - tries = 0 - # Loop until at least 1 path becomes available - while all(not os.path.exists(x) for x in host_devices): - if tries >= self.device_scan_attempts: - raise exception.VolumeDeviceNotFound(device=host_devices) - - LOG.info("ISCSI volume not yet found at: %(host_devices)s. " - "Will rescan & retry. Try number: %(tries)s.", - {'host_devices': host_devices, 'tries': tries}) - + try: if self.use_multipath: - # We need to refresh the paths as the devices may be empty - host_devices, target_props = ( - self._get_potential_volume_paths(connection_properties)) - else: - if tries: - host_devices = self._get_device_path(target_props) - self._run_iscsiadm(target_props, ("--rescan",)) + return self._connect_multipath_volume(connection_properties) + return self._connect_single_volume(connection_properties) + except Exception: + # NOTE(geguileo): By doing the cleanup here we ensure we only do + # the logins once for multipath if they succeed, but retry if they + # don't, which helps on bad network cases. + with excutils.save_and_reraise_exception(): + self._cleanup_connection(connection_properties, force=True) - tries += 1 - if all(not os.path.exists(x) for x in host_devices): - time.sleep(tries ** 2) + @utils.retry(exceptions=(exception.VolumeDeviceNotFound)) + def _connect_single_volume(self, connection_properties): + """Connect to a volume using a single path.""" + data = {'stop_connecting': False, 'num_logins': 0, 'failed_logins': 0, + 'stopped_threads': 0, 'found_devices': [], + 'just_added_devices': []} + + for props in self._iterate_all_targets(connection_properties): + self._connect_vol(self.device_scan_attempts, props, data) + found_devs = data['found_devices'] + if found_devs: + for __ in range(10): + wwn = self._linuxscsi.get_sysfs_wwn(found_devs) + if wwn: + return {'type': 'block', 'scsi_wwn': wwn, + 'path': '/dev/' + found_devs[0]} + time.sleep(1) + LOG.debug('Could not find the WWN for %s.', found_devs[0]) + + # If we failed we must cleanup the connection, as we could be + # leaving the node entry if it's not being used by another device. + ips_iqns_luns = ((props['target_portal'], props['target_iqn'], + props['target_lun']), ) + self._cleanup_connection(props, ips_iqns_luns, force=True, + ignore_errors=True) + # Reset connection result values for next try + data.update(num_logins=0, failed_logins=0, found_devices=[]) + + raise exception.VolumeDeviceNotFound(device='') + + def _connect_vol(self, rescans, props, data): + """Make a connection to a volume, send scans and wait for the device. + + This method is specifically designed to support multithreading and + share the results via a shared dictionary with fixed keys, which is + thread safe. + + Since the heaviest operations are run via subprocesses we don't worry + too much about the GIL or how the eventlets will handle the context + switching. + + The method will only try to log in once, since iscsid's initiator + already tries 8 times by default to do the login, or whatever value we + have as node.session.initial_login_retry_max in our system. + + Shared dictionary has the following keys: + - stop_connecting: When the caller wants us to stop the rescans + - num_logins: Count of how many threads have successfully logged in + - failed_logins: Count of how many threads have failed to log in + - stopped_threads: How many threads have finished. This may be + different than num_logins + failed_logins, since + some threads may still be waiting for a device. + - found_devices: List of devices the connections have found + - just_added_devices: Devices that have been found and still have not + been processed by the main thread that manages + all the connecting threads. + + :param rescans: Number of rescans to perform before giving up. + :param props: Properties of the connection. + :param data: Shared data. + """ + device = hctl = None + portal = props['target_portal'] + session = self._connect_to_iscsi_portal(props) + do_scans = rescans > 0 + retry = 1 + if session: + data['num_logins'] += 1 + LOG.debug('Connected to %s', portal) + while do_scans: + try: + if not hctl: + hctl = self._linuxscsi.get_hctl(session, + props['target_lun']) + # Scan is sent on connect by iscsid, so skip first rescan + if hctl: + if retry > 1: + self._linuxscsi.scan_iscsi(*hctl) + + device = self._linuxscsi.device_name_by_hctl(session, + hctl) + if device: + break + + except Exception: + LOG.exception('Exception scanning %s', portal) + pass + retry += 1 + do_scans = (retry <= rescans and + not (device or data['stop_connecting'])) + if do_scans: + time.sleep(retry ** 2) + if device: + LOG.debug('Connected to %s using %s', device, + strutils.mask_password(props)) else: + LOG.warning('LUN %(lun)s on iSCSI portal %(portal)s not found ' + 'on sysfs after logging in.', + {'lun': props['target_lun'], 'portal': portal}) + else: + LOG.warning('Failed to connect to iSCSI portal %s.', portal) + data['failed_logins'] += 1 + + if device: + data['found_devices'].append(device) + data['just_added_devices'].append(device) + data['stopped_threads'] += 1 + + @utils.retry(exceptions=(exception.VolumeDeviceNotFound)) + def _connect_multipath_volume(self, connection_properties): + """Connect to a multipathed volume launching parallel login requests. + + We will be doing parallel login requests, which will considerably speed + up the process when we have flaky connections. + + We'll always try to return a multipath device even if there's only one + path discovered, that way we can return once we have logged in in all + the portals, because the paths will come up later. + + To make this possible we tell multipathd that the wwid is a multipath + as soon as we have one device, and then hint multipathd to reconsider + that volume for a multipath asking to add the path, because even if + it's already known by multipathd it would have been discarded if it + was the first time this volume was seen here. + """ + wwn = mpath = None + wwn_added = last_try_on = False + found = [] + just_added_devices = [] + # Dict used to communicate with threads as detailed in _connect_vol + data = {'stop_connecting': False, 'num_logins': 0, 'failed_logins': 0, + 'stopped_threads': 0, 'found_devices': found, + 'just_added_devices': just_added_devices} + + ips_iqns_luns = self._get_ips_iqns_luns(connection_properties) + # Launch individual threads for each session with the own properties + retries = self.device_scan_attempts + threads = [] + for ip, iqn, lun in ips_iqns_luns: + props = connection_properties.copy() + props.update(target_portal=ip, target_iqn=iqn, target_lun=lun) + threads.append(threading.Thread(target=self._connect_vol, + args=(retries, props, data))) + for thread in threads: + thread.start() + + # Continue until: + # - All connection attempts have finished and none has logged in + # - Multipath has been found and connection attempts have either + # finished or have already logged in + # - We have finished in all threads, logged in, found some device, and + # 10 seconds have passed, which should be enough with up to 10% + # network package drops. + while not ((len(ips_iqns_luns) == data['stopped_threads'] and + not found) or + (mpath and len(ips_iqns_luns) == data['num_logins'] + + data['failed_logins'])): + # We have devices but we don't know the wwn yet + if not wwn and found: + wwn = self._linuxscsi.get_sysfs_wwn(found) + # We have the wwn but not a multipath + if wwn and not mpath: + mpath = self._linuxscsi.find_sysfs_multipath_dm(found) + if not (mpath or wwn_added): + # Tell multipathd that this wwn is a multipath and hint + # multipathd to recheck all the devices we have just + # connected. We only do this once, since for any new + # device multipathd will already know it is a multipath. + # This is only useful if we have multipathd configured with + # find_multipaths set to yes, and has no effect if it's set + # to no. + wwn_added = self._linuxscsi.multipath_add_wwid(wwn) + while not mpath and just_added_devices: + device_path = '/dev/' + just_added_devices.pop(0) + self._linuxscsi.multipath_add_path(device_path) + mpath = self._linuxscsi.find_sysfs_multipath_dm(found) + # Give some extra time after all threads have finished. + if (not last_try_on and found and + len(ips_iqns_luns) == data['stopped_threads']): + LOG.debug('All connection threads finished, giving 10 seconds ' + 'for dm to appear.') + last_try_on = time.time() + 10 + elif last_try_on and last_try_on < time.time(): break + time.sleep(1) + data['stop_connecting'] = True + for thread in threads: + thread.join() - if tries != 0: - LOG.debug("Found iSCSI node %(host_devices)s " - "(after %(tries)s rescans)", - {'host_devices': host_devices, 'tries': tries}) + # If we haven't found any devices let the caller do the cleanup + if not found: + raise exception.VolumeDeviceNotFound(device='') - # Choose an accessible host device - host_device = next(dev for dev in host_devices if os.path.exists(dev)) + # NOTE(geguileo): If we cannot find the dm it's because all paths are + # really bad, so we might as well raise a not found exception, but + # in our best effort we'll return a device even if it's probably + # useless. + if not mpath: + LOG.warning('No dm was created, connection to volume is probably ' + 'bad and will perform poorly.') + return {'type': 'block', 'scsi_wwn': wwn, + 'path': '/dev/' + found[0]} + return {'type': 'block', 'scsi_wwn': wwn, 'multipath_id': mpath, + 'path': '/dev/' + mpath} - # find out the WWN of the device - device_wwn = self._linuxscsi.get_scsi_wwn(host_device) - LOG.debug("Device WWN = '%(wwn)s'", {'wwn': device_wwn}) - device_info['scsi_wwn'] = device_wwn - - if self.use_multipath: - (host_device, multipath_id) = (super( - ISCSIConnector, self)._discover_mpath_device( - device_wwn, connection_properties, host_device)) - if multipath_id: - device_info['multipath_id'] = multipath_id - - device_info['path'] = host_device - - LOG.debug("connect_volume returning %s", device_info) - return device_info - - def _get_connection_devices(self, connection_properties): + def _get_connection_devices(self, connection_properties, + ips_iqns_luns=None): """Get map of devices by sessions from our connection. For each of the TCP sessions that correspond to our connection @@ -518,8 +636,12 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): We also include all nodes from our connection that don't have a session. + + If ips_iqns_luns parameter is provided connection_properties won't be + used to get them. """ - ips_iqns_luns = self._get_all_targets(connection_properties) + if not ips_iqns_luns: + ips_iqns_luns = self._get_all_targets(connection_properties) nodes = self._get_iscsi_nodes() sessions = self._get_iscsi_sessions_full() # Use (portal, iqn) to map the session value @@ -573,8 +695,31 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): the operation. Default is False. :type ignore_errors: bool """ + return self._cleanup_connection(connection_properties, force=force, + ignore_errors=ignore_errors) + + def _cleanup_connection(self, connection_properties, ips_iqns_luns=None, + force=False, ignore_errors=False): + """Cleans up connection flushing and removing devices and multipath. + + :param connection_properties: The dictionary that describes all + of the target volume attributes. + :type connection_properties: dict that must include: + target_portal(s) - IP and optional port + target_iqn(s) - iSCSI Qualified Name + target_lun(s) - LUN id of the volume + :param ips_iqns_luns: Use this list of tuples instead of information + from the connection_properties. + :param force: Whether to forcefully disconnect even if flush fails. + :type force: bool + :param ignore_errors: When force is True, this will decide whether to + ignore errors or raise an exception once finished + the operation. Default is False. + :type ignore_errors: bool + """ exc = exception.ExceptionChainer() - devices_map = self._get_connection_devices(connection_properties) + devices_map = self._get_connection_devices(connection_properties, + ips_iqns_luns) # Remove devices and multipath from this connection remove_devices = set() @@ -689,24 +834,24 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): return ips, iqns def _connect_to_iscsi_portal(self, connection_properties): + """Connect to an iSCSI portal-target an return the session id.""" + portal = connection_properties['target_portal'].split(",")[0] + target_iqn = connection_properties['target_iqn'] + # NOTE(vish): If we are on the same host as nova volume, the # discovery makes the target so we don't need to # run --op new. Therefore, we check to see if the # target exists, and if we get 255 (Not Found), then # we run --op new. This will also happen if another # volume is using the same target. - LOG.info("Trying to connect to iSCSI portal %(portal)s", - {"portal": connection_properties['target_portal']}) - try: - self._run_iscsiadm(connection_properties, ()) - except putils.ProcessExecutionError as exc: - # iscsiadm returns 21 for "No records found" after version 2.0-871 - if exc.exit_code in [21, 255]: - self._run_iscsiadm(connection_properties, - ('--interface', self._get_transport(), - '--op', 'new')) - else: - raise + # iscsiadm returns 21 for "No records found" after version 2.0-871 + LOG.info("Trying to connect to iSCSI portal %s", portal) + err = self._run_iscsiadm(connection_properties, (), + check_exit_code=(0, 21, 255))[1] + if err: + self._run_iscsiadm(connection_properties, + ('--interface', self._get_transport(), + '--op', 'new')) if connection_properties.get('auth_method'): self._iscsiadm_update(connection_properties, @@ -719,44 +864,31 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): "node.session.auth.password", connection_properties['auth_password']) - # Duplicate logins crash iscsiadm after load, - # so we scan active sessions to see if the node is logged in. - out = self._run_iscsiadm_bare(["-m", "session"], - run_as_root=True, - check_exit_code=[0, 1, 21])[0] or "" + # We exit once we are logged in or once we fail login + while True: + # Duplicate logins crash iscsiadm after load, so we scan active + # sessions to see if the node is logged in. + sessions = self._get_iscsi_sessions_full() + for s in sessions: + # Found our session, return session_id + if 'tcp:' == s[0] and portal == s[2] and s[4] == target_iqn: + return s[1] - portals = [{'portal': p.split(" ")[2], 'iqn': p.split(" ")[3]} - for p in out.splitlines() if p.startswith("tcp:")] - - stripped_portal = connection_properties['target_portal'].split(",")[0] - if len(portals) == 0 or len([s for s in portals - if stripped_portal == - s['portal'].split(",")[0] - and - s['iqn'] == - connection_properties['target_iqn']] - ) == 0: try: - self._run_iscsiadm(connection_properties, - ("--login",), - check_exit_code=[0, 255]) - except putils.ProcessExecutionError as err: # exit_code=15 means the session already exists, so it should # be regarded as successful login. - if err.exit_code not in [15]: - LOG.warning('Failed to login iSCSI target %(iqn)s ' - 'on portal %(portal)s (exit code ' - '%(err)s).', - {'iqn': connection_properties['target_iqn'], - 'portal': connection_properties[ - 'target_portal'], - 'err': err.exit_code}) - return False + self._run_iscsiadm(connection_properties, ("--login",), + check_exit_code=(0, 15, 255)) + except putils.ProcessExecutionError as err: + LOG.warning('Failed to login iSCSI target %(iqn)s on portal ' + '%(portal)s (exit code %(err)s).', + {'iqn': target_iqn, 'portal': portal, + 'err': err.exit_code}) + return None self._iscsiadm_update(connection_properties, "node.startup", "automatic") - return True def _disconnect_from_iscsi_portal(self, connection_properties): self._iscsiadm_update(connection_properties, "node.startup", "manual", @@ -808,72 +940,3 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): {'multipath_command': multipath_command, 'out': out, 'err': err}) return (out, err) - - @utils.retry(exception.HostChannelsTargetsNotFound, backoff_rate=1.5) - def _get_hosts_channels_targets_luns(self, ips_iqns_luns): - iqns = {iqn: lun for ip, iqn, lun in ips_iqns_luns} - LOG.debug('Getting hosts, channels, and targets for iqns: %s', - iqns.keys()) - - # Get all targets indexed by scsi host path - targets_paths = glob.glob('/sys/class/scsi_host/host*/device/session*/' - 'target*') - targets = collections.defaultdict(list) - for path in targets_paths: - target = path.split('/target')[1] - host = path.split('/device/')[0] - targets[host].append(target.split(':')) - - # Get all scsi targets - sessions = glob.glob('/sys/class/scsi_host/host*/device/session*/' - 'iscsi_session/session*/targetname') - - result = [] - for session in sessions: - # Read iSCSI target name - try: - with open(session, 'r') as f: - targetname = f.read().strip('\n') - except Exception: - continue - - # If we are interested in it we store its target information - if targetname in iqns: - host = session.split('/device/')[0] - for __, channel, target_id in targets[host]: - result.append((host, channel, target_id, iqns[targetname])) - # Stop as soon as we have the info of all our iqns, even if - # there are more sessions to check - del iqns[targetname] - if not iqns: - break - - # In some cases the login and udev triggers may not have been fast - # enough to create all sysfs entries, so we want to retry. - else: - raise exception.HostChannelsTargetsNotFound(iqns=iqns.keys(), - found=result) - return result - - def _rescan_iscsi(self, ips_iqns_luns): - try: - hctls = self._get_hosts_channels_targets_luns(ips_iqns_luns) - except exception.HostChannelsTargetsNotFound as e: - if not e.found: - LOG.error('iSCSI scan failed: %s', e) - return - - hctls = e.found - LOG.warning('iSCSI scan: %(error)s\nScanning %(hosts)s', - {'error': e, 'hosts': [h for h, c, t, l in hctls]}) - - for host_path, channel, target_id, target_lun in hctls: - LOG.debug('Scanning host %(host)s c: %(channel)s, ' - 't: %(target)s, l: %(lun)s)', - {'host': host_path, 'channel': channel, - 'target': target_id, 'lun': target_lun}) - self._linuxscsi.echo_scsi_command( - "%s/scan" % host_path, - "%(c)s %(t)s %(l)s" % {'c': channel, - 't': target_id, - 'l': target_lun}) diff --git a/os_brick/initiator/linuxscsi.py b/os_brick/initiator/linuxscsi.py index 6f22c9f98..80c4e0192 100644 --- a/os_brick/initiator/linuxscsi.py +++ b/os_brick/initiator/linuxscsi.py @@ -39,6 +39,9 @@ MULTIPATH_DEVICE_ACTIONS = ['unchanged:', 'reject:', 'reload:', class LinuxSCSI(executor.Executor): + # As found in drivers/scsi/scsi_lib.c + WWN_TYPES = {'t10.': '1', 'eui.': '2', 'naa.': '3'} + def echo_scsi_command(self, path, content): """Used to echo strings to scsi subsystem.""" @@ -103,6 +106,41 @@ class LinuxSCSI(executor.Executor): return dev_info + def get_sysfs_wwn(self, device_names): + """Return the wwid from sysfs in any of devices in udev format.""" + wwid = self.get_sysfs_wwid(device_names) + glob_str = '/dev/disk/by-id/scsi-' + wwn_paths = glob.glob(glob_str + '*') + # If we don't have multiple designators on page 0x83 + if wwid and glob_str + wwid in wwn_paths: + return wwid + + # If we have multiple designators follow the symlinks + for wwn_path in wwn_paths: + try: + if os.path.islink(wwn_path) and os.stat(wwn_path): + path = os.path.realpath(wwn_path) + if path.startswith('/dev/') and path[5:] in device_names: + return wwn_path[len(glob_str):] + except OSError: + continue + return '' + + def get_sysfs_wwid(self, device_names): + """Return the wwid from sysfs in any of devices in udev format.""" + for device_name in device_names: + try: + with open('/sys/block/%s/device/wwid' % device_name) as f: + wwid = f.read().strip() + except IOError: + continue + # The sysfs wwid has the wwn type in string format as a prefix, + # but udev uses its numerical representation as returned by + # scsi_id's page 0x83, so we need to map it + udev_wwid = self.WWN_TYPES.get(wwid[:4], '8') + wwid[4:] + return udev_wwid + return '' + def get_scsi_wwn(self, path): """Read the WWN from page 0x83 value for a SCSI device.""" @@ -186,10 +224,22 @@ class LinuxSCSI(executor.Executor): # Wait until the symlinks are removed with exc.context(force, 'Some devices remain from %s', devices_names): - self.wait_for_volumes_removal(devices_names) - + try: + self.wait_for_volumes_removal(devices_names) + finally: + # Since we use /dev/disk/by-id/scsi- links to get the wwn we + # must ensure they are always removed. + self._remove_scsi_symlinks(devices_names) return multipath_name + def _remove_scsi_symlinks(self, devices_names): + devices = ['/dev/' + dev for dev in devices_names] + links = glob.glob('/dev/disk/by-id/scsi-*') + unlink = [link for link in links + if os.path.realpath(link) in devices] + if unlink: + priv_rootwrap.unlink_root(no_errors=True, *unlink) + def flush_device_io(self, device): """This is used to flush any remaining IO in the buffers.""" if os.path.exists(device): @@ -486,3 +536,82 @@ class LinuxSCSI(executor.Executor): else: return ("0x%04x%04x00000000" % (lun_id & 0xffff, lun_id >> 16 & 0xffff)) + + def get_hctl(self, session, lun): + """Given an iSCSI session return the host, channel, target, and lun.""" + glob_str = '/sys/class/iscsi_host/host*/device/session' + session + paths = glob.glob(glob_str + '/target*') + if paths: + __, channel, target = os.path.split(paths[0])[1].split(':') + # Check if we can get the host + else: + target = channel = '-' + paths = glob.glob(glob_str) + + if not paths: + LOG.debug('No hctl found on session %s with lun %s', session, lun) + return None + + # Extract the host number from the path + host = paths[0][26:paths[0].index('/', 26)] + res = (host, channel, target, lun) + LOG.debug('HCTL %s found on session %s with lun %s', res, session, lun) + return res + + def device_name_by_hctl(self, session, hctl): + """Find the device name given a session and the hctl. + + :param session: A string with the session number + "param hctl: An iterable with the host, channel, target, and lun as + passed to scan. ie: ('5', '-', '-', '0') + """ + if '-' in hctl: + hctl = ['*' if x == '-' else x for x in hctl] + path = ('/sys/class/scsi_host/host%(h)s/device/session%(s)s/target' + '%(h)s:%(c)s:%(t)s/%(h)s:%(c)s:%(t)s:%(l)s/block/*' % + {'h': hctl[0], 'c': hctl[1], 't': hctl[2], 'l': hctl[3], + 's': session}) + # Sort devices and return the first so we don't return a partition + devices = sorted(glob.glob(path)) + device = os.path.split(devices[0])[1] if devices else None + LOG.debug('Searching for a device in session %s and hctl %s yield: %s', + session, hctl, device) + return device + + def scan_iscsi(self, host, channel='-', target='-', lun='-'): + """Send an iSCSI scan request given the host and optionally the ctl.""" + LOG.debug('Scanning host %(host)s c: %(channel)s, ' + 't: %(target)s, l: %(lun)s)', + {'host': host, 'channel': channel, + 'target': target, 'lun': lun}) + self.echo_scsi_command('/sys/class/scsi_host/host%s/scan' % host, + '%(c)s %(t)s %(l)s' % {'c': channel, + 't': target, + 'l': lun}) + + def multipath_add_wwid(self, wwid): + """Add a wwid to the list of know multipath wwids. + + This has the effect of multipathd being willing to create a dm for a + multipath even when there's only 1 device. + """ + out, err = self._execute('multipath', '-a', wwid, + run_as_root=True, + check_exit_code=False, + root_helper=self._root_helper) + return out.strip() == "wwid '" + wwid + "' added" + + def multipath_add_path(self, realpath): + """Add a path to multipathd for monitoring. + + This has the effect of multipathd checking an already checked device + for multipath. + + Together with `multipath_add_wwid` we can create a multipath when + there's only 1 path. + """ + stdout, stderr = self._execute('multipathd', 'add', 'path', realpath, + run_as_root=True, timeout=5, + check_exit_code=False, + root_helper=self._root_helper) + return stdout.strip() == 'ok' diff --git a/os_brick/privileged/rootwrap.py b/os_brick/privileged/rootwrap.py index ef630e74e..87e87536f 100644 --- a/os_brick/privileged/rootwrap.py +++ b/os_brick/privileged/rootwrap.py @@ -36,6 +36,7 @@ the urgency of (1)), then work on the larger refactor that addresses """ +import os import signal import six import threading @@ -191,3 +192,29 @@ def execute(*cmd, **kwargs): def execute_root(*cmd, **kwargs): """NB: Raises processutils.ProcessExecutionError/OSError on failure.""" return custom_execute(*cmd, shell=False, run_as_root=False, **kwargs) + + +@privileged.default.entrypoint +def unlink_root(*links, **kwargs): + """Unlink system links with sys admin privileges. + + By default it will raise an exception if a link does not exist and stop + unlinking remaining links. + + This behavior can be modified passing optional parameters `no_errors` and + `raise_at_end`. + + :param no_errors: Don't raise an exception on error + "param raise_at_end: Don't raise an exception on first error, try to + unlink all links and then raise a ChainedException + with all the errors that where found. + """ + no_errors = kwargs.get('no_errors', False) + raise_at_end = kwargs.get('raise_at_end', False) + exc = exception.ExceptionChainer() + catch_exception = no_errors or raise_at_end + for link in links: + with exc.context(catch_exception, 'Unlink failed for %s', link): + os.unlink(link) + if not no_errors and raise_at_end and exc: + raise exc diff --git a/os_brick/tests/initiator/connectors/test_iscsi.py b/os_brick/tests/initiator/connectors/test_iscsi.py index 2b9e65b71..1c3ded89b 100644 --- a/os_brick/tests/initiator/connectors/test_iscsi.py +++ b/os_brick/tests/initiator/connectors/test_iscsi.py @@ -12,19 +12,14 @@ # License for the specific language governing permissions and limitations # under the License. import collections -import glob import mock import os -import testtools -import time import ddt from oslo_concurrency import processutils as putils from os_brick import exception -from os_brick.initiator.connectors import base from os_brick.initiator.connectors import iscsi -from os_brick.initiator import host_driver from os_brick.initiator import linuxscsi from os_brick.privileged import rootwrap as priv_rootwrap from os_brick.tests.initiator import test_connector @@ -142,6 +137,41 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): ('ip3:port3', 'tgt3'): (set(), set())} self.assertDictEqual(expected, res) + @mock.patch('glob.glob') + @mock.patch.object(iscsi.ISCSIConnector, '_get_iscsi_sessions_full') + @mock.patch.object(iscsi.ISCSIConnector, '_get_iscsi_nodes') + def test_get_connection_devices_with_iqns(self, nodes_mock, sessions_mock, + glob_mock): + ips_iqns_luns = self.connector._get_all_targets(self.CON_PROPS) + + # List sessions from other targets and non tcp sessions + sessions_mock.return_value = [ + ('non-tcp:', '0', 'ip1:port1', '1', 'tgt1'), + ('tcp:', '1', 'ip1:port1', '1', 'tgt1'), + ('tcp:', '2', 'ip2:port2', '-1', 'tgt2'), + ('tcp:', '3', 'ip1:port1', '1', 'tgt4'), + ('tcp:', '4', 'ip2:port2', '-1', 'tgt5')] + # List 1 node without sessions + nodes_mock.return_value = [('ip1:port1', 'tgt1'), + ('ip2:port2', 'tgt2'), + ('ip3:port3', 'tgt3')] + sys_cls = '/sys/class/scsi_host/host' + glob_mock.side_effect = [ + [sys_cls + '1/device/session1/target6/1:2:6:4/block/sda', + sys_cls + '1/device/session1/target6/1:2:6:4/block/sda1'], + [sys_cls + '2/device/session2/target7/2:2:7:5/block/sdb', + sys_cls + '2/device/session2/target7/2:2:7:4/block/sdc'], + ] + with mock.patch.object(iscsi.ISCSIConnector, + '_get_all_targets') as get_targets_mock: + res = self.connector._get_connection_devices(mock.sentinel.props, + ips_iqns_luns) + expected = {('ip1:port1', 'tgt1'): ({'sda'}, set()), + ('ip2:port2', 'tgt2'): ({'sdb'}, {'sdc'}), + ('ip3:port3', 'tgt3'): (set(), set())} + self.assertDictEqual(expected, res) + get_targets_mock.assert_not_called() + def generate_device(self, location, iqn, transport=None, lun=1): dev_format = "ip-%s-iscsi-%s-lun-%s" % (location, iqn, lun) if transport: @@ -264,10 +294,9 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): fake_path = ("/dev/disk/by-path/ip-%(ip)s-iscsi-%(iqn)s-lun-%(lun)s" % {'ip': '10.0.2.15', 'iqn': iqn, 'lun': 1}) - fake_props = {} fake_devices = [fake_path] expected = fake_devices - mock_potential_paths.return_value = (fake_devices, fake_props) + mock_potential_paths.return_value = fake_devices connection_properties = self.iscsi_connection(vol, [location], [iqn]) @@ -336,129 +365,53 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): 'multipath_id': FAKE_SCSI_WWN} self.assertEqual(expected_result, result) - @mock.patch('time.sleep', mock.Mock()) - @mock.patch.object(iscsi.ISCSIConnector, 'disconnect_volume') - def _test_connect_volume(self, extra_props, additional_commands, - disconnect_vol_mock, transport=None): - # for making sure the /dev/disk/by-path is gone - exists_mock = mock.Mock() - exists_mock.return_value = True - os.path.exists = exists_mock + @mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_multipath_volume') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_single_volume') + def test_connect_volume_mp(self, con_single_mock, con_mp_mock, clean_mock): + self.connector.use_multipath = True + res = self.connector.connect_volume(self.CON_PROPS) + self.assertEqual(con_mp_mock.return_value, res) + con_single_mock.assert_not_called() + con_mp_mock.assert_called_once_with(self.CON_PROPS) + clean_mock.assert_not_called() - vol = {'id': 1, 'name': self._name} - connection_info = self.iscsi_connection(vol, self._location, self._iqn) - for key, value in extra_props.items(): - connection_info['data'][key] = value - if transport is not None: - dev_list = self.generate_device(self._location, self._iqn, - transport) - with mock.patch.object(glob, 'glob', return_value=[dev_list]): - device = self.connector.connect_volume(connection_info['data']) - else: - device = self.connector.connect_volume(connection_info['data']) + @mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_multipath_volume') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_single_volume') + def test_connect_volume_mp_failure(self, con_single_mock, con_mp_mock, + clean_mock): + self.connector.use_multipath = True + con_mp_mock.side_effect = exception.BrickException + self.assertRaises(exception.BrickException, + self.connector.connect_volume, self.CON_PROPS) + con_single_mock.assert_not_called() + con_mp_mock.assert_called_once_with(self.CON_PROPS) + clean_mock.assert_called_once_with(self.CON_PROPS, force=True) - dev_str = self.generate_device(self._location, self._iqn, transport) - self.assertEqual(device['type'], 'block') - self.assertEqual(device['path'], dev_str) + @mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_multipath_volume') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_single_volume') + def test_connect_volume_sp(self, con_single_mock, con_mp_mock, clean_mock): + self.connector.use_multipath = False + res = self.connector.connect_volume(self.CON_PROPS) + self.assertEqual(con_single_mock.return_value, res) + con_mp_mock.assert_not_called() + con_single_mock.assert_called_once_with(self.CON_PROPS) + clean_mock.assert_not_called() - self.count = 0 - - # Disconnect has its own tests, should not be tested here - expected_commands = [ - ('iscsiadm -m node -T %s -p %s' % (self._iqn, self._location)), - ('iscsiadm -m session'), - ('iscsiadm -m node -T %s -p %s --login' % (self._iqn, - self._location)), - ('iscsiadm -m node -T %s -p %s --op update' - ' -n node.startup -v automatic' % (self._iqn, - self._location)), - ('/lib/udev/scsi_id --page 0x83 --whitelisted %s' % dev_str), - ] + additional_commands - - self.assertEqual(expected_commands, self.cmds) - - @testtools.skipUnless(os.path.exists('/dev/disk/by-path'), - 'Test requires /dev/disk/by-path') - def test_connect_volume(self): - self._test_connect_volume({}, []) - - @testtools.skipUnless(os.path.exists('/dev/disk/by-path'), - 'Test requires /dev/disk/by-path') - @mock.patch.object(iscsi.ISCSIConnector, '_get_transport') - def test_connect_volume_with_transport(self, mock_transport): - mock_transport.return_value = 'fake_transport' - self._test_connect_volume({}, [], transport='fake_transport') - - @testtools.skipUnless(os.path.exists('/dev/disk/by-path'), - 'Test requires /dev/disk/by-path') - @mock.patch('os.path.exists', side_effect=(True,) * 4 + (False, False)) - @mock.patch.object(iscsi.ISCSIConnector, '_run_iscsiadm') - def test_connect_volume_with_alternative_targets_primary_error( - self, mock_iscsiadm, mock_exists): - location2 = '[2001:db8::1]:3260' - dev_loc2 = '2001:db8::1:3260' # udev location2 - iqn2 = 'iqn.2010-10.org.openstack:%s-2' % self._name - vol = {'id': 1, 'name': self._name} - connection_info = self.iscsi_connection(vol, self._location, self._iqn) - connection_info['data']['target_portals'] = [self._location, location2] - connection_info['data']['target_iqns'] = [self._iqn, iqn2] - connection_info['data']['target_luns'] = [self._lun, 2] - dev_str2 = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-2' % (dev_loc2, iqn2) - - def fake_run_iscsiadm(iscsi_properties, iscsi_command, **kwargs): - if iscsi_properties['target_portal'] == self._location: - if iscsi_command == ('--login',): - raise putils.ProcessExecutionError(None, None, 21) - return mock.DEFAULT - - mock_iscsiadm.side_effect = fake_run_iscsiadm - mock_exists.side_effect = lambda x: x == dev_str2 - device = self.connector.connect_volume(connection_info['data']) - self.assertEqual('block', device['type']) - self.assertEqual(dev_str2, device['path']) - props = connection_info['data'].copy() - for key in ('target_portals', 'target_iqns', 'target_luns'): - props.pop(key, None) - props['target_portal'] = location2 - props['target_iqn'] = iqn2 - props['target_lun'] = 2 - mock_iscsiadm.assert_any_call(props, ('--login',), - check_exit_code=[0, 255]) - - @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn') - @mock.patch.object(iscsi.ISCSIConnector, '_run_iscsiadm_bare') - @mock.patch.object(iscsi.ISCSIConnector, - '_get_target_portals_from_iscsiadm_output') - @mock.patch.object(iscsi.ISCSIConnector, '_connect_to_iscsi_portal') - @mock.patch.object(iscsi.ISCSIConnector, '_rescan_iscsi') - @mock.patch.object(os.path, 'exists', return_value=True) - @mock.patch.object(base.BaseLinuxConnector, '_discover_mpath_device') - def test_connect_volume_with_multipath( - self, mock_discover_mpath_device, exists_mock, - rescan_iscsi_mock, connect_to_mock, - portals_mock, iscsiadm_mock, mock_iscsi_wwn): - mock_iscsi_wwn.return_value = test_connector.FAKE_SCSI_WWN - location = '10.0.2.15:3260' - name = 'volume-00000001' - iqn = 'iqn.2010-10.org.openstack:%s' % name - vol = {'id': 1, 'name': name} - connection_properties = self.iscsi_connection(vol, location, iqn) - mock_discover_mpath_device.return_value = ( - 'iqn.2010-10.org.openstack:%s' % name, - test_connector.FAKE_SCSI_WWN) - - self.connector_with_multipath = \ - iscsi.ISCSIConnector(None, use_multipath=True) - iscsiadm_mock.return_value = "%s %s" % (location, iqn) - portals_mock.return_value = ([location], [iqn]) - - result = self.connector_with_multipath.connect_volume( - connection_properties['data']) - expected_result = {'multipath_id': test_connector.FAKE_SCSI_WWN, - 'path': 'iqn.2010-10.org.openstack:volume-00000001', - 'type': 'block', - 'scsi_wwn': test_connector.FAKE_SCSI_WWN} - self.assertEqual(expected_result, result) + @mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_multipath_volume') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_single_volume') + def test_connect_volume_sp_failure(self, con_single_mock, con_mp_mock, + clean_mock): + self.connector.use_multipath = False + con_single_mock.side_effect = exception.BrickException + self.assertRaises(exception.BrickException, + self.connector.connect_volume, self.CON_PROPS) + con_mp_mock.assert_not_called() + con_single_mock.assert_called_once_with(self.CON_PROPS) + clean_mock.assert_called_once_with(self.CON_PROPS, force=True) def test_discover_iscsi_portals(self): location = '10.0.2.15:3260' @@ -549,229 +502,6 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): self.connector_with_multipath.connect_volume, connection_properties['data']) - @mock.patch.object(iscsi.ISCSIConnector, - '_get_hosts_channels_targets_luns', return_value=[]) - @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn') - @mock.patch('os.path.exists', side_effect=(True,) * 7 + (False, False)) - @mock.patch.object(host_driver.HostDriver, 'get_all_block_devices') - @mock.patch.object(iscsi.ISCSIConnector, '_run_multipath') - @mock.patch.object(base.BaseLinuxConnector, '_discover_mpath_device') - @mock.patch.object(linuxscsi.LinuxSCSI, 'process_lun_id') - def test_connect_volume_with_multiple_portals( - self, mock_process_lun_id, mock_discover_mpath_device, - mock_run_multipath, mock_devices, mock_exists, mock_scsi_wwn, - mock_get_htcls): - mock_scsi_wwn.return_value = test_connector.FAKE_SCSI_WWN - location2 = '[2001:db8::1]:3260' - dev_loc2 = '2001:db8::1:3260' # udev location2 - name2 = 'volume-00000001-2' - iqn2 = 'iqn.2010-10.org.openstack:%s' % name2 - lun2 = 2 - - fake_multipath_dev = '/dev/mapper/fake-multipath-dev' - vol = {'id': 1, 'name': self._name} - connection_properties = self.iscsi_connection_multipath( - vol, [self._location, location2], [self._iqn, iqn2], [self._lun, - lun2]) - devs = ['/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (self._location, - self._iqn), - '/dev/disk/by-path/ip-%s-iscsi-%s-lun-2' % (dev_loc2, iqn2)] - mock_devices.return_value = devs - # mock_iscsi_devices.return_value = devs - # mock_get_iqn.return_value = [self._iqn, iqn2] - mock_discover_mpath_device.return_value = ( - fake_multipath_dev, test_connector.FAKE_SCSI_WWN) - mock_process_lun_id.return_value = [self._lun, lun2] - - result = self.connector_with_multipath.connect_volume( - connection_properties['data']) - expected_result = {'multipath_id': test_connector.FAKE_SCSI_WWN, - 'path': fake_multipath_dev, 'type': 'block', - 'scsi_wwn': test_connector.FAKE_SCSI_WWN} - cmd_format = 'iscsiadm -m node -T %s -p %s --%s' - expected_commands = [cmd_format % (self._iqn, self._location, 'login'), - cmd_format % (iqn2, location2, 'login')] - self.assertEqual(expected_result, result) - for command in expected_commands: - self.assertIn(command, self.cmds) - - mock_get_htcls.assert_called_once_with([(self._location, self._iqn, - self._lun), - (location2, iqn2, lun2)]) - - @mock.patch.object(iscsi.ISCSIConnector, - '_get_hosts_channels_targets_luns', return_value=[]) - @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn') - @mock.patch.object(os.path, 'exists') - @mock.patch.object(host_driver.HostDriver, 'get_all_block_devices') - @mock.patch.object(iscsi.ISCSIConnector, '_run_multipath') - @mock.patch.object(iscsi.ISCSIConnector, '_run_iscsiadm') - @mock.patch.object(base.BaseLinuxConnector, '_discover_mpath_device') - @mock.patch.object(linuxscsi.LinuxSCSI, 'process_lun_id') - def test_connect_volume_with_multiple_portals_primary_error( - self, mock_process_lun_id, mock_discover_mpath_device, - mock_iscsiadm, mock_run_multipath, mock_devices, mock_exists, - mock_scsi_wwn, mock_get_htcls): - mock_scsi_wwn.return_value = test_connector.FAKE_SCSI_WWN - location1 = '10.0.2.15:3260' - location2 = '[2001:db8::1]:3260' - dev_loc2 = '2001:db8::1:3260' # udev location2 - name1 = 'volume-00000001-1' - name2 = 'volume-00000001-2' - iqn1 = 'iqn.2010-10.org.openstack:%s' % name1 - iqn2 = 'iqn.2010-10.org.openstack:%s' % name2 - fake_multipath_dev = '/dev/mapper/fake-multipath-dev' - vol = {'id': 1, 'name': name1} - connection_properties = self.iscsi_connection_multipath( - vol, [location1, location2], [iqn1, iqn2], [1, 2]) - dev1 = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (location1, iqn1) - dev2 = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-2' % (dev_loc2, iqn2) - - def fake_run_iscsiadm(iscsi_properties, iscsi_command, **kwargs): - if iscsi_properties['target_portal'] == location1: - if iscsi_command == ('--login',): - raise putils.ProcessExecutionError(None, None, 21) - return mock.DEFAULT - - mock_exists.side_effect = lambda x: x != dev1 - mock_devices.return_value = [dev2] - mock_iscsiadm.side_effect = fake_run_iscsiadm - - mock_discover_mpath_device.return_value = ( - fake_multipath_dev, test_connector.FAKE_SCSI_WWN) - mock_process_lun_id.return_value = [1, 2] - - props = connection_properties['data'].copy() - result = self.connector_with_multipath.connect_volume( - connection_properties['data']) - - expected_result = {'multipath_id': test_connector.FAKE_SCSI_WWN, - 'path': fake_multipath_dev, 'type': 'block', - 'scsi_wwn': test_connector.FAKE_SCSI_WWN} - self.assertEqual(expected_result, result) - props['target_portal'] = location1 - props['target_iqn'] = iqn1 - mock_iscsiadm.assert_any_call(props, ('--login',), - check_exit_code=[0, 255]) - props['target_portal'] = location2 - props['target_iqn'] = iqn2 - mock_iscsiadm.assert_any_call(props, ('--login',), - check_exit_code=[0, 255]) - - lun1, lun2 = connection_properties['data']['target_luns'] - mock_get_htcls.assert_called_once_with([(location1, iqn1, lun1), - (location2, iqn2, lun2)]) - - @mock.patch.object(iscsi.ISCSIConnector, - '_get_hosts_channels_targets_luns', return_value=[]) - @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn') - @mock.patch.object(os.path, 'exists', return_value=True) - @mock.patch.object(iscsi.ISCSIConnector, - '_get_target_portals_from_iscsiadm_output') - @mock.patch.object(iscsi.ISCSIConnector, '_connect_to_iscsi_portal') - @mock.patch.object(host_driver.HostDriver, 'get_all_block_devices') - @mock.patch.object(iscsi.ISCSIConnector, '_run_multipath') - @mock.patch.object(base.BaseLinuxConnector, '_discover_mpath_device') - def test_connect_volume_with_multipath_connecting( - self, mock_discover_mpath_device, mock_run_multipath, - mock_devices, - mock_connect, mock_portals, mock_exists, mock_scsi_wwn, - mock_get_htcls): - mock_scsi_wwn.return_value = test_connector.FAKE_SCSI_WWN - location1 = '10.0.2.15:3260' - location2 = '[2001:db8::1]:3260' - dev_loc2 = '2001:db8::1:3260' # udev location2 - name1 = 'volume-00000001-1' - name2 = 'volume-00000001-2' - iqn1 = 'iqn.2010-10.org.openstack:%s' % name1 - iqn2 = 'iqn.2010-10.org.openstack:%s' % name2 - fake_multipath_dev = '/dev/mapper/fake-multipath-dev' - vol = {'id': 1, 'name': name1} - connection_properties = self.iscsi_connection(vol, location1, iqn1) - devs = ['/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (location1, iqn1), - '/dev/disk/by-path/ip-%s-iscsi-%s-lun-2' % (dev_loc2, iqn2)] - mock_devices.return_value = devs - mock_portals.return_value = ([location1, location2, location2], - [iqn1, iqn1, iqn2]) - mock_discover_mpath_device.return_value = ( - fake_multipath_dev, test_connector.FAKE_SCSI_WWN) - - result = self.connector_with_multipath.connect_volume( - connection_properties['data']) - expected_result = {'multipath_id': test_connector.FAKE_SCSI_WWN, - 'path': fake_multipath_dev, 'type': 'block', - 'scsi_wwn': test_connector.FAKE_SCSI_WWN} - props1 = connection_properties['data'].copy() - props2 = connection_properties['data'].copy() - locations = list(set([location1, location2])) # order may change - props1['target_portal'] = locations[0] - props2['target_portal'] = locations[1] - expected_calls = [mock.call(props1), mock.call(props2)] - self.assertEqual(expected_result, result) - mock_connect.assert_has_calls(expected_calls, any_order=True) - self.assertEqual(len(expected_calls), mock_connect.call_count) - lun = connection_properties['data']['target_lun'] - self.assertEqual(1, mock_get_htcls.call_count) - # Order of elements in the list is randomized because it comes from - # a set. - self.assertSetEqual({(location1, iqn1, lun), (location2, iqn1, lun)}, - set(mock_get_htcls.call_args[0][0])) - - @mock.patch('retrying.time.sleep', mock.Mock()) - @mock.patch.object(os.path, 'exists', return_value=True) - @mock.patch.object(iscsi.ISCSIConnector, - '_get_target_portals_from_iscsiadm_output') - @mock.patch.object(iscsi.ISCSIConnector, '_connect_to_iscsi_portal') - @mock.patch.object(host_driver.HostDriver, 'get_all_block_devices') - @mock.patch.object(iscsi.ISCSIConnector, '_run_multipath') - def test_connect_volume_multipath_failed_iscsi_login( - self, mock_run_multipath, mock_devices, mock_connect, mock_portals, - mock_exists): - location1 = '10.0.2.15:3260' - location2 = '10.0.3.15:3260' - name1 = 'volume-00000001-1' - name2 = 'volume-00000001-2' - iqn1 = 'iqn.2010-10.org.openstack:%s' % name1 - iqn2 = 'iqn.2010-10.org.openstack:%s' % name2 - vol = {'id': 1, 'name': name1} - connection_properties = self.iscsi_connection(vol, location1, iqn1) - devs = ['/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (location1, iqn1), - '/dev/disk/by-path/ip-%s-iscsi-%s-lun-2' % (location2, iqn2)] - mock_devices.return_value = devs - mock_portals.return_value = ([location1, location2, location2], - [iqn1, iqn1, iqn2]) - - mock_connect.return_value = False - self.assertRaises(exception.FailedISCSITargetPortalLogin, - self.connector_with_multipath.connect_volume, - connection_properties['data']) - - @mock.patch.object(iscsi.ISCSIConnector, '_connect_to_iscsi_portal') - def test_connect_volume_failed_iscsi_login(self, mock_connect): - location1 = '10.0.2.15:3260' - name1 = 'volume-00000001-1' - iqn1 = 'iqn.2010-10.org.openstack:%s' % name1 - vol = {'id': 1, 'name': name1} - connection_properties = self.iscsi_connection(vol, location1, iqn1) - - mock_connect.return_value = False - self.assertRaises(exception.FailedISCSITargetPortalLogin, - self.connector.connect_volume, - connection_properties['data']) - - @mock.patch.object(time, 'sleep') - @mock.patch.object(os.path, 'exists', return_value=False) - def test_connect_volume_with_not_found_device(self, exists_mock, - sleep_mock): - location = '10.0.2.15:3260' - name = 'volume-00000001' - iqn = 'iqn.2010-10.org.openstack:%s' % name - vol = {'id': 1, 'name': name} - connection_info = self.iscsi_connection(vol, location, iqn) - self.assertRaises(exception.VolumeDeviceNotFound, - self.connector.connect_volume, - connection_info['data']) - def test_get_target_portals_from_iscsiadm_output(self): connector = self.connector test_output = '''10.15.84.19:3260 iqn.1992-08.com.netapp:sn.33615311 @@ -783,13 +513,25 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): expected = (ips, iqns) self.assertEqual(expected, res) + @mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection') + def test_disconnect_volume(self, cleanup_mock): + res = self.connector.disconnect_volume(mock.sentinel.con_props, + mock.sentinel.dev_info, + mock.sentinel.Force, + mock.sentinel.ignore_errors) + self.assertEqual(cleanup_mock.return_value, res) + cleanup_mock.assert_called_once_with( + mock.sentinel.con_props, + force=mock.sentinel.Force, + ignore_errors=mock.sentinel.ignore_errors) + @mock.patch.object(iscsi.ISCSIConnector, '_disconnect_connection') @mock.patch.object(iscsi.ISCSIConnector, '_get_connection_devices') @mock.patch.object(linuxscsi.LinuxSCSI, 'flush_multipath_device') @mock.patch.object(linuxscsi.LinuxSCSI, 'remove_connection', return_value=None) - def test_disconnect_volume(self, remove_mock, flush_mock, con_devs_mock, - discon_mock): + def test_cleanup_connection(self, remove_mock, flush_mock, con_devs_mock, + discon_mock): # Return an ordered dicts instead of normal dict for discon_mock.assert con_devs_mock.return_value = collections.OrderedDict(( (('ip1:port1', 'tgt1'), ({'sda'}, set())), @@ -798,10 +540,12 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): with mock.patch.object(self.connector, 'use_multipath') as use_mp_mock: - self.connector.disconnect_volume(self.CON_PROPS, - mock.sentinel.dev_info) + self.connector._cleanup_connection( + self.CON_PROPS, ips_iqns_luns=mock.sentinel.ips_iqns_luns, + force=False, ignore_errors=False) - con_devs_mock.assert_called_once_with(self.CON_PROPS) + con_devs_mock.assert_called_once_with(self.CON_PROPS, + mock.sentinel.ips_iqns_luns) remove_mock.assert_called_once_with({'sda', 'sdb'}, use_mp_mock, False, mock.ANY) discon_mock.assert_called_once_with( @@ -819,8 +563,9 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(linuxscsi.LinuxSCSI, 'flush_multipath_device') @mock.patch.object(linuxscsi.LinuxSCSI, 'remove_connection', return_value=mock.sentinel.mp_name) - def test_disconnect_volume_force_failure(self, remove_mock, flush_mock, - con_devs_mock, discon_mock): + def test_cleanup_connection_force_failure(self, remove_mock, flush_mock, + con_devs_mock, discon_mock): + # Return an ordered dicts instead of normal dict for discon_mock.assert con_devs_mock.return_value = collections.OrderedDict(( (('ip1:port1', 'tgt1'), ({'sda'}, set())), @@ -830,11 +575,13 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): with mock.patch.object(self.connector, 'use_multipath', wraps=True) as use_mp_mock: self.assertRaises(exception.ExceptionChainer, - self.connector.disconnect_volume, - self.CON_PROPS, mock.sentinel.dev_info, - mock.sentinel.force, ignore_errors=False) + self.connector._cleanup_connection, + self.CON_PROPS, + ips_iqns_luns=mock.sentinel.ips_iqns_luns, + force=mock.sentinel.force, ignore_errors=False) - con_devs_mock.assert_called_once_with(self.CON_PROPS) + con_devs_mock.assert_called_once_with(self.CON_PROPS, + mock.sentinel.ips_iqns_luns) remove_mock.assert_called_once_with({'sda', 'sdb'}, use_mp_mock, mock.sentinel.force, mock.ANY) discon_mock.assert_called_once_with( @@ -1020,128 +767,471 @@ Setting up iSCSI targets: unused actual = self.connector.get_all_available_volumes() self.assertItemsEqual(expected, actual) - @mock.patch.object(iscsi.ISCSIConnector, '_discover_iscsi_portals') - def test_get_potential_paths_failure_mpath_single_target(self, - mock_discover): - connection_properties = { - 'target_portal': '10.0.2.15:3260' - } + @mock.patch.object(iscsi.ISCSIConnector, '_get_device_path') + def test_get_potential_paths_mpath(self, get_path_mock): self.connector.use_multipath = True - mock_discover.side_effect = exception.BrickException() - self.assertRaises(exception.TargetPortalNotFound, - self.connector._get_potential_volume_paths, - connection_properties) + res = self.connector._get_potential_volume_paths(self.CON_PROPS) + get_path_mock.assert_called_once_with(self.CON_PROPS) + self.assertEqual(get_path_mock.return_value, res) + self.assertEqual([], self.cmds) + + @mock.patch.object(iscsi.ISCSIConnector, '_get_iscsi_sessions') + @mock.patch.object(iscsi.ISCSIConnector, '_get_device_path') + def test_get_potential_paths_single_path(self, get_path_mock, + get_sessions_mock): + get_path_mock.side_effect = [['path1'], ['path2'], ['path3', 'path4']] + get_sessions_mock.return_value = [ + ('tcp:', 'session1', 'ip1:port1', '1', 'tgt1'), + ('tcp:', 'session2', 'ip2:port2', '-1', 'tgt2'), + ('tcp:', 'session3', 'ip3:port3', '1', 'tgt3')] + + self.connector.use_multipath = False + res = self.connector._get_potential_volume_paths(self.CON_PROPS) + self.assertEqual({'path1', 'path2', 'path3', 'path4'}, set(res)) + get_sessions_mock.assert_called_once_with() @mock.patch.object(iscsi.ISCSIConnector, '_discover_iscsi_portals') - def test_get_potential_paths_failure_mpath_multi_target(self, - mock_discover): - connection_properties = { - 'target_portals': ['10.0.2.15:3260', '10.0.3.15:3260'] - } - self.connector.use_multipath = True - mock_discover.side_effect = exception.BrickException() - self.assertRaises(exception.TargetPortalsNotFound, - self.connector._get_potential_volume_paths, - connection_properties) + def test_get_ips_iqns_luns_with_target_iqns(self, discover_mock): + res = self.connector._get_ips_iqns_luns(self.CON_PROPS) + self.assertEqual(discover_mock.return_value, res) - @mock.patch.object(iscsi.ISCSIConnector, - '_get_hosts_channels_targets_luns') - def test_rescan_iscsi_no_hctls(self, mock_get_htcls): - mock_get_htcls.side_effect = exception.HostChannelsTargetsNotFound( - iqns=['iqn1', 'iqn2'], found=[]) - with mock.patch.object(self.connector, '_linuxscsi') as mock_linuxscsi: - self.connector._rescan_iscsi(mock.sentinel.input) - mock_linuxscsi.echo_scsi_command.assert_not_called() - mock_get_htcls.assert_called_once_with(mock.sentinel.input) + @mock.patch.object(iscsi.ISCSIConnector, '_discover_iscsi_portals') + def test_get_ips_iqns_luns_no_target_iqns_share_iqn(self, discover_mock): + con_props = {'volume_id': 'vol_id', + 'target_portal': 'ip1:port1', + 'target_iqn': 'tgt1', + 'target_lun': '1'} + discover_mock.return_value = [('ip1:port1', 'tgt1', '1'), + ('ip1:port1', 'tgt2', '1'), + ('ip2:port2', 'tgt1', '2'), + ('ip2:port2', 'tgt2', '2')] + res = self.connector._get_ips_iqns_luns(con_props) + expected = {('ip1:port1', 'tgt1', '1'), + ('ip2:port2', 'tgt1', '2')} + self.assertEqual(expected, set(res)) - @mock.patch.object(iscsi.ISCSIConnector, - '_get_hosts_channels_targets_luns') - def test_rescan_iscsi_partial_hctls(self, mock_get_htcls): - mock_get_htcls.side_effect = exception.HostChannelsTargetsNotFound( - iqns=['iqn1'], found=[('h', 'c', 't', 'l')]) - with mock.patch.object(self.connector, '_linuxscsi') as mock_linuxscsi: - self.connector._rescan_iscsi(mock.sentinel.input) - mock_linuxscsi.echo_scsi_command.assert_called_once_with( - 'h/scan', 'c t l') - mock_get_htcls.assert_called_once_with(mock.sentinel.input) + @mock.patch.object(iscsi.ISCSIConnector, '_discover_iscsi_portals') + def test_get_ips_iqns_luns_no_target_iqns_diff_iqn(self, discover_mock): + con_props = {'volume_id': 'vol_id', + 'target_portal': 'ip1:port1', + 'target_iqn': 'tgt1', + 'target_lun': '1'} + discover_mock.return_value = [('ip1:port1', 'tgt1', '1'), + ('ip2:port2', 'tgt2', '2')] + res = self.connector._get_ips_iqns_luns(con_props) + self.assertEqual(discover_mock.return_value, res) - @mock.patch.object(iscsi.ISCSIConnector, - '_get_hosts_channels_targets_luns') - @mock.patch.object(iscsi.ISCSIConnector, '_run_iscsiadm_bare') - def test_rescan_iscsi_hctls(self, mock_iscsiadm, mock_get_htcls): - mock_get_htcls.return_value = [ - ('/sys/class/iscsi_host/host4', '0', '0', '1'), - ('/sys/class/iscsi_host/host5', '0', '0', '2'), + @mock.patch.object(iscsi.ISCSIConnector, '_get_iscsi_sessions_full') + def test_connect_to_iscsi_portal_all_new(self, get_sessions_mock): + """Connect creating node and session.""" + session = 'session2' + get_sessions_mock.side_effect = [ + [('tcp:', 'session1', 'ip1:port1', '1', 'tgt')], + [('tcp:', 'session1', 'ip1:port1', '1', 'tgt'), + ('tcp:', session, 'ip1:port1', '-1', 'tgt1')] ] - - with mock.patch.object(self.connector, '_linuxscsi') as mock_linuxscsi: - self.connector._rescan_iscsi(mock.sentinel.input) - mock_linuxscsi.echo_scsi_command.assert_has_calls(( - mock.call('/sys/class/iscsi_host/host4/scan', '0 0 1'), - mock.call('/sys/class/iscsi_host/host5/scan', '0 0 2'), - )) - mock_get_htcls.assert_called_once_with(mock.sentinel.input) - mock_iscsiadm.assert_not_called() - - @mock.patch('six.moves.builtins.open', create=True) - @mock.patch('glob.glob') - def test_get_hctls(self, mock_glob, mock_open): - host4 = '/sys/class/scsi_host/host4' - host5 = '/sys/class/scsi_host/host5' - host6 = '/sys/class/scsi_host/host6' - host7 = '/sys/class/scsi_host/host7' - - mock_glob.side_effect = ( - (host4 + '/device/session5/target0:1:2', - host5 + '/device/session6/target3:4:5', - host6 + '/device/session7/target6:7:8', - host7 + '/device/session8/target9:10:11'), - (host4 + '/device/session5/iscsi_session/session5/targetname', - host5 + '/device/session6/iscsi_session/session6/targetname', - host6 + '/device/session7/iscsi_session/session7/targetname', - host7 + '/device/session8/iscsi_session/session8/targetname'), - ) - - mock_open.side_effect = ( - mock.mock_open(read_data='iqn0\n').return_value, - mock.mock_open(read_data='iqn1\n').return_value, - mock.mock_open(read_data='iqn2\n').return_value, - mock.mock_open(read_data='iqn3\n').return_value, - ) - - ips_iqns_luns = [('ip1', 'iqn1', 'lun1'), ('ip2', 'iqn2', 'lun2')] - result = self.connector._get_hosts_channels_targets_luns(ips_iqns_luns) - self.assertEqual( - [(host5, '4', '5', 'lun1'), (host6, '7', '8', 'lun2')], - result) - mock_glob.assert_has_calls(( - mock.call('/sys/class/scsi_host/host*/device/session*/target*'), - mock.call('/sys/class/scsi_host/host*/device/session*/' - 'iscsi_session/session*/targetname'), - )) - self.assertEqual(3, mock_open.call_count) - - @mock.patch('retrying.time.sleep', mock.Mock()) - @mock.patch('six.moves.builtins.open', create=True) - @mock.patch('glob.glob', return_value=[]) - def test_get_hctls_not_found(self, mock_glob, mock_open): - host4 = '/sys/class/scsi_host/host4' - mock_glob.side_effect = [ - [(host4 + '/device/session5/target0:1:2')], - [(host4 + '/device/session5/iscsi_session/session5/targetname')], - ] * 3 - # Test exception on open as well as having only half of the htcls - mock_open.side_effect = [ - mock.Mock(side_effect=Exception()), - mock.mock_open(read_data='iqn1\n').return_value, - mock.mock_open(read_data='iqn1\n').return_value, + with mock.patch.object(self.connector, '_execute') as exec_mock: + exec_mock.side_effect = [('', 'error'), ('', None), ('', None), + ('', None)] + res = self.connector._connect_to_iscsi_portal(self.CON_PROPS) + self.assertEqual(session, res) + prefix = 'iscsiadm -m node -T tgt1 -p ip1:port1' + expected_cmds = [ + prefix, + prefix + ' --interface default --op new', + prefix + ' --login', + prefix + ' --op update -n node.startup -v automatic' ] + actual_cmds = [' '.join(args[0]) for args in exec_mock.call_args_list] + self.assertListEqual(expected_cmds, actual_cmds) + self.assertEqual(2, get_sessions_mock.call_count) - ips_iqns_luns = [('ip1', 'iqn1', 'lun1'), ('ip2', 'iqn2', 'lun2')] + @mock.patch.object(iscsi.ISCSIConnector, '_get_iscsi_sessions_full') + def test_connect_to_iscsi_portal_all_exists_chap(self, get_sessions_mock): + """Node and session already exists and we use chap authentication.""" + session = 'session2' + get_sessions_mock.return_value = [('tcp:', session, 'ip1:port1', + '-1', 'tgt1')] + con_props = self.CON_PROPS.copy() + con_props.update(auth_method='CHAP', auth_username='user', + auth_password='pwd') + res = self.connector._connect_to_iscsi_portal(con_props) + self.assertEqual(session, res) + prefix = 'iscsiadm -m node -T tgt1 -p ip1:port1' + expected_cmds = [ + prefix, + prefix + ' --op update -n node.session.auth.authmethod -v CHAP', + prefix + ' --op update -n node.session.auth.username -v user', + prefix + ' --op update -n node.session.auth.password -v pwd', + ] + self.assertListEqual(expected_cmds, self.cmds) + get_sessions_mock.assert_called_once_with() - exc = self.assertRaises( - exception.HostChannelsTargetsNotFound, - self.connector._get_hosts_channels_targets_luns, ips_iqns_luns) + @mock.patch.object(iscsi.ISCSIConnector, '_get_iscsi_sessions_full') + def test_connect_to_iscsi_portal_fail_login(self, get_sessions_mock): + get_sessions_mock.return_value = [] + with mock.patch.object(self.connector, '_execute') as exec_mock: + exec_mock.side_effect = [('', None), putils.ProcessExecutionError] + res = self.connector._connect_to_iscsi_portal(self.CON_PROPS) + self.assertIsNone(res) + expected_cmds = ['iscsiadm -m node -T tgt1 -p ip1:port1', + 'iscsiadm -m node -T tgt1 -p ip1:port1 --login'] + actual_cmds = [' '.join(args[0]) for args in exec_mock.call_args_list] + self.assertListEqual(expected_cmds, actual_cmds) + get_sessions_mock.assert_called_once_with() - # Verify exception contains found results - self.assertEqual([(host4, '1', '2', 'lun1')], exc.found) + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwn', + side_effect=(None, 'tgt2')) + @mock.patch.object(iscsi.ISCSIConnector, '_connect_vol') + @mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection') + @mock.patch('time.sleep') + def test_connect_single_volume(self, sleep_mock, cleanup_mock, + connect_mock, get_wwn_mock): + def my_connect(rescans, props, data): + if props['target_iqn'] == 'tgt2': + # Succeed on second call + data['found_devices'].append('sdz') + + connect_mock.side_effect = my_connect + + res = self.connector._connect_single_volume(self.CON_PROPS) + + expected = {'type': 'block', 'scsi_wwn': 'tgt2', 'path': '/dev/sdz'} + self.assertEqual(expected, res) + get_wwn_mock.assert_has_calls([mock.call(['sdz']), mock.call(['sdz'])]) + sleep_mock.assert_called_once_with(1) + cleanup_mock.assert_called_once_with( + {'target_lun': 4, 'volume_id': 'vol_id', + 'target_portal': 'ip1:port1', 'target_iqn': 'tgt1'}, + (('ip1:port1', 'tgt1', 4),), + force=True, ignore_errors=True) + + @staticmethod + def _get_connect_vol_data(): + return {'stop_connecting': False, 'num_logins': 0, 'failed_logins': 0, + 'stopped_threads': 0, 'found_devices': [], + 'just_added_devices': []} + + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwn', + side_effect=(None, 'tgt2')) + @mock.patch.object(iscsi.ISCSIConnector, '_connect_vol') + @mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection') + @mock.patch('time.sleep') + def test_connect_single_volume_not_found(self, sleep_mock, cleanup_mock, + connect_mock, get_wwn_mock): + + self.assertRaises(exception.VolumeDeviceNotFound, + self.connector._connect_single_volume, + self.CON_PROPS) + + get_wwn_mock.assert_not_called() + + # Called twice by the retry mechanism + self.assertEqual(2, sleep_mock.call_count) + + props = list(self.connector._get_all_targets(self.CON_PROPS)) + calls_per_try = [ + mock.call({'target_portal': prop[0], 'target_iqn': prop[1], + 'target_lun': prop[2], 'volume_id': 'vol_id'}, + (prop,), force=True, ignore_errors=True) + for prop in props + ] + cleanup_mock.assert_has_calls(calls_per_try * 3) + + data = self._get_connect_vol_data() + calls_per_try = [mock.call(self.connector.device_scan_attempts, + {'target_portal': prop[0], + 'target_iqn': prop[1], + 'target_lun': prop[2], + 'volume_id': 'vol_id'}, + data) + for prop in props] + connect_mock.assert_has_calls(calls_per_try * 3) + + @mock.patch.object(linuxscsi.LinuxSCSI, 'find_sysfs_multipath_dm', + side_effect=[None, 'dm-0']) + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwn', + return_value='wwn') + @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_add_path') + @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_add_wwid') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_vol') + @mock.patch('time.sleep') + def test_connect_multipath_volume_all_succeed(self, sleep_mock, + connect_mock, add_wwid_mock, + add_path_mock, get_wwn_mock, + find_dm_mock): + def my_connect(rescans, props, data): + devs = {'tgt1': 'sda', 'tgt2': 'sdb', 'tgt3': 'sdc', 'tgt4': 'sdd'} + data['stopped_threads'] += 1 + data['num_logins'] += 1 + dev = devs[props['target_iqn']] + data['found_devices'].append(dev) + data['just_added_devices'].append(dev) + + connect_mock.side_effect = my_connect + + res = self.connector._connect_multipath_volume(self.CON_PROPS) + + expected = {'type': 'block', 'scsi_wwn': 'wwn', 'multipath_id': 'dm-0', + 'path': '/dev/dm-0'} + self.assertEqual(expected, res) + + self.assertEqual(1, get_wwn_mock.call_count) + result = list(get_wwn_mock.call_args[0][0]) + result.sort() + self.assertEqual(['sda', 'sdb', 'sdc', 'sdd'], result) + add_wwid_mock.assert_called_once_with('wwn') + self.assertNotEqual(0, add_path_mock.call_count) + self.assertGreaterEqual(find_dm_mock.call_count, 2) + self.assertEqual(4, connect_mock.call_count) + + @mock.patch.object(linuxscsi.LinuxSCSI, 'find_sysfs_multipath_dm', + side_effect=[None, 'dm-0']) + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwn', + return_value='wwn') + @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_add_path') + @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_add_wwid') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_vol') + @mock.patch('time.sleep') + def test_connect_multipath_volume_all_fail(self, sleep_mock, connect_mock, + add_wwid_mock, add_path_mock, + get_wwn_mock, find_dm_mock): + def my_connect(rescans, props, data): + data['stopped_threads'] += 1 + data['failed_logins'] += 1 + + connect_mock.side_effect = my_connect + + self.assertRaises(exception.VolumeDeviceNotFound, + self.connector._connect_multipath_volume, + self.CON_PROPS) + + get_wwn_mock.assert_not_called() + add_wwid_mock.assert_not_called() + add_path_mock.assert_not_called() + find_dm_mock.assert_not_called() + self.assertEqual(4 * 3, connect_mock.call_count) + + @mock.patch.object(linuxscsi.LinuxSCSI, 'find_sysfs_multipath_dm', + side_effect=[None, 'dm-0']) + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwn', + return_value='wwn') + @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_add_path') + @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_add_wwid') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_vol') + @mock.patch('time.sleep') + def test_connect_multipath_volume_some_fail_mp_found(self, sleep_mock, + connect_mock, + add_wwid_mock, + add_path_mock, + get_wwn_mock, + find_dm_mock): + def my_connect(rescans, props, data): + devs = {'tgt1': '', 'tgt2': 'sdb', 'tgt3': '', 'tgt4': 'sdd'} + data['stopped_threads'] += 1 + dev = devs[props['target_iqn']] + if dev: + data['num_logins'] += 1 + data['found_devices'].append(dev) + data['just_added_devices'].append(dev) + else: + data['failed_logins'] += 1 + + connect_mock.side_effect = my_connect + + res = self.connector._connect_multipath_volume(self.CON_PROPS) + + expected = {'type': 'block', 'scsi_wwn': 'wwn', 'multipath_id': 'dm-0', + 'path': '/dev/dm-0'} + self.assertEqual(expected, res) + self.assertEqual(1, get_wwn_mock.call_count) + result = list(get_wwn_mock.call_args[0][0]) + result.sort() + self.assertEqual(['sdb', 'sdd'], result) + add_wwid_mock.assert_called_once_with('wwn') + self.assertNotEqual(0, add_path_mock.call_count) + self.assertGreaterEqual(find_dm_mock.call_count, 2) + self.assertEqual(4, connect_mock.call_count) + + @mock.patch.object(linuxscsi.LinuxSCSI, 'find_sysfs_multipath_dm', + return_value=None) + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwn', + return_value='wwn') + @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_add_path') + @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_add_wwid') + @mock.patch.object(iscsi.time, 'time', side_effect=(0, 0, 11, 0)) + @mock.patch.object(iscsi.ISCSIConnector, '_connect_vol') + @mock.patch('time.sleep') + def test_connect_multipath_volume_some_fail_mp_not_found(self, sleep_mock, + connect_mock, + time_mock, + add_wwid_mock, + add_path_mock, + get_wwn_mock, + find_dm_mock): + def my_connect(rescans, props, data): + devs = {'tgt1': '', 'tgt2': 'sdb', 'tgt3': '', 'tgt4': 'sdd'} + data['stopped_threads'] += 1 + dev = devs[props['target_iqn']] + if dev: + data['num_logins'] += 1 + data['found_devices'].append(dev) + data['just_added_devices'].append(dev) + else: + data['failed_logins'] += 1 + + connect_mock.side_effect = my_connect + + res = self.connector._connect_multipath_volume(self.CON_PROPS) + + expected = [{'type': 'block', 'scsi_wwn': 'wwn', 'path': '/dev/sdb'}, + {'type': 'block', 'scsi_wwn': 'wwn', 'path': '/dev/sdd'}] + # It can only be one of the 2 + self.assertIn(res, expected) + self.assertEqual(1, get_wwn_mock.call_count) + result = list(get_wwn_mock.call_args[0][0]) + result.sort() + self.assertEqual(['sdb', 'sdd'], result) + add_wwid_mock.assert_called_once_with('wwn') + self.assertNotEqual(0, add_path_mock.call_count) + self.assertGreaterEqual(find_dm_mock.call_count, 4) + self.assertEqual(4, connect_mock.call_count) + + @mock.patch.object(linuxscsi.LinuxSCSI, 'find_sysfs_multipath_dm', + return_value=None) + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwn', + return_value='wwn') + @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_add_path') + @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_add_wwid') + @mock.patch.object(iscsi.time, 'time', side_effect=(0, 0, 11, 0)) + @mock.patch.object(iscsi.ISCSIConnector, '_connect_vol') + @mock.patch('time.sleep', mock.Mock()) + def test_connect_multipath_volume_all_loging_not_found(self, + connect_mock, + time_mock, + add_wwid_mock, + add_path_mock, + get_wwn_mock, + find_dm_mock): + def my_connect(rescans, props, data): + data['stopped_threads'] += 1 + data['num_logins'] += 1 + + connect_mock.side_effect = my_connect + + self.assertRaises(exception.VolumeDeviceNotFound, + self.connector._connect_multipath_volume, + self.CON_PROPS) + + get_wwn_mock.assert_not_called() + add_wwid_mock.assert_not_called() + add_path_mock.assert_not_called() + find_dm_mock.assert_not_called() + self.assertEqual(12, connect_mock.call_count) + + @mock.patch('time.sleep', mock.Mock()) + @mock.patch.object(linuxscsi.LinuxSCSI, 'scan_iscsi') + @mock.patch.object(linuxscsi.LinuxSCSI, 'device_name_by_hctl', + return_value='sda') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_to_iscsi_portal') + def test_connect_vol(self, connect_mock, dev_name_mock, scan_mock): + lscsi = self.connector._linuxscsi + data = self._get_connect_vol_data() + hctl = [mock.sentinel.host, mock.sentinel.channel, + mock.sentinel.target, mock.sentinel.lun] + + connect_mock.return_value = mock.sentinel.session + + with mock.patch.object(lscsi, 'get_hctl', + side_effect=(None, hctl)) as hctl_mock: + self.connector._connect_vol(3, self.CON_PROPS, data) + + expected = self._get_connect_vol_data() + expected.update(num_logins=1, stopped_threads=1, + found_devices=['sda'], just_added_devices=['sda']) + self.assertDictEqual(expected, data) + + connect_mock.assert_called_once_with(self.CON_PROPS) + hctl_mock.assert_has_calls([mock.call(mock.sentinel.session, + self.CON_PROPS['target_lun']), + mock.call(mock.sentinel.session, + self.CON_PROPS['target_lun'])]) + + scan_mock.assert_called_once_with(*hctl) + dev_name_mock.assert_called_once_with(mock.sentinel.session, hctl) + + @mock.patch.object(iscsi.ISCSIConnector, '_connect_to_iscsi_portal', + return_value=None) + def test_connect_vol_no_session(self, connect_mock): + data = self._get_connect_vol_data() + + self.connector._connect_vol(3, self.CON_PROPS, data) + + expected = self._get_connect_vol_data() + expected.update(failed_logins=1, stopped_threads=1) + self.assertDictEqual(expected, data) + + @mock.patch('time.sleep', mock.Mock()) + @mock.patch.object(linuxscsi.LinuxSCSI, 'scan_iscsi') + @mock.patch.object(linuxscsi.LinuxSCSI, 'device_name_by_hctl', + return_value=None) + @mock.patch.object(iscsi.ISCSIConnector, '_connect_to_iscsi_portal') + def test_connect_vol_not_found(self, connect_mock, dev_name_mock, + scan_mock): + lscsi = self.connector._linuxscsi + data = self._get_connect_vol_data() + hctl = [mock.sentinel.host, mock.sentinel.channel, + mock.sentinel.target, mock.sentinel.lun] + + connect_mock.return_value = mock.sentinel.session + + with mock.patch.object(lscsi, 'get_hctl', + side_effect=(None, hctl)) as hctl_mock: + self.connector._connect_vol(3, self.CON_PROPS, data) + + expected = self._get_connect_vol_data() + expected.update(num_logins=1, stopped_threads=1) + self.assertDictEqual(expected, data) + + hctl_mock.assert_has_calls([mock.call(mock.sentinel.session, + self.CON_PROPS['target_lun']), + mock.call(mock.sentinel.session, + self.CON_PROPS['target_lun'])]) + + scan_mock.assert_has_calls([mock.call(*hctl), mock.call(*hctl)]) + dev_name_mock.assert_has_calls( + [mock.call(mock.sentinel.session, hctl), + mock.call(mock.sentinel.session, hctl)]) + + @mock.patch('time.sleep', mock.Mock()) + @mock.patch.object(linuxscsi.LinuxSCSI, 'scan_iscsi') + @mock.patch.object(iscsi.ISCSIConnector, '_connect_to_iscsi_portal') + def test_connect_vol_stop_connecting(self, connect_mock, scan_mock): + data = self._get_connect_vol_data() + + def device_name_by_hctl(session, hctl): + data['stop_connecting'] = True + return None + + lscsi = self.connector._linuxscsi + hctl = [mock.sentinel.host, mock.sentinel.channel, + mock.sentinel.target, mock.sentinel.lun] + + connect_mock.return_value = mock.sentinel.session + + with mock.patch.object(lscsi, 'get_hctl', + return_value=hctl) as hctl_mock, \ + mock.patch.object( + lscsi, 'device_name_by_hctl', + side_effect=device_name_by_hctl) as dev_name_mock: + + self.connector._connect_vol(3, self.CON_PROPS, data) + + expected = self._get_connect_vol_data() + expected.update(num_logins=1, stopped_threads=1, stop_connecting=True) + self.assertDictEqual(expected, data) + + hctl_mock.assert_called_once_with(mock.sentinel.session, + self.CON_PROPS['target_lun']) + scan_mock.assert_not_called() + dev_name_mock.assert_called_once_with(mock.sentinel.session, hctl) diff --git a/os_brick/tests/initiator/test_linuxscsi.py b/os_brick/tests/initiator/test_linuxscsi.py index 8ed657cfe..f876dadd5 100644 --- a/os_brick/tests/initiator/test_linuxscsi.py +++ b/os_brick/tests/initiator/test_linuxscsi.py @@ -217,6 +217,7 @@ class LinuxSCSITestCase(base.TestCase): @ddt.data({'do_raise': False, 'force': False}, {'do_raise': True, 'force': True}) @ddt.unpack + @mock.patch.object(linuxscsi.LinuxSCSI, '_remove_scsi_symlinks') @mock.patch.object(linuxscsi.LinuxSCSI, 'flush_multipath_device') @mock.patch.object(linuxscsi.LinuxSCSI, 'get_dm_name') @mock.patch.object(linuxscsi.LinuxSCSI, 'find_sysfs_multipath_dm') @@ -226,6 +227,7 @@ class LinuxSCSITestCase(base.TestCase): find_dm_mock, get_dm_name_mock, flush_mp_mock, + remove_link_mock, do_raise, force): if do_raise: flush_mp_mock.side_effect = Exception @@ -245,7 +247,9 @@ class LinuxSCSITestCase(base.TestCase): mock.call('/dev/sdb', mock.sentinel.Force, exc)]) wait_mock.assert_called_once_with(devices_names) self.assertEqual(do_raise, bool(exc)) + remove_link_mock.assert_called_once_with(devices_names) + @mock.patch.object(linuxscsi.LinuxSCSI, '_remove_scsi_symlinks') @mock.patch.object(linuxscsi.LinuxSCSI, 'flush_multipath_device', side_effect=Exception) @mock.patch.object(linuxscsi.LinuxSCSI, 'get_dm_name') @@ -254,7 +258,7 @@ class LinuxSCSITestCase(base.TestCase): @mock.patch.object(linuxscsi.LinuxSCSI, 'remove_scsi_device') def test_remove_connection_multipath_fail(self, remove_mock, wait_mock, find_dm_mock, get_dm_name_mock, - flush_mp_mock): + flush_mp_mock, remove_link_mock): flush_mp_mock.side_effect = exception.ExceptionChainer devices_names = ('sda', 'sdb') exc = exception.ExceptionChainer() @@ -267,11 +271,14 @@ class LinuxSCSITestCase(base.TestCase): flush_mp_mock.assert_called_once_with(get_dm_name_mock.return_value) remove_mock.assert_not_called() wait_mock.assert_not_called() + remove_link_mock.assert_not_called() self.assertTrue(bool(exc)) + @mock.patch.object(linuxscsi.LinuxSCSI, '_remove_scsi_symlinks') @mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_volumes_removal') @mock.patch.object(linuxscsi.LinuxSCSI, 'remove_scsi_device') - def test_remove_connection_singlepath(self, remove_mock, wait_mock): + def test_remove_connection_singlepath(self, remove_mock, wait_mock, + remove_link_mock): devices_names = ('sda', 'sdb') exc = exception.ExceptionChainer() self.linuxscsi.remove_connection(devices_names, is_multipath=False, @@ -281,6 +288,7 @@ class LinuxSCSITestCase(base.TestCase): [mock.call('/dev/sda', mock.sentinel.Force, exc), mock.call('/dev/sdb', mock.sentinel.Force, exc)]) wait_mock.assert_called_once_with(devices_names) + remove_link_mock.assert_called_once_with(devices_names) def test_find_multipath_device_3par_ufn(self): def fake_execute(*cmd, **kwargs): @@ -759,3 +767,177 @@ loop0 0""" False, None, mock_rootwrap.execute)) mock_rootwrap.execute.assert_called_once_with( 'multipathd', 'show', 'status', run_as_root=True, root_helper=None) + + @mock.patch('glob.glob') + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwid') + def test_get_sysfs_wwn_single_designator(self, get_wwid_mock, glob_mock): + glob_mock.return_value = ['/dev/disk/by-id/scsi-wwid1', + '/dev/disk/by-id/scsi-wwid2'] + get_wwid_mock.return_value = 'wwid1' + res = self.linuxscsi.get_sysfs_wwn(mock.sentinel.device_names) + self.assertEqual('wwid1', res) + glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*') + get_wwid_mock.assert_called_once_with(mock.sentinel.device_names) + + @mock.patch('os.path.realpath', side_effect=('/other/path', + '/dev/sda', '/dev/sdb')) + @mock.patch('os.path.islink', side_effect=(False, True, True, True, True)) + @mock.patch('os.stat', side_effect=(False, True, True, True)) + @mock.patch('glob.glob') + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwid') + def test_get_sysfs_wwn_multiple_designators(self, get_wwid_mock, glob_mock, + stat_mock, islink_mock, + realpath_mock): + glob_mock.return_value = ['/dev/disk/by-id/scsi-fail-link', + '/dev/disk/by-id/scsi-fail-stat', + '/dev/disk/by-id/scsi-non-dev', + '/dev/disk/by-id/scsi-wwid1', + '/dev/disk/by-id/scsi-wwid2'] + get_wwid_mock.return_value = 'pre-wwid' + devices = ['sdb', 'sdc'] + res = self.linuxscsi.get_sysfs_wwn(devices) + self.assertEqual('wwid2', res) + glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*') + get_wwid_mock.assert_called_once_with(devices) + + @mock.patch('os.path.realpath', side_effect=('/dev/sda', '/dev/sdb')) + @mock.patch('os.path.islink', return_value=True) + @mock.patch('os.stat', return_value=True) + @mock.patch('glob.glob') + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwid') + def test_get_sysfs_wwn_not_found(self, get_wwid_mock, glob_mock, stat_mock, + islink_mock, realpath_mock): + glob_mock.return_value = ['/dev/disk/by-id/scsi-wwid1', + '/dev/disk/by-id/scsi-wwid2'] + get_wwid_mock.return_value = 'pre-wwid' + devices = ['sdc'] + res = self.linuxscsi.get_sysfs_wwn(devices) + self.assertEqual('', res) + glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*') + get_wwid_mock.assert_called_once_with(devices) + + @ddt.data({'wwn_type': 't10.', 'num_val': '1'}, + {'wwn_type': 'eui.', 'num_val': '2'}, + {'wwn_type': 'naa.', 'num_val': '3'}) + @ddt.unpack + @mock.patch('six.moves.builtins.open') + def test_get_sysfs_wwid(self, open_mock, wwn_type, num_val): + read_fail = mock.MagicMock() + read_fail.__enter__.return_value.read.side_effect = IOError + read_data = mock.MagicMock() + read_data.__enter__.return_value.read.return_value = (wwn_type + + 'wwid1\n') + open_mock.side_effect = (IOError, read_fail, read_data) + + res = self.linuxscsi.get_sysfs_wwid(['sda', 'sdb', 'sdc']) + self.assertEqual(num_val + 'wwid1', res) + open_mock.assert_has_calls([mock.call('/sys/block/sda/device/wwid'), + mock.call('/sys/block/sdb/device/wwid'), + mock.call('/sys/block/sdc/device/wwid')]) + + @mock.patch('six.moves.builtins.open', side_effect=IOError) + def test_get_sysfs_wwid_not_found(self, open_mock): + res = self.linuxscsi.get_sysfs_wwid(['sda', 'sdb']) + self.assertEqual('', res) + open_mock.assert_has_calls([mock.call('/sys/block/sda/device/wwid'), + mock.call('/sys/block/sdb/device/wwid')]) + + @mock.patch.object(linuxscsi.priv_rootwrap, 'unlink_root') + @mock.patch('glob.glob') + @mock.patch('os.path.realpath', side_effect=['/dev/sda', '/dev/sdb', + '/dev/sdc']) + def test_remove_scsi_symlinks(self, realpath_mock, glob_mock, unlink_mock): + paths = ['/dev/disk/by-id/scsi-wwid1', '/dev/disk/by-id/scsi-wwid2', + '/dev/disk/by-id/scsi-wwid3'] + glob_mock.return_value = paths + self.linuxscsi._remove_scsi_symlinks(['sdb', 'sdc', 'sdd']) + glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*') + realpath_mock.assert_has_calls([mock.call(g) for g in paths]) + unlink_mock.assert_called_once_with(no_errors=True, *paths[1:]) + + @mock.patch.object(linuxscsi.priv_rootwrap, 'unlink_root') + @mock.patch('glob.glob') + @mock.patch('os.path.realpath', side_effect=['/dev/sda', '/dev/sdb']) + def test_remove_scsi_symlinks_no_links(self, realpath_mock, glob_mock, + unlink_mock): + paths = ['/dev/disk/by-id/scsi-wwid1', '/dev/disk/by-id/scsi-wwid2'] + glob_mock.return_value = paths + self.linuxscsi._remove_scsi_symlinks(['/dev/sdd', '/dev/sde']) + glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*') + realpath_mock.assert_has_calls([mock.call(g) for g in paths]) + unlink_mock.assert_not_called() + + @mock.patch('glob.glob') + def test_get_hctl_with_target(self, glob_mock): + glob_mock.return_value = [ + '/sys/class/iscsi_host/host3/device/session1/target3:4:5', + '/sys/class/iscsi_host/host3/device/session1/target3:4:6'] + res = self.linuxscsi.get_hctl('1', '2') + self.assertEqual(('3', '4', '5', '2'), res) + glob_mock.assert_called_once_with( + '/sys/class/iscsi_host/host*/device/session1/target*') + + @mock.patch('glob.glob') + def test_get_hctl_no_target(self, glob_mock): + glob_mock.side_effect = [ + [], + ['/sys/class/iscsi_host/host3/device/session1', + '/sys/class/iscsi_host/host3/device/session1']] + res = self.linuxscsi.get_hctl('1', '2') + self.assertEqual(('3', '-', '-', '2'), res) + glob_mock.assert_has_calls( + [mock.call('/sys/class/iscsi_host/host*/device/session1/target*'), + mock.call('/sys/class/iscsi_host/host*/device/session1')]) + + @mock.patch('glob.glob', return_value=[]) + def test_get_hctl_no_paths(self, glob_mock): + res = self.linuxscsi.get_hctl('1', '2') + self.assertIsNone(res) + glob_mock.assert_has_calls( + [mock.call('/sys/class/iscsi_host/host*/device/session1/target*'), + mock.call('/sys/class/iscsi_host/host*/device/session1')]) + + @mock.patch('glob.glob') + def test_device_name_by_hctl(self, glob_mock): + glob_mock.return_value = [ + '/sys/class/scsi_host/host3/device/session1/target3:4:5/3:4:5:2/' + 'block/sda2', + '/sys/class/scsi_host/host3/device/session1/target3:4:5/3:4:5:2/' + 'block/sda'] + res = self.linuxscsi.device_name_by_hctl('1', ('3', '4', '5', '2')) + self.assertEqual('sda', res) + glob_mock.assert_called_once_with( + '/sys/class/scsi_host/host3/device/session1/target3:4:5/3:4:5:2/' + 'block/*') + + @mock.patch('glob.glob') + def test_device_name_by_hctl_wildcards(self, glob_mock): + glob_mock.return_value = [ + '/sys/class/scsi_host/host3/device/session1/target3:4:5/3:4:5:2/' + 'block/sda2', + '/sys/class/scsi_host/host3/device/session1/target3:4:5/3:4:5:2/' + 'block/sda'] + res = self.linuxscsi.device_name_by_hctl('1', ('3', '-', '-', '2')) + self.assertEqual('sda', res) + glob_mock.assert_called_once_with( + '/sys/class/scsi_host/host3/device/session1/target3:*:*/3:*:*:2/' + 'block/*') + + @mock.patch('glob.glob', mock.Mock(return_value=[])) + def test_device_name_by_hctl_no_devices(self): + res = self.linuxscsi.device_name_by_hctl('1', ('4', '5', '6', '2')) + self.assertIsNone(res) + + @mock.patch.object(linuxscsi.LinuxSCSI, 'echo_scsi_command') + def test_scsi_iscsi(self, echo_mock): + self.linuxscsi.scan_iscsi('host', 'channel', 'target', 'lun') + echo_mock.assert_called_once_with('/sys/class/scsi_host/hosthost/scan', + 'channel target lun') + + def test_multipath_add_wwid(self): + self.linuxscsi.multipath_add_wwid('wwid1') + self.assertEqual(['multipath -a wwid1'], self.cmds) + + def test_multipath_add_path(self): + self.linuxscsi.multipath_add_path('/dev/sda') + self.assertEqual(['multipathd add path /dev/sda'], self.cmds) diff --git a/os_brick/tests/privileged/test_rootwrap.py b/os_brick/tests/privileged/test_rootwrap.py index 4f59fda74..cc48f6282 100644 --- a/os_brick/tests/privileged/test_rootwrap.py +++ b/os_brick/tests/privileged/test_rootwrap.py @@ -125,3 +125,33 @@ class PrivRootwrapTestCase(base.TestCase): check_exit_code=False) self.assertEqual('', out) self.assertIsInstance(err, six.string_types) + + @mock.patch.object(priv_rootwrap.unlink_root.privsep_entrypoint, + 'client_mode', False) + @mock.patch('os.unlink', side_effect=IOError) + def test_unlink_root(self, unlink_mock): + links = ['/dev/disk/by-id/link1', '/dev/disk/by-id/link2'] + priv_rootwrap.unlink_root(*links, no_errors=True) + unlink_mock.assert_has_calls([mock.call(links[0]), + mock.call(links[1])]) + + @mock.patch.object(priv_rootwrap.unlink_root.privsep_entrypoint, + 'client_mode', False) + @mock.patch('os.unlink', side_effect=IOError) + def test_unlink_root_raise(self, unlink_mock): + links = ['/dev/disk/by-id/link1', '/dev/disk/by-id/link2'] + self.assertRaises(IOError, + priv_rootwrap.unlink_root, + *links, no_errors=False) + unlink_mock.assert_called_once_with(links[0]) + + @mock.patch.object(priv_rootwrap.unlink_root.privsep_entrypoint, + 'client_mode', False) + @mock.patch('os.unlink', side_effect=IOError) + def test_unlink_root_raise_at_end(self, unlink_mock): + links = ['/dev/disk/by-id/link1', '/dev/disk/by-id/link2'] + self.assertRaises(exception.ExceptionChainer, + priv_rootwrap.unlink_root, + *links, raise_at_end=True) + unlink_mock.assert_has_calls([mock.call(links[0]), + mock.call(links[1])]) diff --git a/releasenotes/notes/refactor_iscsi_connect-dfbb24305a954783.yaml b/releasenotes/notes/refactor_iscsi_connect-dfbb24305a954783.yaml new file mode 100644 index 000000000..dddb903d1 --- /dev/null +++ b/releasenotes/notes/refactor_iscsi_connect-dfbb24305a954783.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + iSCSI connect mechanism refactoring to be faster, more robust, more + reliable.