LGTM, thanks On Tue, 1 Dec 2015 at 12:24 Oleg Ponomarev <[email protected]> wrote:
> Sorry, but there should be one more interdiff. The last one:) > > --- a/lib/cmdlib/node.py > +++ b/lib/cmdlib/node.py > @@ -1595,7 +1595,7 @@ class LUNodeRemove(LogicalUnit): > self.cfg.RemoveNode(self.node.uuid) > > # Run post hooks on the node before it's removed > - RunPostHook(self, self.node.name) > + RunPostHook(self, self.node.uuid) > > # we have to call this by name rather than by UUID, as the node is no > longer > # in the config > > > On 11/27/2015 04:30 PM, Helga Velroyen wrote: > > LGTM, thanks > > On Fri, 27 Nov 2015 at 16:29 Oleg Ponomarev <[email protected]> wrote: > >> In the initial patch, local hooks were forgotten. >> >> Thus, please consider the following interdiff: >> >> diff --git a/lib/backend.py b/lib/backend.py >> index bd638a3..a8d4170 100644 >> --- a/lib/backend.py >> +++ b/lib/backend.py >> @@ -333,8 +333,9 @@ def RunLocalHooks(hook_opcode, hooks_path, >> env_builder_fn): >> """ >> def decorator(fn): >> def wrapper(*args, **kwargs): >> - _, myself = ssconf.GetMasterAndMyself() >> - nodes = ([myself], [myself]) # these hooks run locally >> + # Despite the hooks run locally, we still have to pass an uuid >> which >> + # will be ignored in RunLocalHooks then. >> + nodes = ([constants.DUMMY_UUID], [constants.DUMMY_UUID]) >> >> env_fn = compat.partial(env_builder_fn, *args, **kwargs) >> >> @@ -5662,8 +5663,7 @@ class HooksRunner(object): >> """ >> assert len(node_list) == 1 >> node = node_list[0] >> - _, myself = ssconf.GetMasterAndMyself() >> - assert node == myself >> + assert node == constants.DUMMY_UUID >> >> results = self.RunHooks(hpath, phase, env) >> >> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs >> index 105004c..ae33b7b 100644 >> --- a/src/Ganeti/Constants.hs >> +++ b/src/Ganeti/Constants.hs >> @@ -5063,6 +5063,9 @@ diskRemoveRetryInterval = 3 >> uuidRegex :: String >> uuidRegex = >> "^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$" >> >> +dummyUuid :: String >> +dummyUuid = "deadbeef-dead-beed-dead-beefdeadbeef" >> + >> -- * Luxi constants >> >> luxiSocketPerms :: Int >> >> >> >> On 11/26/2015 09:12 AM, Helga Velroyen wrote: >> >> 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. >> >> >> -- > > 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. > > > -- 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.
