]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
vmware: intermittent failure in test_vmware_vmdk
authorSubramanian Neelakantan <subramanian.neelakantan@gmail.com>
Thu, 6 Feb 2014 06:07:27 +0000 (11:37 +0530)
committerSubramanian Neelakantan <subramanian.neelakantan@gmail.com>
Thu, 6 Feb 2014 10:47:52 +0000 (16:17 +0530)
There was a race condition between the two Event() objects being
created within loopingcall and Retry in api module. Did away with
the Event object in Retry and used loopingcall the right way.

Change-Id: I2534bad9294747fcd618ba3634abb01ceef92213

cinder/tests/test_vmware_vmdk.py
cinder/volume/drivers/vmware/api.py

index 3642454f4ca9f7bc2863e3cb7c389468028d3795..6ff8f286be7d92bbda08033b5d7b31b88b75142c 100644 (file)
@@ -162,6 +162,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
 
             def __init__(self):
                 self.counter1 = 0
+                self.counter2 = 0
 
             @api.Retry(max_retry_count=2, inc_sleep_time=0.001,
                        exceptions=(Exception))
@@ -169,9 +170,16 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
                 self.counter1 += 1
                 raise exception.CinderException('Fail')
 
+            @api.Retry(max_retry_count=2)
+            def success(self):
+                self.counter2 += 1
+                return self.counter2
+
         test_obj = TestClass()
         self.assertRaises(exception.CinderException, test_obj.fail)
         self.assertEqual(test_obj.counter1, 3)
+        ret = test_obj.success()
+        self.assertEqual(1, ret)
 
     def test_create_session(self):
         """Test create_session."""
index 7b300193ae1ddb126313cee319a564b0618fae60..e7ca916b002bb6b827d03d19660da930993ac3ea 100644 (file)
@@ -20,8 +20,6 @@ Session and API call management for VMware ESX/VC server.
 Provides abstraction over cinder.volume.drivers.vmware.vim.Vim SOAP calls.
 """
 
-from eventlet import event
-
 from cinder.openstack.common import log as logging
 from cinder.openstack.common import loopingcall
 from cinder.volume.drivers.vmware import error_util
@@ -67,32 +65,29 @@ class Retry(object):
 
     def __call__(self, f):
 
-        def _func(done, *args, **kwargs):
+        def _func(*args, **kwargs):
             try:
                 result = f(*args, **kwargs)
-                done.send(result)
             except self._exceptions as excep:
                 LOG.exception(_("Failure while invoking function: "
                                 "%(func)s. Error: %(excep)s.") %
                               {'func': f.__name__, 'excep': excep})
                 if (self._max_retry_count != -1 and
                         self._retry_count >= self._max_retry_count):
-                    done.send_exception(excep)
+                    raise excep
                 else:
                     self._retry_count += 1
                     self._sleep_time += self._inc_sleep_time
                     return self._sleep_time
             except Exception as excep:
-                done.send_exception(excep)
-            return 0
+                raise excep
+            # got result. Stop the loop.
+            raise loopingcall.LoopingCallDone(result)
 
         def func(*args, **kwargs):
-            done = event.Event()
-            loop = loopingcall.DynamicLoopingCall(_func, done, *args, **kwargs)
-            loop.start(periodic_interval_max=self._max_sleep_time)
-            result = done.wait()
-            loop.stop()
-            return result
+            loop = loopingcall.DynamicLoopingCall(_func, *args, **kwargs)
+            timer = loop.start(periodic_interval_max=self._max_sleep_time)
+            return timer.wait()
 
         return func
 
@@ -231,15 +226,10 @@ class VMwareAPISession(object):
         :param task: Managed object reference of the task
         :return: Task info upon successful completion of the task
         """
-        done = event.Event()
-        loop = loopingcall.FixedIntervalLoopingCall(self._poll_task,
-                                                    task, done)
-        loop.start(self._task_poll_interval)
-        task_info = done.wait()
-        loop.stop()
-        return task_info
-
-    def _poll_task(self, task, done):
+        loop = loopingcall.FixedIntervalLoopingCall(self._poll_task, task)
+        return loop.start(self._task_poll_interval).wait()
+
+    def _poll_task(self, task):
         """Poll the given task.
 
         If the task completes successfully then returns task info.
@@ -260,36 +250,29 @@ class VMwareAPISession(object):
                 return
             elif task_info.state == 'success':
                 LOG.debug(_("Task %s status: success.") % task)
-                done.send(task_info)
             else:
                 error_msg = str(task_info.error.localizedMessage)
                 LOG.exception(_("Task: %(task)s failed with error: %(err)s.") %
                               {'task': task, 'err': error_msg})
-                done.send_exception(error_util.VimFaultException([],
-                                    error_msg))
+                raise error_util.VimFaultException([], error_msg)
         except Exception as excep:
             LOG.exception(_("Task: %(task)s failed with error: %(err)s.") %
                           {'task': task, 'err': excep})
-            done.send_exception(excep)
+            raise excep
+        # got the result. So stop the loop.
+        raise loopingcall.LoopingCallDone(task_info)
 
     def wait_for_lease_ready(self, lease):
-        done = event.Event()
-        loop = loopingcall.FixedIntervalLoopingCall(self._poll_lease,
-                                                    lease,
-                                                    done)
-        loop.start(self._task_poll_interval)
-        done.wait()
-        loop.stop()
-
-    def _poll_lease(self, lease, done):
+        loop = loopingcall.FixedIntervalLoopingCall(self._poll_lease, lease)
+        return loop.start(self._task_poll_interval).wait()
+
+    def _poll_lease(self, lease):
         try:
             state = self.invoke_api(vim_util, 'get_object_property',
                                     self.vim, lease, 'state')
             if state == 'ready':
                 # done
                 LOG.debug(_("Lease is ready."))
-                done.send()
-                return
             elif state == 'initializing':
                 LOG.debug(_("Lease initializing..."))
                 return
@@ -298,11 +281,13 @@ class VMwareAPISession(object):
                                             self.vim, lease, 'error')
                 LOG.exception(error_msg)
                 excep = error_util.VimFaultException([], error_msg)
-                done.send_exception(excep)
+                raise excep
             else:
                 # unknown state - complain
                 error_msg = _("Error: unknown lease state %s.") % state
                 raise error_util.VimFaultException([], error_msg)
         except Exception as excep:
             LOG.exception(excep)
-            done.send_exception(excep)
+            raise excep
+        # stop the loop since state is ready
+        raise loopingcall.LoopingCallDone()