Merge "LVM: Fix delete volume error due to lvs failure"

cinder/brick/local_dev/lvm.py

@@ -286,6 +286,8 @@ class LVM(executor.Executor):
         return LVM._supports_pvs_ignoreskippedcluster
 
     @staticmethod
+    @utils.retry(retry=utils.retry_if_exit_code, retry_param=139, interval=0.5,
+                 backoff_rate=0.5)  # Bug#1901783
     def get_lv_info(root_helper, vg_name=None, lv_name=None):
         """Retrieve info about LVs (all, in a VG, or a single LV).
 
@@ -653,7 +655,7 @@ class LVM(executor.Executor):
         # order to prevent a race condition.
         self._wait_for_volume_deactivation(name)
 
-    @utils.retry(exceptions=exception.VolumeNotDeactivated, retries=5,
+    @utils.retry(retry_param=exception.VolumeNotDeactivated, retries=5,
                  backoff_rate=2)
     def _wait_for_volume_deactivation(self, name):
         LOG.debug("Checking to see if volume %s has been deactivated.",

cinder/tests/unit/brick/test_brick_lvm.py

@@ -231,6 +231,27 @@ class BrickLvmTestCase(test.TestCase):
                 'sudo', vg_name='fake-vg')
         )
 
+    @mock.patch('tenacity.nap.sleep', mock.Mock())
+    @mock.patch.object(brick.putils, 'execute')
+    def test_get_lv_info_retry(self, exec_mock):
+        exec_mock.side_effect = (
+            processutils.ProcessExecutionError('', '', exit_code=139),
+            ('vg name size', ''),
+        )
+
+        self.assertEqual(
+            [{'name': 'name', 'vg': 'vg', 'size': 'size'}],
+            self.vg.get_lv_info('sudo', vg_name='vg', lv_name='name')
+        )
+
+        self.assertEqual(2, exec_mock.call_count)
+        args = ['env', 'LC_ALL=C', 'lvs', '--noheadings', '--unit=g', '-o',
+                'vg_name,name,size', '--nosuffix', '--readonly', 'vg/name']
+        if self.configuration.lvm_suppress_fd_warnings:
+            args.insert(2, 'LVM_SUPPRESS_FD_WARNINGS=1')
+        lvs_call = mock.call(*args, root_helper='sudo', run_as_root=True)
+        exec_mock.assert_has_calls([lvs_call, lvs_call])
+
     def test_get_all_physical_volumes(self):
         # Filtered VG version
         pvs = self.vg.get_all_physical_volumes('sudo', 'fake-vg')

cinder/tests/unit/test_utils.py

@@ -1056,6 +1056,33 @@ class TestRetryDecorator(test.TestCase):
             self.assertRaises(WrongException, raise_unexpected_error)
             self.assertFalse(mock_sleep.called)
 
+    @mock.patch('tenacity.nap.sleep')
+    def test_retry_exit_code(self, sleep_mock):
+
+        exit_code = 5
+        exception = utils.processutils.ProcessExecutionError
+
+        @utils.retry(retry=utils.retry_if_exit_code, retry_param=exit_code)
+        def raise_retriable_exit_code():
+            raise exception(exit_code=exit_code)
+
+        self.assertRaises(exception, raise_retriable_exit_code)
+        self.assertEqual(2, sleep_mock.call_count)
+        sleep_mock.assert_has_calls([mock.call(1), mock.call(2)])
+
+    @mock.patch('tenacity.nap.sleep')
+    def test_retry_exit_code_non_retriable(self, sleep_mock):
+
+        exit_code = 5
+        exception = utils.processutils.ProcessExecutionError
+
+        @utils.retry(retry=utils.retry_if_exit_code, retry_param=exit_code)
+        def raise_non_retriable_exit_code():
+            raise exception(exit_code=exit_code + 1)
+
+        self.assertRaises(exception, raise_non_retriable_exit_code)
+        sleep_mock.assert_not_called()
+
 
 @ddt.ddt
 class TestCalculateVirtualFree(test.TestCase):

cinder/utils.py

@@ -575,8 +575,19 @@ class ComparableMixin(object):
         return self._compare(other, lambda s, o: s != o)
 
 
-def retry(exceptions, interval=1, retries=3, backoff_rate=2,
+class retry_if_exit_code(tenacity.retry_if_exception):
-          wait_random=False):
+    """Retry on ProcessExecutionError specific exit codes."""
+    def __init__(self, codes):
+        self.codes = (codes,) if isinstance(codes, int) else codes
+        super(retry_if_exit_code, self).__init__(self._check_exit_code)
+
+    def _check_exit_code(self, exc):
+        return (exc and isinstance(exc, processutils.ProcessExecutionError) and
+                exc.exit_code in self.codes)
+
+
+def retry(retry_param, interval=1, retries=3, backoff_rate=2,
+          wait_random=False, retry=tenacity.retry_if_exception_type):
 
     if retries < 1:
         raise ValueError('Retries must be greater than or '
@@ -598,7 +609,7 @@ def retry(exceptions, interval=1, retries=3, backoff_rate=2,
                 after=tenacity.after_log(LOG, logging.DEBUG),
                 stop=tenacity.stop_after_attempt(retries),
                 reraise=True,
-                retry=tenacity.retry_if_exception_type(exceptions),
+                retry=retry(retry_param),
                 wait=wait)
             return r.call(f, *args, **kwargs)
 

cinder/volume/drivers/dell_emc/sc/storagecenter_api.py

@@ -232,8 +232,8 @@ class HttpClient(object):
                 raise exception.VolumeBackendAPIException(message=msg)
         return rest_response
 
-    @utils.retry(exceptions=(requests.ConnectionError,
+    @utils.retry(retry_param=(requests.ConnectionError,
-                             DellDriverRetryableException))
+                              DellDriverRetryableException))
     def get(self, url):
         LOG.debug('get: %(url)s', {'url': url})
         rest_response = self.session.get(self.__formatUrl(url),
@@ -247,7 +247,7 @@ class HttpClient(object):
             raise DellDriverRetryableException()
         return rest_response
 
-    @utils.retry(exceptions=(requests.ConnectionError,))
+    @utils.retry(retry_param=(requests.ConnectionError,))
     def post(self, url, payload, async_call=False):
         LOG.debug('post: %(url)s data: %(payload)s',
                   {'url': url,
@@ -261,7 +261,7 @@ class HttpClient(object):
                 self.asynctimeout if async_call else self.synctimeout)),
             async_call)
 
-    @utils.retry(exceptions=(requests.ConnectionError,))
+    @utils.retry(retry_param=(requests.ConnectionError,))
     def put(self, url, payload, async_call=False):
         LOG.debug('put: %(url)s data: %(payload)s',
                   {'url': url,
@@ -275,7 +275,7 @@ class HttpClient(object):
                 self.asynctimeout if async_call else self.synctimeout)),
             async_call)
 
-    @utils.retry(exceptions=(requests.ConnectionError,))
+    @utils.retry(retry_param=(requests.ConnectionError,))
     def delete(self, url, payload=None, async_call=False):
         LOG.debug('delete: %(url)s data: %(payload)s',
                   {'url': url, 'payload': payload})

cinder/volume/drivers/dell_emc/vnx/client.py

@@ -200,7 +200,7 @@ class Client(object):
     def modify_lun(self):
         pass
 
-    @cinder_utils.retry(exceptions=const.VNXTargetNotReadyError,
+    @cinder_utils.retry(retry_param=const.VNXTargetNotReadyError,
                         interval=15,
                         retries=5, backoff_rate=1)
     def migrate_lun(self, src_id, dst_id,

cinder/volume/drivers/inspur/as13000/as13000_driver.py

@@ -704,7 +704,7 @@ class AS13000Driver(san.SanISCSIDriver):
                                  request_type=request_type)
 
     @volume_utils.trace
-    @utils.retry(exceptions=exception.VolumeDriverException,
+    @utils.retry(retry_param=exception.VolumeDriverException,
                  interval=1,
                  retries=3)
     def _add_lun_to_target(self, target_name, volume):

releasenotes/notes/lvm-delete-error-f12da00c1b3859dc.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    LVM driver `bug #1901783
+    <https://bugs.launchpad.net/cinder/+bug/1901783>`_: Fix unexpected delete
+    volume failure due to unexpected exit code 139 on ``lvs`` command call.