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.
