QA: Convert nodes to objects

Up until now nodes were stored as a dictionary. The keys were hardcoded
in a lot of places and entries modified directly.

This patch introduces a new class for nodes in QA named “_QaNode”. It
still supports accessing details via dictionary syntax, but that will be
removed after a couple of patches changing all users. Unit tests for
“qa_config.AcquireNode” are also included.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

diff --git a/qa/qa_config.py b/qa/qa_config.py
index 4fe6a92..7036a15 100644 (file)
--- a/qa/qa_config.py
+++ b/qa/qa_config.py
@@ -113,8 +113,85 @@ class _QaInstance(object):
       return default
 
 
+class _QaNode(object):
+  __slots__ = [
+    "primary",
+    "secondary",
+    "_added",
+    "use_count",
+    ]
+
+  def __init__(self, primary, secondary):
+    """Initializes instances of this class.
+
+    """
+    self.primary = primary
+    self.secondary = secondary
+    self.use_count = 0
+    self._added = False
+
+  @classmethod
+  def FromDict(cls, data):
+    """Creates node object from JSON dictionary.
+
+    """
+    return cls(primary=data["primary"], secondary=data.get("secondary"))
+
+  def __getitem__(self, key):
+    """Legacy dict-like interface.
+
+    """
+    if key == "primary":
+      return self.primary
+    elif key == "secondary":
+      return self.secondary
+    else:
+      raise KeyError(key)
+
+  def get(self, key, default):
+    """Legacy dict-like interface.
+
+    """
+    try:
+      return self[key]
+    except KeyError:
+      return default
+
+  def Use(self):
+    """Marks a node as being in use.
+
+    """
+    assert self.use_count >= 0
+
+    self.use_count += 1
+
+    return self
+
+  def MarkAdded(self):
+    """Marks node as having been added to a cluster.
+
+    """
+    assert not self._added
+    self._added = True
+
+  def MarkRemoved(self):
+    """Marks node as having been removed from a cluster.
+
+    """
+    assert self._added
+    self._added = False
+
+  @property
+  def added(self):
+    """Returns whether a node is part of a cluster.
+
+    """
+    return self._added
+
+
 _RESOURCE_CONVERTER = {
   "instances": _QaInstance.FromDict,
+  "nodes": _QaNode.FromDict,
   }
 
 
@@ -470,12 +547,16 @@ def IsTemplateSupported(templ):
   return GetConfig().IsTemplateSupported(templ)
 
 
-def AcquireNode(exclude=None):
+def AcquireNode(exclude=None, _cfg=None):
   """Returns the least used node.
 
   """
-  master = GetMasterNode()
-  cfg = GetConfig()
+  if _cfg is None:
+    cfg = GetConfig()
+  else:
+    cfg = _cfg
+
+  master = cfg.GetMasterNode()
 
   # Filter out unwanted nodes
   # TODO: Maybe combine filters
@@ -486,25 +567,22 @@ def AcquireNode(exclude=None):
   else:
     nodes = filter(lambda node: node != exclude, cfg["nodes"])
 
-  tmp_flt = lambda node: node.get("_added", False) or node == master
-  nodes = filter(tmp_flt, nodes)
-  del tmp_flt
+  nodes = filter(lambda node: node.added or node == master, nodes)
 
-  if len(nodes) == 0:
+  if not nodes:
     raise qa_error.OutOfNodesError("No nodes left")
 
   # Get node with least number of uses
+  # TODO: Switch to computing sort key instead of comparing directly
   def compare(a, b):
-    result = cmp(a.get("_count", 0), b.get("_count", 0))
+    result = cmp(a.use_count, b.use_count)
     if result == 0:
-      result = cmp(a["primary"], b["primary"])
+      result = cmp(a.primary, b.primary)
     return result
 
   nodes.sort(cmp=compare)
 
-  node = nodes[0]
-  node["_count"] = node.get("_count", 0) + 1
-  return node
+  return nodes[0].Use()
 
 
 def AcquireManyNodes(num, exclude=None):
@@ -539,7 +617,9 @@ def AcquireManyNodes(num, exclude=None):
 
 
 def ReleaseNode(node):
