Currently hooksmaster is able to deal with both uuids and names.
While uuids are passed in most cases, the node list uses the legacy
naming *node_names* inside the hooksmaster implementation which leads
to misunderstandings. This patch modifies the existing code to make
it always pass node uuids to hooksmaster. It also renames the
misleading *node_names* parameter to *node_uuids*.

Signed-off-by: Oleg Ponomarev <[email protected]>
---
 lib/cmdlib/cluster/__init__.py     |  2 +-
 lib/cmdlib/common.py               |  6 +++---
 lib/hooksmaster.py                 | 33 ++++++++++++++++++++-------------
 test/py/cmdlib/cluster_unittest.py |  4 ++--
 test/py/cmdlib/node_unittest.py    | 12 ++++++------
 test/py/ganeti.hooks_unittest.py   | 18 +++++++++++-------
 6 files changed, 43 insertions(+), 32 deletions(-)

diff --git a/lib/cmdlib/cluster/__init__.py b/lib/cmdlib/cluster/__init__.py
index 951ad54..15631cb 100644
--- a/lib/cmdlib/cluster/__init__.py
+++ b/lib/cmdlib/cluster/__init__.py
@@ -298,7 +298,7 @@ class LUClusterDestroy(LogicalUnit):
     master_params = self.cfg.GetMasterNetworkParameters()
 
     # Run post hooks on master node before it's removed
-    RunPostHook(self, self.cfg.GetNodeName(master_params.uuid))
+    RunPostHook(self, master_params.uuid)
 
     ems = self.cfg.GetUseExternalMipScript()
     result = self.rpc.call_node_deactivate_master_ip(master_params.uuid,
diff --git a/lib/cmdlib/common.py b/lib/cmdlib/common.py
index 696a331..edd00af 100644
--- a/lib/cmdlib/common.py
+++ b/lib/cmdlib/common.py
@@ -179,16 +179,16 @@ def GetWantedInstances(lu, short_inst_names):
   return (inst_uuids, [lu.cfg.GetInstanceName(uuid) for uuid in inst_uuids])
 
 
-def RunPostHook(lu, node_name):
+def RunPostHook(lu, node_uuid):
   """Runs the post-hook for an opcode on a single node.
 
   """
   hm = lu.proc.BuildHooksManager(lu)
   try:
-    hm.RunPhase(constants.HOOKS_PHASE_POST, node_names=[node_name])
+    hm.RunPhase(constants.HOOKS_PHASE_POST, node_uuids=[node_uuid])
   except Exception, err: # pylint: disable=W0703
     lu.LogWarning("Errors occurred running hooks on %s: %s",
-                  node_name, err)
+                  node_uuid, err)
 
 
 def RedistributeAncillaryFiles(lu):
diff --git a/lib/hooksmaster.py b/lib/hooksmaster.py
index c23d857..4f1d7bd 100644
--- a/lib/hooksmaster.py
+++ b/lib/hooksmaster.py
@@ -56,7 +56,8 @@ def _RpcResultsToHooksResults(rpc_results):
 class HooksMaster(object):
   def __init__(self, opcode, hooks_path, nodes, hooks_execution_fn,
                hooks_results_adapt_fn, build_env_fn, prepare_post_nodes_fn,
-               log_fn, htype=None, cluster_name=None, master_name=None):
+               log_fn, htype=None, cluster_name=None, master_name=None,
+               master_uuid=None):
     """Base class for hooks masters.
 
     This class invokes the execution of hooks according to the behaviour
@@ -93,6 +94,8 @@ class HooksMaster(object):
     @param cluster_name: name of the cluster
     @type master_name: string
     @param master_name: name of the master
+    @type master_uuid: string
+    @param master_uuid: uuid of the master
 
     """
     self.opcode = opcode
@@ -105,6 +108,7 @@ class HooksMaster(object):
     self.htype = htype
     self.cluster_name = cluster_name
     self.master_name = master_name
+    self.master_uuid = master_uuid
 
     self.pre_env = self._BuildEnv(constants.HOOKS_PHASE_PRE)
     (self.pre_nodes, self.post_nodes) = nodes
@@ -183,10 +187,12 @@ class HooksMaster(object):
 
     assert compat.all(key == "PATH" or key.startswith("GANETI_")
                       for key in env)
+    for node in node_list:
+      assert utils.UUID_RE.match(node), "Invalid node uuid %s" % node
 
     return self.hooks_execution_fn(node_list, hpath, phase, env)
 
-  def RunPhase(self, phase, node_names=None):
+  def RunPhase(self, phase, node_uuids=None):
     """Run all the scripts for a phase.
 
     This is the main function of the HookMaster.
@@ -196,7 +202,7 @@ class HooksMaster(object):
 
     @param phase: one of L{constants.HOOKS_PHASE_POST} or
         L{constants.HOOKS_PHASE_PRE}; it denotes the hooks phase
-    @param node_names: overrides the predefined list of nodes for the given
+    @param node_uuids: overrides the predefined list of nodes for the given
         phase
     @return: the processed results of the hooks multi-node rpc call
     @raise errors.HooksFailure: on communication failure to the nodes
@@ -204,25 +210,25 @@ class HooksMaster(object):
 
     """
     if phase == constants.HOOKS_PHASE_PRE:
