hv_xen: Refactor running & parsing "xm list", add tests

This patch refactors “_RunXmList” and adds some tests.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

Makefile.am

@@ -1038,6 +1038,8 @@ TEST_FILES = \
 	test/data/vgreduce-removemissing-2.02.66-ok.txt \
 	test/data/vgs-missing-pvs-2.02.02.txt \
 	test/data/vgs-missing-pvs-2.02.66.txt \
+	test/data/xen-xm-list-4.0.1-dom0-only.txt \
+	test/data/xen-xm-list-4.0.1-four-instances.txt \
 	test/py/ganeti-cli.test \
 	test/py/gnt-cli.test \
 	test/py/import-export_unittest-helper

lib/hypervisor/hv_xen.py

@@ -75,6 +75,89 @@ def _CreateConfigCpus(cpu_mask):
     return "cpus = [ %s ]" % ", ".join(map(_GetCPUMap, cpu_list))
 
 
+def _RunXmList(fn, xmllist_errors):
+  """Helper function for L{_GetXmList} to run "xm list".
+
+  @type fn: callable
+  @param fn: Function returning result of running C{xm list}
+  @type xmllist_errors: list
+  @param xmllist_errors: Error list
+  @rtype: list
+
+  """
+  result = fn()
+  if result.failed:
+    logging.error("xm list failed (%s): %s", result.fail_reason,
+                  result.output)
+    xmllist_errors.append(result)
+    raise utils.RetryAgain()
+
+  # skip over the heading
+  return result.stdout.splitlines()
+
+
+def _ParseXmList(lines, include_node):
+  """Parses the output of C{xm list}.
+
+  @type lines: list
+  @param lines: Output lines of C{xm list}
+  @type include_node: boolean
+  @param include_node: If True, return information for Dom0
+  @return: list of tuple containing (name, id, memory, vcpus, state, time
+    spent)
+
+  """
+  result = []
+
+  # Iterate through all lines while ignoring header
+  for line in lines[1:]:
+    # The format of lines is:
+    # Name      ID Mem(MiB) VCPUs State  Time(s)
+    # Domain-0   0  3418     4 r-----    266.2
+    data = line.split()
+    if len(data) != 6:
+      raise errors.HypervisorError("Can't parse output of xm list,"
+                                   " line: %s" % line)
+    try:
+      data[1] = int(data[1])
+      data[2] = int(data[2])
+      data[3] = int(data[3])
+      data[5] = float(data[5])
+    except (TypeError, ValueError), err:
+      raise errors.HypervisorError("Can't parse output of xm list,"
+                                   " line: %s, error: %s" % (line, err))
+
+    # skip the Domain-0 (optional)
+    if include_node or data[0] != _DOM0_NAME:
+      result.append(data)
+
+  return result
+
+
+def _GetXmList(fn, include_node, _timeout=5):
+  """Return the list of running instances.
+
+  See L{_RunXmList} and L{_ParseXmList} for parameter details.
+
+  """
+  xmllist_errors = []
+  try:
+    lines = utils.Retry(_RunXmList, (0.3, 1.5, 1.0), _timeout,
+                        args=(fn, xmllist_errors))
+  except utils.RetryTimeout:
+    if xmllist_errors:
+      xmlist_result = xmllist_errors.pop()
+
+      errmsg = ("xm list failed, timeout exceeded (%s): %s" %
+                (xmlist_result.fail_reason, xmlist_result.output))
+    else:
+      errmsg = "xm list failed"
+
+    raise errors.HypervisorError(errmsg)
+
+  return _ParseXmList(lines, include_node)
+
+
 class XenHypervisor(hv_base.BaseHypervisor):
   """Xen generic hypervisor interface
 
@@ -154,73 +237,19 @@ class XenHypervisor(hv_base.BaseHypervisor):
     utils.RemoveFile(XenHypervisor._ConfigFileName(instance_name))
 
   @staticmethod
-  def _RunXmList(xmlist_errors):
-    """Helper function for L{_GetXMList} to run "xm list".
+  def _GetXmList(include_node):
+    """Wrapper around module level L{_GetXmList}.
 
     """
