LGTM, thanks On Wed, 25 Nov 2015 at 14:10 'Oleg Ponomarev' via ganeti-devel < [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]> > --- > 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 > > -- Helga Velroyen Software Engineer [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.
