LGTM, thanks
On Wed, 25 Nov 2015 at 14:10 'Oleg Ponomarev' via ganeti-devel
<[email protected] <mailto:[email protected]>>
wrote:
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]
<mailto:[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
<http://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
<http://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
<http://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
<http://ganeti.hooks_unittest.py>
b/test/py/ganeti.hooks_unittest.py <http://ganeti.hooks_unittest.py>
index ab7ddda..9e297a3 100755
--- a/test/py/ganeti.hooks_unittest.py
<http://ganeti.hooks_unittest.py>
+++ b/test/py/ganeti.hooks_unittest.py
<http://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 <http://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
--
Helga Velroyen
Software Engineer
[email protected] <mailto:[email protected]>
Google Germany GmbH
Dienerstraße 12
80331 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat
sind, leiten Sie diese bitte nicht weiter, informieren Sie den
Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank.
This e-mail is confidential. If you are not the right addressee please
do not forward it, please inform the sender, and please erase this
e-mail including any attachments. Thanks.
