Fix iSCSI disconnect_volume when flush fails

The purpose of providing force=True and ignore_errors=True
is to tell os-brick that we want to disconnect the volume
even if flush fails (force) and not raise any exceptions
(ignore_errors). Currently, in an iSCSI multipath environment,
disconnect_volume can fail when both parameters are True.

The current flow when disconnecting an iSCSI volume is
that if flushing a multipath device fails, we manually
remove the device, logout from the target portals,
and try the flush again.

There are two problems here:

1) code problem: The second flush is not wrapped by
ExceptionChainer. This causes it to raise the exception
immediately after flush fails irrespective of the value
of the ignore_errors flag.

2) conceptual problem: In this situation, there is no point
in making the second flush attempt. Instead, we should just
remove the multipath map from multipathd monitoring since
we have already removed the paths manually.

This patch fixes the conceptual problem, as we don't make a second
flush call and ignore any errors on the execution ``multipathd del map``
thereby also fixing the code problem.

Closes-Bug: #2012251
Change-Id: I828911495a2de550ea997e6f51cc039a7b7fa8cd

os_brick/initiator/connectors/iscsi.py

@@ -943,13 +943,13 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector):
         self._disconnect_connection(connection_properties, disconnect, force,
                                     exc)  # type:ignore
 
-        # If flushing the multipath failed before, try now after we have
-        # removed the devices and we may have even logged off (only reaches
-        # here with multipath_name if force=True).
+        # If flushing the multipath failed before, remove the multipath device
+        # map from multipathd monitoring (only reaches here with multipath_name
+        # if force=True).
         if multipath_name:
-            LOG.debug('Flushing again multipath %s now that we removed the '
-                      'devices.', multipath_name)
-            self._linuxscsi.flush_multipath_device(multipath_name)
+            LOG.debug('Removing multipath device map %s to stop multipathd '
+                      'from monitoring the device.', multipath_name)
+            self._linuxscsi.multipath_del_map(multipath_name)
 
         if exc:  # type: ignore
             LOG.warning('There were errors removing %s, leftovers may remain '

os_brick/initiator/linuxscsi.py

@@ -759,3 +759,12 @@ class LinuxSCSI(executor.Executor):
                                        check_exit_code=False,
                                        root_helper=self._root_helper)
         return stdout.strip() == 'ok'
+
+    def multipath_del_map(self, mpath_device):
+        """Remove a map from multipathd for monitoring."""
+        stdout, stderr = self._execute('multipathd', 'del', 'map',
+                                       mpath_device,
+                                       run_as_root=True, timeout=5,
+                                       check_exit_code=False,
+                                       root_helper=self._root_helper)
+        return stdout.strip() == 'ok'

os_brick/tests/initiator/connectors/test_iscsi.py

@@ -721,11 +721,12 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase):
                 mock.Mock(return_value=True))
     @mock.patch.object(iscsi.ISCSIConnector, '_disconnect_connection')
     @mock.patch.object(iscsi.ISCSIConnector, '_get_connection_devices')
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'flush_multipath_device')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'remove_connection',
                        return_value=mock.sentinel.mp_name)
-    def test_cleanup_connection_force_failure(self, remove_mock, flush_mock,
-                                              con_devs_mock, discon_mock):
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_del_map')
+    def test_cleanup_connection_force_failure(self, remove_map_mock,
+                                              remove_mock, con_devs_mock,
+                                              discon_mock):
 
         # Return an ordered dicts instead of normal dict for discon_mock.assert
         con_devs_mock.return_value = collections.OrderedDict((
@@ -749,7 +750,7 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase):
             self.CON_PROPS,
             [('ip1:port1', 'tgt1'), ('ip3:port3', 'tgt3')],
             mock.sentinel.force, mock.ANY)
-        flush_mock.assert_called_once_with(mock.sentinel.mp_name)
+        remove_map_mock.assert_called_once_with(mock.sentinel.mp_name)
 
     def test_cleanup_connection_no_data_discoverydb(self):
         self.connector.use_multipath = True

os_brick/tests/initiator/test_linuxscsi.py

@@ -1299,6 +1299,11 @@ loop0                                     0"""
         self.linuxscsi.multipath_del_path('/dev/sda')
         self.assertEqual(['multipathd del path /dev/sda'], self.cmds)
 
+    @ddt.data('mpatha', '3600140598d2e10957eb444ab09805555')
+    def test_multipath_del_map(self, mpath_device):
+        self.linuxscsi.multipath_del_map(mpath_device)
+        self.assertEqual(['multipathd del map %s' % (mpath_device)], self.cmds)
+
     @ddt.data(('/dev/sda', '/dev/sda', False, True, None),
               # This checks that we ignore the was_multipath parameter if it
               # doesn't make sense (because the used path is the one we are

releasenotes/notes/fix-bug-2012251-381ad16653472c50.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    `Bug #2012251 <https://bugs.launchpad.net/os-brick/+bug/2012251>`_: Fixed
+    issue when disconnecting iSCSI volume when ``force`` and ``ignore_errors``
+    are set to ``True`` and flushing multipath device fails.