-      if node_names is None:
-        node_names = self.pre_nodes
+      if node_uuids is None:
+        node_uuids = self.pre_nodes
       env = self.pre_env
     elif phase == constants.HOOKS_PHASE_POST:
-      if node_names is None:
-        node_names = self.post_nodes
-        if node_names is not None and self.prepare_post_nodes_fn is not None:
-          node_names = frozenset(self.prepare_post_nodes_fn(list(node_names)))
+      if node_uuids is None:
+        node_uuids = self.post_nodes
+        if node_uuids is not None and self.prepare_post_nodes_fn is not None:
+          node_uuids = frozenset(self.prepare_post_nodes_fn(list(node_uuids)))
       env = self._BuildEnv(phase)
     else:
       raise AssertionError("Unknown phase '%s'" % phase)
 
-    if not node_names:
+    if not node_uuids:
       # empty node list, we should not attempt to run this as either
       # we're in the cluster init phase and the rpc client part can't
       # even attempt to run, or this LU doesn't do hooks at all
       return
 
-    results = self._RunWrapper(node_names, self.hooks_path, phase, env)
+    results = self._RunWrapper(node_uuids, self.hooks_path, phase, env)
     if not results:
       msg = "Communication Failure"
       if phase == constants.HOOKS_PHASE_PRE:
