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

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

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

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

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 '<Anything>'
+
+    __lt__ = __gt__ = __le__ = __ge__ = __ne__
+    __repr__ = __str__
+
+
+ANY = Anything()

releasenotes/notes/nvmeof-consolidate-004dbe3a98f6f815.yaml

@@ -0,0 +1,46 @@
+---
+fixes:
+  - |
+    NVMe-oF connector `bug #1964395
+    <https://bugs.launchpad.net/os-brick/+bug/1964395>`_: Fixed dependence on a
+    specific nvme cli version for proper detection of devices when attaching a
+    volume.
+
+  - |
+    NVMe-oF connector `bug #1964388
+    <https://bugs.launchpad.net/os-brick/+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
+    <https://bugs.launchpad.net/os-brick/+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
+    <https://bugs.launchpad.net/os-brick/+bug/1964380>`_: Fixed support for
+    newer nvme cli exit code when trying to connect to an already
+    subsystem-portal.
+
+  - |
+    NVMe-oF connector `bug #1964383
+    <https://bugs.launchpad.net/os-brick/+bug/1964383>`_: Fixed not being able
+    to attach a volume if there was already a controller for the subsystem.
+
+  - |
+    NVMe-oF connector `bug #1965954
+    <https://bugs.launchpad.net/os-brick/+bug/1965954>`_: Fixed extend of
+    in-use replicated volumes with a single replica not growing the RAID
+
+  - |
+    NVMe-oF connector `bug #1964590
+    <https://bugs.launchpad.net/os-brick/+bug/1964590>`_: Fixed extend failure
+    of in-use volumes with some Cinder drivers.
+
+  - |
+    NVMe-oF connector `bug #1903032
+    <https://bugs.launchpad.net/os-brick/+bug/1903032>`_: Fixed not flushing
+    single connection volumes on some Cinder drivers.