-  node["_count"] = node.get("_count", 0) - 1
+  assert node.use_count > 0
+
+  node.use_count -= 1
 
 
 def ReleaseManyNodes(nodes):

diff --git a/qa/qa_node.py b/qa/qa_node.py
index 647af19..c1cfc17 100644 (file)
--- a/qa/qa_node.py
+++ b/qa/qa_node.py
@@ -36,9 +36,9 @@ from qa_utils import AssertCommand, AssertEqual
 
 
 def _NodeAdd(node, readd=False):
-  if not readd and node.get("_added", False):
+  if not readd and node.added:
     raise qa_error.Error("Node %s already in cluster" % node["primary"])
-  elif readd and not node.get("_added", False):
+  elif readd and not node.added:
     raise qa_error.Error("Node %s not yet in cluster" % node["primary"])
 
   cmd = ["gnt-node", "add", "--no-ssh-key-check"]
@@ -50,12 +50,15 @@ def _NodeAdd(node, readd=False):
 
   AssertCommand(cmd)
 
-  node["_added"] = True
+  if readd:
+    assert node.added
+  else:
+    node.MarkAdded()
 
 
 def _NodeRemove(node):
   AssertCommand(["gnt-node", "remove", node["primary"]])
-  node["_added"] = False
+  node.MarkRemoved()
 
 
 def MakeNodeOffline(node, value):
@@ -81,7 +84,7 @@ def MarkNodeAddedAll():
   master = qa_config.GetMasterNode()
   for node in qa_config.get("nodes"):
     if node != master:
-      node["_added"] = True
+      node.MarkAdded()
 
 
 def TestNodeRemoveAll():

diff --git a/test/py/qa.qa_config_unittest.py b/test/py/qa.qa_config_unittest.py
index c76cbce..70b5965 100755 (executable)
--- a/test/py/qa.qa_config_unittest.py
+++ b/test/py/qa.qa_config_unittest.py
@@ -277,6 +277,10 @@ class TestQaConfig(unittest.TestCase):
     self.assertTrue(isinstance(self.config["instances"][0],
                                qa_config._QaInstance))
 
+  def testNodeConversion(self):
+    self.assertTrue(isinstance(self.config["nodes"][0],
+                               qa_config._QaNode))
+
   def testAcquireAndReleaseInstance(self):
     self.assertFalse(compat.any(map(operator.attrgetter("used"),
                                     self.config["instances"])))
@@ -304,6 +308,42 @@ class TestQaConfig(unittest.TestCase):
     self.assertRaises(qa_error.OutOfInstancesError,
                       qa_config.AcquireInstance, _cfg=self.config)
 
+  def testAcquireNodeNoneAdded(self):
+    self.assertFalse(compat.any(map(operator.attrgetter("added"),
+                                    self.config["nodes"])))
+
+    # First call must return master node
+    node = qa_config.AcquireNode(_cfg=self.config)
+    self.assertEqual(node, self.config.GetMasterNode())
+
+    # Next call with exclusion list fails
+    self.assertRaises(qa_error.OutOfNodesError, qa_config.AcquireNode,
+                      exclude=[node], _cfg=self.config)
+
+  def testAcquireNodeTooMany(self):
+    # Mark all nodes as marked (master excluded)
+    for node in self.config["nodes"]:
+      if node != self.config.GetMasterNode():
+        node.MarkAdded()
+
+    nodecount = len(self.config["nodes"])
+
+    self.assertTrue(nodecount > 1)
+
+    acquired = []
+
+    for _ in range(nodecount):
+      node = qa_config.AcquireNode(exclude=acquired, _cfg=self.config)
+      if node == self.config.GetMasterNode():
+        self.assertFalse(node.added)
+      else:
+        self.assertTrue(node.added)
+      self.assertEqual(node.use_count, 1)
+      acquired.append(node)
+
+    self.assertRaises(qa_error.OutOfNodesError, qa_config.AcquireNode,
+                      exclude=acquired, _cfg=self.config)
+
 
 if __name__ == "__main__":
   testutils.GanetiTestProgram()