@@ -268,7 +274,7 @@ class HooksMaster(object):
     """
     phase = constants.HOOKS_PHASE_POST
     hpath = constants.HOOKS_NAME_CFGUPDATE
-    nodes = [self.master_name]
+    nodes = [self.master_uuid]
     self._RunWrapper(nodes, hpath, phase, self.pre_env)
 
   @staticmethod
@@ -285,9 +291,10 @@ class HooksMaster(object):
     master_name = cluster_name = None
     if lu.cfg:
       master_name = lu.cfg.GetMasterNodeName()
+      master_uuid = lu.cfg.GetMasterNode()
       cluster_name = lu.cfg.GetClusterName()
 
     return HooksMaster(lu.op.OP_ID, lu.HPATH, nodes, hooks_execution_fn,
                        _RpcResultsToHooksResults, lu.BuildHooksEnv,
                        lu.PreparePostHookNodes, lu.LogWarning, lu.HTYPE,
-                       cluster_name, master_name)
+                       cluster_name, master_name, master_uuid)
diff --git a/test/py/cmdlib/cluster_unittest.py 
b/test/py/cmdlib/cluster_unittest.py
index 24c1ca9..7b4b4e7 100644
--- a/test/py/cmdlib/cluster_unittest.py
+++ b/test/py/cmdlib/cluster_unittest.py
@@ -232,7 +232,7 @@ class TestLUClusterDestroy(CmdlibTestCase):
 
     self.ExecOpCode(op)
 
-    self.assertSingleHooksCall([self.master.name],
+    self.assertSingleHooksCall([self.master.uuid],
                                "cluster-destroy",
                                constants.HOOKS_PHASE_POST)
 
@@ -1208,7 +1208,7 @@ class TestLUClusterVerifyClientCerts(CmdlibTestCase):
   def _AddNormalNode(self):
     self.normalnode = copy.deepcopy(self.master)
     self.normalnode.master_candidate = False
-    self.normalnode.uuid = "normal-node-uuid"
+    self.normalnode.uuid = "deadbeef-dead-beef-dead-beefdeadbeef"
     self.cfg.AddNode(self.normalnode, None)
 
   def testVerifyMasterCandidate(self):
diff --git a/test/py/cmdlib/node_unittest.py b/test/py/cmdlib/node_unittest.py
index 8dcad78..d6dc66a 100644
--- a/test/py/cmdlib/node_unittest.py
+++ b/test/py/cmdlib/node_unittest.py
@@ -291,12 +291,12 @@ class TestLUNodeSetParams(CmdlibTestCase):
     self.node = self.cfg.AddNewNode(
         primary_ip='192.168.168.191',
         secondary_ip='192.168.168.192',
-        master_candidate=True, uuid='blue_bunny')
+        master_candidate=True, uuid='00000000-dead-beef-dead-beefdeadbeef')
 
     self.snode = self.cfg.AddNewNode(
         primary_ip='192.168.168.193',
         secondary_ip='192.168.168.194',
-        master_candidate=True, uuid='pink_bunny')
+        master_candidate=True, uuid='11111111-dead-beef-dead-beefdeadbeef')
 
   def testSetSecondaryIp(self):
     self.instance = self.cfg.AddNewInstance(primary_node=self.node,
@@ -310,8 +310,8 @@ class TestLUNodeSetParams(CmdlibTestCase):
     self.assertEqual(sorted(self.wconfd.all_locks.items()), [
         ('cluster/BGL', 'shared'),
         ('instance/mock_inst_1.example.com', 'shared'),
-        ('node-res/blue_bunny', 'exclusive'),
-        ('node/blue_bunny', 'exclusive')])
+        ('node-res/00000000-dead-beef-dead-beefdeadbeef', 'exclusive'),
+        ('node/00000000-dead-beef-dead-beefdeadbeef', 'exclusive')])
 
   def testSetSecondaryIpNoLock(self):
     self.instance = self.cfg.AddNewInstance(primary_node=self.node,
@@ -324,8 +324,8 @@ class TestLUNodeSetParams(CmdlibTestCase):
     self.assertEqual('254.254.254.254', self.node.secondary_ip)
     self.assertEqual(sorted(self.wconfd.all_locks.items()), [
         ('cluster/BGL', 'shared'),
-        ('node-res/blue_bunny', 'exclusive'),
-        ('node/blue_bunny', 'exclusive')])
+        ('node-res/00000000-dead-beef-dead-beefdeadbeef', 'exclusive'),
+        ('node/00000000-dead-beef-dead-beefdeadbeef', 'exclusive')])
 
 
 if __name__ == "__main__":
diff --git a/test/py/ganeti.hooks_unittest.py b/test/py/ganeti.hooks_unittest.py
index ab7ddda..9e297a3 100755
--- a/test/py/ganeti.hooks_unittest.py
+++ b/test/py/ganeti.hooks_unittest.py
@@ -60,7 +60,8 @@ class FakeLU(cmdlib.LogicalUnit):
     return {}
 
   def BuildHooksNodes(self):
-    return ["a"], ["a"]
+    return (["aaaaaaaa-dead-beef-dead-beefdeadbeef"],
+            ["aaaaaaaa-dead-beef-dead-beefdeadbeef"])
 
 
 class TestHooksRunner(unittest.TestCase):
@@ -299,7 +300,8 @@ class FakeEnvLU(cmdlib.LogicalUnit):
     return self.hook_env
 
   def BuildHooksNodes(self):
-    return (["a"], ["a"])
+    return (["aaaaaaaa-dead-beef-dead-beefdeadbeef"],
+            ["aaaaaaaa-dead-beef-dead-beefdeadbeef"])
 
 
 class FakeNoHooksLU(cmdlib.NoHooksLU):
@@ -414,7 +416,7 @@ class TestHooksRunnerEnv(unittest.TestCase):
   def testNoNodes(self):
     self.lu.hook_env = {}
     hm = hooksmaster.HooksMaster.BuildFromLu(self._HooksRpc, self.lu)
-    hm.RunPhase(constants.HOOKS_PHASE_PRE, node_names=[])
+    hm.RunPhase(constants.HOOKS_PHASE_PRE, node_uuids=[])
     self.assertRaises(IndexError, self._rpcs.pop)
 
   def testSpecificNodes(self):
@@ -515,10 +517,11 @@ class 
FakeEnvWithCustomPostHookNodesLU(cmdlib.LogicalUnit):
     return {}
 
   def BuildHooksNodes(self):
-    return (["a"], ["a"])
+    return (["aaaaaaaa-dead-beef-dead-beefdeadbeef"],
+            ["aaaaaaaa-dead-beef-dead-beefdeadbeef"])
 
   def PreparePostHookNodes(self, post_hook_node_uuids):
-    return post_hook_node_uuids + ["b"]
+    return post_hook_node_uuids + ["bbbbbbbb-dead-beef-dead-beefdeadbeef"]
 
 
 class TestHooksRunnerEnv(unittest.TestCase):
@@ -542,13 +545,14 @@ class TestHooksRunnerEnv(unittest.TestCase):
     hm.RunPhase(constants.HOOKS_PHASE_PRE)
 
     (node_list, hpath, phase, env) = self._rpcs.pop(0)
-    self.assertEqual(node_list, set(["a"]))
+    self.assertEqual(node_list, set(["aaaaaaaa-dead-beef-dead-beefdeadbeef"]))
 
     # Check post-phase hook
     hm.RunPhase(constants.HOOKS_PHASE_POST)
 
     (node_list, hpath, phase, env) = self._rpcs.pop(0)
-    self.assertEqual(node_list, set(["a", "b"]))
+    self.assertEqual(node_list, set(["aaaaaaaa-dead-beef-dead-beefdeadbeef",
+                                     "bbbbbbbb-dead-beef-dead-beefdeadbeef"]))
 
     self.assertRaises(IndexError, self._rpcs.pop)
 
-- 
2.6.0.rc2.230.g3dd15c0

Reply via email to