Merge "Avoid volume extension errors caused by multipath-tools version"

os_brick/initiator/linuxscsi.py

@@ -24,6 +24,7 @@ from typing import Dict, List, Optional  # noqa: H301
 
 from oslo_concurrency import processutils as putils
 from oslo_log import log as logging
+from oslo_utils import excutils
 
 from os_brick import exception
 from os_brick import executor
@@ -37,6 +38,7 @@ MULTIPATH_WWID_REGEX = re.compile(r"\((?P<wwid>.+)\)")
 MULTIPATH_DEVICE_ACTIONS = ['unchanged:', 'reject:', 'reload:',
                             'switchpg:', 'rename:', 'create:',
                             'resize:']
+MULTIPATHD_RESIZE_TIMEOUT = 120
 
 
 class LinuxSCSI(executor.Executor):
@@ -571,16 +573,44 @@ class LinuxSCSI(executor.Executor):
                                     root_helper=self._root_helper)
         return out
 
+    def _multipath_resize_map(self, mpath_id):
+        cmd = ('multipathd', 'resize', 'map', mpath_id)
+        (out, _err) = self._execute(*cmd,
+                                    run_as_root=True,
+                                    root_helper=self._root_helper)
+        if 'fail' in out or 'timeout' in out:
+            raise putils.ProcessExecutionError(
+                stdout=out, stderr=_err,
+                exit_code=1, cmd=cmd)
+
+        return out
+
     def multipath_resize_map(self, mpath_id):
         """Issue a multipath resize map on device.
 
         This forces the multipath daemon to update it's
         size information a particular multipath device.
         """
-        (out, _err) = self._execute('multipathd', 'resize', 'map', mpath_id,
-                                    run_as_root=True,
-                                    root_helper=self._root_helper)
-        return out
+
+        # "multipathd reconfigure" is async since 0.6.1. While the
+        # operation is in progress, "multipathd resize map" returns
+        # "timeout".
+        tstart = time.time()
+        while True:
+            try:
+                self._multipath_resize_map(mpath_id)
+                break
+            except putils.ProcessExecutionError as err:
+                with excutils.save_and_reraise_exception(reraise=True) as ctx:
+                    elapsed = time.time() - tstart
+                    if 'timeout' in err.stdout and (
+                            elapsed < MULTIPATHD_RESIZE_TIMEOUT):
+                        LOG.debug(
+                            "multipathd resize map timed out. "
+                            "Elapsed: %s, timeout: %s. Retrying...",
+                            elapsed, MULTIPATHD_RESIZE_TIMEOUT)
+                        ctx.reraise = False
+                        time.sleep(1)
 
     def extend_volume(self, volume_paths, use_multipath=False):
         """Signal the SCSI subsystem to test for volume resize.
@@ -621,13 +651,8 @@ class LinuxSCSI(executor.Executor):
                 size = self.get_device_size(mpath_device)
                 LOG.info("mpath(%(device)s) current size %(size)s",
                          {'device': mpath_device, 'size': size})
-                result = self.multipath_resize_map(scsi_wwn)
-                if 'fail' in result:
-                    LOG.error("Multipathd failed to update the size mapping "
-                              "of multipath device %(scsi_wwn)s volume "
-                              "%(volume)s",
-                              {'scsi_wwn': scsi_wwn, 'volume': volume_paths})
-                    return None
+
+                self.multipath_resize_map(scsi_wwn)
 
                 new_size = self.get_device_size(mpath_device)
                 LOG.info("mpath(%(device)s) new size %(size)s",

os_brick/tests/initiator/test_linuxscsi.py

@@ -851,7 +851,7 @@ loop0                                     0"""
                          'multipathd resize map %s' % wwn]
         self.assertEqual(expected_cmds, self.cmds)
 
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_resize_map')
+    @mock.patch.object(linuxscsi.LinuxSCSI, '_multipath_resize_map')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
@@ -873,11 +873,14 @@ loop0                                     0"""
         mock_find_mpath_path.return_value = ('/dev/mapper/dm-uuid-mpath-%s' %
                                              wwn)
 
-        mock_mpath_resize_map.return_value = 'fail'
+        mock_mpath_resize_map.side_effect = putils.ProcessExecutionError(
+            stdout="fail")
 
-        ret_size = self.linuxscsi.extend_volume(['/dev/fake1', '/dev/fake2'],
-                                                use_multipath=True)
-        self.assertIsNone(ret_size)
+        self.assertRaises(
+            putils.ProcessExecutionError,
+            self.linuxscsi.extend_volume,
+            volume_paths=['/dev/fake1', '/dev/fake2'],
+            use_multipath=True)
 
         # because we don't mock out the echo_scsi_command
         expected_cmds = ['tee -a /sys/bus/scsi/drivers/sd/0:0:0:1/rescan',
@@ -885,6 +888,91 @@ loop0                                     0"""
                          'multipathd reconfigure']
         self.assertEqual(expected_cmds, self.cmds)
 
