Support force disconnect for FC

This patch adds support for the force and ignore_errors on the
disconnect_volume of the FC connector like we have in the iSCSI
connector.

Related-Bug: #2004555
Change-Id: Ia74ecfba03ba23de9d30eb33706245a7f85e1d66
(cherry picked from commit 570df49)
Conflicts:
	os_brick/initiator/connectors/fibre_channel.py
(cherry picked from commit 111b393)
(cherry picked from commit 7049373)
(cherry picked from commit c00bd3f)

os_brick/initiator/connectors/fibre_channel.py

@@ -333,7 +333,7 @@ def disconnect_volume(self, connection_properties, device_info,
         target_wwn - World Wide Name
         target_lun - LUN id of the volume
         """
-
+        exc = exception.ExceptionChainer()
         devices = []
         wwn = None
 
@@ -349,14 +349,30 @@ def disconnect_volume(self, connection_properties, device_info,
                 wwn = self._linuxscsi.get_scsi_wwn(path)
                 mpath_path = self._linuxscsi.find_multipath_device_path(wwn)
                 if mpath_path:
-                    self._linuxscsi.flush_multipath_device(mpath_path)
+                    with exc.context(force, 'Flushing %s failed', mpath_path):
+                        self._linuxscsi.flush_multipath_device(mpath_path)
             dev_info = self._linuxscsi.get_device_info(real_path)
             devices.append(dev_info)
 
+        # If flush failed, then remove it forcefully since force=True
+        if mpath_path and exc:
+            with exc.context(force, 'Removing multipath %s failed',
+                             mpath_path):
+                mpath_name = os.path.basename(os.path.realpath(mpath_path))
+                self._linuxscsi.multipath_del_map(mpath_name)
+
         LOG.debug("devices to remove = %s", devices)
-        self._remove_devices(connection_properties, devices, device_info)
+        self._remove_devices(connection_properties, devices, device_info,
+                             force, exc)
+
+        if exc:  # type: ignore
+            LOG.warning('There were errors removing %s, leftovers may remain '
+                        'in the system', volume_paths)
+            if not ignore_errors:
+                raise exc  # type: ignore
 
-    def _remove_devices(self, connection_properties, devices, device_info):
+    def _remove_devices(self, connection_properties, devices, device_info,
+                        force, exc):
         # There may have been more than 1 device mounted
         # by the kernel for this volume.  We have to remove
         # all of them
@@ -371,11 +387,12 @@ def _remove_devices(self, connection_properties, devices, device_info):
         # paths, whereas for multipaths we have multiple link formats.
         was_multipath = '/pci-' not in path_used and was_symlink
         for device in devices:
-            device_path = device['device']
-            flush = self._linuxscsi.requires_flush(device_path,
-                                                   path_used,
-                                                   was_multipath)
-            self._linuxscsi.remove_scsi_device(device_path, flush=flush)
+            with exc.context(force, 'Removing device %s failed', device):
+                device_path = device['device']
+                flush = self._linuxscsi.requires_flush(device_path,
+                                                       path_used,
+                                                       was_multipath)
+                self._linuxscsi.remove_scsi_device(device_path, flush=flush)
 
     def _get_pci_num(self, hba):
         # NOTE(walter-boring)

os_brick/initiator/connectors/fibre_channel_s390x.py

@@ -87,12 +87,15 @@ def _get_device_file_path(self, pci_num, target_wwn, lun):
         ]
         return host_device
 
-    def _remove_devices(self, connection_properties, devices, device_info):
+    def _remove_devices(self, connection_properties, devices, device_info,
+                        force, exc):
         hbas = self._linuxfc.get_fc_hbas_info()
         targets = connection_properties['targets']
         possible_devs = self._get_possible_devices(hbas, targets)
         for platform, pci_num, target_wwn, lun in possible_devs:
             target_lun = self._get_lun_string(lun)
-            self._linuxfc.deconfigure_scsi_device(pci_num,
-                                                  target_wwn,
-                                                  target_lun)
+            with exc.context(force, 'Removing device %s:%s:%s failed',
+                             pci_num, target_wwn, target_lun):
+                self._linuxfc.deconfigure_scsi_device(pci_num,
+                                                      target_wwn,
+                                                      target_lun)

os_brick/initiator/linuxscsi.py

@@ -735,3 +735,21 @@ def multipath_del_path(self, realpath):
                                        check_exit_code=False,
                                        root_helper=self._root_helper)
         return stdout.strip() == 'ok'
+
+    @utils.retry((putils.ProcessExecutionError, exception.BrickException),
+                 retries=3)
+    def multipath_del_map(self, mpath):
+        """Stop monitoring a multipath given its device name (eg: dm-7).
+
+        Method ensures that the multipath device mapper actually dissapears
+        from sysfs.
+        """
+        map_name = self.get_dm_name(mpath)
+        if map_name:
+            self._execute('multipathd', 'del', 'map', map_name,
+                          run_as_root=True, timeout=5,
+                          root_helper=self._root_helper)
+
+        if map_name and self.get_dm_name(mpath):
+            raise exception.BrickException("Multipath doesn't go away")
+        LOG.debug('Multipath %s no longer present', mpath)

os_brick/tests/initiator/connectors/test_fibre_channel.py

@@ -744,17 +744,53 @@ def test__add_targets_to_connection_properties(self, target_info,
     @mock.patch('os_brick.utils.get_dev_path')
     def test__remove_devices(self, path_used, was_multipath, get_dev_path_mock,
                              flush_mock, remove_mock):
+        exc = exception.ExceptionChainer()
         get_dev_path_mock.return_value = path_used
         self.connector._remove_devices(mock.sentinel.con_props,
                                        [{'device': '/dev/sda'}],
-                                       mock.sentinel.device_info)
+                                       mock.sentinel.device_info,
+                                       force=False, exc=exc)
+        self.assertFalse(bool(exc))
         get_dev_path_mock.assert_called_once_with(mock.sentinel.con_props,
                                                   mock.sentinel.device_info)
         flush_mock.assert_called_once_with('/dev/sda', path_used,
                                            was_multipath)
         remove_mock.assert_called_once_with('/dev/sda',
                                             flush=flush_mock.return_value)
 
+    @ddt.data(('/dev/mapper/<WWN>', True),
+              ('/dev/mapper/mpath0', True),
+              # Check real devices are properly detected as non multipaths
+              ('/dev/sda', False),
+              ('/dev/disk/by-path/pci-1-fc-1-lun-1', False))
+    @ddt.unpack
+    @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.remove_scsi_device')
+    @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.requires_flush')
+    @mock.patch('os_brick.utils.get_dev_path')
+    def test__remove_devices_fails(self, path_used, was_multipath,
+                                   get_dev_path_mock, flush_mock, remove_mock):
+        exc = exception.ExceptionChainer()
+        get_dev_path_mock.return_value = path_used
+        remove_mock.side_effect = Exception
+        self.connector._remove_devices(mock.sentinel.con_props,
+                                       [{'device': '/dev/sda'},
+                                        {'device': '/dev/sdb'}],
+                                       mock.sentinel.device_info,
+                                       force=True, exc=exc)
+        self.assertTrue(bool(exc))
+        get_dev_path_mock.assert_called_once_with(mock.sentinel.con_props,
+                                                  mock.sentinel.device_info)
+
+        expect_flush = [mock.call('/dev/sda', path_used, was_multipath),
+                        mock.call('/dev/sdb', path_used, was_multipath)]
+        self.assertEqual(len(expect_flush), flush_mock.call_count)
+        flush_mock.assert_has_calls(expect_flush)
+
+        expect_remove = [mock.call('/dev/sda', flush=flush_mock.return_value),
+                         mock.call('/dev/sdb', flush=flush_mock.return_value)]
+        self.assertEqual(len(expect_remove), remove_mock.call_count)
+        remove_mock.assert_has_calls(expect_remove)
+
     @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_rw')
     @mock.patch.object(os.path, 'exists', return_value=True)
@@ -816,3 +852,94 @@ def test_disconnect_volume(self, check_valid_device_mock,
             'tee -a /sys/block/sdc/device/delete',
         ]
         self.assertEqual(expected_commands, self.cmds)
+
+    @ddt.data((False, Exception), (True, Exception), (False, None))
+    @ddt.unpack
+    @mock.patch.object(fibre_channel.FibreChannelConnector, '_remove_devices')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_del_map')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'flush_multipath_device')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_rw')
+    @mock.patch.object(os.path, 'exists', return_value=True)
+    @mock.patch.object(os.path, 'realpath')
+    @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas')
+    @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas_info')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
+    @mock.patch.object(base.BaseLinuxConnector, 'check_valid_device')
+    def test_disconnect_volume_fails(self, ignore_exc, side_effect,
+                                     check_valid_device_mock,
+                                     find_mp_device_path_mock,
+                                     get_device_info_mock,
+                                     get_scsi_wwn_mock,
+                                     get_fc_hbas_info_mock,
+                                     get_fc_hbas_mock,
+                                     realpath_mock,
+                                     exists_mock,
+                                     wait_for_rw_mock,
+                                     flush_mock,
+                                     del_map_mock,
+                                     remove_mock):
+
+        flush_mock.side_effect = side_effect
+        del_map_mock.side_effect = side_effect
+
+        check_valid_device_mock.return_value = True
+        self.connector.use_multipath = True
+        get_fc_hbas_mock.side_effect = self.fake_get_fc_hbas
+        get_fc_hbas_info_mock.side_effect = self.fake_get_fc_hbas_info
+
+        wwn = '360002ac00000000000000b860000741c'
+        multipath_devname = f'/dev/disk/by-id/dm-uuid-mpath-{wwn}'
+        realpath_mock.return_value = '/dev/dm-1'
+        devices = {"device": multipath_devname,
+                   "id": wwn,
+                   "devices": [{'device': '/dev/sdb',
+                                'address': '1:0:0:1',
+                                'host': 1, 'channel': 0,
+                                'id': 0, 'lun': 1},
+                               {'device': '/dev/sdc',
+                                'address': '1:0:0:2',
+                                'host': 1, 'channel': 0,
+                                'id': 0, 'lun': 1}]}
+        get_device_info_mock.side_effect = devices['devices']
+        get_scsi_wwn_mock.return_value = wwn
+
+        location = '10.0.2.15:3260'
+        name = 'volume-00000001'
+        vol = {'id': 1, 'name': name}
+        initiator_wwn = ['1234567890123456', '1234567890123457']
+
+        find_mp_device_path_mock.return_value = '/dev/mapper/mpatha'
+
+        connection_info = self.fibrechan_connection(vol, location,
+                                                    initiator_wwn)
+        if side_effect and not ignore_exc:
+            self.assertRaises(exception.ExceptionChainer,
+                              self.connector.disconnect_volume,
+                              connection_info['data'], devices['devices'][0],
+                              force=True, ignore_errors=ignore_exc)
+        else:
+            self.connector.disconnect_volume(connection_info['data'],
+                                             devices['devices'][0],
+                                             force=True,
+                                             ignore_errors=ignore_exc)
+
+        flush_mock.assert_called_once_with(
+            find_mp_device_path_mock.return_value)
+
+        expected = [
+            mock.call(f'/dev/disk/by-path/pci-0000:05:00.2-fc-0x{wwn}-lun-1')
+            for wwn in initiator_wwn]
+        if side_effect:
+            del_map_mock.assert_called_once_with('dm-1')
+            expected.append(mock.call(find_mp_device_path_mock.return_value))
+        else:
+            del_map_mock.assert_not_called()
+
+        remove_mock.assert_called_once_with(connection_info['data'],
+                                            devices['devices'],
+                                            devices['devices'][0],
+                                            True, mock.ANY)
+        self.assertEqual(len(expected), realpath_mock.call_count)
+        realpath_mock.assert_has_calls(expected)

os_brick/tests/initiator/connectors/test_fibre_channel_s390x.py

@@ -13,6 +13,7 @@
 #    under the License.
 from unittest import mock
 
+from os_brick import exception
 from os_brick.initiator.connectors import fibre_channel_s390x
 from os_brick.initiator import linuxfc
 from os_brick.tests.initiator import test_connector
@@ -66,10 +67,32 @@ def test_get_lun_string(self):
                        'deconfigure_scsi_device')
     def test_remove_devices(self, mock_deconfigure_scsi_device,
                             mock_get_fc_hbas_info, mock_get_possible_devices):
+        exc = exception.ExceptionChainer()
         connection_properties = {'targets': [5, 2]}
         self.connector._remove_devices(connection_properties, devices=None,
-                                       device_info=None)
+                                       device_info=None, force=False, exc=exc)
         mock_deconfigure_scsi_device.assert_called_with(3, 5,
                                                         "0x0002000000000000")
         mock_get_fc_hbas_info.assert_called_once_with()
         mock_get_possible_devices.assert_called_once_with([], [5, 2])
+        self.assertFalse(bool(exc))
+
+    @mock.patch.object(fibre_channel_s390x.FibreChannelConnectorS390X,
+                       '_get_possible_devices', return_value=[('', 3, 5, 2), ])
+    @mock.patch.object(linuxfc.LinuxFibreChannelS390X, 'get_fc_hbas_info',
+                       return_value=[])
+    @mock.patch.object(linuxfc.LinuxFibreChannelS390X,
+                       'deconfigure_scsi_device')
+    def test_remove_devices_force(self, mock_deconfigure_scsi_device,
+                                  mock_get_fc_hbas_info,
+                                  mock_get_possible_devices):
+        exc = exception.ExceptionChainer()
+        mock_deconfigure_scsi_device.side_effect = Exception
+        connection_properties = {'targets': [5, 2]}
+        self.connector._remove_devices(connection_properties, devices=None,
+                                       device_info=None, force=True, exc=exc)
+        mock_deconfigure_scsi_device.assert_called_with(3, 5,
+                                                        "0x0002000000000000")
+        mock_get_fc_hbas_info.assert_called_once_with()
+        mock_get_possible_devices.assert_called_once_with([], [5, 2])
+        self.assertTrue(bool(exc))

os_brick/tests/initiator/test_linuxscsi.py

@@ -1231,6 +1231,64 @@ def test_multipath_del_path(self):
         self.linuxscsi.multipath_del_path('/dev/sda')
         self.assertEqual(['multipathd del path /dev/sda'], self.cmds)
 
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_dm_name', return_value=None)
+    def test_multipath_del_map_not_present(self, name_mock):
+        self.linuxscsi.multipath_del_map('dm-7')
+        self.assertEqual([], self.cmds)
+        name_mock.assert_called_once_with('dm-7')
+
+    @mock.patch.object(linuxscsi.LinuxSCSI, '_execute')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_dm_name', return_value=None)
+    def test_multipath_del_map(self, name_mock, exec_mock):
+        exec_mock.side_effect = [putils.ProcessExecutionError, None]
+
+        mpath_name = '3600d0230000000000e13955cc3757800'
+        name_mock.side_effect = [mpath_name, mpath_name, None]
+        self.linuxscsi.multipath_del_map('dm-7')
+
+        self.assertEqual(2, exec_mock.call_count)
+        exec_mock.assert_has_calls(
+            [mock.call('multipathd', 'del', 'map', mpath_name,
+                       run_as_root=True, timeout=5,
+                       root_helper=self.linuxscsi._root_helper)] * 2)
+        self.assertEqual(3, name_mock.call_count)
+        name_mock.assert_has_calls([mock.call('dm-7')] * 3)
+
+    @mock.patch.object(linuxscsi.LinuxSCSI, '_execute')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_dm_name')
+    def test_multipath_del_map_retries_cmd_fails(self, name_mock, exec_mock):
+        exec_mock.side_effect = putils.ProcessExecutionError
+        mpath_name = '3600d0230000000000e13955cc3757800'
+        name_mock.return_value = mpath_name
+        self.assertRaises(putils.ProcessExecutionError,
+                          self.linuxscsi.multipath_del_map, 'dm-7')
+
+        self.assertEqual(3, exec_mock.call_count)
+        exec_mock.assert_has_calls(
+            [mock.call('multipathd', 'del', 'map', mpath_name,
+                       run_as_root=True, timeout=5,
+                       root_helper=self.linuxscsi._root_helper)] * 3)
+
+        self.assertEqual(3, name_mock.call_count)
+        name_mock.assert_has_calls([mock.call('dm-7')] * 3)
+
+    @mock.patch.object(linuxscsi.LinuxSCSI, '_execute')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_dm_name')
+    def test_multipath_del_map_retries_remains(self, name_mock, exec_mock):
+        mpath_name = '3600d0230000000000e13955cc3757800'
+        name_mock.return_value = mpath_name
+        self.assertRaises(exception.BrickException,
+                          self.linuxscsi.multipath_del_map, 'dm-7')
+
+        self.assertEqual(3, exec_mock.call_count)
+        exec_mock.assert_has_calls(
+            [mock.call('multipathd', 'del', 'map', mpath_name,
+                       run_as_root=True, timeout=5,
+                       root_helper=self.linuxscsi._root_helper)] * 3)
+
+        self.assertEqual(6, name_mock.call_count)
+        name_mock.assert_has_calls([mock.call('dm-7')] * 6)
+
     @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/fc-force-disconnect-1a33cf46c233dd04.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    FC connector: Added support to ``force`` and ``ignore_errors`` parameters
+    on ``disconnect_volume`` method.