-    result = utils.RunCmd([constants.XEN_CMD, "list"])
-    if result.failed:
-      logging.error("xm list failed (%s): %s", result.fail_reason,
-                    result.output)
-      xmlist_errors.append(result)
-      raise utils.RetryAgain()
-
-    # skip over the heading
-    return result.stdout.splitlines()[1:]
-
-  @classmethod
-  def _GetXMList(cls, include_node):
-    """Return the list of running instances.
-
-    If the include_node argument is True, then we return information
-    for dom0 also, otherwise we filter that from the return value.
-
-    @return: list of (name, id, memory, vcpus, state, time spent)
-
-    """
-    xmlist_errors = []
-    try:
-      lines = utils.Retry(cls._RunXmList, 1, 5, args=(xmlist_errors, ))
-    except utils.RetryTimeout:
-      if xmlist_errors:
-        xmlist_result = xmlist_errors.pop()
-
-        errmsg = ("xm list failed, timeout exceeded (%s): %s" %
-                  (xmlist_result.fail_reason, xmlist_result.output))
-      else:
-        errmsg = "xm list failed"
-
-      raise errors.HypervisorError(errmsg)
-
-    result = []
-    for line in lines:
-      # The format of lines is:
-      # Name      ID Mem(MiB) VCPUs State  Time(s)
-      # Domain-0   0  3418     4 r-----    266.2
-      data = line.split()
-      if len(data) != 6:
-        raise errors.HypervisorError("Can't parse output of xm list,"
-                                     " line: %s" % line)
-      try:
-        data[1] = int(data[1])
-        data[2] = int(data[2])
-        data[3] = int(data[3])
-        data[5] = float(data[5])
-      except (TypeError, ValueError), err:
-        raise errors.HypervisorError("Can't parse output of xm list,"
-                                     " line: %s, error: %s" % (line, err))
-
-      # skip the Domain-0 (optional)
-      if include_node or data[0] != _DOM0_NAME:
-        result.append(data)
-
-    return result
+    # TODO: Abstract running Xen command for testing
+    return _GetXmList(lambda: utils.RunCmd([constants.XEN_CMD, "list"]),
+                      include_node)
 
   def ListInstances(self):
     """Get the list of running instances.
 
     """
-    xm_list = self._GetXMList(False)
+    xm_list = self._GetXmList(False)
     names = [info[0] for info in xm_list]
     return names
 
@@ -232,7 +261,7 @@ class XenHypervisor(hv_base.BaseHypervisor):
     @return: tuple (name, id, memory, vcpus, stat, times)
 
     """
-    xm_list = self._GetXMList(instance_name == _DOM0_NAME)
+    xm_list = self._GetXmList(instance_name == _DOM0_NAME)
     result = None
     for data in xm_list:
       if data[0] == instance_name:
@@ -246,7 +275,7 @@ class XenHypervisor(hv_base.BaseHypervisor):
     @return: list of tuples (name, id, memory, vcpus, stat, times)
 
     """
-    xm_list = self._GetXMList(False)
+    xm_list = self._GetXmList(False)
     return xm_list
 
   def StartInstance(self, instance, block_devices, startup_paused):
@@ -395,7 +424,7 @@ class XenHypervisor(hv_base.BaseHypervisor):
       result["cpu_sockets"] = nr_cpus / (cores_per_socket * threads_per_core)
 
     total_instmem = 0
-    for (name, _, mem, vcpus, _, _) in self._GetXMList(True):
+    for (name, _, mem, vcpus, _, _) in self._GetXmList(True):
       if name == _DOM0_NAME:
         result["memory_dom0"] = mem
         result["dom0_cpus"] = vcpus

test/data/xen-xm-list-4.0.1-dom0-only.txt

+Name                                        ID   Mem VCPUs      State   Time(s)
+Domain-0                                     0  1023     1     r----- 121152.6

