From 4c21b40df20ef497c7a3c82ffcf32fe21621034c Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Tue, 7 Jun 2022 18:26:13 +0200 Subject: [PATCH] NVMe-oF: Consolidate code paths We currently have 2 different connection information formats: The initial one and the new one. Within the new format we have 2 variants: Replicated and Not replicated. Currently the nvmeof connector has multiple code paths and we end up finding bugs that affect one path and not the other, and bugs that are present in both may end up getting fixed in only one of them. This patch consolidates both formats by converting the connection information into a common internal representation, thus reducing the number of code paths. Thanks to this consolidation the Cinder drivers are less restricted on how they can identify a volume in the connection information. They are no longer forced to pass the NVMe UUID (in case they cannot get that information or the backend doesn't support it) and they can provide other information instead (nguid or namespace id). This connection properties format consolidation is explained in the new NVMeOFConnProps class docstring. By consolidating the code paths a number of bugs get fixed automatically, because they were broken in one path but not in the other. As examples, the old code path didn't have rescans but the new one did, and the old code path had device flush but the new one didn't. Given the big refactoring needed to consolidate everything this patch also accomplishes the following things: - Uses Portal, Target, and NVMeOFConnProps classes instead of using the harder to read and error prone dictionaries and tuples. - Adds method annotations. - Documents most methods (exect the raid ones which I'm not familiar with) - Adds the connector to the docs. - Makes method signatures conform with Cinder team standards: no more static methods passing self and no more calling class methods using the class name instead of self. Closes-Bug: #1964590 Closes-Bug: #1964395 Closes-Bug: #1964388 Closes-Bug: #1964385 Closes-Bug: #1964380 Closes-Bug: #1964383 Closes-Bug: #1965954 Closes-Bug: #1903032 Related-Bug: #1961102 Change-Id: I6c2a952f7e286f3e3890e3f2fcb2fdd1063f0c17 --- .../os_brick/initiator/connector.rst | 8 + mypy-files.txt | 1 + os_brick/initiator/connectors/nvmeof.py | 1606 ++++++++----- .../tests/initiator/connectors/test_nvmeof.py | 2031 +++++++++++------ os_brick/tests/test_utils.py | 17 + os_brick/utils.py | 18 + .../nvmeof-consolidate-004dbe3a98f6f815.yaml | 46 + 7 files changed, 2499 insertions(+), 1228 deletions(-) create mode 100644 releasenotes/notes/nvmeof-consolidate-004dbe3a98f6f815.yaml diff --git a/doc/source/reference/os_brick/initiator/connector.rst b/doc/source/reference/os_brick/initiator/connector.rst index 1c251e0c6..a2dbfea19 100644 --- a/doc/source/reference/os_brick/initiator/connector.rst +++ b/doc/source/reference/os_brick/initiator/connector.rst @@ -27,3 +27,11 @@ .. automethod:: connect_volume .. automethod:: disconnect_volume + + .. autoclass:: os_brick.initiator.connectors.nvmeof.NVMeOFConnector + + .. automethod:: connect_volume + .. automethod:: disconnect_volume + .. automethod:: extend_volume + .. automethod:: get_volume_paths + .. automethod:: get_connector_properties diff --git a/mypy-files.txt b/mypy-files.txt index f38c724dc..e8a2b4e68 100644 --- a/mypy-files.txt +++ b/mypy-files.txt @@ -6,6 +6,7 @@ os_brick/initiator/linuxscsi.py os_brick/initiator/connectors/base.py os_brick/initiator/connectors/base_iscsi.py os_brick/initiator/connectors/iscsi.py +os_brick/initiator/connectors/nvmeof.py os_brick/remotefs/remotefs.py os_brick/initiator/linuxrbd.py os_brick/encryptors/luks.py diff --git a/os_brick/initiator/connectors/nvmeof.py b/os_brick/initiator/connectors/nvmeof.py index 4bfa1132e..0366fe53b 100644 --- a/os_brick/initiator/connectors/nvmeof.py +++ b/os_brick/initiator/connectors/nvmeof.py @@ -10,12 +10,19 @@ # License for the specific language governing permissions and limitations # under the License. +# Look in the NVMeOFConnProps class docstring to see the format of the NVMe-oF +# connection properties + import errno +import functools import glob import json import os.path -import re import time +from typing import (Callable, Dict, List, Optional, Sequence, # noqa: H301 + Tuple, Type, Union) # noqa: H301 +import uuid as uuid_lib + from oslo_concurrency import processutils as putils from oslo_log import log as logging @@ -31,60 +38,673 @@ from os_brick.privileged import rootwrap as priv_rootwrap from os_brick import utils DEV_SEARCH_PATH = '/dev/' +RAID_PATH = '/dev/md/' +NVME_CTRL_SYSFS_PATH = '/sys/class/nvme-fabrics/ctl/' +BLK_SYSFS_PATH = '/sys/class/block/' DEVICE_SCAN_ATTEMPTS_DEFAULT = 5 LOG = logging.getLogger(__name__) +# ######################################################### +# CONNECTION PROPERTIES KEYS start + +# Only present in the old connection info format +OLD_NQN = 'nqn' +TRANSPORT = 'transport_type' +PORTAL = 'target_portal' +PORT = 'target_port' + +# These were only present in the old connection info, but now we'll allowed +# both in the old format, the new format, and as part of a volume_replicas +# element. +NGUID = 'volume_nguid' +NSID = 'ns_id' +HOST_NQN = 'host_nqn' + +# Present in the new new connection info format +UUID = 'vol_uuid' +NQN = 'target_nqn' +PORTALS = 'portals' +ALIAS = 'alias' +REPLICAS = 'volume_replicas' +REPLICA_COUNT = 'replica_count' + +# CONNECTION PROPERTIES KEYS end +# ######################################################### + + +# ######################################################### +# UTILITY METHODS start + +def ctrl_property(prop_name: str, ctrl_name: str) -> Optional[str]: + """Get a sysfs property of an nvme controller.""" + return sysfs_property(prop_name, NVME_CTRL_SYSFS_PATH + ctrl_name) + + +def blk_property(prop_name: str, blk_name: str) -> Optional[str]: + """Get a sysfs property of a block device.""" + return sysfs_property(prop_name, BLK_SYSFS_PATH + blk_name) + + +def sysfs_property(prop_name: str, path_or_name: str) -> Optional[str]: + """Get sysfs property by path returning None if not possible.""" + filename = os.path.join(path_or_name, prop_name) + LOG.debug('Checking property at %s', filename) + try: + with open(filename, 'r') as f: + result = f.read().strip() + LOG.debug('Contents: %s', result) + return result + + except (FileNotFoundError, IOError) as exc: + # May happens on race conditions with device removals + LOG.debug('Error reading file %s', exc) + return None + + +def nvme_basename(path: str) -> str: + """Convert a sysfs control path into a namespace device. + + We can have a basic namespace devices such as nvme0n10 which is already in + the desired form, but there's also channels when ANA is enabled on the + kernel which have the form nvme0c2n10 which need to be converted to + nvme0n10 to get the actual device. + """ + basename = os.path.basename(path) + if 'c' not in basename: # nvme0n10 + return basename + + # nvme0c1n10 ==> nvme0n10 + ctrl, rest = basename.split('c', 1) + ns = rest[rest.index('n'):] + return ctrl + ns + +# UTILITY METHODS end +# ######################################################### + + +# ######################################################### +# AUXILIARY CLASSES start + +class Portal(object): + """Representation of an NVMe-oF Portal with some related operations.""" + LIVE = 'live' + MISSING = None # Unkown or not present in the system + controller: Optional[str] = None # Don't know controller name on start + + def __str__(self) -> str: + return (f'Portal {self.transport} at {self.address}:{self.port} ' + f'(ctrl: {self.controller})') + + __repr__ = __str__ + + def __init__(self, + parent_target: 'Target', + address: str, + port: Union[str, int], + transport: str) -> None: + self.parent_target = parent_target + self.address = address + self.port = str(port) + + # Convert the transport into our internal representation + if transport in ('RoCEv2', 'rdma'): + self.transport = 'rdma' + else: + self.transport = 'tcp' + + @property + def is_live(self) -> bool: + """Check if the portal is live. + + Not being live can mean many things, such as being connecting because + the connection to the backend was lost, not knowing the controller name + because we haven't searched for it, or not being connected to the + backend. + """ + LOG.debug('Checking if %s is live', self) + return self.state == self.LIVE + + @property + def state(self) -> Optional[str]: + """Return the state if the controller is known, None otherwise.""" + # Does not automatically search for the controller + if self.controller: + return ctrl_property('state', self.controller) + return None + + def get_device(self) -> Optional[str]: + """Get a device path using available volume identification markers. + + Priority is given to the uuid, since that must be unique across ALL + devices, then nguid which is backend specific but a backend can reuse + in other connections after it has been disconnected, and then using + the namespace id (nsid) which changes based on the subsystem and the + moment the volume is connected. + + If the target in the connection information didn't have any of those + identifiers, then let the parent Target instance try to figure out the + device based on the devices that existed when we started connecting and + the ones available now. + + None is returned when a device cannot be found. + """ + target = self.parent_target + if target.uuid: + return self.get_device_by_property('uuid', target.uuid) + + if target.nguid: + return self.get_device_by_property('nguid', target.nguid) + + if target.ns_id: + return self.get_device_by_property('nsid', target.ns_id) + + # Fallback to using the target to do the search + LOG.warning('Using unreliable mechanism to find device: ' + '"devices_on_start"') + return target.get_device_path_by_initial_devices() + + def get_device_by_property(self, + prop_name: str, + value: str) -> Optional[str]: + """Look for a specific device (namespace) within a controller. + + Use a specific property to identify the namespace within the + controller and returns the device path under /dev. + + Returns None if device is not found. + """ + LOG.debug('Looking for device where %s=%s on controller %s', + prop_name, value, self.controller) + + # Look under the controller, where we will have normal devices and ANA + # channel devices. For nvme1 we could find nvme1n1 or nvme0c1n1) + paths = glob.glob(f'{NVME_CTRL_SYSFS_PATH}{self.controller}/nvme*') + for path in paths: + prop_value = sysfs_property(prop_name, path) + if prop_value == value: + # Convert path to the namespace device name + result = DEV_SEARCH_PATH + nvme_basename(path) + LOG.debug('Device found at %s, using %s', path, result) + return result + + LOG.debug('Block %s is not the one we look for (%s != %s)', + path, prop_value, value) + + LOG.debug('No device Found on controller %s', self.controller) + return None + + +class Target(object): + """Representation of an NVMe-oF Target and some related operations.""" + # Only used if the target has no uuid, nguid, or ns_id information + devices_on_start = None + # Cache the device we find for cases where we do retries + _device = None + + def __str__(self) -> str: + return f'Target {self.nqn} at {self.portals}' + + __repr__ = __str__ + + @classmethod + def factory(cls: Type['Target'], + source_conn_props: 'NVMeOFConnProps', + target_nqn: str, + portals: List[str], + vol_uuid: Optional[str] = None, + volume_nguid: Optional[str] = None, + ns_id: Optional[str] = None, + host_nqn: Optional[str] = None, + find_controllers=False, + **ignore) -> 'Target': + """Create an instance from the connection properties keys. + + Extra keyword arguments are accepted (and ignored) for convenience, so + they don't need to be removed when calling the factory. + """ + target = cls(source_conn_props, target_nqn, portals, vol_uuid, + volume_nguid, ns_id, host_nqn, find_controllers) + return target + + def __init__(self, + source_conn_props: 'NVMeOFConnProps', + nqn: str, + portals: List[str], + uuid: str = None, + nguid: str = None, + ns_id: str = None, + host_nqn=None, + find_controllers=False) -> None: + """Initialize instance. + + Portals are converted from a list of length 3 tuple/list into a list of + Portal instances. + + The find_controllers parameter controls the search of the controller + names in the system for each of the portals. + """ + self.source_conn_props = source_conn_props + self.nqn = nqn + self.portals = [Portal(self, *portal) for portal in portals] + self.uuid = uuid and str(uuid_lib.UUID(uuid)) + self.nguid = nguid and str(uuid_lib.UUID(nguid)) + self.ns_id = ns_id + self.host_nqn = host_nqn + + if find_controllers: + self.set_portals_controllers() + + # This only happens with some old connection properties format, where + # we may not have a way to identify the new volume and we'll have to + # try to guess it looking at existing volumes before the attach. + if not (uuid or nguid or ns_id): + self.devices_on_start = self._get_nvme_devices() + LOG.debug('Devices on start are: %s', self.devices_on_start) + + @staticmethod + def _get_nvme_devices() -> List[str]: + """Get all NVMe devices present in the system.""" + pattern = '/dev/nvme*n*' # e.g. /dev/nvme10n10 + return glob.glob(pattern) + + @property + def live_portals(self) -> List[Portal]: + """Get live portals. + + Must have called set_portals_controllers first since portals without a + controller name will be skipped. + """ + return [p for p in self.portals if p.is_live] + + @property + def present_portals(self) -> List[Portal]: + """Get present portals. + + Must have called set_portals_controllers first since portals without a + controller name will be skipped. + """ + return [p for p in self.portals if p.state is not None] + + def set_portals_controllers(self) -> None: + """Search and set controller names in the target's portals. + + Compare the address, port, and transport protocol for each portal + against existing nvme subsystem controllers. + """ + if all(p.controller for p in self.portals): # all have been found + return + + hostnqn: str = self.host_nqn or utils.get_host_nqn() + + # List of portal addresses and transports for this target + # Unlike "nvme list-subsys -o json" sysfs addr is separated by a coma + sysfs_portals: List[Tuple[Optional[str], + Optional[str], + Optional[Union[str, utils.Anything]]]] = [ + (f'traddr={p.address},trsvcid={p.port}', p.transport, hostnqn) + for p in self.portals + ] + known_names: List[str] = [p.controller for p in self.portals + if p.controller] + + warned = False + LOG.debug('Search controllers for portals %s', sysfs_portals) + ctrl_paths = glob.glob(NVME_CTRL_SYSFS_PATH + 'nvme*') + for ctrl_path in ctrl_paths: + ctrl_name = os.path.basename(ctrl_path) + if ctrl_name in known_names: + continue + LOG.debug('Checking controller %s', ctrl_name) + + nqn = sysfs_property('subsysnqn', ctrl_path) + if nqn != self.nqn: + LOG.debug("Skipping %s, doesn't match %s", nqn, self.nqn) + continue + + # The right subsystem, but must also be the right portal + ctrl_transport = sysfs_property('transport', ctrl_path) + ctrl_addr = sysfs_property('address', ctrl_path) + # hostnqn value not present in all OSs. Ignore when not present + ctrl_hostnqn = sysfs_property('hostnqn', ctrl_path) or utils.ANY + # Warn once per target for OS not presenting the hostnqn on sysfs + if ctrl_hostnqn is utils.ANY and not warned: + LOG.warning("OS doesn't present the host nqn information. " + "Controller may be incorrectly matched.") + warned = True + + ctrl_portal = (ctrl_addr, ctrl_transport, ctrl_hostnqn) + try: + index = sysfs_portals.index(ctrl_portal) + + LOG.debug('Found a valid portal at %s', ctrl_portal) + # Update the portal with the found controller name + self.portals[index].controller = ctrl_name + known_names.append(ctrl_name) # One more controller found + except ValueError: + # If it's not one of our controllers ignore it + LOG.debug('Skipping %s, not part of portals %s', + ctrl_portal, sysfs_portals) + + # short circuit if no more portals to find + if len(known_names) == len(sysfs_portals): + return + + def get_devices(self, only_live=False, get_one=False) -> List[str]: + """Return devices for this target + + Optionally only return devices from portals that are live and also + optionally return on first device found. + + Returns an empty list when not found. + """ + LOG.debug('Looking for volume at %s', self.nqn) + + # Use a set because multiple portals can return the same device when + # using ANA (even if we are not intentionally doing multipathing) + result = set() + portals = self.live_portals if only_live else self.present_portals + + for portal in portals: + device = portal.get_device() + if device: + result.add(device) + if get_one: + break + + return list(result) + + # NOTE: Don't change to a property, as it would hide VolumeDeviceNotFound + @utils.retry(exception.VolumeDeviceNotFound, retries=5) + def find_device(self) -> str: + """Search for a device that is on a live subsystem + + Must have called set_portals_controllers first since portals without a + controller name will be skipped. + + Retries up to 5 times with exponential backoff to give time in case the + subsystem is currently reconnecting. Raises VolumeDeviceNotFound when + finally gives up trying. + """ + if not self._device: + devices = self.get_devices(only_live=True, get_one=True) + if not devices: + raise exception.VolumeDeviceNotFound(device=self.nqn) + self._device = devices[0] + return self._device + + def get_device_path_by_initial_devices(self) -> Optional[str]: + """Find target's device path from devices that were present before.""" + ctrls = [p.controller for p in self.portals if p.controller] + + def discard(devices): + """Discard devices that don't belong to our controllers.""" + if not devices: + return set() + + return set(dev for dev in devices + if os.path.basename(dev).rsplit('n', 1)[0] in ctrls) + + current_devices = self._get_nvme_devices() + LOG.debug('Initial devices: %s. Current devices %s. Controllers: %s', + self.devices_on_start, current_devices, ctrls) + devices = discard(current_devices) - discard(self.devices_on_start) + if not devices: + return None + + if len(devices) == 1: + return devices.pop() + + # With multiple devices they must all have the same uuid + if (len(devices) > 1 and + 1 < len(set(blk_property('uuid', os.path.basename(d)) + for d in devices))): + msg = _('Too many different volumes found for %s') % ctrls + LOG.error(msg) + return None + + return devices.pop() # They are the same volume, return any of them + + +class NVMeOFConnProps(object): + """Internal representation of the NVMe-oF connection properties + + There is an old and a newer connection properties format, which result + in 2 variants for replicated connections and 2 for non replicated: + + 1- New format with multiples replicas information + 2- New format with single replica information + 3- New format with no replica information + 4- Original format + + Case #1 and #2 format: + { + 'vol_uuid': , + 'alias': , + 'volume_replicas': [ , ... ], + 'replica_count': len(volume_replicas), + } + + Where: + cinder_volume_id ==> Cinder id, could be different from NVMe UUID. + with/without hyphens, uppper/lower cased. + target :== { + 'target_nqn': , + 'vol_uuid': , + 'portals': [ , ... ], + } + + nvme_volume_uuid ==> NVMe UUID. Can be different than cinder's id. + With/without hyphens, uppper/lower cased + portal ::= tuple/list( + , + , + + ) + transport_type ::= 'RoCEv2' | # anything => tcp + + Case #3 format: + ==> As defined in case #1 & #2 + + Case #4 format: + { + 'nqn': , + 'transport_type': , + 'target_portal': , + 'target_port': , + 'volume_nguid': , + 'ns_id': , + 'host_nqn': , + } + + Where: + transport_type ::= 'rdma' | 'tcp' + volume_nguid ==> Optional, with/without hyphens, uppper/lower cased + target_namespace_id ==> Optional + connector_host_nqn> ==> Optional + + This class unifies the representation of all these in the following + attributes: + + replica_count: None for non replicated + alias: None for non replicated + cinder_volume_id: None for non replicated + is_replicated: True if replica count > 1, None if count = 1 else False + targets: List of Target instances + device_path: None if not present (it's set by Nova) + + In this unification case#4 is treated as case#3 where the vol_uuid is None + and leaving all the additional information in the dictionary. This way non + replicated cases are always handled in the same way and we have a common + " definition for all cases: + + target :== { + 'target_nqn': , + 'vol_uuid': , + 'portals': [ , ... ], + 'volume_nguid': , + 'ns_id': , + 'host_nqn': , + } + new_portal ::= tuple/list( + , + , + + ) + new_transport_type ::= 'rdma' | 'tcp' + + Portals change the transport_type to the internal representation where: + 'RoCEv2' ==> 'rdma' + ==> 'tcp' + + This means that the new connection format now accepts vol_uuid set to None, + and accepts ns_id, volume_nguid, and host_nqn parameters, as described in + the connect_volume docstring. + """ + RO = 'ro' + RW = 'rw' + replica_count = None + cinder_volume_id: Optional[str] = None + + def __init__(self, conn_props: Dict, + find_controllers: bool = False) -> None: + # Generic connection properties fields used by Nova + self.qos_specs = conn_props.get('qos_specs') + self.readonly = conn_props.get('access_mode', self.RW) == self.RO + self.encrypted = conn_props.get('encrypted', False) + self.cacheable = conn_props.get('cacheable', False) + self.discard = conn_props.get('discard', False) + + # old connection properties format + if REPLICAS not in conn_props and NQN not in conn_props: + LOG.debug('Converting old connection info to new format') + conn_props[UUID] = None + conn_props[NQN] = conn_props.pop(OLD_NQN) + conn_props[PORTALS] = [(conn_props.pop(PORTAL), + conn_props.pop(PORT), + conn_props.pop(TRANSPORT))] + # Leave other fields as they are: volume_nguid, ns_id, host_nqn + + # NVMe-oF specific fields below + self.alias = conn_props.get(ALIAS) + + if REPLICAS in conn_props: + self.replica_count = (conn_props[REPLICA_COUNT] or + len(conn_props[REPLICAS])) + self.is_replicated = True if self.replica_count > 1 else None + targets = conn_props[REPLICAS] + self.cinder_volume_id = str(uuid_lib.UUID(conn_props[UUID])) + else: + self.is_replicated = False + targets = [conn_props] + + self.targets = [Target.factory(source_conn_props=self, + find_controllers=find_controllers, + **target) for target in targets] + + # Below fields may have been added by nova + self.device_path = conn_props.get('device_path') + + def get_devices(self, only_live: bool = False) -> List[str]: + """Get all device paths present in the system for all targets.""" + result = [] + for target in self.targets: + result.extend(target.get_devices(only_live)) + return result + + @classmethod + def from_dictionary_parameter(cls: Type['NVMeOFConnProps'], + func: Callable) -> Callable: + """Decorator to convert connection properties dictionary. + + It converts the connection properties into a NVMeOFConnProps instance + and finds the controller names for all portals present in the system. + """ + @functools.wraps(func) + def wrapper(self, connection_properties, *args, **kwargs): + conn_props = cls(connection_properties, find_controllers=True) + return func(self, conn_props, *args, **kwargs) + return wrapper + +# AUXILIARY CLASSES end +# ######################################################### + + +# ######################################################### +# CONNECTOR CLASS start + class NVMeOFConnector(base.BaseLinuxConnector): """Connector class to attach/detach NVMe-oF volumes.""" + # Use a class attribute since a restart is needed to change it on the host native_multipath_supported = None - def __init__(self, root_helper, driver=None, use_multipath=False, - device_scan_attempts=DEVICE_SCAN_ATTEMPTS_DEFAULT, - *args, **kwargs): + def __init__(self, + root_helper: str, + driver: Optional[base.host_driver.HostDriver] = None, + use_multipath: bool = False, + device_scan_attempts: int = DEVICE_SCAN_ATTEMPTS_DEFAULT, + *args, **kwargs) -> None: super(NVMeOFConnector, self).__init__( root_helper, - driver=driver, - device_scan_attempts=device_scan_attempts, + driver, + device_scan_attemps=device_scan_attempts, *args, **kwargs) self.use_multipath = use_multipath self._set_native_multipath_supported() - if self.use_multipath and not \ - NVMeOFConnector.native_multipath_supported: + if self.use_multipath and not self.native_multipath_supported: LOG.warning('native multipath is not enabled') @staticmethod - def get_search_path(): + def get_search_path() -> str: + """Necessary implementation for an os-brick connector.""" return DEV_SEARCH_PATH - def get_volume_paths(self, connection_properties): - device_path = connection_properties.get('device_path') + def get_volume_paths( + self, + connection_properties: NVMeOFConnProps, + device_info: Optional[Dict[str, str]] = None) -> List[str]: + """Return paths where the volume is present.""" + # Priority is on the value returned by connect_volume method + if device_info and device_info.get('path'): + return [device_info['path']] + + # Nova may add the path on the connection properties as a device_path + device_path = connection_properties.device_path if device_path: return [device_path] - volume_replicas = connection_properties.get('volume_replicas') - if not volume_replicas: # compatibility - return [] - replica_count = connection_properties.get('replica_count') - try: - if volume_replicas and replica_count > 1: - return ['/dev/md/' + connection_properties.get('alias')] - if volume_replicas and replica_count == 1: - return [NVMeOFConnector.get_nvme_device_path( - self, volume_replicas[0]['target_nqn'], - volume_replicas[0]['vol_uuid'])] - else: - return [NVMeOFConnector.get_nvme_device_path( - self, connection_properties.get('target_nqn'), - connection_properties.get('vol_uuid'))] - except exception.VolumeDeviceNotFound: - return [] + # If we don't get the info on the connection properties it could be + # problematic because we could have multiple devices and not know which + # one we used. + LOG.warning('We are being called without the path information!') + # TODO: For raids it would be good to ensure they are actually there + if connection_properties.is_replicated: + if connection_properties.alias is None: + raise exception.BrickException('Alias missing in connection ' + 'info') + return [RAID_PATH + connection_properties.alias] + + # TODO: Return live devices first? + devices = connection_properties.get_devices() + + # If we are not sure if it's replicated or not, find out + if connection_properties.is_replicated is None: + if any(self._is_raid_device(dev) for dev in devices): + if connection_properties.alias is None: + raise exception.BrickException('Alias missing in ' + 'connection info') + return [RAID_PATH + connection_properties.alias] + + return devices + + # ####### Connector Properties methods ######## @classmethod - def nvme_present(cls): + def nvme_present(cls: type) -> bool: + """Check if the nvme command is present.""" try: priv_rootwrap.custom_execute('nvme', 'version') return True @@ -97,11 +717,12 @@ class NVMeOFConnector(base.BaseLinuxConnector): return False @classmethod - def get_connector_properties(cls, root_helper, *args, **kwargs): + def get_connector_properties(cls, root_helper, *args, **kwargs) -> Dict: """The NVMe-oF connector properties (initiator uuid and nqn.)""" execute = kwargs.get('execute') or priv_rootwrap.execute nvmf = NVMeOFConnector(root_helper=root_helper, execute=execute) ret = {} + nqn = None uuid = nvmf._get_host_uuid() suuid = nvmf._get_system_uuid() @@ -116,7 +737,8 @@ class NVMeOFConnector(base.BaseLinuxConnector): ret['nvme_native_multipath'] = cls._set_native_multipath_supported() return ret - def _get_host_uuid(self): + def _get_host_uuid(self) -> Optional[str]: + """Get the UUID of the first mounted filesystem.""" cmd = ('findmnt', '/', '-n', '-o', 'SOURCE') try: lines, err = self._execute( @@ -131,7 +753,12 @@ class NVMeOFConnector(base.BaseLinuxConnector): "Process execution error in _get_host_uuid: %s" % str(e)) return None - def _get_system_uuid(self): + def _get_system_uuid(self) -> str: + """Get the system's UUID from DMI + + First try reading it from sysfs and if not possible use the dmidecode + tool. + """ # RSD requires system_uuid to let Cinder RSD Driver identify # Nova node for later RSD volume attachment. try: @@ -167,221 +794,188 @@ class NVMeOFConnector(base.BaseLinuxConnector): LOG.warning("Could not find nvme_core/parameters/multipath") return False - def _get_nvme_devices(self): - nvme_devices = [] - # match nvme devices like /dev/nvme10n10 - pattern = r'/dev/nvme[0-9]+n[0-9]+' - cmd = ['nvme', 'list'] - for retry in range(1, self.device_scan_attempts + 1): - try: - (out, err) = self._execute(*cmd, - root_helper=self._root_helper, - run_as_root=True) - for line in out.split('\n'): - result = re.match(pattern, line) - if result: - nvme_devices.append(result.group(0)) - LOG.debug("_get_nvme_devices returned %(nvme_devices)s", - {'nvme_devices': nvme_devices}) - return nvme_devices - - except putils.ProcessExecutionError: - LOG.warning( - "Failed to list available NVMe connected controllers, " - "retrying.") - time.sleep(retry ** 2) - else: - msg = _("Failed to retrieve available connected NVMe controllers " - "when running nvme list.") - raise exception.CommandExecutionFailed(message=msg) - - @utils.retry(exception.VolumePathsNotFound) - def _get_device_path(self, current_nvme_devices): - all_nvme_devices = self._get_nvme_devices() - LOG.debug("all_nvme_devices are %(all_nvme_devices)s", - {'all_nvme_devices': all_nvme_devices}) - path = set(all_nvme_devices) - set(current_nvme_devices) - if not path: - raise exception.VolumePathsNotFound() - return list(path)[0] - - @utils.retry(exception.VolumeDeviceNotFound) - def _get_device_path_by_nguid(self, nguid): - device_path = os.path.join(DEV_SEARCH_PATH, - 'disk', - 'by-id', - 'nvme-eui.%s' % nguid) - LOG.debug("Try to retrieve symlink to %(device_path)s.", - {"device_path": device_path}) - try: - path, _err = self._execute('readlink', - '-e', - device_path, - run_as_root=True, - root_helper=self._root_helper) - if not path: - raise exception.VolumePathsNotFound() - return path.rstrip() - except putils.ProcessExecutionError as e: - LOG.exception(e) - raise exception.VolumeDeviceNotFound(device=device_path) - - @utils.retry(putils.ProcessExecutionError) - def _try_connect_nvme(self, cmd): - try: - self._execute(*cmd, root_helper=self._root_helper, - run_as_root=True) - except putils.ProcessExecutionError as e: - # Idempotent connection to target. - # Exit code 70 means that target is already connected. - if e.exit_code == 70: - return - raise - - def _get_nvme_subsys(self): - # Example output: - # { - # 'Subsystems' : [ - # { - # 'Name' : 'nvme-subsys0', - # 'NQN' : 'nqn.2016-06.io.spdk:cnode1' - # }, - # { - # 'Paths' : [ - # { - # 'Name' : 'nvme0', - # 'Transport' : 'rdma', - # 'Address' : 'traddr=10.0.2.15 trsvcid=4420' - # } - # ] - # } - # ] - # } - # - - cmd = ['nvme', 'list-subsys', '-o', 'json'] - ret_val = self._execute(*cmd, root_helper=self._root_helper, - run_as_root=True) - - return ret_val - - @staticmethod - def _filter_nvme_devices(current_nvme_devices, nvme_controller): - LOG.debug("Filter NVMe devices belonging to controller " - "%(nvme_controller)s.", - {"nvme_controller": nvme_controller}) - nvme_name_pattern = "/dev/%sn[0-9]+" % nvme_controller - nvme_devices_filtered = list( - filter( - lambda device: re.match(nvme_name_pattern, device), - current_nvme_devices - ) - ) - return nvme_devices_filtered - - @utils.retry(exception.NotFound, retries=5) - def _is_nvme_available(self, nvme_name): - current_nvme_devices = self._get_nvme_devices() - if self._filter_nvme_devices(current_nvme_devices, nvme_name): - return True - else: - LOG.error("Failed to find nvme device") - raise exception.NotFound() - - def _wait_for_blk(self, nvme_transport_type, conn_nqn, - target_portal, port): - # Find nvme name in subsystem list and wait max 15 seconds - # until new volume will be available in kernel - nvme_name = "" - nvme_address = "traddr=%s trsvcid=%s" % (target_portal, port) - - # Get nvme subsystems in order to find - # nvme name for connected nvme - try: - (out, err) = self._get_nvme_subsys() - except putils.ProcessExecutionError: - LOG.error("Failed to get nvme subsystems") - raise - - # Get subsystem list. Throw exception if out is currupt or empty - try: - subsystems = json.loads(out)['Subsystems'] - except Exception: - return False - - # Find nvme name among subsystems - for i in range(0, int(len(subsystems) / 2)): - subsystem = subsystems[i * 2] - if 'NQN' in subsystem and subsystem['NQN'] == conn_nqn: - for path in subsystems[i * 2 + 1]['Paths']: - if (path['Transport'] == nvme_transport_type - and path['Address'] == nvme_address): - nvme_name = path['Name'] - break - - if not nvme_name: - return False - - # Wait until nvme will be available in kernel - return self._is_nvme_available(nvme_name) + # ####### Connect Volume methods ######## @utils.trace @utils.connect_volume_prepare_result @base.synchronized('connect_volume', external=True) - def connect_volume(self, connection_properties): - """Discover and attach the volume. + @NVMeOFConnProps.from_dictionary_parameter + def connect_volume( + self, connection_properties: NVMeOFConnProps) -> Dict[str, str]: + """Attach and discover the volume.""" + if connection_properties.is_replicated is False: + LOG.debug('Starting non replicated connection') + path = self._connect_target(connection_properties.targets[0]) - :param connection_properties: The dictionary that describes all - of the target volume attributes. - connection_properties must include: - nqn - NVMe subsystem name to the volume to be connected - target_port - NVMe target port that hosts the nqn sybsystem - target_portal - NVMe target ip that hosts the nqn sybsystem - :type connection_properties: dict - :returns: dict + else: # If we know it's replicated or we don't yet know + LOG.debug('Starting replicated connection') + path = self._connect_volume_replicated(connection_properties) + + return {'type': 'block', 'path': path} + + def _do_multipath(self): + return self.use_multipath and self.native_multipath_supported + + @utils.retry(exception.VolumeDeviceNotFound, interval=2) + def _connect_target(self, target: Target) -> str: + """Attach a specific target to present a volume on the system + + If we are already connected to any of the portals (and it's live) we + send a rescan (because the backend may not support AER messages), + otherwise we iterate through the portals trying to do an nvme-of + connection. + + This method assumes that the controllers for the portals have already + been set. For example using the from_dictionary_parameter decorator + in the NVMeOFConnProps class. + + Returns the path of the connected device. """ - if connection_properties.get('vol_uuid'): # compatibility - return self._connect_volume_by_uuid(connection_properties) + connected = False + missing_portals = [] - current_nvme_devices = self._get_nvme_devices() - device_info = {'type': 'block'} - conn_nqn = connection_properties['nqn'] - target_portal = connection_properties['target_portal'] - port = connection_properties['target_port'] - nvme_transport_type = connection_properties['transport_type'] - host_nqn = connection_properties.get('host_nqn') - device_nguid = connection_properties.get('volume_nguid') - cmd = [ - 'nvme', 'connect', - '-t', nvme_transport_type, - '-n', conn_nqn, - '-a', target_portal, - '-s', port] - if host_nqn: - cmd.extend(['-q', host_nqn]) + for portal in target.portals: + state = portal.state # store it so we read only once from sysfs + # Rescan live controllers in case backend doesn't support AER + if state == portal.LIVE: + connected = True + self.rescan(portal.controller) # type: ignore + # Remember portals that are not present in the system + elif state == portal.MISSING: + missing_portals.append(portal) + # Ignore reconnecting/dead portals + else: + LOG.debug('%s exists but is %s', portal, state) - self._try_connect_nvme(cmd) - try: - self._wait_for_blk(nvme_transport_type, conn_nqn, - target_portal, port) - except exception.NotFound: - LOG.error("Waiting for nvme failed") - raise exception.NotFound(message="nvme connect: NVMe device " - "not found") - if device_nguid: - path = self._get_device_path_by_nguid(device_nguid) + # If no live portals exist or if we want to use multipath + do_multipath = self._do_multipath() + if do_multipath or not connected: + for portal in missing_portals: + cmd = ['connect', '-a', portal.address, '-s', portal.port, + '-t', portal.transport, '-n', target.nqn, '-Q', '128', + '-l', '-1'] + if target.host_nqn: + cmd.extend(['-q', target.host_nqn]) + try: + self.run_nvme_cli(cmd) + connected = True + except putils.ProcessExecutionError as exc: + # In some nvme versions code 70 means target is already + # connected, but in newer versions code is EALREADY. + # Those should only happen if there is a race condition + # because something is incorrectly configured (n-cpu and + # c-vol running on same node with different lock paths) or + # an admin is touching things manually. Not passing these + # exit codes in check_exit_code parameter to _execute so we + # can log it. + if exc.exit_code not in (70, errno.EALREADY): + LOG.error('Could not connect to %s: exit_code: %s, ' + 'stdout: "%s", stderr: "%s",', portal, + exc.exit_code, exc.stdout, exc.stderr) + continue + + LOG.warning('Race condition with some other application ' + 'when connecting to %s, please check your ' + 'system configuration.', portal) + connected = True + + if not do_multipath: + break # We are connected + + if not connected: + raise exception.VolumeDeviceNotFound(device=target.nqn) + + # Ensure controller names of new connections are set + target.set_portals_controllers() + dev_path = target.find_device() + return dev_path + + @utils.trace + def _connect_volume_replicated( + self, connection_properties: NVMeOFConnProps) -> str: + """Connect to a replicated volume and prepare the RAID + + Connection properties must contain all the necessary replica + information, even if there is only 1 replica. + + Returns the /dev/md path + + Raises VolumeDeviceNotFound when cannot present the device in the + system. + """ + host_device_paths = [] + + if not connection_properties.alias: + raise exception.BrickException('Alias missing in connection info') + + for replica in connection_properties.targets: + try: + rep_host_device_path = self._connect_target(replica) + host_device_paths.append(rep_host_device_path) + except Exception as ex: + LOG.error("_connect_target: %s", ex) + if not host_device_paths: + raise exception.VolumeDeviceNotFound( + device=connection_properties.targets) + + if connection_properties.is_replicated: + device_path = self._handle_replicated_volume(host_device_paths, + connection_properties) else: - path = self._get_device_path(current_nvme_devices) - device_info['path'] = path - LOG.debug("NVMe device to be connected to is %(path)s", - {'path': path}) - return device_info + device_path = self._handle_single_replica( + host_device_paths, connection_properties.alias) + + if nvmeof_agent: + nvmeof_agent.NVMeOFAgent.ensure_running(self) + + return device_path + + def _handle_replicated_volume(self, + host_device_paths: List[str], + conn_props: NVMeOFConnProps) -> str: + """Assemble the raid from found devices.""" + path_in_raid = False + for dev_path in host_device_paths: + path_in_raid = self._is_device_in_raid(dev_path) + if path_in_raid: + break + device_path = RAID_PATH + conn_props.alias # type: ignore + if path_in_raid: + self.stop_and_assemble_raid(host_device_paths, device_path, False) + else: + paths_found = len(host_device_paths) + if conn_props.replica_count > paths_found: # type: ignore + LOG.error( + 'Cannot create MD as %s out of %s legs were found.', + paths_found, conn_props.replica_count) + raise exception.VolumeDeviceNotFound(device=conn_props.alias) + self.create_raid(host_device_paths, '1', + conn_props.alias, # type: ignore + conn_props.alias, # type: ignore + False) + + return device_path + + def _handle_single_replica(self, + host_device_paths: List[str], + volume_alias: str) -> str: + """Assemble the raid from a single device.""" + if self._is_raid_device(host_device_paths[0]): + md_path = RAID_PATH + volume_alias + self.stop_and_assemble_raid(host_device_paths, md_path, False) + return md_path + return host_device_paths[0] + + # ####### Disconnect methods ######## @utils.trace @base.synchronized('connect_volume', external=True) @utils.connect_volume_undo_prepare_result(unlink_after=True) - def disconnect_volume(self, connection_properties, device_info, - force=False, ignore_errors=False): + def disconnect_volume(self, + connection_properties: Dict, + device_info: Dict[str, str], + force: bool = False, + ignore_errors: bool = False) -> None: """Flush the volume. Disconnect of volumes happens on storage system side. Here we could @@ -390,309 +984,162 @@ class NVMeOFConnector(base.BaseLinuxConnector): left is flushing or disassembly of a correspondng RAID device. :param connection_properties: The dictionary that describes all - of the target volume attributes. - connection_properties must include: - device_path - path to the volume to be connected + of the target volume attributes as + described in connect_volume but also with + the "device_path" key containing the path + to the volume that was connected (this is + added by Nova). :type connection_properties: dict :param device_info: historical difference, but same as connection_props :type device_info: dict - """ - if connection_properties.get('vol_uuid'): # compatibility - return self._disconnect_volume_replicated( - connection_properties, device_info, - force=force, ignore_errors=ignore_errors) + # NOTE: Cannot use NVMeOFConnProps's decorator to create instance from + # conn props because "connect_volume_undo_prepare_result" must be the + # first decorator and it expects a dictionary. + conn_props = NVMeOFConnProps(connection_properties) + try: + device_path = self.get_volume_paths(conn_props, device_info)[0] + except IndexError: + LOG.warning( + "Cannot find the device for %s, assuming it's not there.", + conn_props.cinder_volume_id or conn_props.targets[0].nqn) + return - conn_nqn = connection_properties['nqn'] - if device_info and device_info.get('path'): - device_path = device_info.get('path') - else: - device_path = connection_properties['device_path'] or '' - current_nvme_devices = self._get_nvme_devices() - if device_path not in current_nvme_devices: - LOG.warning("Trying to disconnect device %(device_path)s with " - "subnqn %(conn_nqn)s that is not connected.", - {'device_path': device_path, 'conn_nqn': conn_nqn}) + if not os.path.exists(device_path): + LOG.warning("Trying to disconnect device %(device_path)s, but " + "it is not connected. Skipping.", + {'device_path': device_path}) return try: - self._linuxscsi.flush_device_io(device_path) - except putils.ProcessExecutionError: + # We assume that raid devices are flushed when ending the raid + if device_path.startswith(RAID_PATH): + self.end_raid(device_path) + + else: + self._linuxscsi.flush_device_io(device_path) + except Exception: if not ignore_errors: raise + # ####### Extend methods ######## + + @staticmethod + def _get_sizes_from_lba(ns_data: Dict) -> Tuple[Optional[int], + Optional[int]]: + """Return size in bytes and the nsze of the volume from NVMe NS data. + + nsze is the namespace size that defines the total size of the namespace + in logical blocks (LBA 0 through n-1), as per NVMe-oF specs. + + Returns a tuple of nsze and size + """ + try: + lbads = ns_data['lbafs'][0]['ds'] + + # Don't know what to do with more than 1 LBA and as per NVMe specs + # if LBADS < 9 then LBA is not supported + if len(ns_data['lbafs']) != 1 or lbads < 9: + LOG.warning("Cannot calculate new size with LBAs") + return None, None + nsze = ns_data['nsze'] + new_size = nsze * (1 << lbads) + except Exception: + return None, None + + LOG.debug('New volume size is %s and nsze is %s', new_size, nsze) + return nsze, new_size + @utils.trace @base.synchronized('extend_volume', external=True) @utils.connect_volume_undo_prepare_result - def extend_volume(self, connection_properties): - """Update the local kernel's size information. + def extend_volume(self, connection_properties: Dict[str, str]) -> int: + """Update an attached volume to reflect the current size after extend - Try and update the local kernel's size information - for an LVM volume. + The only way to reflect the new size of an NVMe-oF volume on the host + is a rescan, which rescans the whole subsystem. This is a problem on + attach_volume and detach_volume, but not here, since we will have at + least the namespace we are operating on in the subsystem. + + The tricky part is knowing when a rescan has already been completed and + the volume size on sysfs is final. The rescan may already have + happened before this method is called due to an AER message or we may + need to trigger it here. + + Scans can be triggered manually with 'nvme ns-rescan' or writing 1 in + configf's rescan file, or they can be triggered indirectly when calling + the 'nvme list', 'nvme id-ns', or even using the 'nvme admin-passthru' + command. + + Even after getting the new size with any of the NVMe commands above, we + still need to wait until this is reflected on the host device, because + we cannot return to the caller until the new size is in effect. + + If we don't see the new size taking effect on the system after 5 + seconds, or if we cannot get the new size with nvme, then we rescan in + the latter and in both cases we blindly wait 5 seconds and return + whatever size is present. + + For replicated volumes, the RAID needs to be extended. """ - if connection_properties.get('vol_uuid'): # compatibility - return self._extend_volume_replicated(connection_properties) - volume_paths = self.get_volume_paths(connection_properties) - if volume_paths: - if connection_properties.get('volume_nguid'): - for path in volume_paths: - return utils.get_device_size(self, path) - return self._linuxscsi.extend_volume( - volume_paths, use_multipath=self.use_multipath) + # NOTE: Cannot use NVMeOFConnProps's decorator to create instance from + # conn props because "connect_volume_undo_prepare_result" must be the + # first decorator and it expects a dictionary. + conn_props = NVMeOFConnProps(connection_properties) + try: + device_path = self.get_volume_paths(conn_props)[0] + except IndexError: + raise exception.VolumeDeviceNotFound() + + # Replicated needs to grow the raid, even if there's only 1 device + if device_path.startswith(RAID_PATH): + # NOTE: May not work without backend AER support and may have races + self.run_mdadm(('mdadm', '--grow', '--size', 'max', device_path)) + else: - LOG.warning("Couldn't find any volume paths on the host to " - "extend volume for %(props)s", - {'props': connection_properties}) - raise exception.VolumePathsNotFound() + dev_name = os.path.basename(device_path) + ctrl_name = dev_name.rsplit('n', 1)[0] + nsze: Optional[Union[str, int]] = None + try: + # With many devices, id-ns command generates less data + out, err = self._execute('nvme', 'id-ns', '-ojson', + device_path, run_as_root=True, + root_helper=self._root_helper) + ns_data = json.loads(out) + nsze, new_size = self._get_sizes_from_lba(ns_data) + except Exception as exc: + LOG.warning('Failed to get id-ns %s', exc) + # Assume that nvme command failed, so didn't scan + self.rescan(ctrl_name) - @utils.trace - def _connect_volume_by_uuid(self, connection_properties): - """connect to volume on host + if nsze: # Wait for the system to reflect the new size + nsze = str(nsze) # To compare with contents of sysfs + for x in range(10): # Wait 5 secs for size to appear in sysfs + current_nsze = blk_property('size', dev_name) + if current_nsze == nsze: + return new_size # type: ignore - connection_properties for NVMe-oF must include: - target_portals - list of ip,port,transport for each portal - target_nqn - NVMe-oF Qualified Name - vol_uuid - UUID for volume/replica - """ - volume_replicas = connection_properties.get('volume_replicas') - replica_count = connection_properties.get('replica_count') - volume_alias = connection_properties.get('alias') + LOG.debug('Sysfs size is still %s', current_nsze) + time.sleep(0.5) + LOG.warning('Timeout waiting for sysfs to reflect the right ' + 'volume size.') - if volume_replicas: - host_device_paths = [] - - for replica in volume_replicas: - try: - rep_host_device_path = self._connect_target_volume( - replica['target_nqn'], replica['vol_uuid'], - replica['portals']) - if rep_host_device_path: - host_device_paths.append(rep_host_device_path) - except Exception as ex: - LOG.error("_connect_target_volume: %s", ex) - if not host_device_paths: - raise exception.VolumeDeviceNotFound( - device=volume_replicas) - - if replica_count > 1: - device_path = self._handle_replicated_volume( - host_device_paths, volume_alias, replica_count) - else: - device_path = self._handle_single_replica( - host_device_paths, volume_alias) - else: - device_path = self._connect_target_volume( - connection_properties['target_nqn'], - connection_properties['vol_uuid'], - connection_properties['portals']) - - if nvmeof_agent: - nvmeof_agent.NVMeOFAgent.ensure_running(self) - - return {'type': 'block', 'path': device_path} - - @utils.trace - def _disconnect_volume_replicated(self, connection_properties, device_info, - force=False, ignore_errors=False): - device_path = None - volume_replicas = connection_properties.get('volume_replicas') - replica_count = connection_properties.get('replica_count') - if device_info and device_info.get('path'): - device_path = device_info['path'] - elif connection_properties.get('device_path'): - device_path = connection_properties['device_path'] - elif volume_replicas and replica_count > 1: - device_path = '/dev/md/' + connection_properties['alias'] - - if volume_replicas and replica_count > 1: - NVMeOFConnector.end_raid(self, device_path) - else: - if self._get_fs_type(device_path) == 'linux_raid_member': - NVMeOFConnector.end_raid(self, device_path) - - def _extend_volume_replicated(self, connection_properties): - volume_replicas = connection_properties.get('volume_replicas') - replica_count = connection_properties.get('replica_count') - - if volume_replicas and replica_count > 1: - device_path = '/dev/md/' + connection_properties['alias'] - NVMeOFConnector.run_mdadm( - self, ['mdadm', '--grow', '--size', 'max', device_path]) - else: - target_nqn = None - vol_uuid = None - if not volume_replicas: - target_nqn = connection_properties['target_nqn'] - vol_uuid = connection_properties['vol_uuid'] - elif len(volume_replicas) == 1: - target_nqn = volume_replicas[0]['target_nqn'] - vol_uuid = volume_replicas[0]['vol_uuid'] - device_path = NVMeOFConnector.get_nvme_device_path( - self, target_nqn, vol_uuid) + # Last resort when id-ns failed or system didn't reflect new size + LOG.info('Wait 5 seconds and return whatever size is present') + time.sleep(5) return utils.get_device_size(self, device_path) - def _connect_target_volume(self, target_nqn, vol_uuid, portals): - nvme_ctrls = NVMeOFConnector.rescan(self, target_nqn) - any_new_connect = NVMeOFConnector.connect_to_portals(self, target_nqn, - portals, - nvme_ctrls) - if not any_new_connect and len(nvme_ctrls) == 0: - # no new connections and any pre-exists controllers - LOG.error("No successful connections to: %s", target_nqn) - raise exception.VolumeDeviceNotFound(device=target_nqn) - if any_new_connect: - # new connections - refresh controllers map - nvme_ctrls = \ - NVMeOFConnector.get_live_nvme_controllers_map(self, target_nqn) - nvme_ctrls_values = list(nvme_ctrls.values()) - dev_path = NVMeOFConnector.get_nvme_device_path(self, target_nqn, - vol_uuid, - nvme_ctrls_values) - if not dev_path: - LOG.error("Target %s volume %s not found", target_nqn, vol_uuid) - raise exception.VolumeDeviceNotFound(device=vol_uuid) - return dev_path + # ####### RAID methods ######## - @staticmethod - def connect_to_portals(executor, target_nqn, target_portals, nvme_ctrls): - # connect to any of NVMe-oF target portals - - # check if the controller exist before trying to connect - # in multipath connect all given target portals - any_new_connect = False - no_multipath = (not executor.use_multipath - or not NVMeOFConnector.native_multipath_supported) - for portal in target_portals: - portal_address = portal[0] - portal_port = portal[1] - if NVMeOFConnector.is_portal_connected(portal_address, portal_port, - nvme_ctrls): - if no_multipath: - break - continue - if portal[2] == 'RoCEv2': - portal_transport = 'rdma' - else: - portal_transport = 'tcp' - nvme_command = ( - 'connect', '-a', portal_address, '-s', portal_port, '-t', - portal_transport, '-n', target_nqn, '-Q', '128', '-l', '-1') - try: - NVMeOFConnector.run_nvme_cli(executor, nvme_command) - any_new_connect = True - if no_multipath: - break - except Exception: - LOG.exception("Could not connect to portal %s", portal) - return any_new_connect - - @staticmethod - def is_portal_connected(portal_address, portal_port, nvme_ctrls): - address = f"traddr={portal_address},trsvcid={portal_port}" - return address in nvme_ctrls - - @staticmethod - def get_nvme_controllers(executor, target_nqn): - nvme_controllers = \ - NVMeOFConnector.get_live_nvme_controllers_map(executor, target_nqn) - if len(nvme_controllers) > 0: - return nvme_controllers.values() - raise exception.VolumeDeviceNotFound(device=target_nqn) - - @staticmethod - def get_live_nvme_controllers_map(executor, target_nqn): - """returns map of all live controllers and their addresses """ - nvme_controllers = dict() - ctrls = glob.glob('/sys/class/nvme-fabrics/ctl/nvme*') - for ctrl in ctrls: - try: - lines, _err = executor._execute( - 'cat', ctrl + '/subsysnqn', run_as_root=True, - root_helper=executor._root_helper) - for line in lines.split('\n'): - if line == target_nqn: - state, _err = executor._execute( - 'cat', ctrl + '/state', run_as_root=True, - root_helper=executor._root_helper) - if 'live' in state: - address_file = ctrl + '/address' - try: - with open(address_file, 'rt') as f: - address = f.read().strip() - except Exception: - LOG.warning("Failed to read file %s", - address_file) - continue - ctrl_name = os.path.basename(ctrl) - LOG.debug("[!] address: %s|%s", address, ctrl_name) - nvme_controllers[address] = ctrl_name - else: - LOG.debug("nvmeof ctrl device not live: %s", ctrl) - except putils.ProcessExecutionError as e: - LOG.exception(e) - return nvme_controllers - - @staticmethod - @utils.retry(exception.VolumeDeviceNotFound, retries=5) - def get_nvme_device_path(executor, target_nqn, vol_uuid, nvme_ctrls=None): - if not nvme_ctrls: - nvme_ctrls = NVMeOFConnector.get_nvme_controllers(executor, - target_nqn) - LOG.debug("[!] nvme_ctrls: %s", nvme_ctrls) - for nvme_ctrl in nvme_ctrls: - uuid_paths = glob.glob('/sys/class/block/' + nvme_ctrl + 'n*/uuid') - for uuid_path in uuid_paths: - try: - uuid_lines, _err = executor._execute( - 'cat', uuid_path, run_as_root=True, - root_helper=executor._root_helper) - if uuid_lines.split('\n')[0] == vol_uuid: - ignore = len('/uuid') - ns_ind = uuid_path.rfind('/', 0, -ignore) - nvme_device = uuid_path[ns_ind + 1: -ignore] - return '/dev/' + nvme_device - except putils.ProcessExecutionError as e: - LOG.exception(e) - raise exception.VolumeDeviceNotFound(device=vol_uuid) - - def _handle_replicated_volume(self, host_device_paths, - volume_alias, num_of_replicas): - path_in_raid = False - for dev_path in host_device_paths: - path_in_raid = NVMeOFConnector._is_device_in_raid(self, dev_path) - if path_in_raid: - break - device_path = '/dev/md/' + volume_alias - if path_in_raid: - NVMeOFConnector.stop_and_assemble_raid( - self, host_device_paths, device_path, False) - else: - paths_found = len(host_device_paths) - if num_of_replicas > paths_found: - LOG.error( - 'Cannot create MD as %s out of %s legs were found.', - paths_found, num_of_replicas) - raise exception.VolumeDeviceNotFound(device=volume_alias) - NVMeOFConnector.create_raid(self, host_device_paths, '1', - volume_alias, volume_alias, False) - - return device_path - - def _handle_single_replica(self, host_device_paths, volume_alias): - if self._get_fs_type(host_device_paths[0]) == 'linux_raid_member': - md_path = '/dev/md/' + volume_alias - NVMeOFConnector.stop_and_assemble_raid( - self, host_device_paths, md_path, False) - return md_path - return host_device_paths[0] - - @staticmethod - def run_mdadm(executor, cmd, raise_exception=False): + def run_mdadm(self, + cmd: Sequence[str], + raise_exception: bool = False) -> Optional[str]: cmd_output = None try: - lines, err = executor._execute( - *cmd, run_as_root=True, root_helper=executor._root_helper) + lines, err = self._execute( + *cmd, run_as_root=True, root_helper=self._root_helper) for line in lines.split('\n'): cmd_output = line break @@ -702,8 +1149,7 @@ class NVMeOFConnector(base.BaseLinuxConnector): raise ex return cmd_output - @staticmethod - def _is_device_in_raid(self, device_path): + def _is_device_in_raid(self, device_path: str) -> bool: cmd = ['mdadm', '--examine', device_path] raid_expected = device_path + ':' try: @@ -715,17 +1161,17 @@ class NVMeOFConnector(base.BaseLinuxConnector): else: return False except putils.ProcessExecutionError: - return False + pass + return False @staticmethod - def ks_readlink(dest): + def ks_readlink(dest: str) -> str: try: return os.readlink(dest) except Exception: return '' - @staticmethod - def get_md_name(executor, device_name): + def get_md_name(self, device_name: str) -> Optional[str]: get_md_cmd = ( 'cat /proc/mdstat | grep ' + device_name + ' | awk \'{print $1;}\'') @@ -734,14 +1180,14 @@ class NVMeOFConnector(base.BaseLinuxConnector): cmd_output = None try: - lines, err = executor._execute( - *cmd, run_as_root=True, root_helper=executor._root_helper) + lines, err = self._execute( + *cmd, run_as_root=True, root_helper=self._root_helper) for line in lines.split('\n'): cmd_output = line break - LOG.debug("[!] cmd_output = " + cmd_output) + LOG.debug("[!] cmd_output = %s", cmd_output) if err: return None @@ -750,8 +1196,10 @@ class NVMeOFConnector(base.BaseLinuxConnector): LOG.warning("[!] Could not run cmd: %s", str(ex)) return None - @staticmethod - def stop_and_assemble_raid(executor, drives, md_path, read_only): + def stop_and_assemble_raid(self, + drives: List[str], + md_path: str, + read_only: bool) -> None: md_name = None i = 0 assembled = False @@ -759,7 +1207,7 @@ class NVMeOFConnector(base.BaseLinuxConnector): while i < 5 and not assembled: for drive in drives: device_name = drive[5:] - md_name = NVMeOFConnector.get_md_name(executor, device_name) + md_name = self.get_md_name(device_name) link = NVMeOFConnector.ks_readlink(md_path) if link != '': link = os.path.basename(link) @@ -771,16 +1219,17 @@ class NVMeOFConnector(base.BaseLinuxConnector): time.sleep(1) if md_name and md_name != link: - NVMeOFConnector.stop_raid(executor, md_name) + self.stop_raid(md_name) try: - assembled = NVMeOFConnector.assemble_raid( - executor, drives, md_path, read_only) + assembled = self.assemble_raid(drives, md_path, read_only) except Exception: i += 1 - @staticmethod - def assemble_raid(executor, drives, md_path, read_only): + def assemble_raid(self, + drives: List[str], + md_path: str, + read_only: bool) -> bool: cmd = ['mdadm', '--assemble', '--run', md_path] if read_only: @@ -790,15 +1239,19 @@ class NVMeOFConnector(base.BaseLinuxConnector): cmd.append(drives[i]) try: - NVMeOFConnector.run_mdadm(executor, cmd, True) + self.run_mdadm(cmd, True) except putils.ProcessExecutionError as ex: LOG.warning("[!] Could not _assemble_raid: %s", str(ex)) raise ex return True - @staticmethod - def create_raid(executor, drives, raid_type, device_name, name, read_only): + def create_raid(self, + drives: List[str], + raid_type: str, + device_name: str, + name: str, + read_only: bool) -> None: cmd = ['mdadm'] num_drives = len(drives) cmd.append('-C') @@ -825,11 +1278,11 @@ class NVMeOFConnector(base.BaseLinuxConnector): cmd.append(drives[i]) LOG.debug('[!] cmd = ' + str(cmd)) - NVMeOFConnector.run_mdadm(executor, cmd) - # sometimes under load, md is not created right away, so we wait + self.run_mdadm(cmd) + # sometimes under load, md is not created right away so we wait for i in range(60): try: - is_exist = os.path.exists("/dev/md/" + name) + is_exist = os.path.exists(RAID_PATH + name) LOG.debug("[!] md is_exist = %s", is_exist) if is_exist: return @@ -840,14 +1293,12 @@ class NVMeOFConnector(base.BaseLinuxConnector): LOG.error(msg) raise exception.NotFound(message=msg) - @staticmethod - def end_raid(executor, device_path): - raid_exists = NVMeOFConnector.is_raid_exists(executor, device_path) + def end_raid(self, device_path: str) -> None: + raid_exists = self.is_raid_exists(device_path) if raid_exists: for i in range(10): try: - cmd_out = NVMeOFConnector.stop_raid( - executor, device_path, True) + cmd_out = self.stop_raid(device_path, True) if not cmd_out: break except Exception: @@ -856,26 +1307,26 @@ class NVMeOFConnector(base.BaseLinuxConnector): is_exist = os.path.exists(device_path) LOG.debug("[!] is_exist = %s", is_exist) if is_exist: - NVMeOFConnector.remove_raid(executor, device_path) + self.remove_raid(device_path) os.remove(device_path) except Exception: LOG.debug('[!] Exception_stop_raid!') - @staticmethod - def stop_raid(executor, md_path, raise_exception=False): + def stop_raid(self, + md_path: str, + raise_exception: bool = False) -> Optional[str]: cmd = ['mdadm', '--stop', md_path] LOG.debug("[!] cmd = " + str(cmd)) - cmd_out = NVMeOFConnector.run_mdadm(executor, cmd, raise_exception) + cmd_out = self.run_mdadm(cmd, raise_exception) return cmd_out - @staticmethod - def is_raid_exists(executor, device_path): + def is_raid_exists(self, device_path: str) -> bool: cmd = ['mdadm', '--detail', device_path] LOG.debug("[!] cmd = " + str(cmd)) raid_expected = device_path + ':' try: - lines, err = executor._execute( - *cmd, run_as_root=True, root_helper=executor._root_helper) + lines, err = self._execute( + *cmd, run_as_root=True, root_helper=self._root_helper) for line in lines.split('\n'): LOG.debug("[!] line = " + line) @@ -884,40 +1335,19 @@ class NVMeOFConnector(base.BaseLinuxConnector): else: return False except putils.ProcessExecutionError: - return False + pass + return False - @staticmethod - def remove_raid(executor, device_path): + def remove_raid(self, device_path: str) -> None: cmd = ['mdadm', '--remove', device_path] LOG.debug("[!] cmd = " + str(cmd)) - NVMeOFConnector.run_mdadm(executor, cmd) + self.run_mdadm(cmd) - @staticmethod - def run_nvme_cli(executor, nvme_command, **kwargs): - (out, err) = executor._execute('nvme', *nvme_command, run_as_root=True, - root_helper=executor._root_helper, - check_exit_code=True) - msg = ("nvme %(nvme_command)s: stdout=%(out)s stderr=%(err)s" % - {'nvme_command': nvme_command, 'out': out, 'err': err}) - LOG.debug("[!] " + msg) + def _is_raid_device(self, device: str) -> bool: + return self._get_fs_type(device) == 'linux_raid_member' - return out, err - - @staticmethod - def rescan(executor, target_nqn): - nvme_ctrls = NVMeOFConnector.get_live_nvme_controllers_map(executor, - target_nqn) - for nvme_ctrl in nvme_ctrls.values(): - ctr_device = (NVMeOFConnector.get_search_path() + nvme_ctrl) - nvme_command = ('ns-rescan', ctr_device) - try: - NVMeOFConnector.run_nvme_cli(executor, nvme_command) - except Exception as e: - LOG.exception(e) - return nvme_ctrls - - def _get_fs_type(self, device_path): - cmd = ('blkid', device_path, '-s', 'TYPE', '-o', 'value') + def _get_fs_type(self, device_path: str) -> Optional[str]: + cmd = ['blkid', device_path, '-s', 'TYPE', '-o', 'value'] LOG.debug("[!] cmd = " + str(cmd)) fs_type = None @@ -927,3 +1357,29 @@ class NVMeOFConnector(base.BaseLinuxConnector): check_exit_code=False) fs_type = lines.split('\n')[0] return fs_type or None + + # ####### NVMe methods ######## + + def run_nvme_cli(self, + nvme_command: Sequence[str], + **kwargs) -> Tuple[str, str]: + """Run an nvme cli command and return stdout and stderr output.""" + (out, err) = self._execute('nvme', *nvme_command, run_as_root=True, + root_helper=self._root_helper, + check_exit_code=True) + msg = ("nvme %(nvme_command)s: stdout=%(out)s stderr=%(err)s" % + {'nvme_command': nvme_command, 'out': out, 'err': err}) + LOG.debug("[!] %s", msg) + + return out, err + + def rescan(self, controller_name: str) -> None: + """Rescan an nvme controller.""" + nvme_command = ('ns-rescan', DEV_SEARCH_PATH + controller_name) + try: + self.run_nvme_cli(nvme_command) + except Exception as e: + raise exception.CommandExecutionFailed(e, cmd=nvme_command) + +# CONNECTOR CLASS end +# ######################################################### diff --git a/os_brick/tests/initiator/connectors/test_nvmeof.py b/os_brick/tests/initiator/connectors/test_nvmeof.py index 0cb52d4c9..1b2ba1ae4 100644 --- a/os_brick/tests/initiator/connectors/test_nvmeof.py +++ b/os_brick/tests/initiator/connectors/test_nvmeof.py @@ -11,7 +11,7 @@ # under the License. import builtins -import glob +import errno import os.path from unittest import mock @@ -21,75 +21,40 @@ from oslo_concurrency import processutils as putils from os_brick import exception from os_brick import executor from os_brick.initiator.connectors import nvmeof -from os_brick.initiator import linuxscsi from os_brick.privileged import rootwrap as priv_rootwrap +from os_brick.tests import base as test_base from os_brick.tests.initiator import test_connector from os_brick import utils TARGET_NQN = 'target.nqn' -EXECUTOR = executor.Executor(None) VOL_UUID = 'c20aba21-6ef6-446b-b374-45733b4883ba' +VOL_UUID_NO_HYPHENS = 'c20aba216ef6446bb37445733b4883ba' NVME_DEVICE_PATH = '/dev/nvme1' NVME_NS_PATH = '/dev/nvme1n1' -NVME_DEVICE_NGUID = '4941ef7595b8ee978ccf096800f205c6' +NGUID = '4941ef75-95b8-ee97-8ccf-096800f205c6' +NGUID_NO_HYPHENS = '4941ef7595b8ee978ccf096800f205c6' SYS_UUID = '9126E942-396D-11E7-B0B7-A81E84C186D1' HOST_UUID = 'c20aba21-6ef6-446b-b374-45733b4883ba' HOST_NQN = 'nqn.2014-08.org.nvmexpress:uuid:' \ 'beaae2de-3a97-4be1-a739-6ac4bc5bf138' -volume_replicas = [{'target_nqn': 'fakenqn1', 'vol_uuid': 'fakeuuid1', +VOL_UUID1 = '9b30ec12-75b9-4a53-be32-111111111111' +VOL_UUID2 = '9b30ec12-75b9-4a53-be32-222222222222' +VOL_UUID3 = '9b30ec12-75b9-4a53-be32-333333333333' +volume_replicas = [{'target_nqn': 'fakenqn1', 'vol_uuid': VOL_UUID1, 'portals': [('10.0.0.1', 4420, 'tcp')]}, - {'target_nqn': 'fakenqn2', 'vol_uuid': 'fakeuuid2', + {'target_nqn': 'fakenqn2', 'vol_uuid': VOL_UUID2, 'portals': [('10.0.0.2', 4420, 'tcp')]}, - {'target_nqn': 'fakenqn3', 'vol_uuid': 'fakeuuid3', + {'target_nqn': 'fakenqn3', 'vol_uuid': VOL_UUID3, 'portals': [('10.0.0.3', 4420, 'tcp')]}] connection_properties = { 'alias': 'fakealias', - 'vol_uuid': 'fakevoluuid', + 'vol_uuid': VOL_UUID, 'volume_replicas': volume_replicas, 'replica_count': 3 } +CONN_PROPS = nvmeof.NVMeOFConnProps(connection_properties) fake_portal = ('fake', 'portal', 'tcp') -fake_controller = '/sys/class/nvme-fabrics/ctl/nvme1' -fake_controllers_map = {'traddr=fakeaddress,trsvcid=4430': 'nvme1'} -nvme_list_subsystems_stdout = """ - { - "Subsystems" : [ - { - "Name" : "nvme-subsys0", - "NQN" : "nqn.2016-06.io.spdk:cnode1" - }, - { - "Paths" : [ - { - "Name" : "nvme0", - "Transport" : "tcp", - "Address" : "traddr=10.0.2.15 trsvcid=4420" - } - ] - }, - { - "Name" : "nvme-subsys1", - "NQN" : "nqn.2016-06.io.spdk:cnode2" - }, - { - "Paths" : [ - { - "Name" : "nvme1", - "Transport" : "rdma", - "Address" : "traddr=10.0.2.16 trsvcid=4420" - }, - { - "Name" : "nvme2", - "Transport" : "rdma", - "Address" : "traddr=10.0.2.17 trsvcid=4420" - } - ] - } - ] - } -""" - nvme_list_stdout = """ Node SN Model Namespace Usage Format FW Rev ------------- ------- ----- --------- ---------------- ----------- ------- @@ -99,8 +64,747 @@ Node SN Model Namespace Usage Format FW Rev @ddt.ddt -class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): +class UtilityMethodsTestCase(test_base.TestCase): + @mock.patch.object(nvmeof, 'sysfs_property', return_value='live') + def test_ctrl_property(self, mock_sysfs): + """Controller properties just read from nvme fabrics in sysfs.""" + res = nvmeof.ctrl_property('state', 'nvme0') + self.assertEqual('live', res) + mock_sysfs.assert_called_once_with('state', + '/sys/class/nvme-fabrics/ctl/nvme0') + @mock.patch.object(nvmeof, 'sysfs_property', return_value='uuid_value') + def test_blk_property(self, mock_sysfs): + """Block properties just read from block devices in sysfs.""" + res = nvmeof.blk_property('uuid', 'nvme0n1') + + self.assertEqual('uuid_value', res) + mock_sysfs.assert_called_once_with('uuid', '/sys/class/block/nvme0n1') + + @mock.patch.object(builtins, 'open') + def test_sysfs_property(self, mock_open): + """Method is basically an open an read method.""" + mock_read = mock_open.return_value.__enter__.return_value.read + mock_read.return_value = ' uuid ' + res = nvmeof.sysfs_property('uuid', '/sys/class/block/nvme0n1') + self.assertEqual('uuid', res) + mock_open.assert_called_once_with('/sys/class/block/nvme0n1/uuid', 'r') + mock_read.assert_called_once_with() + + @mock.patch.object(builtins, 'open', side_effect=FileNotFoundError) + def test_sysfs_property_not_found(self, mock_open): + """Failure to open file returns None.""" + mock_read = mock_open.return_value.__enter__.return_value.read + res = nvmeof.sysfs_property('uuid', '/sys/class/block/nvme0n1') + self.assertIsNone(res) + mock_open.assert_called_once_with('/sys/class/block/nvme0n1/uuid', 'r') + mock_read.assert_not_called() + + @mock.patch.object(builtins, 'open') + def test_sysfs_property_ioerror(self, mock_open): + """Failure to read file returns None.""" + mock_read = mock_open.return_value.__enter__.return_value.read + mock_read.side_effect = IOError + res = nvmeof.sysfs_property('uuid', '/sys/class/block/nvme0n1') + self.assertIsNone(res) + mock_open.assert_called_once_with('/sys/class/block/nvme0n1/uuid', 'r') + mock_read.assert_called_once_with() + + @ddt.data('/dev/nvme0n10', + '/sys/class/block/nvme0c1n10', + '/sys/class/nvme-fabrics/ctl/nvme1/nvme0c1n10') + def test_nvme_basename(self, name): + """ANA devices are transformed to the right name.""" + res = nvmeof.nvme_basename(name) + self.assertEqual('nvme0n10', res) + + +@ddt.ddt +class PortalTestCase(test_base.TestCase): + def setUp(self): + self.conn_props_dict = {'target_nqn': 'nqn_value', + 'vol_uuid': VOL_UUID, + 'portals': [('portal1', 'port1', 'RoCEv2')]} + self.conn_props = nvmeof.NVMeOFConnProps(self.conn_props_dict) + self.target = self.conn_props.targets[0] + self.portal = self.target.portals[0] + super().setUp() + + @ddt.data(('RoCEv2', 'rdma'), ('rdma', 'rdma'), ('tcp', 'tcp'), + ('TCP', 'tcp'), ('other', 'tcp')) + @ddt.unpack + def test_init(self, transport, expected_transport): + """Init changes conn props transport into rdma or tcp.""" + portal = nvmeof.Portal(self.target, 'address', 'port', transport) + self.assertEqual(self.target, portal.parent_target) + self.assertEqual('address', portal.address) + self.assertEqual('port', portal.port) + self.assertIsNone(portal.controller) + self.assertEqual(expected_transport, portal.transport) + + @ddt.data(('live', True), ('connecting', False), (None, False)) + @ddt.unpack + @mock.patch.object(nvmeof.Portal, 'state', + new_callable=mock.PropertyMock) + def test_is_live(self, state, expected, mock_state): + """Is live only returns True if the state is 'live'.""" + mock_state.return_value = state + self.assertIs(expected, self.portal.is_live) + mock_state.assert_called_once_with() + + @mock.patch.object(nvmeof, 'ctrl_property') + def test_state(self, mock_property): + """State uses sysfs to check the value.""" + self.portal.controller = 'nvme0' + self.assertEqual(mock_property.return_value, self.portal.state) + mock_property.assert_called_once_with('state', 'nvme0') + + @mock.patch.object(nvmeof, 'ctrl_property') + def test_state_no_controller(self, mock_property): + """Cannot read the state if the controller name has not been found.""" + self.portal.controller = None + self.assertIsNone(self.portal.state) + mock_property.assert_not_called() + + @mock.patch.object(nvmeof.Portal, 'get_device_by_property') + def test_get_device(self, mock_property): + """UUID has priority over everything else.""" + mock_property.return_value = 'result' + self.target.nguid = 'nguid' # will be ignored + res = self.portal.get_device() + self.assertEqual('result', res) + mock_property.assert_called_once_with('uuid', self.target.uuid) + + @mock.patch.object(nvmeof.Portal, 'get_device_by_property') + def test_get_device_by_nguid(self, mock_property): + """nguid takes priority over ns_id if no UUID.""" + mock_property.return_value = 'result' + self.target.uuid = None + self.target.nguid = 'nguid_value' + self.target.ns_id = 'ns_id_value' # will be ignored + res = self.portal.get_device() + self.assertEqual('result', res) + mock_property.assert_called_once_with('nguid', 'nguid_value') + + @mock.patch.object(nvmeof.Portal, 'get_device_by_property') + def test_get_device_by_ns_id(self, mock_property): + """nguid takes priority over ns_id if no UUID.""" + mock_property.return_value = 'result' + self.target.uuid = None + self.target.nguid = None + self.target.ns_id = 'ns_id_value' # will be ignored + res = self.portal.get_device() + self.assertEqual('result', res) + mock_property.assert_called_once_with('nsid', 'ns_id_value') + + @mock.patch.object(nvmeof.Target, 'get_device_path_by_initial_devices') + @mock.patch.object(nvmeof.Portal, 'get_device_by_property') + def test_get_device_by_initial_devices(self, mock_property, mock_get_dev): + """With no id, calls target to get device from initial devices.""" + mock_get_dev.return_value = 'result' + self.target.uuid = None + self.target.nguid = None + self.target.ns_id = None + res = self.portal.get_device() + self.assertEqual('result', res) + mock_get_dev.assert_called_once_with() + + @mock.patch.object(nvmeof, 'nvme_basename', return_value='nvme1n2') + @mock.patch.object(nvmeof, 'sysfs_property') + @mock.patch('glob.glob') + def test_get_device_by_property(self, mock_glob, mock_property, mock_name): + """Searches all devices for the right one and breaks when found.""" + mock_glob.return_value = [ + '/sys/class/nvme-fabrics/ctl/nvme0/nvme0n1', + '/sys/class/nvme-fabrics/ctl/nvme0/nvme1c1n2', + '/sys/class/nvme-fabrics/ctl/nvme0/nvme0n3' + ] + mock_property.side_effect = ['uuid1', 'uuid2'] + self.portal.controller = 'nvme0' + + res = self.portal.get_device_by_property('uuid', 'uuid2') + + self.assertEqual('/dev/nvme1n2', res) + + mock_glob.assert_called_once_with( + '/sys/class/nvme-fabrics/ctl/nvme0/nvme*') + self.assertEqual(2, mock_property.call_count) + mock_property.assert_has_calls( + [mock.call('uuid', '/sys/class/nvme-fabrics/ctl/nvme0/nvme0n1'), + mock.call('uuid', '/sys/class/nvme-fabrics/ctl/nvme0/nvme1c1n2')] + ) + mock_name.assert_called_once_with( + '/sys/class/nvme-fabrics/ctl/nvme0/nvme1c1n2') + + @mock.patch.object(nvmeof, 'nvme_basename', return_value='nvme1n2') + @mock.patch.object(nvmeof, 'sysfs_property') + @mock.patch('glob.glob') + def test_get_device_by_property_not_found( + self, mock_glob, mock_property, mock_name): + """Exhausts devices searching before returning None.""" + mock_glob.return_value = ['/sys/class/nvme-fabrics/ctl/nvme0/nvme0n1', + '/sys/class/nvme-fabrics/ctl/nvme0/nvme0n2'] + mock_property.side_effect = ['uuid1', 'uuid2'] + self.portal.controller = 'nvme0' + + res = self.portal.get_device_by_property('uuid', 'uuid3') + + self.assertIsNone(res) + + mock_glob.assert_called_once_with( + '/sys/class/nvme-fabrics/ctl/nvme0/nvme*') + self.assertEqual(2, mock_property.call_count) + mock_property.assert_has_calls( + [mock.call('uuid', '/sys/class/nvme-fabrics/ctl/nvme0/nvme0n1'), + mock.call('uuid', '/sys/class/nvme-fabrics/ctl/nvme0/nvme0n2')] + ) + mock_name.assert_not_called() + + +@ddt.ddt +class TargetTestCase(test_base.TestCase): + def setUp(self): + self.conn_props_dict = { + 'target_nqn': 'nqn_value', + 'vol_uuid': VOL_UUID, + 'portals': [('portal1', 'port1', 'RoCEv2'), + ('portal2', 'port2', 'anything')], + } + self.conn_props = nvmeof.NVMeOFConnProps(self.conn_props_dict) + self.target = self.conn_props.targets[0] + super().setUp() + + @mock.patch.object(nvmeof.Target, '__init__', return_value=None) + def test_factory(self, mock_init): + """Test Target factory + + The factory's parameter names take after the keys in the connection + + properties, and then calls the class init method that uses different + names. + """ + res = nvmeof.Target.factory(self.conn_props, **self.conn_props_dict) + mock_init.assert_called_once_with( + self.conn_props, + self.conn_props_dict['target_nqn'], + self.conn_props_dict['portals'], + self.conn_props_dict['vol_uuid'], + None, # nguid + None, # ns_id + None, # host_nqn + False) # find_controllers + self.assertIsInstance(res, nvmeof.Target) + + @ddt.data(True, False) + @mock.patch.object(nvmeof.Target, 'set_portals_controllers') + @mock.patch.object(nvmeof.Portal, '__init__', return_value=None) + def test_init(self, find_controllers, mock_init, mock_set_ctrls): + """Init instantiates portals and may call set_portals_controllers.""" + target = nvmeof.Target(self.conn_props, + 'nqn', + self.conn_props_dict['portals'], + # Confirm they get converted to hyphenated + VOL_UUID_NO_HYPHENS, + NGUID_NO_HYPHENS, + 'ns_id', + 'host_nqn', + find_controllers) + + self.assertEqual(self.conn_props, target.source_conn_props) + self.assertEqual('nqn', target.nqn) + self.assertEqual(VOL_UUID, target.uuid) + self.assertEqual(NGUID, target.nguid) + self.assertEqual('ns_id', target.ns_id) + self.assertEqual('host_nqn', target.host_nqn) + + self.assertIsInstance(target.portals[0], nvmeof.Portal) + self.assertIsInstance(target.portals[1], nvmeof.Portal) + + if find_controllers: + mock_set_ctrls.assert_called_once_with() + else: + mock_set_ctrls.assert_not_called() + + self.assertEqual(2, mock_init.call_count) + mock_init.assert_has_calls( + [mock.call(target, 'portal1', 'port1', 'RoCEv2'), + mock.call(target, 'portal2', 'port2', 'anything')] + ) + + @mock.patch.object(nvmeof.Target, '_get_nvme_devices') + @mock.patch.object(nvmeof.Target, 'set_portals_controllers') + @mock.patch.object(nvmeof.Portal, '__init__', return_value=None) + def test_init_no_id(self, mock_init, mock_set_ctrls, mock_get_devs): + """With no ID parameters query existing nvme devices.""" + target = nvmeof.Target(self.conn_props, + 'nqn', + self.conn_props_dict['portals']) + + self.assertEqual(self.conn_props, target.source_conn_props) + self.assertEqual('nqn', target.nqn) + for name in ('uuid', 'nguid', 'ns_id'): + self.assertIsNone(getattr(target, name)) + + self.assertIsInstance(target.portals[0], nvmeof.Portal) + self.assertIsInstance(target.portals[1], nvmeof.Portal) + + mock_set_ctrls.assert_not_called() + mock_get_devs.assert_called_once_with() + + self.assertEqual(2, mock_init.call_count) + mock_init.assert_has_calls( + [mock.call(target, 'portal1', 'port1', 'RoCEv2'), + mock.call(target, 'portal2', 'port2', 'anything')] + ) + + @mock.patch('glob.glob', return_value=['/dev/nvme0n1', '/dev/nvme1n1']) + def test__get_nvme_devices(self, mock_glob): + """Test getting all nvme devices present in system.""" + res = self.target._get_nvme_devices() + self.assertEqual(mock_glob.return_value, res) + mock_glob.assert_called_once_with('/dev/nvme*n*') + + @mock.patch.object(nvmeof.Portal, 'is_live', + new_callable=mock.PropertyMock) + def test_live_portals(self, mock_is_live): + """List with only live portals should be returned.""" + mock_is_live.side_effect = (True, False) + res = self.target.live_portals + self.assertListEqual([self.target.portals[0]], res) + + @mock.patch.object(nvmeof.Portal, 'state', + new_callable=mock.PropertyMock) + def test_present_portals(self, mock_state): + """List with only live portals should be returned.""" + # Duplicate number of portals + self.target.portals.extend(self.target.portals) + mock_state.side_effect = (None, 'live', 'connecting', 'live') + res = self.target.present_portals + self.assertListEqual(self.target.portals[1:], res) + + @mock.patch('glob.glob') + def test_set_portals_controllers_do_nothing(self, mock_glob): + """Do nothing if all protals already have the controller name.""" + self.target.portals[0].controller = 'nvme0' + self.target.portals[1].controller = 'nvme1' + self.target.set_portals_controllers() + mock_glob.assert_not_called() + + @mock.patch.object(nvmeof, 'sysfs_property') + @mock.patch('glob.glob') + def test_set_portals_controllers(self, mock_glob, mock_sysfs): + """Look in sysfs for the device paths.""" + portal = nvmeof.Portal(self.target, 'portal4', 'port4', 'tcp') + portal.controller = 'nvme0' + self.target.portals.insert(0, portal) + self.target.portals.append(nvmeof.Portal(self.target, 'portal5', + 'port5', 'tcp')) + self.target.host_nqn = 'nqn' + mock_glob.return_value = ['/sys/class/nvme-fabrics/ctl/nvme0', + '/sys/class/nvme-fabrics/ctl/nvme1', + '/sys/class/nvme-fabrics/ctl/nvme2', + '/sys/class/nvme-fabrics/ctl/nvme3', + '/sys/class/nvme-fabrics/ctl/nvme4', + '/sys/class/nvme-fabrics/ctl/nvme5'] + mock_sysfs.side_effect = [ + # nvme0 is skipped because it already belongs to the first portal + # nvme1 nqn doesn't match + 'wrong-nqn', + # nvme2 matches nqn but not the address + self.target.nqn, 'rdma', 'traddr=portal5,trsvcid=port5', 'nqn', + # nvme3 matches first portal but not the host_nqn + self.target.nqn, 'rdma', 'traddr=portal2,trsvcid=port2', 'badnqn', + # nvme4 matches first portal + self.target.nqn, 'tcp', 'traddr=portal2,trsvcid=port2', 'nqn', + # nvme5 simulates OS doesn't have the hostnqn attribute + self.target.nqn, 'tcp', 'traddr=portal5,trsvcid=port5', None, + ] + + self.target.set_portals_controllers() + + mock_glob.assert_called_once_with('/sys/class/nvme-fabrics/ctl/nvme*') + + expected_calls = [ + mock.call('subsysnqn', '/sys/class/nvme-fabrics/ctl/nvme1'), + mock.call('subsysnqn', '/sys/class/nvme-fabrics/ctl/nvme2'), + mock.call('transport', '/sys/class/nvme-fabrics/ctl/nvme2'), + mock.call('address', '/sys/class/nvme-fabrics/ctl/nvme2'), + mock.call('hostnqn', '/sys/class/nvme-fabrics/ctl/nvme2'), + mock.call('subsysnqn', '/sys/class/nvme-fabrics/ctl/nvme3'), + mock.call('transport', '/sys/class/nvme-fabrics/ctl/nvme3'), + mock.call('address', '/sys/class/nvme-fabrics/ctl/nvme3'), + mock.call('hostnqn', '/sys/class/nvme-fabrics/ctl/nvme3'), + mock.call('subsysnqn', '/sys/class/nvme-fabrics/ctl/nvme4'), + mock.call('transport', '/sys/class/nvme-fabrics/ctl/nvme4'), + mock.call('address', '/sys/class/nvme-fabrics/ctl/nvme4'), + mock.call('hostnqn', '/sys/class/nvme-fabrics/ctl/nvme4'), + mock.call('subsysnqn', '/sys/class/nvme-fabrics/ctl/nvme5'), + mock.call('transport', '/sys/class/nvme-fabrics/ctl/nvme5'), + mock.call('address', '/sys/class/nvme-fabrics/ctl/nvme5'), + mock.call('hostnqn', '/sys/class/nvme-fabrics/ctl/nvme5'), + ] + self.assertEqual(len(expected_calls), mock_sysfs.call_count) + mock_sysfs.assert_has_calls(expected_calls) + + # Confirm we didn't touch the first two portals + self.assertEqual('nvme0', self.target.portals[0].controller) + self.assertIsNone(self.target.portals[1].controller) + self.assertEqual('nvme4', self.target.portals[2].controller) + self.assertEqual('nvme5', self.target.portals[3].controller) + + @mock.patch('os_brick.utils.get_host_nqn', mock.Mock(return_value='nqn')) + @mock.patch.object(nvmeof, 'sysfs_property') + @mock.patch('glob.glob') + def test_set_portals_controllers_short_circuit( + self, mock_glob, mock_sysfs): + """Stops looking once we have found names for all portals.""" + self.target.portals[0].controller = 'nvme0' + mock_glob.return_value = ['/sys/class/nvme-fabrics/ctl/nvme0', + '/sys/class/nvme-fabrics/ctl/nvme1', + '/sys/class/nvme-fabrics/ctl/nvme2', + '/sys/class/nvme-fabrics/ctl/nvme3'] + mock_sysfs.side_effect = [ + self.target.nqn, 'tcp', 'traddr=portal2,trsvcid=port2', 'nqn', + ] + + self.target.set_portals_controllers() + + mock_glob.assert_called_once_with('/sys/class/nvme-fabrics/ctl/nvme*') + + expected_calls = [ + mock.call('subsysnqn', '/sys/class/nvme-fabrics/ctl/nvme1'), + mock.call('transport', '/sys/class/nvme-fabrics/ctl/nvme1'), + mock.call('address', '/sys/class/nvme-fabrics/ctl/nvme1'), + mock.call('hostnqn', '/sys/class/nvme-fabrics/ctl/nvme1'), + ] + self.assertEqual(len(expected_calls), mock_sysfs.call_count) + mock_sysfs.assert_has_calls(expected_calls) + + # We set the first portal with the newly found controller name + self.assertEqual('nvme0', self.target.portals[0].controller) + # Confirm we didn't touch second portal + self.assertEqual('nvme1', self.target.portals[1].controller) + + @mock.patch.object(nvmeof.Target, 'present_portals', + new_callable=mock.PropertyMock) + @mock.patch.object(nvmeof.Target, 'live_portals', + new_callable=mock.PropertyMock) + def test_get_devices_first_live(self, mock_live, mock_present): + """Return on first live portal with a device.""" + portal1 = mock.Mock(**{'get_device.return_value': None}) + portal2 = mock.Mock(**{'get_device.return_value': '/dev/nvme0n1'}) + portal3 = mock.Mock(**{'get_device.return_value': None}) + mock_live.return_value = [portal1, portal2] + + res = self.target.get_devices(only_live=True, get_one=True) + + self.assertListEqual(['/dev/nvme0n1'], res) + + mock_live.assert_called_once_with() + mock_present.assert_not_called() + portal1.get_device.assert_called_once_with() + portal2.get_device.assert_called_once_with() + portal3.get_device.assert_not_called() + + @mock.patch.object(nvmeof.Target, 'present_portals', + new_callable=mock.PropertyMock) + @mock.patch.object(nvmeof.Target, 'live_portals', + new_callable=mock.PropertyMock) + def test_get_devices_get_present(self, mock_live, mock_present): + """Return all devices that are found.""" + portal1 = mock.Mock(**{'get_device.return_value': '/dev/nvme0n1'}) + portal2 = mock.Mock(**{'get_device.return_value': None}) + portal3 = mock.Mock(**{'get_device.return_value': '/dev/nvme1n1'}) + mock_present.return_value = [portal1, portal2, portal3] + + res = self.target.get_devices(only_live=False) + + self.assertIsInstance(res, list) + self.assertEqual({'/dev/nvme0n1', '/dev/nvme1n1'}, set(res)) + + mock_present.assert_called_once_with() + mock_live.assert_not_called() + portal1.get_device.assert_called_once_with() + portal2.get_device.assert_called_once_with() + portal3.get_device.assert_called_once_with() + + @mock.patch.object(nvmeof.Target, 'get_devices') + def test_find_device_not_found(self, mock_get_devs): + """Finding a devices tries up to 5 times before giving up.""" + mock_get_devs.return_value = [] + self.assertRaises(exception.VolumeDeviceNotFound, + self.target.find_device) + + self.assertEqual(5, mock_get_devs.call_count) + mock_get_devs.assert_has_calls( + 5 * [mock.call(only_live=True, get_one=True)] + ) + + @mock.patch.object(nvmeof.Target, 'get_devices') + def test_find_device_first_found(self, mock_get_devs): + """Returns the first device found.""" + mock_get_devs.return_value = ['/dev/nvme0n1'] + res = self.target.find_device() + mock_get_devs.assert_called_once_with(only_live=True, get_one=True) + self.assertEqual('/dev/nvme0n1', res) + + @mock.patch.object(nvmeof.Target, '_get_nvme_devices') + def test_get_device_path_by_initial_devices(self, mock_get_devs): + """There's a new device since we started, return it.""" + self.target.portals[0].controller = 'nvme0' + self.target.portals[1].controller = 'nvme1' + mock_get_devs.return_value = ['/dev/nvme0n1', '/dev/nvme0n2', + '/dev/nvme1n2', '/dev/nvme2n1'] + self.target.devices_on_start = ['/dev/nvme0n1', '/dev/nvme1n2'] + + res = self.target.get_device_path_by_initial_devices() + + mock_get_devs.assert_called_once_with() + self.assertEqual('/dev/nvme0n2', res) + + @mock.patch.object(nvmeof.Target, '_get_nvme_devices') + def test_get_device_path_by_initial_devices_not_found(self, mock_get_devs): + """There are now new devices since we started, return None.""" + self.target.portals[0].controller = 'nvme0' + self.target.portals[1].controller = 'nvme1' + mock_get_devs.return_value = ['/dev/nvme0n1', '/dev/nvme1n2'] + self.target.devices_on_start = ['/dev/nvme0n1', '/dev/nvme1n2'] + + res = self.target.get_device_path_by_initial_devices() + + mock_get_devs.assert_called_once_with() + self.assertIsNone(res) + + @mock.patch.object(nvmeof, 'blk_property') + @mock.patch.object(nvmeof.Target, '_get_nvme_devices') + def test_get_device_path_by_initial_devices_multiple(self, mock_get_devs, + mock_property): + """There are multiple new devices, but they are the same volume.""" + self.target.portals[0].controller = 'nvme0' + self.target.portals[1].controller = 'nvme1' + mock_property.return_value = 'uuid' + mock_get_devs.return_value = ['/dev/nvme0n1', '/dev/nvme0n2', + '/dev/nvme1n1', '/dev/nvme1n2'] + self.target.devices_on_start = ['/dev/nvme0n1', '/dev/nvme1n1'] + + res = self.target.get_device_path_by_initial_devices() + + mock_get_devs.assert_called_once_with() + self.assertEqual(2, mock_property.call_count) + mock_property.assert_has_calls([mock.call('uuid', 'nvme0n2'), + mock.call('uuid', 'nvme1n2')], + any_order=True) + # The result is any of the 2 volumes, since they are the same + self.assertIn(res, ['/dev/nvme0n2', '/dev/nvme1n2']) + + @mock.patch.object(nvmeof, 'blk_property') + @mock.patch.object(nvmeof.Target, '_get_nvme_devices') + def test_get_device_path_by_initial_devices_multiple_different( + self, mock_get_devs, mock_property): + """There are multiple new devices and they are different.""" + self.target.portals[0].controller = 'nvme0' + self.target.portals[1].controller = 'nvme1' + mock_property.side_effect = ('uuid1', 'uuid2') + mock_get_devs.return_value = ['/dev/nvme0n1', '/dev/nvme0n2', + '/dev/nvme1n1', '/dev/nvme1n2'] + self.target.devices_on_start = ['/dev/nvme0n1', '/dev/nvme1n1'] + + res = self.target.get_device_path_by_initial_devices() + + mock_get_devs.assert_called_once_with() + self.assertEqual(2, mock_property.call_count) + mock_property.assert_has_calls([mock.call('uuid', 'nvme0n2'), + mock.call('uuid', 'nvme1n2')], + any_order=True) + self.assertIsNone(res) + + +@ddt.ddt +class NVMeOFConnPropsTestCase(test_base.TestCase): + @mock.patch.object(nvmeof.Target, 'factory') + def test_init_old_props(self, mock_target): + """Test init with old format connection properties.""" + conn_props = {'nqn': 'nqn_value', + 'transport_type': 'rdma', + 'target_portal': 'portal_value', + 'target_port': 'port_value', + 'volume_nguid': 'nguid', + 'ns_id': 'nsid', + 'host_nqn': 'host_nqn_value', + 'qos_specs': None, + 'access_mode': 'rw', + 'encrypted': False, + 'cacheable': True, + 'discard': True} + res = nvmeof.NVMeOFConnProps(conn_props, + mock.sentinel.find_controllers) + + self.assertFalse(res.is_replicated) + self.assertIsNone(res.qos_specs) + self.assertFalse(res.readonly) + self.assertFalse(res.encrypted) + self.assertTrue(res.cacheable) + self.assertTrue(res.discard) + self.assertIsNone(res.alias) + self.assertIsNone(res.cinder_volume_id) + + mock_target.assert_called_once_with( + source_conn_props=res, + find_controllers=mock.sentinel.find_controllers, + volume_nguid='nguid', ns_id='nsid', host_nqn='host_nqn_value', + portals=[('portal_value', 'port_value', 'rdma')], vol_uuid=None, + target_nqn='nqn_value', + # These parameters are no necessary for the Target, but for + # convenience they are accepted and ignored. + qos_specs=None, access_mode='rw', encrypted=False, cacheable=True, + discard=True) + + self.assertListEqual([mock_target.return_value], res.targets) + + @ddt.data('vol_uuid', 'ns_id', 'volume_nguid') + @mock.patch.object(nvmeof.Target, 'factory') + def test_init_new_props_unreplicated(self, id_name, mock_target): + """Test init with new format connection properties but no replicas.""" + conn_props = {'target_nqn': 'nqn_value', + id_name: 'uuid', + 'portals': [('portal1', 'port_value', 'RoCEv2'), + ('portal2', 'port_value', 'anything')], + 'qos_specs': None, + 'access_mode': 'rw', + 'encrypted': False, + 'cacheable': True, + 'discard': True} + + res = nvmeof.NVMeOFConnProps(conn_props, + mock.sentinel.find_controllers) + + self.assertFalse(res.is_replicated) + self.assertIsNone(res.qos_specs) + self.assertFalse(res.readonly) + self.assertFalse(res.encrypted) + self.assertTrue(res.cacheable) + self.assertTrue(res.discard) + self.assertIsNone(res.alias) + self.assertIsNone(res.cinder_volume_id) + + kw_id_arg = {id_name: 'uuid'} + mock_target.assert_called_once_with( + source_conn_props=res, + find_controllers=mock.sentinel.find_controllers, + target_nqn='nqn_value', + portals=[('portal1', 'port_value', 'RoCEv2'), + ('portal2', 'port_value', 'anything')], + + # These parameters are no necessary for the Target, but for + # convenience they are accepted and ignored. + qos_specs=None, access_mode='rw', encrypted=False, cacheable=True, + discard=True, + + **kw_id_arg + ) + self.assertListEqual([mock_target.return_value], res.targets) + + @mock.patch.object(nvmeof.Target, 'factory') + def test_init_new_props_replicated(self, mock_target): + """Test init with new format connection properties with replicas.""" + conn_props = { + 'vol_uuid': VOL_UUID_NO_HYPHENS, + 'alias': 'raid_alias', + 'replica_count': 2, + 'volume_replicas': [ + {'target_nqn': 'nqn1', + 'vol_uuid': VOL_UUID1, + 'portals': [['portal1', 'port_value', 'RoCEv2'], + ['portal2', 'port_value', 'anything']]}, + {'target_nqn': 'nqn2', + 'vol_uuid': VOL_UUID2, + 'portals': [['portal4', 'port_value', 'anything'], + ['portal3', 'port_value', 'RoCEv2']]} + ], + 'qos_specs': None, + 'access_mode': 'ro', + 'encrypted': True, + 'cacheable': False, + 'discard': False + } + targets = [mock.Mock(), mock.Mock()] + mock_target.side_effect = targets + res = nvmeof.NVMeOFConnProps(conn_props, + mock.sentinel.find_controllers) + + self.assertTrue(res.is_replicated) + self.assertIsNone(res.qos_specs) + self.assertTrue(res.readonly) + self.assertTrue(res.encrypted) + self.assertFalse(res.cacheable) + self.assertFalse(res.discard) + self.assertEqual('raid_alias', res.alias) + self.assertEqual(VOL_UUID, res.cinder_volume_id) + + self.assertEqual(2, mock_target.call_count) + call_1 = dict(source_conn_props=res, + find_controllers=mock.sentinel.find_controllers, + vol_uuid=VOL_UUID1, target_nqn='nqn1', + portals=[['portal1', 'port_value', 'RoCEv2'], + ['portal2', 'port_value', 'anything']]) + call_2 = dict(source_conn_props=res, + find_controllers=mock.sentinel.find_controllers, + vol_uuid=VOL_UUID2, target_nqn='nqn2', + portals=[['portal4', 'port_value', 'anything'], + ['portal3', 'port_value', 'RoCEv2']]) + mock_target.assert_has_calls([mock.call(**call_1), + mock.call(**call_2)]) + self.assertListEqual(targets, res.targets) + + @mock.patch.object(nvmeof.Target, 'factory') + def test_get_devices(self, mock_target): + """Connector get devices gets devices from all its portals.""" + conn_props = { + 'vol_uuid': VOL_UUID, + 'alias': 'raid_alias', + 'replica_count': 2, + 'volume_replicas': [ + {'target_nqn': 'nqn1', + 'vol_uuid': VOL_UUID1, + 'portals': [['portal1', 'port_value', 'RoCEv2'], + ['portal2', 'port_value', 'anything']]}, + {'target_nqn': VOL_UUID2, + 'vol_uuid': 'uuid2', + 'portals': [['portal4', 'port_value', 'anything'], + ['portal3', 'port_value', 'RoCEv2']]} + ], + } + targets = [mock.Mock(), mock.Mock()] + targets[0].get_devices.return_value = [] + targets[1].get_devices.return_value = ['/dev/nvme0n1', '/dev/nvme0n2'] + mock_target.side_effect = targets + conn_props_instance = nvmeof.NVMeOFConnProps(conn_props) + + res = conn_props_instance.get_devices(mock.sentinel.only_live) + self.assertListEqual(['/dev/nvme0n1', '/dev/nvme0n2'], res) + + @mock.patch.object(nvmeof.Target, 'factory') + def test_from_dictionary_parameter(self, mock_target): + """Decorator converts dict into connection properties instance.""" + class Connector(object): + @nvmeof.NVMeOFConnProps.from_dictionary_parameter + def connect_volume(my_self, connection_properties): + self.assertIsInstance(connection_properties, + nvmeof.NVMeOFConnProps) + return 'result' + + conn = Connector() + + conn_props = {'target_nqn': 'nqn_value', 'vol_uuid': 'uuid', + 'portals': [('portal1', 'port_value', 'RoCEv2'), + ('portal2', 'port_value', 'anything')]} + res = conn.connect_volume(conn_props) + + self.assertEqual('result', res) + + +@ddt.ddt +class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): """Test cases for NVMe initiator class.""" def setUp(self): @@ -108,6 +812,13 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): self.connector = nvmeof.NVMeOFConnector(None, execute=self.fake_execute, use_multipath=False) + self.conn_props_dict = {'target_nqn': 'nqn_value', + 'vol_uuid': VOL_UUID, + 'portals': [('portal1', 'port1', 'RoCEv2'), + ('portal2', 'port2', 'tcp'), + ('portal3', 'port3', 'rdma')]} + self.conn_props = nvmeof.NVMeOFConnProps(self.conn_props_dict) + self.patch('oslo_concurrency.lockutils.external_lock') @mock.patch.object(priv_rootwrap, 'custom_execute', autospec=True) def test_nvme_present(self, mock_execute): @@ -178,453 +889,556 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): "uuid": HOST_UUID, 'nvme_native_multipath': False} self.assertEqual(expected_props, props) - def test_get_volume_paths_unreplicated(self): - self.assertEqual(self.connector.get_volume_paths( - {'target_nqn': 'fakenqn', 'vol_uuid': 'fakeuuid', - 'portals': [('fake', 'portal', 'tcp')]}), []) + def test_get_volume_paths_device_info(self): + """Device info path has highest priority.""" + dev_path = '/dev/nvme0n1' + device_info = {'type': 'block', 'path': dev_path} + conn_props = connection_properties.copy() + conn_props['device_path'] = 'lower_priority' + conn_props = nvmeof.NVMeOFConnProps(conn_props) + res = self.connector.get_volume_paths(conn_props, device_info) + self.assertEqual([dev_path], res) - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_device_path') - def test_get_volume_paths_single(self, mock_get_device_path): - mock_get_device_path.return_value = '/dev/nvme1n1' + def test_get_volume_paths_nova_conn_props(self): + """Second highest priority is device_path nova puts in conn props.""" + dev_path = '/dev/nvme0n1' + device_info = None + conn_props = connection_properties.copy() + conn_props['device_path'] = dev_path + conn_props = nvmeof.NVMeOFConnProps(conn_props) + res = self.connector.get_volume_paths(conn_props, device_info) + self.assertEqual([dev_path], res) + + @mock.patch.object(nvmeof.NVMeOFConnector, '_is_raid_device') + @mock.patch.object(nvmeof.NVMeOFConnProps, 'get_devices') + def test_get_volume_paths_unreplicated(self, mock_get_devs, mock_is_raid): + """Search for device from unreplicated connection properties.""" + mock_get_devs.return_value = ['/dev/nvme0n1'] + conn_props = nvmeof.NVMeOFConnProps(volume_replicas[0]) + + res = self.connector.get_volume_paths(conn_props, None) + self.assertEqual(mock_get_devs.return_value, res) + mock_is_raid.assert_not_called() + mock_get_devs.assert_called_once_with() + + @mock.patch.object(nvmeof.NVMeOFConnector, '_is_raid_device') + @mock.patch.object(nvmeof.NVMeOFConnProps, 'get_devices') + def test_get_volume_paths_single_replica(self, mock_get_devs, + mock_is_raid): + """Search for device from replicated conn props with 1 replica.""" + dev_path = '/dev/nvme1n1' + mock_get_devs.return_value = [dev_path] + target_props = volume_replicas[0] connection_properties = { + 'vol_uuid': VOL_UUID, 'alias': 'fakealias', - 'volume_replicas': [volume_replicas[0]], + 'volume_replicas': [target_props], 'replica_count': 1 } - self.assertEqual(self.connector.get_volume_paths( - connection_properties), - ['/dev/nvme1n1']) - mock_get_device_path.assert_called_with( - self.connector, volume_replicas[0]['target_nqn'], - volume_replicas[0]['vol_uuid']) + conn_props = nvmeof.NVMeOFConnProps(connection_properties) + res = self.connector.get_volume_paths(conn_props, None) + self.assertEqual(['/dev/md/fakealias'], res) + mock_is_raid.assert_called_once_with(dev_path) + mock_get_devs.assert_called_once_with() + + @mock.patch.object(nvmeof.NVMeOFConnector, '_is_raid_device') + @mock.patch.object(nvmeof.NVMeOFConnProps, 'get_devices') + def test_get_volume_paths_single_replica_not_replicated( + self, mock_get_devs, mock_is_raid): + """Search for device from unreplicated conn props with 1 replica.""" + mock_is_raid.return_value = False + dev_path = '/dev/nvme1n1' + mock_get_devs.return_value = [dev_path] + target_props = volume_replicas[0] + connection_properties = { + 'vol_uuid': VOL_UUID, + 'alias': 'fakealias', + 'volume_replicas': [target_props], + 'replica_count': 1 + } + conn_props = nvmeof.NVMeOFConnProps(connection_properties) + + res = self.connector.get_volume_paths(conn_props, None) + self.assertEqual([dev_path], res) + mock_is_raid.assert_called_once_with(dev_path) + mock_get_devs.assert_called_once_with() def test_get_volume_paths_replicated(self): - self.assertEqual(self.connector.get_volume_paths( - connection_properties), - ['/dev/md/fakealias']) + """Search for device from replicated conn props with >1 replica.""" + conn_props = nvmeof.NVMeOFConnProps(connection_properties) + self.assertEqual(['/dev/md/fakealias'], + self.connector.get_volume_paths(conn_props)) - def test_get_volume_paths(self): - connection_properties = { - 'device_path': '/dev/md/fakealias' - } - self.assertEqual(self.connector.get_volume_paths( - connection_properties), - [connection_properties['device_path']]) + @mock.patch.object(nvmeof.Target, 'set_portals_controllers', mock.Mock()) + @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target') + def test_connect_volume_not_replicated(self, mock_connect_target): + """Single vol attach.""" + connection_properties = volume_replicas[0].copy() + mock_connect_target.return_value = '/dev/nvme0n1' + self.assertEqual({'type': 'block', 'path': '/dev/nvme0n1'}, + self.connector.connect_volume(connection_properties)) - @mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True) - def test__try_connect_nvme_idempotent(self, mock_execute): - cmd = [ - 'nvme', 'connect', - '-t', 'tcp', - '-n', TARGET_NQN, - '-a', 'portal', - '-s', 4420] - mock_execute.side_effect = putils.ProcessExecutionError(exit_code=70) - self.connector._try_connect_nvme(cmd) - mock_execute.assert_called_once_with(self.connector, - *cmd, - root_helper=None, - run_as_root=True) + mock_connect_target.assert_called_with(mock.ANY) + self.assertIsInstance(mock_connect_target.call_args[0][0], + nvmeof.Target) - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices') - def test__get_device_path(self, mock_nvme_devices): - mock_nvme_devices.return_value = ['/dev/nvme0n1', - '/dev/nvme1n1', - '/dev/nvme0n2'] - current_devices = ['/dev/nvme0n1', '/dev/nvme0n2'] - self.assertEqual(self.connector._get_device_path(current_devices), - '/dev/nvme1n1') - - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices') - def test__get_device_path_no_new_device(self, mock_nvme_devices): - current_devices = ['/dev/nvme0n1', '/dev/nvme0n2'] - mock_nvme_devices.return_value = current_devices - self.assertRaises(exception.VolumePathsNotFound, - self.connector._get_device_path, - current_devices) - - @mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True) - def test__get_device_path_by_nguid(self, mock_execute): - mock_execute.return_value = '/dev/nvme0n1\n', None - res = self.connector._get_device_path_by_nguid(NVME_DEVICE_NGUID) - self.assertEqual(res, '/dev/nvme0n1') - - @mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True) - def test__get_device_path_by_nguid_empty_response(self, mock_execute): - mock_execute.return_value = None, None - self.assertRaises(exception.VolumePathsNotFound, - self.connector._get_device_path_by_nguid, - NVME_DEVICE_NGUID) - - @mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True) - def test__get_device_path_by_nguid_exception(self, mock_execute): - mock_execute.side_effect = putils.ProcessExecutionError() - self.assertRaises(exception.VolumeDeviceNotFound, - self.connector._get_device_path_by_nguid, - NVME_DEVICE_NGUID) - - @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target_volume') - def test_connect_volume_single_rep( - self, mock_connect_target_volume): - connection_properties1 = { - 'target_nqn': 'fakenqn', - 'vol_uuid': 'fakeuuid', - 'volume_replicas': [volume_replicas[0]], - 'replica_count': 1 - } - mock_connect_target_volume.return_value = '/dev/nvme0n1' - self.assertEqual( - self.connector.connect_volume(connection_properties1), - {'type': 'block', 'path': '/dev/nvme0n1'}) - mock_connect_target_volume.assert_called_with( - connection_properties1['volume_replicas'][0]['target_nqn'], - connection_properties1['volume_replicas'][0]['vol_uuid'], - connection_properties1['volume_replicas'][0]['portals']) - - @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target_volume') - def test_connect_volume_unreplicated( - self, mock_connect_target_volume): - mock_connect_target_volume.return_value = '/dev/nvme0n1' - self.assertEqual( - self.connector._connect_volume_by_uuid( - { - 'target_nqn': 'fakenqn', - 'vol_uuid': 'fakeuuid', - 'portals': [('fake', 'portal', 'tcp')] - } - ), - {'type': 'block', 'path': '/dev/nvme0n1'}) - mock_connect_target_volume.assert_called_with( - 'fakenqn', 'fakeuuid', [('fake', 'portal', 'tcp')]) - - @mock.patch.object(nvmeof.NVMeOFConnector, '_handle_replicated_volume') - @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target_volume') + @mock.patch.object(nvmeof.Target, 'set_portals_controllers', mock.Mock()) + @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_volume_replicated') + @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target') def test_connect_volume_replicated( - self, mock_connect_target_volume, mock_replicated_volume): - mock_connect_target_volume.side_effect = ( - '/dev/nvme0n1', '/dev/nvme1n2', '/dev/nvme2n1') + self, mock_connect_target, mock_replicated_volume): mock_replicated_volume.return_value = '/dev/md/md1' - actual = self.connector.connect_volume(connection_properties) - mock_connect_target_volume.assert_any_call( - 'fakenqn1', 'fakeuuid1', [('10.0.0.1', 4420, 'tcp')]) - mock_connect_target_volume.assert_any_call( - 'fakenqn2', 'fakeuuid2', [('10.0.0.2', 4420, 'tcp')]) - mock_connect_target_volume.assert_any_call( - 'fakenqn3', 'fakeuuid3', [('10.0.0.3', 4420, 'tcp')]) - mock_replicated_volume.assert_called_with( - ['/dev/nvme0n1', '/dev/nvme1n2', '/dev/nvme2n1'], - connection_properties['alias'], - len(connection_properties['volume_replicas'])) - self.assertEqual(actual, {'type': 'block', 'path': '/dev/md/md1'}) + actual = self.connector.connect_volume(connection_properties) + + expected = {'type': 'block', 'path': '/dev/md/md1'} + self.assertEqual(expected, actual) + + mock_replicated_volume.assert_called_once_with(mock.ANY) + self.assertIsInstance(mock_replicated_volume.call_args[0][0], + nvmeof.NVMeOFConnProps) + mock_connect_target.assert_not_called() + + @mock.patch.object(nvmeof.Target, 'set_portals_controllers', mock.Mock()) @mock.patch.object(nvmeof.NVMeOFConnector, '_handle_replicated_volume') - @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target_volume') + @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target') def test_connect_volume_replicated_exception( - self, mock_connect_target_volume, mock_replicated_volume): - mock_connect_target_volume.side_effect = Exception() + self, mock_connect_target, mock_replicated_volume): + mock_connect_target.side_effect = Exception() self.assertRaises(exception.VolumeDeviceNotFound, self.connector.connect_volume, connection_properties) - @mock.patch.object(linuxscsi.LinuxSCSI, 'flush_device_io', autospec=True) - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices', - autospec=True) - @mock.patch('os_brick.utils._time_sleep') - def test_disconnect_volume_nova(self, mock_sleep, - mock_devices, - mock_flush): - device = '/dev/nvme0n1' - connection_properties = {'target_portal': 'portal', - 'target_port': 1, - 'nqn': 'nqn.volume_123', - 'device_path': device, - 'transport_type': 'rdma'} - mock_devices.return_value = [device] - self.connector.disconnect_volume(connection_properties, None) - - mock_flush.assert_called_once_with(mock.ANY, device) - - @mock.patch.object(linuxscsi.LinuxSCSI, 'flush_device_io', autospec=True) - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices', - autospec=True) - @mock.patch('os_brick.utils._time_sleep') - def test_disconnect_volume_cinder(self, mock_sleep, - mock_devices, - mock_flush): - device = '/dev/nvme0n1' - connection_properties = {'target_portal': 'portal', - 'target_port': 1, - 'nqn': 'nqn.volume_123', - 'transport_type': 'rdma'} - device_info = {'path': device} - mock_devices.return_value = [device] - - self.connector.disconnect_volume(connection_properties, - device_info, - ignore_errors=True) - - mock_flush.assert_called_once_with(mock.ANY, device) - - @mock.patch.object(linuxscsi.LinuxSCSI, 'flush_device_io', autospec=True) - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices', - autospec=True) - @mock.patch('os_brick.utils._time_sleep') - def test_disconnect_volume_force_ignore_errors(self, mock_sleep, - mock_devices, - mock_flush): - device = '/dev/nvme0n1' - mock_flush.side_effect = putils.ProcessExecutionError - mock_devices.return_value = [device] - connection_properties = {'target_portal': 'portal', - 'target_port': 1, - 'nqn': 'nqn.volume_123', - 'device_path': device, - 'transport_type': 'rdma'} - + @mock.patch.object(nvmeof.NVMeOFConnector, 'get_volume_paths') + @mock.patch('os.path.exists', return_value=True) + def test_disconnect_volume_path_not_found( + self, mock_exists, mock_get_paths): + """Disconnect can't find device path from conn props and dev info.""" + mock_get_paths.return_value = [] res = self.connector.disconnect_volume(connection_properties, - None, - force=True, - ignore_errors=True) + mock.sentinel.device_info) self.assertIsNone(res) - mock_flush.assert_called_once_with(mock.ANY, device) + mock_get_paths.assert_called_once_with(mock.ANY, + mock.sentinel.device_info) + self.assertIsInstance(mock_get_paths.call_args[0][0], + nvmeof.NVMeOFConnProps) + mock_exists.assert_not_called() - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_fs_type') - def test_disconnect_unreplicated_volume_nova(self, mock_get_fs_type): - connection_properties = { - 'vol_uuid': 'fakeuuid', - 'portals': [('10.0.0.1', 4420, 'tcp')], - 'target_nqn': 'fakenqn', - 'device_path': '/dev/nvme0n1' - } - mock_get_fs_type.return_value = 'linux_raid_member' - self.connector._disconnect_volume_replicated( - connection_properties, None) - mock_get_fs_type.assert_called_with( - connection_properties['device_path']) + @mock.patch.object(nvmeof.NVMeOFConnector, 'get_volume_paths') + @mock.patch('os.path.exists', return_value=True) + def test_disconnect_volume_path_doesnt_exist( + self, mock_exists, mock_get_paths): + """Disconnect path doesn't exist""" + dev_path = '/dev/nvme0n1' + mock_get_paths.return_value = [dev_path] + mock_exists.return_value = False + res = self.connector.disconnect_volume(connection_properties, + mock.sentinel.device_info) + self.assertIsNone(res) + mock_get_paths.assert_called_once_with(mock.ANY, + mock.sentinel.device_info) + self.assertIsInstance(mock_get_paths.call_args[0][0], + nvmeof.NVMeOFConnProps) + mock_exists.assert_called_once_with(dev_path) + @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.flush_device_io') + @mock.patch.object(nvmeof.NVMeOFConnector, 'get_volume_paths') @mock.patch.object(nvmeof.NVMeOFConnector, 'end_raid') - def test_disconnect_replicated_volume_no_device_path(self, mock_end_raid): - - mock_end_raid.return_value = None - self.connector.disconnect_volume(connection_properties, None) - device_path = '/dev/md/' + connection_properties['alias'] - mock_end_raid.assert_called_with(self.connector, device_path) - - @mock.patch.object(nvmeof.NVMeOFConnector, 'end_raid') - def test_disconnect_replicated_volume_nova(self, mock_end_raid): - connection_properties = { - 'vol_uuid': 'fakeuuid', - 'volume_replicas': volume_replicas, - 'replica_count': 3, - 'device_path': '/dev/md/md1' - } - self.connector.disconnect_volume(connection_properties, None) - mock_end_raid.assert_called_with(self.connector, '/dev/md/md1') - - def test_disconnect_unreplicated_volume_cinder(self): - connection_properties = { - 'vol_uuid': 'fakeuuid', - 'portals': [('10.0.0.1', 4420, 'tcp')], - 'target_nqn': 'fakenqn', - } - device_info = {'path': '/dev/nvme0n1'} - self.connector._disconnect_volume_replicated( - connection_properties, device_info, ignore_errors=True) - - @mock.patch.object(nvmeof.NVMeOFConnector, 'end_raid') - def test_disconnect_replicated_volume_cinder(self, mock_end_raid): - device_info = {'path': '/dev/md/md1'} + @mock.patch('os.path.exists', return_value=True) + def test_disconnect_volume_unreplicated( + self, mock_exists, mock_end_raid, mock_get_paths, mock_flush): + """Disconnect a single device.""" + dev_path = '/dev/nvme0n1' + mock_get_paths.return_value = [dev_path] self.connector.disconnect_volume(connection_properties, - device_info, + mock.sentinel.device_info, ignore_errors=True) - mock_end_raid.assert_called_with(self.connector, '/dev/md/md1') + mock_get_paths.assert_called_once_with(mock.ANY, + mock.sentinel.device_info) + self.assertIsInstance(mock_get_paths.call_args[0][0], + nvmeof.NVMeOFConnProps) + mock_exists.assert_called_once_with(dev_path) + mock_end_raid.assert_not_called() + mock_flush.assert_called_with(dev_path) - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_device_path') + @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.flush_device_io') + @mock.patch.object(nvmeof.NVMeOFConnector, 'get_volume_paths') + @mock.patch.object(nvmeof.NVMeOFConnector, 'end_raid') + @mock.patch('os.path.exists', return_value=True) + def test_disconnect_volume_replicated( + self, mock_exists, mock_end_raid, mock_get_paths, mock_flush): + """Disconnect a raid.""" + raid_path = '/dev/md/md1' + mock_get_paths.return_value = [raid_path] + self.connector.disconnect_volume(connection_properties, + mock.sentinel.device_info, + ignore_errors=True) + mock_get_paths.assert_called_once_with(mock.ANY, + mock.sentinel.device_info) + self.assertIsInstance(mock_get_paths.call_args[0][0], + nvmeof.NVMeOFConnProps) + mock_exists.assert_called_once_with(raid_path) + mock_end_raid.assert_called_with(raid_path) + mock_flush.assert_not_called() + + def test__get_sizes_from_lba(self): + """Get nsze and new size using nvme LBA information.""" + nsze = 6291456 + ns_data = {"nsze": nsze, "ncap": nsze, "nuse": nsze, + "lbafs": [{"ms": 0, "ds": 9, "rp": 0}]} + res_nsze, res_size = self.connector._get_sizes_from_lba(ns_data) + self.assertEqual(nsze, res_nsze) + self.assertEqual(nsze * 1 << 9, res_size) + + @ddt.data([{"ms": 0, "ds": 6, "rp": 0}], + [{"ms": 0, "ds": 9, "rp": 0}, {"ms": 0, "ds": 9, "rp": 0}]) + def test__get_sizes_from_lba_error(self, lbafs): + """Incorrect data returned in LBA information.""" + nsze = 6291456 + ns_data = {"nsze": nsze, "ncap": nsze, "nuse": nsze, "lbafs": lbafs} + res_nsze, res_size = self.connector._get_sizes_from_lba(ns_data) + self.assertIsNone(res_nsze) + self.assertIsNone(res_size) + + @mock.patch.object(nvmeof, 'blk_property') + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_sizes_from_lba') + @mock.patch.object(nvmeof.NVMeOFConnector, '_execute') + @mock.patch.object(nvmeof.NVMeOFConnector, 'get_volume_paths') @mock.patch('os_brick.utils.get_device_size') - def test_extend_volume_unreplicated( - self, mock_device_size, mock_device_path): - connection_properties = { - 'target_nqn': 'fakenqn', - 'vol_uuid': 'fakeuuid', - 'volume_replicas': [volume_replicas[0]], - 'replica_count': 1 - } - mock_device_path.return_value = '/dev/nvme0n1' + def test_extend_volume_unreplicated(self, mock_device_size, mock_paths, + mock_exec, mock_lba, mock_property): + """Uses nvme to get expected size and waits until sysfs shows it.""" + new_size = 3221225472 + new_nsze = int(new_size / 512) # nsze is size / block-size + old_nsze = int(new_nsze / 2) + dev_path = '/dev/nvme0n1' + + mock_paths.return_value = [dev_path] + stdout = '{"data": "jsondata"}' + mock_exec.return_value = (stdout, '') + mock_lba.return_value = (new_nsze, new_size) + # Simulate a delay before the new value is present in sysfs + mock_property.side_effect = (str(old_nsze), str(new_nsze)) + + self.assertEqual(new_size, + self.connector.extend_volume(connection_properties)) + + mock_paths.assert_called_with(mock.ANY) + self.assertIsInstance(mock_paths.call_args[0][0], + nvmeof.NVMeOFConnProps) + mock_exec.assert_called_once_with( + 'nvme', 'id-ns', '-ojson', dev_path, + run_as_root=True, root_helper=self.connector._root_helper) + mock_lba.assert_called_once_with({"data": "jsondata"}) + self.assertEqual(2, mock_property.call_count) + mock_property.assert_has_calls([mock.call('size', 'nvme0n1'), + mock.call('size', 'nvme0n1')]) + mock_device_size.assert_not_called() + + @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') + @mock.patch.object(nvmeof, 'blk_property') + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_sizes_from_lba') + @mock.patch.object(nvmeof.NVMeOFConnector, '_execute') + @mock.patch.object(nvmeof.NVMeOFConnector, 'get_volume_paths') + @mock.patch('os_brick.utils.get_device_size') + def test_extend_volume_unreplicated_nvme_fails( + self, mock_device_size, mock_paths, mock_exec, mock_lba, + mock_property, mock_rescan): + """nvme command fails, so it rescans, waits, and reads size.""" + dev_path = '/dev/nvme0n1' mock_device_size.return_value = 100 - self.assertEqual( - self.connector.extend_volume(connection_properties), - 100) - mock_device_path.assert_called_with( - self.connector, volume_replicas[0]['target_nqn'], - volume_replicas[0]['vol_uuid']) - mock_device_size.assert_called_with(self.connector, '/dev/nvme0n1') - - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_device_path') - @mock.patch('os_brick.utils.get_device_size') - def test_extend_volume_unreplicated_no_replica( - self, mock_device_size, mock_device_path): - connection_properties = { - 'target_nqn': 'fakenqn', - 'vol_uuid': 'fakeuuid' - } - mock_device_path.return_value = '/dev/nvme0n1' - mock_device_size.return_value = 100 - self.assertEqual( - self.connector._extend_volume_replicated( - connection_properties), 100) - mock_device_path.assert_called_with( - self.connector, 'fakenqn', 'fakeuuid') + mock_paths.return_value = [dev_path] + mock_exec.side_effect = putils.ProcessExecutionError() + + self.assertEqual(100, + self.connector.extend_volume(connection_properties)) + + mock_paths.assert_called_with(mock.ANY) + self.assertIsInstance(mock_paths.call_args[0][0], + nvmeof.NVMeOFConnProps) + mock_exec.assert_called_once_with( + 'nvme', 'id-ns', '-ojson', dev_path, + run_as_root=True, root_helper=self.connector._root_helper) + mock_lba.assert_not_called() + mock_property.assert_not_called() + mock_rescan.assert_called_once_with('nvme0') mock_device_size.assert_called_with(self.connector, '/dev/nvme0n1') + @mock.patch.object(nvmeof.NVMeOFConnector, 'get_volume_paths') @mock.patch.object(nvmeof.NVMeOFConnector, 'run_mdadm') @mock.patch('os_brick.utils.get_device_size') def test_extend_volume_replicated( - self, mock_device_size, mock_mdadm): - mock_device_size.return_value = 100 - self.assertEqual( - self.connector.extend_volume(connection_properties), - 100) + self, mock_device_size, mock_mdadm, mock_paths): device_path = '/dev/md/' + connection_properties['alias'] - mock_mdadm.assert_called_with( - self.connector, ['mdadm', '--grow', '--size', 'max', device_path]) - mock_device_size.assert_called_with(self.connector, device_path) - - @mock.patch('os_brick.utils.get_device_size') - def test_extend_volume_with_nguid(self, mock_device_size): - device_path = '/dev/nvme0n1' - connection_properties = { - 'volume_nguid': NVME_DEVICE_NGUID, - 'device_path': device_path, - } + mock_paths.return_value = [device_path] mock_device_size.return_value = 100 self.assertEqual( - self.connector.extend_volume(connection_properties), - 100 - ) + 100, + self.connector.extend_volume(connection_properties)) + mock_paths.assert_called_once_with(mock.ANY) + self.assertIsInstance(mock_paths.call_args[0][0], + nvmeof.NVMeOFConnProps) + mock_mdadm.assert_called_with( + ('mdadm', '--grow', '--size', 'max', device_path)) mock_device_size.assert_called_with(self.connector, device_path) - @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_device_path') - def test__connect_target_volume_with_connected_device( - self, mock_device_path, mock_rescan): - mock_device_path.return_value = '/dev/nvme0n1' - self.assertEqual( - self.connector._connect_target_volume( - 'fakenqn', 'fakeuuid', [('fake', 'portal', 'tcp')]), - '/dev/nvme0n1') - mock_rescan.assert_called_with(self.connector, 'fakenqn') - mock_device_path.assert_called_with( - self.connector, 'fakenqn', 'fakeuuid', list({}.values())) - - @mock.patch.object(nvmeof.NVMeOFConnector, 'connect_to_portals') - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_device_path') - def test__connect_target_volume_not_connected( - self, mock_device_path, mock_portals): - mock_device_path.side_effect = exception.VolumeDeviceNotFound() - mock_portals.return_value = False - self.assertRaises(exception.VolumeDeviceNotFound, - self.connector._connect_target_volume, TARGET_NQN, - VOL_UUID, [('fake', 'portal', 'tcp')]) - - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_controllers') - @mock.patch.object(nvmeof.NVMeOFConnector, 'connect_to_portals') - def test__connect_target_volume_no_portals_con( - self, mock_portals, mock_controller): - mock_controller.side_effect = exception.VolumeDeviceNotFound() - mock_portals.return_value = None - self.assertRaises(exception.VolumeDeviceNotFound, - self.connector._connect_target_volume, 'fakenqn', - 'fakeuuid', [fake_portal]) - - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_device_path') - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_live_nvme_controllers_map') - @mock.patch.object(nvmeof.NVMeOFConnector, 'connect_to_portals') - @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') - def test__connect_target_volume_new_device_path( - self, mock_rescan, mock_connect_portal, - mock_get_live_nvme_controllers_map, mock_device_path): - mock_device_path.return_value = '/dev/nvme0n1' - mock_rescan.return_value = {} - mock_connect_portal.return_value = True - mock_get_live_nvme_controllers_map.return_value = fake_controllers_map - self.assertEqual( - self.connector._connect_target_volume( - 'fakenqn', 'fakeuuid', [('fake', 'portal', 'tcp')]), - '/dev/nvme0n1') - mock_rescan.assert_called_with(self.connector, 'fakenqn') - mock_connect_portal.assert_called_with( - self.connector, 'fakenqn', [('fake', 'portal', 'tcp')], {}) - mock_get_live_nvme_controllers_map.assert_called_with(self.connector, - 'fakenqn') - fake_controllers_map_values = fake_controllers_map.values() - mock_device_path.assert_called_with( - self.connector, 'fakenqn', 'fakeuuid', - list(fake_controllers_map_values)) - + @mock.patch.object(nvmeof.Target, 'find_device') + @mock.patch.object(nvmeof.Target, 'set_portals_controllers') @mock.patch.object(nvmeof.NVMeOFConnector, 'run_nvme_cli') - def test_connect_to_portals(self, mock_nvme_cli): - nvme_command = ( - 'connect', '-a', '10.0.0.1', '-s', 4420, '-t', - 'tcp', '-n', 'fakenqn', '-Q', '128', '-l', '-1') - self.assertEqual( - self.connector.connect_to_portals( - self.connector, 'fakenqn', [('10.0.0.1', 4420, 'tcp')], {}), - True) - mock_nvme_cli.assert_called_with(self.connector, nvme_command) + @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') + @mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock) + def test__connect_target_with_connected_device( + self, mock_state, mock_rescan, mock_cli, mock_set_ctrls, + mock_find_dev): + """Test connect target when there's a connection to the subsystem.""" + self.conn_props.targets[0].portals[-1].controller = 'nvme0' + mock_state.side_effect = ('connecting', None, 'live') + dev_path = '/dev/nvme0n1' + mock_find_dev.return_value = dev_path + res = self.connector._connect_target(self.conn_props.targets[0]) + self.assertEqual(dev_path, res) + + self.assertEqual(3, mock_state.call_count) + mock_state.assert_has_calls(3 * [mock.call()]) + mock_rescan.assert_called_once_with('nvme0') + mock_set_ctrls.assert_called_once_with() + mock_find_dev.assert_called_once_with() + mock_cli.assert_not_called() + + @ddt.data(True, False) + @mock.patch.object(nvmeof.Target, 'find_device') + @mock.patch.object(nvmeof.Target, 'set_portals_controllers') + @mock.patch.object(nvmeof.NVMeOFConnector, '_do_multipath') @mock.patch.object(nvmeof.NVMeOFConnector, 'run_nvme_cli') - def test_connect_to_portals_rdma_no_conn(self, mock_nvme_cli): - mock_nvme_cli.side_effect = Exception() - nvme_command = ( - 'connect', '-a', '10.0.0.1', '-s', 4420, '-t', - 'rdma', '-n', 'fakenqn', '-Q', '128', '-l', '-1') - self.assertEqual( - self.connector.connect_to_portals( - self.connector, 'fakenqn', [('10.0.0.1', 4420, 'RoCEv2')], {}), - False) - mock_nvme_cli.assert_called_with(self.connector, nvme_command) + @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') + @mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock) + def test__connect_target_not_found(self, do_multipath, mock_state, + mock_rescan, mock_cli, doing_multipath, + mock_set_ctrls, mock_find_dev): + """Test connect target fails to find device after connecting.""" + self.conn_props.targets[0].portals[-1].controller = 'nvme0' + doing_multipath.return_value = do_multipath + retries = 3 + mock_state.side_effect = retries * ['connecting', None, 'live'] + + mock_find_dev.side_effect = exception.VolumeDeviceNotFound() + + self.assertRaises(exception.VolumeDeviceNotFound, + self.connector._connect_target, + self.conn_props.targets[0]) + + self.assertEqual(retries * 3, mock_state.call_count) + mock_state.assert_has_calls(retries * 3 * [mock.call()]) + self.assertEqual(retries, mock_rescan.call_count) + mock_rescan.assert_has_calls(retries * [mock.call('nvme0')]) + + self.assertEqual(retries, mock_set_ctrls.call_count) + mock_set_ctrls.assert_has_calls(retries * [mock.call()]) + self.assertEqual(retries, mock_find_dev.call_count) + mock_find_dev.assert_has_calls(retries * [mock.call()]) + if do_multipath: + self.assertEqual(retries, mock_cli.call_count) + mock_cli.assert_has_calls( + retries * [mock.call(['connect', '-a', 'portal2', '-s', + 'port2', '-t', 'tcp', '-n', 'nqn_value', + '-Q', '128', '-l', '-1'])]) + else: + mock_cli.assert_not_called() + + @mock.patch.object(nvmeof.Target, 'find_device') + @mock.patch.object(nvmeof.Target, 'set_portals_controllers') + @mock.patch.object(nvmeof.NVMeOFConnector, 'run_nvme_cli') + @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') + @mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock) + def test__connect_target_portals_down( + self, mock_state, mock_rescan, mock_cli, mock_set_ctrls, + mock_find_dev): + """Test connect target has all portal connections down.""" + retries = 3 + mock_state.side_effect = retries * 3 * ['connecting'] + + self.assertRaises(exception.VolumeDeviceNotFound, + self.connector._connect_target, + self.conn_props.targets[0]) + + self.assertEqual(retries * 3, mock_state.call_count) + mock_state.assert_has_calls(retries * 3 * [mock.call()]) + mock_rescan.assert_not_called() + mock_set_ctrls.assert_not_called() + mock_find_dev.assert_not_called() + mock_cli.assert_not_called() + + @mock.patch.object(nvmeof.LOG, 'error') + @mock.patch.object(nvmeof.Target, 'find_device') + @mock.patch.object(nvmeof.Target, 'set_portals_controllers') + @mock.patch.object(nvmeof.NVMeOFConnector, 'run_nvme_cli') + @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') + @mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock) + def test__connect_target_no_portals_connect( + self, mock_state, mock_rescan, mock_cli, mock_set_ctrls, + mock_find_dev, mock_log): + """Test connect target when fails to connect to any portal.""" + retries = 3 + mock_state.side_effect = retries * ['connecting', 'connecting', None] + mock_cli.side_effect = putils.ProcessExecutionError() + + target = self.conn_props.targets[0] + + self.assertRaises(exception.VolumeDeviceNotFound, + self.connector._connect_target, target) + + self.assertEqual(retries, mock_log.call_count) + self.assertEqual(retries * 3, mock_state.call_count) + mock_state.assert_has_calls(retries * 3 * [mock.call()]) + mock_rescan.assert_not_called() + mock_set_ctrls.assert_not_called() + mock_find_dev.assert_not_called() + self.assertEqual(3, mock_cli.call_count) + portal = target.portals[-1] + mock_cli.assert_has_calls( + retries * [mock.call(['connect', '-a', portal.address, + '-s', portal.port, '-t', portal.transport, + '-n', target.nqn, '-Q', '128', '-l', '-1'])]) + + @mock.patch.object(nvmeof.Target, 'find_device') + @mock.patch.object(nvmeof.Target, 'set_portals_controllers') + @mock.patch.object(nvmeof.NVMeOFConnector, 'run_nvme_cli') + @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') + @mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock) + def test__connect_target_new_device_path( + self, mock_state, mock_rescan, mock_cli, mock_set_ctrls, + mock_find_dev): + """Test connect when we do a new connection and find the device.""" + mock_state.side_effect = ['connecting', 'connecting', None] + dev_path = '/dev/nvme0n1' + mock_find_dev.return_value = dev_path + + target = self.conn_props.targets[0] + target.host_nqn = 'host_nqn' + + res = self.connector._connect_target(target) + self.assertEqual(dev_path, res) + + self.assertEqual(3, mock_state.call_count) + mock_state.assert_has_calls(3 * [mock.call()]) + mock_rescan.assert_not_called() + mock_set_ctrls.assert_called_once_with() + mock_find_dev.assert_called_once_with() + + portal = target.portals[-1] + mock_cli.called_once_with(['connect', '-a', portal.address, + '-s', portal.port, '-t', portal.transport, + '-n', target.nqn, '-Q', '128', '-l', '-1', + '-q', 'host_nqn']) + + @mock.patch.object(nvmeof.NVMeOFConnector, '_do_multipath', + mock.Mock(return_value=True)) + @mock.patch.object(nvmeof.Target, 'find_device') + @mock.patch.object(nvmeof.Target, 'set_portals_controllers') + @mock.patch.object(nvmeof.NVMeOFConnector, 'run_nvme_cli') + @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') + @mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock) + def test__connect_target_multipath( + self, mock_state, mock_rescan, mock_cli, mock_set_ctrls, + mock_find_dev): + """Test connect when we do a new connection and find the device.""" + target = self.conn_props.targets[0] + + mock_state.side_effect = [None, None, None] + dev_path = '/dev/nvme0n1' + mock_find_dev.return_value = dev_path + + res = self.connector._connect_target(target) + self.assertEqual(dev_path, res) + + self.assertEqual(3, mock_state.call_count) + mock_state.assert_has_calls(3 * [mock.call()]) + mock_rescan.assert_not_called() + mock_set_ctrls.assert_called_once_with() + mock_find_dev.assert_called_once_with() + + self.assertEqual(len(target.portals), mock_cli.call_count) + mock_cli.assert_has_calls( + [mock.call(['connect', '-a', portal.address, + '-s', portal.port, '-t', portal.transport, + '-n', target.nqn, '-Q', '128', '-l', '-1']) + for portal in target.portals]) + + @ddt.data(70, errno.EALREADY) + @mock.patch.object(nvmeof.LOG, 'warning') + @mock.patch.object(nvmeof.Target, 'find_device') + @mock.patch.object(nvmeof.Target, 'set_portals_controllers') + @mock.patch.object(nvmeof.NVMeOFConnector, 'run_nvme_cli') + @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') + @mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock) + def test__connect_target_race( + self, exit_code, mock_state, mock_rescan, mock_cli, + mock_set_ctrls, mock_find_dev, mock_log): + """Treat race condition with sysadmin as success.""" + mock_state.side_effect = ['connecting', 'connecting', None] + dev_path = '/dev/nvme0n1' + mock_find_dev.return_value = dev_path + mock_cli.side_effect = putils.ProcessExecutionError( + exit_code=exit_code) + + target = self.conn_props.targets[0] + + res = self.connector._connect_target(target) + self.assertEqual(dev_path, res) + + self.assertEqual(3, mock_state.call_count) + mock_state.assert_has_calls(3 * [mock.call()]) + mock_rescan.assert_not_called() + mock_set_ctrls.assert_called_once_with() + mock_find_dev.assert_called_once_with() + + portal = target.portals[-1] + mock_cli.called_once_with(['connect', '-a', portal.address, + '-s', portal.port, '-t', portal.transport, + '-n', target.nqn, '-Q', '128', '-l', '-1']) + self.assertEqual(1, mock_log.call_count) @mock.patch.object(nvmeof.NVMeOFConnector, 'stop_and_assemble_raid') @mock.patch.object(nvmeof.NVMeOFConnector, '_is_device_in_raid') def test_handle_replicated_volume_existing( self, mock_device_raid, mock_stop_assemble_raid): mock_device_raid.return_value = True - self.assertEqual( - self.connector._handle_replicated_volume( - ['/dev/nvme1n1', '/dev/nvme1n2', '/dev/nvme1n3'], - 'fakealias', 3), - '/dev/md/fakealias') - mock_device_raid.assert_called_with(self.connector, '/dev/nvme1n1') + conn_props = nvmeof.NVMeOFConnProps(connection_properties) + result = self.connector._handle_replicated_volume( + ['/dev/nvme1n1', '/dev/nvme1n2', '/dev/nvme1n3'], conn_props) + self.assertEqual('/dev/md/fakealias', result) + mock_device_raid.assert_called_with('/dev/nvme1n1') mock_stop_assemble_raid.assert_called_with( - self.connector, ['/dev/nvme1n1', '/dev/nvme1n2', '/dev/nvme1n3'], + ['/dev/nvme1n1', '/dev/nvme1n2', '/dev/nvme1n3'], '/dev/md/fakealias', False) @mock.patch.object(nvmeof.NVMeOFConnector, '_is_device_in_raid') - def test_handle_replicated_volume_not_found( - self, mock_device_raid): + def test_handle_replicated_volume_not_found(self, mock_device_raid): mock_device_raid.return_value = False + conn_props = nvmeof.NVMeOFConnProps(connection_properties) + conn_props.replica_count = 4 self.assertRaises(exception.VolumeDeviceNotFound, self.connector._handle_replicated_volume, ['/dev/nvme1n1', '/dev/nvme1n2', '/dev/nvme1n3'], - 'fakealias', 4) - mock_device_raid.assert_any_call(self.connector, '/dev/nvme1n1') - mock_device_raid.assert_any_call(self.connector, '/dev/nvme1n2') - mock_device_raid.assert_any_call(self.connector, '/dev/nvme1n3') + conn_props) + mock_device_raid.assert_any_call('/dev/nvme1n1') + mock_device_raid.assert_any_call('/dev/nvme1n2') + mock_device_raid.assert_any_call('/dev/nvme1n3') @mock.patch.object(nvmeof.NVMeOFConnector, 'create_raid') @mock.patch.object(nvmeof.NVMeOFConnector, '_is_device_in_raid') def test_handle_replicated_volume_new( self, mock_device_raid, mock_create_raid): + conn_props = nvmeof.NVMeOFConnProps(connection_properties) mock_device_raid.return_value = False - self.assertEqual( - self.connector._handle_replicated_volume( - ['/dev/nvme1n1', '/dev/nvme1n2', '/dev/nvme1n3'], - 'fakealias', 3), - '/dev/md/fakealias') - mock_device_raid.assert_any_call(self.connector, '/dev/nvme1n1') - mock_device_raid.assert_any_call(self.connector, '/dev/nvme1n2') - mock_device_raid.assert_any_call(self.connector, '/dev/nvme1n3') + res = self.connector._handle_replicated_volume( + ['/dev/nvme1n1', '/dev/nvme1n2', '/dev/nvme1n3'], conn_props) + self.assertEqual('/dev/md/fakealias', res) + mock_device_raid.assert_any_call('/dev/nvme1n1') + mock_device_raid.assert_any_call('/dev/nvme1n2') + mock_device_raid.assert_any_call('/dev/nvme1n3') mock_create_raid.assert_called_with( - self.connector, ['/dev/nvme1n1', '/dev/nvme1n2', '/dev/nvme1n3'], + ['/dev/nvme1n1', '/dev/nvme1n2', '/dev/nvme1n3'], '1', 'fakealias', 'fakealias', False) @mock.patch.object(nvmeof.NVMeOFConnector, 'ks_readlink') @@ -634,8 +1448,8 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): mock_readlink.return_value = '' mock_md_name.return_value = 'mdalias' self.assertIsNone(self.connector.stop_and_assemble_raid( - self.connector, ['/dev/sda'], '/dev/md/mdalias', False)) - mock_md_name.assert_called_with(self.connector, 'sda') + ['/dev/sda'], '/dev/md/mdalias', False)) + mock_md_name.assert_called_with('sda') mock_readlink.assert_called_with('/dev/md/mdalias') @mock.patch.object(nvmeof.NVMeOFConnector, 'ks_readlink') @@ -645,8 +1459,8 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): mock_readlink.return_value = '/dev/md/mdalias' mock_md_name.return_value = 'mdalias' self.assertIsNone(self.connector.stop_and_assemble_raid( - self.connector, ['/dev/sda'], '/dev/md/mdalias', False)) - mock_md_name.assert_called_with(self.connector, 'sda') + ['/dev/sda'], '/dev/md/mdalias', False)) + mock_md_name.assert_called_with('sda') mock_readlink.assert_called_with('/dev/md/mdalias') @mock.patch.object(nvmeof.NVMeOFConnector, 'assemble_raid') @@ -658,18 +1472,17 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): mock_md_name.return_value = 'dummy' mock_assemble.side_effect = Exception() self.assertIsNone(self.connector.stop_and_assemble_raid( - self.connector, ['/dev/sda'], '/dev/md/mdalias', False)) - mock_md_name.assert_called_with(self.connector, 'sda') + ['/dev/sda'], '/dev/md/mdalias', False)) + mock_md_name.assert_called_with('sda') mock_readlink.assert_called_with('/dev/md/mdalias') - mock_assemble.assert_called_with(self.connector, ['/dev/sda'], - '/dev/md/mdalias', False) + mock_assemble.assert_called_with( + ['/dev/sda'], '/dev/md/mdalias', False) @mock.patch.object(nvmeof.NVMeOFConnector, 'run_mdadm') def test_assemble_raid_simple(self, mock_run_mdadm): self.assertEqual(self.connector.assemble_raid( - self.connector, ['/dev/sda'], '/dev/md/md1', True), True) + ['/dev/sda'], '/dev/md/md1', True), True) mock_run_mdadm.assert_called_with( - self.connector, ['mdadm', '--assemble', '--run', '/dev/md/md1', '-o', '/dev/sda'], True) @@ -677,10 +1490,9 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): def test_assemble_raid_simple_err(self, mock_run_mdadm): mock_run_mdadm.side_effect = putils.ProcessExecutionError() self.assertRaises(putils.ProcessExecutionError, - self.connector.assemble_raid, self.connector, + self.connector.assemble_raid, ['/dev/sda'], '/dev/md/md1', True) mock_run_mdadm.assert_called_with( - self.connector, ['mdadm', '--assemble', '--run', '/dev/md/md1', '-o', '/dev/sda'], True) @@ -689,9 +1501,8 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): def test_create_raid_cmd_simple(self, mock_run_mdadm, mock_os): mock_os.return_value = True self.assertIsNone(self.connector.create_raid( - self.connector, ['/dev/sda'], '1', 'md1', 'name', True)) + ['/dev/sda'], '1', 'md1', 'name', True)) mock_run_mdadm.assert_called_with( - self.connector, ['mdadm', '-C', '-o', 'md1', '-R', '-N', 'name', '--level', '1', '--raid-devices=1', '--bitmap=internal', '--homehost=any', '--failfast', '--assume-clean', '/dev/sda']) @@ -702,10 +1513,9 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): def test_end_raid_simple(self, mock_raid_exists, mock_stop_raid): mock_raid_exists.return_value = True mock_stop_raid.return_value = False - self.assertIsNone(self.connector.end_raid( - self.connector, '/dev/md/md1')) - mock_raid_exists.assert_called_with(self.connector, '/dev/md/md1') - mock_stop_raid.assert_called_with(self.connector, '/dev/md/md1', True) + self.assertIsNone(self.connector.end_raid('/dev/md/md1')) + mock_raid_exists.assert_called_with('/dev/md/md1') + mock_stop_raid.assert_called_with('/dev/md/md1', True) @mock.patch.object(os.path, 'exists') @mock.patch.object(nvmeof.NVMeOFConnector, 'stop_raid') @@ -714,10 +1524,9 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): mock_raid_exists.return_value = True mock_stop_raid.return_value = False mock_os.return_value = True - self.assertIsNone(self.connector.end_raid( - self.connector, '/dev/md/md1')) - mock_raid_exists.assert_called_with(self.connector, '/dev/md/md1') - mock_stop_raid.assert_called_with(self.connector, '/dev/md/md1', True) + self.assertIsNone(self.connector.end_raid('/dev/md/md1')) + mock_raid_exists.assert_called_with('/dev/md/md1') + mock_stop_raid.assert_called_with('/dev/md/md1', True) mock_os.assert_called_with('/dev/md/md1') @mock.patch.object(os.path, 'exists') @@ -727,57 +1536,46 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): mock_raid_exists.return_value = True mock_stop_raid.side_effect = Exception() mock_os.return_value = True - self.assertIsNone(self.connector.end_raid( - self.connector, '/dev/md/md1')) - mock_raid_exists.assert_called_with(self.connector, '/dev/md/md1') - mock_stop_raid.assert_called_with(self.connector, '/dev/md/md1', True) + self.assertIsNone(self.connector.end_raid('/dev/md/md1')) + mock_raid_exists.assert_called_with('/dev/md/md1') + mock_stop_raid.assert_called_with('/dev/md/md1', True) mock_os.assert_called_with('/dev/md/md1') @mock.patch.object(nvmeof.NVMeOFConnector, 'run_mdadm') def test_stop_raid_simple(self, mock_run_mdadm): mock_run_mdadm.return_value = 'mdadm output' - self.assertEqual(self.connector.stop_raid( - self.connector, '/dev/md/md1', True), 'mdadm output') - mock_run_mdadm.assert_called_with( - self.connector, ['mdadm', '--stop', '/dev/md/md1'], True) + self.assertEqual(self.connector.stop_raid('/dev/md/md1', True), + 'mdadm output') + mock_run_mdadm.assert_called_with(['mdadm', '--stop', '/dev/md/md1'], + True) @mock.patch.object(nvmeof.NVMeOFConnector, 'run_mdadm') def test_remove_raid_simple(self, mock_run_mdadm): - self.assertIsNone(self.connector.remove_raid( - self.connector, '/dev/md/md1')) - mock_run_mdadm.assert_called_with( - self.connector, ['mdadm', '--remove', '/dev/md/md1']) + self.assertIsNone(self.connector.remove_raid('/dev/md/md1')) + mock_run_mdadm.assert_called_with(['mdadm', '--remove', '/dev/md/md1']) @mock.patch.object(nvmeof.NVMeOFConnector, 'run_nvme_cli') - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_live_nvme_controllers_map') - def test_rescan(self, mock_get_live_nvme_controllers_map, - mock_run_nvme_cli): - mock_get_live_nvme_controllers_map.return_value = fake_controllers_map + def test_rescan(self, mock_run_nvme_cli): + """Test successful nvme rescan.""" mock_run_nvme_cli.return_value = None - result = self.connector.rescan(EXECUTOR, TARGET_NQN) - self.assertEqual(fake_controllers_map, result) - mock_get_live_nvme_controllers_map.assert_called_with(EXECUTOR, - TARGET_NQN) + result = self.connector.rescan('nvme1') + self.assertIsNone(result) nvme_command = ('ns-rescan', NVME_DEVICE_PATH) - mock_run_nvme_cli.assert_called_with(EXECUTOR, nvme_command) + mock_run_nvme_cli.assert_called_with(nvme_command) @mock.patch.object(nvmeof.NVMeOFConnector, 'run_nvme_cli') - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_live_nvme_controllers_map') - def test_rescan_err(self, mock_get_live_nvme_controllers_map, - mock_run_nvme_cli): - mock_get_live_nvme_controllers_map.return_value = fake_controllers_map + def test_rescan_err(self, mock_run_nvme_cli): + """Test failure on nvme rescan subprocess execution.""" mock_run_nvme_cli.side_effect = Exception() - result = self.connector.rescan(EXECUTOR, TARGET_NQN) - self.assertEqual(fake_controllers_map, result) - mock_get_live_nvme_controllers_map.assert_called_with(EXECUTOR, - TARGET_NQN) + self.assertRaises(exception.CommandExecutionFailed, + self.connector.rescan, 'nvme1') nvme_command = ('ns-rescan', NVME_DEVICE_PATH) - mock_run_nvme_cli.assert_called_with(EXECUTOR, nvme_command) + mock_run_nvme_cli.assert_called_with(nvme_command) @mock.patch.object(executor.Executor, '_execute') def test_is_raid_exists_not(self, mock_execute): mock_execute.return_value = (VOL_UUID + "\n", "") - result = self.connector.is_raid_exists(EXECUTOR, NVME_DEVICE_PATH) + result = self.connector.is_raid_exists(NVME_DEVICE_PATH) self.assertEqual(False, result) cmd = ['mdadm', '--detail', NVME_DEVICE_PATH] args, kwargs = mock_execute.call_args @@ -788,7 +1586,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(executor.Executor, '_execute') def test_is_raid_exists(self, mock_execute): mock_execute.return_value = (NVME_DEVICE_PATH + ':' + "\n", "") - result = self.connector.is_raid_exists(EXECUTOR, NVME_DEVICE_PATH) + result = self.connector.is_raid_exists(NVME_DEVICE_PATH) self.assertEqual(True, result) cmd = ['mdadm', '--detail', NVME_DEVICE_PATH] args, kwargs = mock_execute.call_args @@ -799,7 +1597,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(executor.Executor, '_execute') def test_is_raid_exists_err(self, mock_execute): mock_execute.side_effect = putils.ProcessExecutionError - result = self.connector.is_raid_exists(EXECUTOR, NVME_DEVICE_PATH) + result = self.connector.is_raid_exists(NVME_DEVICE_PATH) self.assertEqual(False, result) cmd = ['mdadm', '--detail', NVME_DEVICE_PATH] args, kwargs = mock_execute.call_args @@ -810,7 +1608,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(executor.Executor, '_execute') def test_get_md_name(self, mock_execute): mock_execute.return_value = ('nvme1' + "\n", "") - result = self.connector.get_md_name(EXECUTOR, NVME_DEVICE_PATH) + result = self.connector.get_md_name(NVME_DEVICE_PATH) self.assertEqual('nvme1', result) get_md_cmd = 'cat /proc/mdstat | grep /dev/nvme1 | awk \'{print $1;}\'' cmd = ['bash', '-c', get_md_cmd] @@ -822,7 +1620,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(executor.Executor, '_execute') def test_get_md_name_err(self, mock_execute): mock_execute.side_effect = putils.ProcessExecutionError() - result = self.connector.get_md_name(EXECUTOR, NVME_DEVICE_PATH) + result = self.connector.get_md_name(NVME_DEVICE_PATH) self.assertIsNone(result) get_md_cmd = 'cat /proc/mdstat | grep /dev/nvme1 | awk \'{print $1;}\'' cmd = ['bash', '-c', get_md_cmd] @@ -834,8 +1632,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(executor.Executor, '_execute') def test_is_device_in_raid(self, mock_execute): mock_execute.return_value = (NVME_DEVICE_PATH + ':' + "\n", "") - result = self.connector._is_device_in_raid(self.connector, - NVME_DEVICE_PATH) + result = self.connector._is_device_in_raid(NVME_DEVICE_PATH) self.assertEqual(True, result) cmd = ['mdadm', '--examine', NVME_DEVICE_PATH] args, kwargs = mock_execute.call_args @@ -846,8 +1643,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(executor.Executor, '_execute') def test_is_device_in_raid_not_found(self, mock_execute): mock_execute.return_value = (VOL_UUID + "\n", "") - result = self.connector._is_device_in_raid(self.connector, - NVME_DEVICE_PATH) + result = self.connector._is_device_in_raid(NVME_DEVICE_PATH) self.assertEqual(False, result) cmd = ['mdadm', '--examine', NVME_DEVICE_PATH] args, kwargs = mock_execute.call_args @@ -858,8 +1654,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch.object(executor.Executor, '_execute') def test_is_device_in_raid_err(self, mock_execute): mock_execute.side_effect = putils.ProcessExecutionError() - result = self.connector._is_device_in_raid(self.connector, - NVME_DEVICE_PATH) + result = self.connector._is_device_in_raid(NVME_DEVICE_PATH) self.assertEqual(False, result) cmd = ['mdadm', '--examine', NVME_DEVICE_PATH] args, kwargs = mock_execute.call_args @@ -871,7 +1666,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): def test_run_mdadm(self, mock_execute): mock_execute.return_value = (VOL_UUID + "\n", "") cmd = ['mdadm', '--examine', NVME_DEVICE_PATH] - result = self.connector.run_mdadm(EXECUTOR, cmd) + result = self.connector.run_mdadm(cmd) self.assertEqual(VOL_UUID, result) args, kwargs = mock_execute.call_args self.assertEqual(args[0], cmd[0]) @@ -882,89 +1677,13 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): def test_run_mdadm_err(self, mock_execute): mock_execute.side_effect = putils.ProcessExecutionError() cmd = ['mdadm', '--examine', NVME_DEVICE_PATH] - result = self.connector.run_mdadm(EXECUTOR, cmd) + result = self.connector.run_mdadm(cmd) self.assertIsNone(result) args, kwargs = mock_execute.call_args self.assertEqual(args[0], cmd[0]) self.assertEqual(args[1], cmd[1]) self.assertEqual(args[2], cmd[2]) - @mock.patch.object(executor.Executor, '_execute') - @mock.patch.object(glob, 'glob') - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_controllers') - def test_get_nvme_device_path(self, mock_get_nvme_controllers, mock_glob, - mock_execute): - mock_get_nvme_controllers.return_value = ['nvme1'] - block_dev_path = '/sys/class/block/nvme1n*/uuid' - mock_glob.side_effect = [['/sys/class/block/nvme1n1/uuid']] - mock_execute.return_value = (VOL_UUID + "\n", "") - cmd = ['cat', '/sys/class/block/nvme1n1/uuid'] - result = self.connector.get_nvme_device_path(EXECUTOR, TARGET_NQN, - VOL_UUID) - mock_get_nvme_controllers.assert_called_with(EXECUTOR, TARGET_NQN) - self.assertEqual(NVME_NS_PATH, result) - mock_glob.assert_any_call(block_dev_path) - args, kwargs = mock_execute.call_args - self.assertEqual(args[0], cmd[0]) - self.assertEqual(args[1], cmd[1]) - - def execute_side_effect(self, value, run_as_root, root_helper): - if 'nqn' in value: - return TARGET_NQN + "\n", "" - if 'state' in value: - return 'live' + "\n", "" - - def execute_side_effect_not_live(self, value, run_as_root, root_helper): - if 'nqn' in value: - return TARGET_NQN + "\n", "" - if 'state' in value: - return 'dead' + "\n", "" - - def execute_side_effect_not_found(self, value, run_as_root, root_helper): - if 'nqn' in value: - return "dummy" + "\n", "" - if 'state' in value: - return 'live' + "\n", "" - - @mock.patch.object(nvmeof.NVMeOFConnector, 'get_live_nvme_controllers_map') - def test_get_nvme_controllers(self, mock_get_live_nvme_controllers_map): - mock_get_live_nvme_controllers_map.return_value = fake_controllers_map - result = self.connector.get_nvme_controllers(EXECUTOR, TARGET_NQN) - fake_controllers_map_values = fake_controllers_map.values() - self.assertEqual(list(fake_controllers_map_values)[0][1], - list(result)[0][1]) - mock_get_live_nvme_controllers_map.assert_called_with(EXECUTOR, - TARGET_NQN) - - @mock.patch.object(executor.Executor, '_execute', - side_effect=execute_side_effect_not_live) - @mock.patch.object(glob, 'glob') - def test_get_nvme_controllers_not_live(self, mock_glob, mock_execute): - ctrl_path = '/sys/class/nvme-fabrics/ctl/nvme*' - mock_glob.side_effect = [['/sys/class/nvme-fabrics/ctl/nvme1']] - cmd = ['cat', '/sys/class/nvme-fabrics/ctl/nvme1/state'] - self.assertRaises(exception.VolumeDeviceNotFound, - self.connector.get_nvme_controllers, EXECUTOR, - TARGET_NQN) - mock_glob.assert_any_call(ctrl_path) - args, kwargs = mock_execute.call_args - self.assertEqual(args[0], cmd[0]) - self.assertEqual(args[1], cmd[1]) - - @mock.patch.object(executor.Executor, '_execute', - side_effect=execute_side_effect_not_found) - @mock.patch.object(glob, 'glob') - def test_get_nvme_controllers_not_found(self, mock_glob, mock_execute): - ctrl_path = '/sys/class/nvme-fabrics/ctl/nvme*' - mock_glob.side_effect = [['/sys/class/nvme-fabrics/ctl/nvme1']] - cmd = ['cat', '/sys/class/nvme-fabrics/ctl/nvme1/state'] - self.assertRaises(exception.VolumeDeviceNotFound, - self.connector.get_nvme_controllers, EXECUTOR, - TARGET_NQN) - mock_glob.assert_any_call(ctrl_path) - args, kwargs = mock_execute.call_args - self.assertEqual(args[0], cmd[0]) - @mock.patch.object(builtins, 'open') def test_get_host_nqn_file_available(self, mock_open): mock_open.return_value.__enter__.return_value.read = ( @@ -996,7 +1715,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): def test_run_nvme_cli(self, mock_execute): mock_execute.return_value = ("\n", "") cmd = 'dummy command' - result = self.connector.run_nvme_cli(EXECUTOR, cmd) + result = self.connector.run_nvme_cli(cmd) self.assertEqual(("\n", ""), result) def test_ks_readlink(self): @@ -1004,9 +1723,19 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): result = self.connector.ks_readlink(dest) self.assertEqual('', result) + @mock.patch.object(executor.Executor, '_execute') + def test__get_fs_type(self, mock_execute): + mock_execute.return_value = ('expected\n', '') + result = self.connector._get_fs_type(NVME_DEVICE_PATH) + self.assertEqual('expected', result) + mock_execute.assert_called_once_with( + 'blkid', NVME_DEVICE_PATH, '-s', 'TYPE', '-o', 'value', + run_as_root=True, root_helper=self.connector._root_helper, + check_exit_code=False) + @mock.patch.object(executor.Executor, '_execute', return_value=('', 'There was a big error')) - def test_get_fs_type_err(self, mock_execute): + def test__get_fs_type_err(self, mock_execute): result = self.connector._get_fs_type(NVME_DEVICE_PATH) self.assertIsNone(result) mock_execute.assert_called_once_with( @@ -1014,85 +1743,19 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): run_as_root=True, root_helper=self.connector._root_helper, check_exit_code=False) - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices') - def test__is_nvme_available(self, mock_nvme_devices): - mock_nvme_devices.return_value = {'/dev/nvme0n1', - '/dev/nvme2n1', - '/dev/nvme2n2', - '/dev/nvme3n1'} - result = self.connector._is_nvme_available('nvme2') + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_fs_type') + def test__is_raid_device(self, mock_get_fs_type): + mock_get_fs_type.return_value = 'linux_raid_member' + result = self.connector._is_raid_device(NVME_DEVICE_PATH) self.assertTrue(result) + mock_get_fs_type.assert_called_once_with(NVME_DEVICE_PATH) - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices') - def test__is_nvme_available_wrong_name(self, mock_nvme_devices): - mock_nvme_devices.return_value = {'/dev/nvme0n1', - '/dev/nvme2n1', - '/dev/nvme2n2', - '/dev/nvme3n1'} - self.assertRaises(exception.NotFound, - self.connector._is_nvme_available, - 'nvme1') - - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices') - def test__is_nvme_available_no_devices(self, mock_nvme_devices): - mock_nvme_devices.return_value = [] - self.assertRaises(exception.NotFound, - self.connector._is_nvme_available, - 'nvme1') - - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices') - def test__is_nvme_available_fail_to_get_devices(self, mock_nvme_devices): - mock_nvme_devices.side_effect = exception.CommandExecutionFailed() - self.assertRaises(exception.CommandExecutionFailed, - self.connector._is_nvme_available, - 'nvme1') - - @mock.patch.object(executor.Executor, '_execute') - def test__get_nvme_devices(self, mock_execute): - mock_execute.return_value = nvme_list_stdout, None - res = self.connector._get_nvme_devices() - self.assertEqual(set(res), {'/dev/nvme0n1', '/dev/nvme0n2'}) - - @mock.patch.object(executor.Executor, '_execute') - def test__get_nvme_devices_failed(self, mock_execute): - mock_execute.side_effect = putils.ProcessExecutionError() - self.assertRaises(exception.CommandExecutionFailed, - self.connector._get_nvme_devices) - - @mock.patch.object(nvmeof.NVMeOFConnector, '_is_nvme_available') - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_subsys') - def test__wait_for_blk(self, mock_nvme_subsys, mock_nvme_avail): - mock_nvme_subsys.return_value = nvme_list_subsystems_stdout, None - mock_nvme_avail.return_value = True - result = self.connector._wait_for_blk('rdma', - 'nqn.2016-06.io.spdk:cnode2', - '10.0.2.16', '4420') - self.assertTrue(result) - - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_subsys') - def test__wait_for_blk_cli_exception(self, mock_nvme_subsys): - mock_nvme_subsys.side_effect = putils.ProcessExecutionError() - self.assertRaises(putils.ProcessExecutionError, - self.connector._wait_for_blk, - 'rdma', - 'nqn.2016-06.io.spdk:cnode2', - '10.0.2.16', '4420') - - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_subsys') - def test__wait_for_blk_bad_json(self, mock_nvme_subsys): - mock_nvme_subsys.return_value = ".", None - result = self.connector._wait_for_blk('rdma', - 'nqn.2016-06.io.spdk:cnode2', - '10.0.2.16', '4420') - self.assertFalse(result) - - @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_subsys') - def test__wait_for_blk_ip_not_found(self, mock_nvme_subsys): - mock_nvme_subsys.return_value = nvme_list_subsystems_stdout, None - result = self.connector._wait_for_blk('rdma', - 'nqn.2016-06.io.spdk:cnode2', - '10.0.2.18', '4420') + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_fs_type') + def test__is_raid_device_not(self, mock_get_fs_type): + mock_get_fs_type.return_value = 'xfs' + result = self.connector._is_raid_device(NVME_DEVICE_PATH) self.assertFalse(result) + mock_get_fs_type.assert_called_once_with(NVME_DEVICE_PATH) def _get_host_nqn(self): host_nqn = None @@ -1124,3 +1787,65 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): res = self.connector._set_native_multipath_supported() mock_ana.assert_not_called() self.assertTrue(res) + + @mock.patch.object(nvmeof.NVMeOFConnector, '_handle_single_replica') + @mock.patch.object(nvmeof.NVMeOFConnector, '_handle_replicated_volume') + @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target') + def test__connect_volume_replicated( + self, mock_connect, mock_replicated, mock_single): + """Connect to replicated backend handles connection failures.""" + found_devices = ['/dev/nvme0n1', '/dev/nvme1n1'] + mock_connect.side_effect = [Exception] + found_devices + res = self.connector._connect_volume_replicated(CONN_PROPS) + + self.assertEqual(mock_replicated.return_value, res) + + mock_replicated.assert_called_once_with(found_devices, CONN_PROPS) + mock_single.assert_not_called() + + @mock.patch.object(nvmeof.NVMeOFConnector, '_handle_single_replica') + @mock.patch.object(nvmeof.NVMeOFConnector, '_handle_replicated_volume') + @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target') + def test__connect_volume_replicated_single_replica( + self, mock_connect, mock_replicated, mock_single): + """Connect to single repica backend.""" + conn_props = nvmeof.NVMeOFConnProps({ + 'alias': 'fakealias', + 'vol_uuid': VOL_UUID, + 'volume_replicas': [volume_replicas[0]], + 'replica_count': 1 + }) + + found_devices = ['/dev/nvme0n1'] + mock_connect.side_effect = found_devices + res = self.connector._connect_volume_replicated(conn_props) + + self.assertEqual(mock_single.return_value, res) + + mock_replicated.assert_not_called() + mock_single.assert_called_once_with(found_devices, 'fakealias') + + @mock.patch.object(nvmeof.NVMeOFConnector, '_handle_single_replica') + @mock.patch.object(nvmeof.NVMeOFConnector, '_handle_replicated_volume') + @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target') + def test__connect_volume_replicated_no_device_paths_found( + self, mock_connect, mock_replicated, mock_single): + """Fail if cannot connect to any replica.""" + mock_connect.side_effect = 3 * [Exception] + + self.assertRaises(exception.VolumeDeviceNotFound, + self.connector._connect_volume_replicated, + CONN_PROPS) + + mock_replicated.assert_not_called() + mock_single.assert_not_called() + + @ddt.data({'result': False, 'use_multipath': False, 'ana_support': True}, + {'result': False, 'use_multipath': False, 'ana_support': False}, + {'result': False, 'use_multipath': True, 'ana_support': False}, + {'result': True, 'use_multipath': True, 'ana_support': True}) + @ddt.unpack + def test__do_multipath(self, result, use_multipath, ana_support): + self.connector.use_multipath = use_multipath + self.connector.native_multipath_supported = ana_support + self.assertIs(result, self.connector._do_multipath()) diff --git a/os_brick/tests/test_utils.py b/os_brick/tests/test_utils.py index fb83f8726..e52a71fe5 100644 --- a/os_brick/tests/test_utils.py +++ b/os_brick/tests/test_utils.py @@ -612,3 +612,20 @@ class ConnectionPropertiesDecoratorsTestCase(base.TestCase): outer_self.assertEqual('extend_volume', res) mock_dev_path.assert_called_once_with(symlink_path) mock_unlink.assert_not_called() + + +@ddt.ddt +class AnyTestCase(base.TestCase): + @ddt.data('hola', 1, None, {'a': 1}, {1, 2}, False) + def test_equal(self, what): + self.assertEqual(what, utils.ANY) + self.assertEqual(utils.ANY, what) + + @ddt.data('hola', 1, None, {'a': 1}, {1, 2}, False) + def test_different(self, what): + self.assertFalse(what != utils.ANY) # noqa + self.assertFalse(utils.ANY != what) # noqa + self.assertFalse(utils.ANY > what) # noqa + self.assertFalse(utils.ANY < what) # noqa + self.assertFalse(utils.ANY <= what) # noqa + self.assertFalse(utils.ANY >= what) # noqa diff --git a/os_brick/utils.py b/os_brick/utils.py index 595efded5..4b52dea7b 100644 --- a/os_brick/utils.py +++ b/os_brick/utils.py @@ -406,3 +406,21 @@ def get_device_size(executor, device): return int(var) else: return None + + +class Anything(object): + """Object equal to everything.""" + def __eq__(self, other): + return True + + def __ne__(self, other): + return False + + def __str__(self): + return '' + + __lt__ = __gt__ = __le__ = __ge__ = __ne__ + __repr__ = __str__ + + +ANY = Anything() diff --git a/releasenotes/notes/nvmeof-consolidate-004dbe3a98f6f815.yaml b/releasenotes/notes/nvmeof-consolidate-004dbe3a98f6f815.yaml new file mode 100644 index 000000000..3cd1408d6 --- /dev/null +++ b/releasenotes/notes/nvmeof-consolidate-004dbe3a98f6f815.yaml @@ -0,0 +1,46 @@ +--- +fixes: + - | + NVMe-oF connector `bug #1964395 + `_: Fixed dependence on a + specific nvme cli version for proper detection of devices when attaching a + volume. + + - | + NVMe-oF connector `bug #1964388 + `_: Fixed corner case + where it could return the wrong path for a volume, resulting in attaching + in Nova the wrong volume to an instance, destroying volume data in Cinder, + and other similarly dangerous scenarios. + + - | + NVMe-oF connector `bug #1964385 + `_: Fixed disappearance + of volumes/devices from the host, with potential data loss of unflushed + data, when network issues last longer than 10 minutes. + + - | + NVMe-oF connector `bug #1964380 + `_: Fixed support for + newer nvme cli exit code when trying to connect to an already + subsystem-portal. + + - | + NVMe-oF connector `bug #1964383 + `_: Fixed not being able + to attach a volume if there was already a controller for the subsystem. + + - | + NVMe-oF connector `bug #1965954 + `_: Fixed extend of + in-use replicated volumes with a single replica not growing the RAID + + - | + NVMe-oF connector `bug #1964590 + `_: Fixed extend failure + of in-use volumes with some Cinder drivers. + + - | + NVMe-oF connector `bug #1903032 + `_: Fixed not flushing + single connection volumes on some Cinder drivers.