Run PRE and POST global hooks in case of succesful job execution on the master node and on the node_list provided by the logical unit. POST hooks are always executed with status equal to SUCCESS.
Also fix the python tests: - as currently at least global hooks are always executed and hooksmaster account the rpc results, call_hooks_runner mock modified in order to always return successful results; - the tests ensuring that hooks are executed only once are now useless because of global hooks. They are replaced by the separate tests per each hooks execution. Signed-off-by: Oleg Ponomarev <[email protected]> --- lib/cmdlib/common.py | 3 +++ lib/hooksmaster.py | 5 ++--- lib/mcpu.py | 22 +++++++++++++++++----- src/Ganeti/Constants.hs | 9 +++++++++ test/py/cmdlib/cluster_unittest.py | 18 ++++++++++++------ test/py/cmdlib/testsupport/cmdlib_testcase.py | 19 ++----------------- test/py/cmdlib/testsupport/rpc_runner_mock.py | 23 +++++++++++++++++++---- test/py/ganeti.hooks_unittest.py | 6 +++--- 8 files changed, 67 insertions(+), 38 deletions(-) diff --git a/lib/cmdlib/common.py b/lib/cmdlib/common.py index 696a331..04ba7a3 100644 --- a/lib/cmdlib/common.py +++ b/lib/cmdlib/common.py @@ -185,7 +185,10 @@ def RunPostHook(lu, node_name): """ hm = lu.proc.BuildHooksManager(lu) try: + # Execute usual post hooks, then global post hooks. hm.RunPhase(constants.HOOKS_PHASE_POST, node_names=[node_name]) + hm.RunPhase(constants.HOOKS_PHASE_POST, [node_name], is_global=True, + post_status=constants.POST_HOOKS_STATUS_SUCCESS) except Exception, err: # pylint: disable=W0703 lu.LogWarning("Errors occurred running hooks on %s: %s", node_name, err) diff --git a/lib/hooksmaster.py b/lib/hooksmaster.py index 1472ea7..1326dc0 100644 --- a/lib/hooksmaster.py +++ b/lib/hooksmaster.py @@ -202,12 +202,11 @@ class HooksMaster(object): ret = dict() if is_global: env["GANETI_IS_MASTER"] = constants.GLOBAL_HOOKS_MASTER - master_set = set([self.master_name]) - ret.update(self.hooks_execution_fn(master_set, hpath, phase, env)) + ret.update(self.hooks_execution_fn([self.master_name], hpath, phase, env)) if node_list: # Master node might be either listed by the uuid or by the name - master_set.add(self.master_uuid) + master_set = frozenset([self.master_uuid, self.master_name]) node_list = frozenset(set(node_list) - master_set) if not node_list: return ret diff --git a/lib/mcpu.py b/lib/mcpu.py index 209e859..8bb7969 100644 --- a/lib/mcpu.py +++ b/lib/mcpu.py @@ -305,6 +305,7 @@ class Processor(object): self.cfg = context.GetConfig(ec_id) self.rpc = context.GetRpc(self.cfg) self.hmclass = hooksmaster.HooksMaster + self._hm = None self._enable_locks = enable_locks self.wconfd = wconfd # Indirection to allow testing self._wconfdcontext = context.GetWConfdContext(ec_id) @@ -483,8 +484,11 @@ class Processor(object): lu.cfg.OutDate() lu.CheckPrereq() - hm = self.BuildHooksManager(lu) - h_results = hm.RunPhase(constants.HOOKS_PHASE_PRE) + self._hm = self.BuildHooksManager(lu) + # Run hooks twice: first for the global hooks, then for the usual hooks. + self._hm.RunPhase(constants.HOOKS_PHASE_PRE, node_names=None, + is_global=True) + h_results = self._hm.RunPhase(constants.HOOKS_PHASE_PRE) lu.HooksCallBack(constants.HOOKS_PHASE_PRE, h_results, self.Log, None) @@ -504,19 +508,20 @@ class Processor(object): lusExecuting[0] += 1 try: result = _ProcessResult(submit_mj_fn, lu.op, lu.Exec(self.Log)) - h_results = hm.RunPhase(constants.HOOKS_PHASE_POST) + h_results = self._hm.RunPhase(constants.HOOKS_PHASE_POST) result = lu.HooksCallBack(constants.HOOKS_PHASE_POST, h_results, self.Log, result) finally: # FIXME: This needs locks if not lu_class.REQ_BGL lusExecuting[0] -= 1 if write_count != self.cfg.write_count: - hm.RunConfigUpdate() + self._hm.RunConfigUpdate() return result def BuildHooksManager(self, lu): - return self.hmclass.BuildFromLu(lu.rpc.call_hooks_runner, lu) + return self.hmclass.BuildFromLu(lu.rpc.call_hooks_runner, lu, + self.GetECId()) def _LockAndExecLU(self, lu, level, calc_timeout, pending=None): """Execute a Logical Unit, with the needed locks. @@ -715,6 +720,13 @@ class Processor(object): self._CheckLUResult(op, result) + # The post hooks below are always executed with a SUCCESS status because + # all the possible errors during pre hooks and LU execution cause + # exception and therefore the statement below will be skipped. + if self._hm is not None: + self._hm.RunPhase(constants.HOOKS_PHASE_POST, node_names=None, + is_global=True, + post_status=constants.POST_HOOKS_STATUS_SUCCESS) return result def Log(self, *args): diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs index 1f96c0d..1dddefe 100644 --- a/src/Ganeti/Constants.hs +++ b/src/Ganeti/Constants.hs @@ -735,6 +735,15 @@ globalHooksMaster = "master" globalHooksNotMaster :: String globalHooksNotMaster = "not_master" +postHooksStatusSuccess :: String +postHooksStatusSuccess = "success" + +postHooksStatusError :: String +postHooksStatusError = "error" + +postHooksStatusDisappear :: String +postHooksStatusDisappear = "disappear" + -- * Hooks subject type (what object type does the LU deal with) htypeCluster :: String diff --git a/test/py/cmdlib/cluster_unittest.py b/test/py/cmdlib/cluster_unittest.py index 24c1ca9..8124555 100644 --- a/test/py/cmdlib/cluster_unittest.py +++ b/test/py/cmdlib/cluster_unittest.py @@ -232,9 +232,12 @@ class TestLUClusterDestroy(CmdlibTestCase): self.ExecOpCode(op) - self.assertSingleHooksCall([self.master.name], - "cluster-destroy", - constants.HOOKS_PHASE_POST) + self.assertHooksCall([self.master.name], constants.GLOBAL_HOOKS_DIR, + constants.HOOKS_PHASE_PRE, index=0) + self.assertHooksCall([self.master.name], "cluster-destroy", + constants.HOOKS_PHASE_POST, index=1) + self.assertHooksCall([self.master.name], constants.GLOBAL_HOOKS_DIR, + constants.HOOKS_PHASE_POST, index=2) class TestLUClusterPostInit(CmdlibTestCase): @@ -244,9 +247,12 @@ class TestLUClusterPostInit(CmdlibTestCase): self.ExecOpCode(op) - self.assertSingleHooksCall([self.master.uuid], - "cluster-init", - constants.HOOKS_PHASE_POST) + self.assertHooksCall([self.master.name], constants.GLOBAL_HOOKS_DIR, + constants.HOOKS_PHASE_PRE, index=0) + self.assertHooksCall([self.master.uuid], "cluster-init", + constants.HOOKS_PHASE_POST, index=1) + self.assertHooksCall([self.master.name], constants.GLOBAL_HOOKS_DIR, + constants.HOOKS_PHASE_POST, index=2) class TestLUClusterQuery(CmdlibTestCase): diff --git a/test/py/cmdlib/testsupport/cmdlib_testcase.py b/test/py/cmdlib/testsupport/cmdlib_testcase.py index 4e459f3..f98b751 100644 --- a/test/py/cmdlib/testsupport/cmdlib_testcase.py +++ b/test/py/cmdlib/testsupport/cmdlib_testcase.py @@ -326,7 +326,7 @@ class CmdlibTestCase(testutils.GanetiTestCase): self.mcpu.assertLogContainsRegex(expected_regex) def assertHooksCall(self, nodes, hook_path, phase, - environment=None, count=None, index=0): + environment=None, index=0): """Asserts a call to C{rpc.call_hooks_runner} @type nodes: list of string @@ -338,16 +338,11 @@ class CmdlibTestCase(testutils.GanetiTestCase): @type environment: dict @param environment: the environment passed to the hooks. C{None} to skip asserting it - @type count: int - @param count: the number of hook invocations. C{None} to skip asserting it @type index: int @param index: the index of the hook invocation to assert """ - if count is not None: - self.assertEqual(count, self.rpc.call_hooks_runner.call_count) - - args = self.rpc.call_hooks_runner.call_args[index] + args = self.rpc.call_hooks_runner.mock_calls[index][1] self.assertEqual(set(nodes), set(args[0])) self.assertEqual(hook_path, args[1]) @@ -355,16 +350,6 @@ class CmdlibTestCase(testutils.GanetiTestCase): if environment is not None: self.assertEqual(environment, args[3]) - def assertSingleHooksCall(self, nodes, hook_path, phase, - environment=None): - """Asserts a single call to C{rpc.call_hooks_runner} - - @see L{assertHooksCall} for parameter description. - - """ - self.assertHooksCall(nodes, hook_path, phase, - environment=environment, count=1) - def CopyOpCode(self, opcode, **kwargs): """Creates a copy of the given opcode and applies modifications to it diff --git a/test/py/cmdlib/testsupport/rpc_runner_mock.py b/test/py/cmdlib/testsupport/rpc_runner_mock.py index 9658963..10e40b4 100644 --- a/test/py/cmdlib/testsupport/rpc_runner_mock.py +++ b/test/py/cmdlib/testsupport/rpc_runner_mock.py @@ -39,11 +39,24 @@ from ganeti.rpc import node as rpc from cmdlib.testsupport.util import patchModule +# Disable 'Unused argument' because it's a mock function +# pylint: disable=W0613 +def MockHooksExecutionFn(nodes, hpath, phase, env): + """Helper function that generate rpc results for call_hooks_runner mock + + """ + results = RpcResultsBuilder() + for node in nodes: + results.AddSuccessfulNode(node, data=None, node_id_passed=True) + return results.Build() + + def CreateRpcRunnerMock(): """Creates a new L{mock.MagicMock} tailored for L{rpc.RpcRunner} """ ret = mock.MagicMock(spec=rpc.RpcRunner) + ret.call_hooks_runner.side_effect = MockHooksExecutionFn return ret @@ -106,7 +119,7 @@ class RpcResultsBuilder(object): else: return node.uuid - def CreateSuccessfulNodeResult(self, node, data=None): + def CreateSuccessfulNodeResult(self, node, data=None, node_id_passed=False): """@see L{RpcResultsBuilder} @param node: @see L{RpcResultsBuilder}. @@ -116,7 +129,8 @@ class RpcResultsBuilder(object): """ if data is None: data = {} - return rpc.RpcResult(data=(True, data), node=self._GetNodeId(node)) + res_node = node if node_id_passed else self._GetNodeId(node) + return rpc.RpcResult(data=(True, data), node=res_node) def CreateFailedNodeResult(self, node): """@see L{RpcResultsBuilder} @@ -144,14 +158,15 @@ class RpcResultsBuilder(object): """ return rpc.RpcResult(data=(False, error_msg), node=self._GetNodeId(node)) - def AddSuccessfulNode(self, node, data=None): + def AddSuccessfulNode(self, node, data=None, node_id_passed=False): """@see L{CreateSuccessfulNode} @rtype: L{RpcResultsBuilder} @return: self for chaining """ - self._results.append(self.CreateSuccessfulNodeResult(node, data)) + self._results.append(self.CreateSuccessfulNodeResult(node, data, + node_id_passed)) return self def AddFailedNode(self, node): diff --git a/test/py/ganeti.hooks_unittest.py b/test/py/ganeti.hooks_unittest.py index ab7ddda..3de3bfa 100755 --- a/test/py/ganeti.hooks_unittest.py +++ b/test/py/ganeti.hooks_unittest.py @@ -222,9 +222,9 @@ def FakeHooksRpcSuccess(node_list, hpath, phase, env): class TestHooksMaster(unittest.TestCase): """Testing case for HooksMaster""" - def _call_false(*args): + def _call_fail(*args): """Fake call_hooks_runner function which returns False.""" - return False + return {} @staticmethod def _call_nodes_false(node_list, hpath, phase, env): @@ -259,7 +259,7 @@ class TestHooksMaster(unittest.TestCase): def testTotalFalse(self): """Test complete rpc failure""" - hm = hooksmaster.HooksMaster.BuildFromLu(self._call_false, self.lu) + hm = hooksmaster.HooksMaster.BuildFromLu(self._call_fail, self.lu) self.failUnlessRaises(errors.HooksFailure, hm.RunPhase, constants.HOOKS_PHASE_PRE) hm.RunPhase(constants.HOOKS_PHASE_POST) -- 2.6.0.rc2.230.g3dd15c0