test/data/xen-xm-list-4.0.1-four-instances.txt

+Name                                        ID   Mem VCPUs      State   Time(s)
+Domain-0                                     0  1023     1     r----- 154706.1
+server01.example.com                         1  1024     1     -b---- 167643.2
+web3106215069.example.com                    3  4096     1     -b---- 466690.9
+testinstance.example.com                     2  2048     2     r----- 244443.0

test/py/ganeti.hypervisor.hv_xen_unittest.py

 #!/usr/bin/python
 #
 
-# Copyright (C) 2011 Google Inc.
+# Copyright (C) 2011, 2013 Google Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -26,6 +26,9 @@ import unittest
 from ganeti import constants
 from ganeti import objects
 from ganeti import hypervisor
+from ganeti import utils
+from ganeti import errors
+from ganeti import compat
 
 from ganeti.hypervisor import hv_xen
 
@@ -63,5 +66,91 @@ class TestCreateConfigCpus(unittest.TestCase):
                       constants.CPU_PINNING_ALL_XEN))
 
 
+class TestParseXmList(testutils.GanetiTestCase):
+  def test(self):
+    data = testutils.ReadTestData("xen-xm-list-4.0.1-dom0-only.txt")
+
+    # Exclude node
+    self.assertEqual(hv_xen._ParseXmList(data.splitlines(), False), [])
+
+    # Include node
+    result = hv_xen._ParseXmList(data.splitlines(), True)
+    self.assertEqual(len(result), 1)
+    self.assertEqual(len(result[0]), 6)
+
+    # Name
+    self.assertEqual(result[0][0], hv_xen._DOM0_NAME)
+
+    # ID
+    self.assertEqual(result[0][1], 0)
+
+    # Memory
+    self.assertEqual(result[0][2], 1023)
+
+    # VCPUs
+    self.assertEqual(result[0][3], 1)
+
+    # State
+    self.assertEqual(result[0][4], "r-----")
+
+    # Time
+    self.assertAlmostEqual(result[0][5], 121152.6)
+
+  def testWrongLineFormat(self):
+    tests = [
+      ["three fields only"],
+      ["name InvalidID 128 1 r----- 12345"],
+      ]
+
+    for lines in tests:
+      try:
+        hv_xen._ParseXmList(["Header would be here"] + lines, False)
+      except errors.HypervisorError, err:
+        self.assertTrue("Can't parse output of xm list" in str(err))
+      else:
+        self.fail("Exception was not raised")
+
+
+class TestGetXmList(testutils.GanetiTestCase):
+  def _Fail(self):
+    return utils.RunResult(constants.EXIT_FAILURE, None,
+                           "stdout", "stderr", None,
+                           NotImplemented, NotImplemented)
+
+  def testTimeout(self):
+    fn = testutils.CallCounter(self._Fail)
+    try:
+      hv_xen._GetXmList(fn, False, _timeout=0.1)
+    except errors.HypervisorError, err:
+      self.assertTrue("timeout exceeded" in str(err))
+    else:
+      self.fail("Exception was not raised")
+
+    self.assertTrue(fn.Count() < 10,
+                    msg="'xm list' was called too many times")
+
+  def _Success(self, stdout):
+    return utils.RunResult(constants.EXIT_SUCCESS, None, stdout, "", None,
+                           NotImplemented, NotImplemented)
+
+  def testSuccess(self):
+    data = testutils.ReadTestData("xen-xm-list-4.0.1-four-instances.txt")
+
+    fn = testutils.CallCounter(compat.partial(self._Success, data))
+
+    result = hv_xen._GetXmList(fn, True, _timeout=0.1)
+
+    self.assertEqual(len(result), 4)
+
+    self.assertEqual(map(compat.fst, result), [
+      "Domain-0",
+      "server01.example.com",
+      "web3106215069.example.com",
+      "testinstance.example.com",
+      ])
+
+    self.assertEqual(fn.Count(), 1)
+
+
 if __name__ == "__main__":
   testutils.GanetiTestProgram()