LGTM, thanks On Wed, 18 Mar 2015 at 14:28 'Hrvoje Ribicic' via ganeti-devel < [email protected]> wrote:
> Due to hard-coded timeouts used when listing instances, the Xen > unit tests took around 60s to execute. The key offender was a timeout > of five seconds used for an unsuccessful listing of instances. This > patch refactors the code slightly to define the timeout and delays used > in one place, allowing these to be changed during testing to a more > acceptable value. As a result, these tests take around 5s to execute. > > Signed-off-by: Hrvoje Ribicic <[email protected]> > --- > lib/hypervisor/hv_xen.py | 21 +++++++++++++-------- > test/py/ganeti.hypervisor.hv_xen_unittest.py | 15 +++++++++++++-- > 2 files changed, 26 insertions(+), 10 deletions(-) > > diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py > index 1da5acb..8de82f0 100644 > --- a/lib/hypervisor/hv_xen.py > +++ b/lib/hypervisor/hv_xen.py > @@ -154,7 +154,7 @@ def _ParseInstanceList(lines, include_node): > return result > > > -def _GetAllInstanceList(fn, include_node, _timeout=5): > +def _GetAllInstanceList(fn, include_node, delays, timeout): > """Return the list of instances including running and shutdown. > > See L{_RunInstanceList} and L{_ParseInstanceList} for parameter details. > @@ -162,7 +162,7 @@ def _GetAllInstanceList(fn, include_node, _timeout=5): > """ > instance_list_errors = [] > try: > - lines = utils.Retry(_RunInstanceList, (0.3, 1.5, 1.0), _timeout, > + lines = utils.Retry(_RunInstanceList, delays, timeout, > args=(fn, instance_list_errors)) > except utils.RetryTimeout: > if instance_list_errors: > @@ -260,23 +260,23 @@ def _XenToHypervisorInstanceState(instance_info): > instance_info) > > > -def _GetRunningInstanceList(fn, include_node, _timeout=5): > +def _GetRunningInstanceList(fn, include_node, delays, timeout): > """Return the list of running instances. > > See L{_GetAllInstanceList} for parameter details. > > """ > - instances = _GetAllInstanceList(fn, include_node, _timeout) > + instances = _GetAllInstanceList(fn, include_node, delays, timeout) > return [i for i in instances if hv_base.HvInstanceState. > IsRunning(i[4])] > > > -def _GetShutdownInstanceList(fn, include_node, _timeout=5): > +def _GetShutdownInstanceList(fn, include_node, delays, timeout): > """Return the list of shutdown instances. > > See L{_GetAllInstanceList} for parameter details. > > """ > - instances = _GetAllInstanceList(fn, include_node, _timeout) > + instances = _GetAllInstanceList(fn, include_node, delays, timeout) > return [i for i in instances if hv_base.HvInstanceState. > IsShutdown(i[4])] > > > @@ -445,6 +445,9 @@ class XenHypervisor(hv_base.BaseHypervisor): > _NICS_DIR = _ROOT_DIR + "/nic" # contains NICs' info > _DIRS = [_ROOT_DIR, _NICS_DIR] > > + _INSTANCE_LIST_DELAYS = (0.3, 1.5, 1.0) > + _INSTANCE_LIST_TIMEOUT = 5 > + > ANCILLARY_FILES = [ > XEND_CONFIG_FILE, > XL_CONFIG_FILE, > @@ -649,7 +652,8 @@ class XenHypervisor(hv_base.BaseHypervisor): > > """ > return _GetAllInstanceList(lambda: self._RunXen(["list"], hvparams), > - include_node) > + include_node, delays=self._INSTANCE_LIST_ > DELAYS, > + timeout=self._INSTANCE_LIST_TIMEOUT) > > def ListInstances(self, hvparams=None): > """Get the list of running instances. > @@ -663,7 +667,8 @@ class XenHypervisor(hv_base.BaseHypervisor): > """ > instance_list = _GetRunningInstanceList( > lambda: self._RunXen(["list"], hvparams), > - False) > + False, delays=self._INSTANCE_LIST_DELAYS, > + timeout=self._INSTANCE_LIST_TIMEOUT) > return [info[0] for info in instance_list] > > def GetInstanceInfo(self, instance_name, hvparams=None): > diff --git a/test/py/ganeti.hypervisor.hv_xen_unittest.py b/test/py/ > ganeti.hypervisor.hv_xen_unittest.py > index 7247e74..ddb92d4 100755 > --- a/test/py/ganeti.hypervisor.hv_xen_unittest.py > +++ b/test/py/ganeti.hypervisor.hv_xen_unittest.py > @@ -208,7 +208,8 @@ class TestGetInstanceList(testutils.GanetiTestCase): > def testTimeout(self): > fn = testutils.CallCounter(self._Fail) > try: > - hv_xen._GetRunningInstanceList(fn, False, _timeout=0.1) > + hv_xen._GetRunningInstanceList(fn, False, delays=(0.02, 1.0, 0.03), > + timeout=0.1) > except errors.HypervisorError, err: > self.assertTrue("timeout exceeded" in str(err)) > else: > @@ -226,7 +227,8 @@ class TestGetInstanceList(testutils.GanetiTestCase): > > fn = testutils.CallCounter(compat.partial(self._Success, data)) > > - result = hv_xen._GetRunningInstanceList(fn, True, _timeout=0.1) > + result = hv_xen._GetRunningInstanceList(fn, True, delays=(0.02, 1.0, > 0.03), > + timeout=0.1) > > self.assertEqual(len(result), 4) > > @@ -584,6 +586,12 @@ class _TestXenHypervisor(object): > > self.vncpw = "".join(random.sample(string.ascii_letters, 10)) > > + self._xen_delay = self.TARGET._INSTANCE_LIST_DELAYS > + self.TARGET._INSTANCE_LIST_DELAYS = (0.01, 1.0, 0.05) > + > + self._list_timeout = self.TARGET._INSTANCE_LIST_TIMEOUT > + self.TARGET._INSTANCE_LIST_TIMEOUT = 0.1 > + > self.vncpw_path = utils.PathJoin(self.tmpdir, "vncpw") > utils.WriteFile(self.vncpw_path, data=self.vncpw) > > @@ -592,6 +600,9 @@ class _TestXenHypervisor(object): > > shutil.rmtree(self.tmpdir) > > + self.TARGET._INSTANCE_LIST_DELAYS = self._xen_delay > + self.TARGET._INSTANCE_LIST_TIMEOUT = self._list_timeout > + > def _GetHv(self, run_cmd=NotImplemented): > return self.TARGET(_cfgdir=self.tmpdir, _run_cmd_fn=run_cmd, > _cmd=self.CMD) > > -- > 2.2.0.rc0.207.ga3a616c > >