+    @mock.patch('time.sleep')
+    @mock.patch.object(linuxscsi.LinuxSCSI, '_multipath_resize_map')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
+    def test_extend_volume_with_mpath_pending(self, mock_device_info,
+                                              mock_device_size,
+                                              mock_scsi_wwn,
+                                              mock_find_mpath_path,
+                                              mock_mpath_resize_map,
+                                              mock_sleep):
+        """Test extending a volume where there is a multipath device fail."""
+        mock_device_info.side_effect = [{'host': host,
+                                         'channel': '0',
+                                         'id': '0',
+                                         'lun': '1'} for host in ['0', '1']]
+
+        mock_device_size.side_effect = [1024, 2048, 1024, 2048, 1024, 2048]
+        wwn = '1234567890123456'
+        mock_scsi_wwn.return_value = wwn
+        mock_find_mpath_path.return_value = ('/dev/mapper/dm-uuid-mpath-%s' %
+                                             wwn)
+        mock_mpath_resize_map.side_effect = (
+            putils.ProcessExecutionError(stdout="timeout"),
+            "success")
+
+        ret_size = self.linuxscsi.extend_volume(['/dev/fake1', '/dev/fake2'],
+                                                use_multipath=True)
+        self.assertEqual(2048, ret_size)
+
+        # because we don't mock out the echo_scsi_command
+        expected_cmds = ['tee -a /sys/bus/scsi/drivers/sd/0:0:0:1/rescan',
+                         'tee -a /sys/bus/scsi/drivers/sd/1:0:0:1/rescan',
+                         'multipathd reconfigure']
+        self.assertEqual(expected_cmds, self.cmds)
+        mock_mpath_resize_map.assert_has_calls([mock.call(wwn)] * 2)
+
+    @mock.patch('time.sleep')
+    @mock.patch('time.time')
+    @mock.patch.object(linuxscsi.LinuxSCSI, '_multipath_resize_map')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
+    def test_extend_volume_with_mpath_timeout(self, mock_device_info,
+                                              mock_device_size,
+                                              mock_scsi_wwn,
+                                              mock_find_mpath_path,
+                                              mock_mpath_resize_map,
+                                              mock_currtime,
+                                              mock_sleep):
+        """Test extending a volume where there is a multipath device fail."""
+        mock_device_info.side_effect = [{'host': host,
+                                         'channel': '0',
+                                         'id': '0',
+                                         'lun': '1'} for host in ['0', '1']]
+
+        mock_device_size.side_effect = [1024, 2048, 1024, 2048, 1024, 2048]
+        wwn = '1234567890123456'
+        mock_scsi_wwn.return_value = wwn
+        mock_find_mpath_path.return_value = ('/dev/mapper/dm-uuid-mpath-%s' %
+                                             wwn)
+
+        # time.time is used to check if our own timeout has been exceeded,
+        # which is why it has to be mocked.
+        fake_time = 0
+
+        def get_fake_time():
+            nonlocal fake_time
+            fake_time += 10
+            return fake_time
+
+        mock_currtime.side_effect = get_fake_time
+        # We're testing the scenario in which the multipath resize map
+        # call times out indefinitely.
+        mock_mpath_resize_map.side_effect = putils.ProcessExecutionError(
+            stdout="timeout")
+
+        self.assertRaises(
+            putils.ProcessExecutionError,
+            self.linuxscsi.extend_volume,
+            ['/dev/fake1', '/dev/fake2'],
+            use_multipath=True)
+
     def test_process_lun_id_list(self):
         lun_list = [2, 255, 88, 370, 5, 256]
         result = self.linuxscsi.process_lun_id(lun_list)

releasenotes/notes/bug-1888675-mpath-resize-6013ce39fa2b8401.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    `Bug #1888675 <https://bugs.launchpad.net/os-brick/+bug/1888675>`_: Fixed
+    in-use volume resize issues caused by the multipath-tools version.